Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added GERRIT_EVENT_FILE_BASED environment variable #358

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added GERRIT_EVENT_FILE_BASED environment variable #358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 41 additions & 0 deletions ...main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTrigger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ public class GerritTrigger extends Trigger<Job> {
* abort running jobs by setting this to 'false'.
*/
public static final String JOB_ABORT = GerritTrigger.class.getName() + "_job_abort";
private transient GerritTriggerRuntimeParam triggerParamRuntime = new GerritTriggerRuntimeParam();

//! Association between patches and the jobs that we're running for them
private transient RunningJobs runningJobs = new RunningJobs();
Expand Down Expand Up @@ -988,6 +989,10 @@ public boolean isInteresting(GerritTriggeredEvent event) {
changeBasedEvent.getFiles(
new GerritQueryHandler(getServerConfig(event))))) {
logger.trace("According to {} the event is interesting.", p);
if (triggerParamRuntime == null) {
triggerParamRuntime = new GerritTriggerRuntimeParam();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is bad as the GerritTrigger object is one instance per job so adding a field to it means adding a field to the job not the build.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I can't think of another way of doing it. Please give me some clues to which file is the right one for this purpose.

As far as why I thought this was the only option: This is the only file that knows about list of files involved in a given Gerrit change(changeBasedEvent.getFiles), thus, able to check if the affected-files in a gerrit change match the filtered files of file-based-trigger configuration.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You would need to carry the information over from somewhere around here to the build. data in builds are stored via actions, int this case the simplest would be to put the information into the GerritCause object that is added to the build when scheduled.
Also, here I don't know if you can be sure the event was interesting because of the file, but maybe :)

}
triggerParamRuntime.setfileBasedMatch(true);
return true;
}
} else {
Expand Down Expand Up @@ -1658,6 +1663,14 @@ public String getBuildNotBuiltMessage() {
return buildNotBuiltMessage;
}

/**
*
* @return True or False
*/
public boolean getfileBasedMatch() {
return this.triggerParamRuntime.getfileBasedMatch();
}

/**
* Message to write to Gerrit when all builds are not built.
*
Expand Down Expand Up @@ -2215,6 +2228,34 @@ public List<PluginGerritEvent.PluginGerritEventDescriptor> getGerritEventDescrip
}
}

/**
* Class for maintaining the Gerrit Trigger parameters.
* These are parameters whose values are known only after querying Gerrit, for example,
* the criterion by which something was 'isInteresting', say because the file modified
* in a changeset matched the file-based trigger file list.
*/
public class GerritTriggerRuntimeParam {
private boolean fileBasedMatch = false;

/**
* Set file based match if found so.
*
* @param i true or false
*/
public void setfileBasedMatch(boolean i) {
this.fileBasedMatch = i;
}

/**
* Returns true or false.
*
* @return true or false
*/
public boolean getfileBasedMatch() {
return this.fileBasedMatch;
}
}

/**
* Class for maintaining and synchronizing the runningJobs info.
* Association between patches and the jobs that we're running for them.
Expand Down
10 changes: 9 additions & 1 deletion ...com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerParameters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,11 @@ public enum GerritTriggerParameters {
/**
* Comment posted to Gerrit in a comment-added event.
*/
GERRIT_EVENT_COMMENT_TEXT;
GERRIT_EVENT_COMMENT_TEXT,
/**
* Set to true if file based match.
*/
GERRIT_EVENT_FILE_BASED;

private static final Logger logger = LoggerFactory.getLogger(GerritTriggerParameters.class);

Expand Down Expand Up @@ -373,6 +377,7 @@ public static void setOrCreateParameters(GerritTriggeredEvent gerritEvent, Job p
boolean escapeQuotes = false;
ParameterMode commitMessageMode = ParameterMode.BASE64;
ParameterMode changeSubjectMode = ParameterMode.PLAIN;
boolean getfileBasedMatch = false;
ParameterMode commentTextMode = ParameterMode.BASE64;
if (project != null) {
GerritTrigger trigger = GerritTrigger.getTrigger(project);
Expand All @@ -382,9 +387,12 @@ public static void setOrCreateParameters(GerritTriggeredEvent gerritEvent, Job p
commitMessageMode = trigger.getCommitMessageParameterMode();
changeSubjectMode = trigger.getChangeSubjectParameterMode();
commentTextMode = trigger.getCommentTextParameterMode();
getfileBasedMatch = trigger.getfileBasedMatch();
}
}

GERRIT_EVENT_FILE_BASED.setOrCreateStringParameterValue(
parameters, String.valueOf(getfileBasedMatch), escapeQuotes);
GERRIT_EVENT_TYPE.setOrCreateStringParameterValue(
parameters, gerritEvent.getEventType().getTypeValue(), escapeQuotes);
GERRIT_EVENT_HASH.setOrCreateStringParameterValue(
Expand Down