Skip to content

Commit

Permalink
[FIXES JENKINS-2494] Restore correct behaviour
Browse files Browse the repository at this point in the history
- Fixes a regression in core where the display name clear on copy was triggering a save
- More than one way to do this, could also have used the marker interface approach
  This route seems slightly less fragile, though people could still add ItemListeners
  with order == -Double.MAX_VALUE which would then introduce intdeterminism.
  A marker interface would remove that indeterminism as the onCopyComplete method would
  be only called on the Job as the last method... but it could be hard to
  ensure that all ItemGroupMixin's respect the calling of onCopyComplete contract
  hence this approach seems better to me for that reason
  • Loading branch information
stephenc committed Jul 3, 2013
1 parent 817237d commit 503c3bd
Showing 1 changed file with 28 additions and 3 deletions.
31 changes: 28 additions & 3 deletions core/src/main/java/hudson/model/Job.java
Expand Up @@ -125,6 +125,13 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
*/ */
private transient volatile boolean holdOffBuildUntilSave; private transient volatile boolean holdOffBuildUntilSave;


/**
* {@link ItemListener}s can, and do, modify the job with a corresponding save which will clear
* {@link #holdOffBuildUntilSave} prematurely. The {@link LastItemListener} is responsible for
* clearing this flag as the last item listener.
*/
private transient volatile boolean holdOffBuildUntilUserSave;

private volatile BuildDiscarder logRotator; private volatile BuildDiscarder logRotator;


/** /**
Expand All @@ -150,7 +157,7 @@ protected Job(ItemGroup parent, String name) {
@Override @Override
public synchronized void save() throws IOException { public synchronized void save() throws IOException {
super.save(); super.save();
holdOffBuildUntilSave = false; holdOffBuildUntilSave = holdOffBuildUntilUserSave;
} }


@Override @Override
Expand Down Expand Up @@ -204,7 +211,25 @@ public void onCopiedFrom(Item src) {
super.onCopiedFrom(src); super.onCopiedFrom(src);
synchronized (this) { synchronized (this) {
this.nextBuildNumber = 1; // reset the next build number this.nextBuildNumber = 1; // reset the next build number
this.holdOffBuildUntilSave = true; this.holdOffBuildUntilUserSave = true;
this.holdOffBuildUntilSave = this.holdOffBuildUntilUserSave;
}
}

@Extension(ordinal = -Double.MAX_VALUE)
public static class LastItemListener extends ItemListener {

@Override
public void onCopied(Item src, Item item) {
// If any of the other ItemListeners modify the job, they effect
// a save, which will clear the holdOffBuildUntilUserSave and
// causing a regression of JENKINS-2494
if (item instanceof Job) {
Job job = (Job) item;
synchronized (job) {
job.holdOffBuildUntilUserSave = false;
}
}
} }
} }


Expand All @@ -226,7 +251,7 @@ protected void performDelete() throws IOException, InterruptedException {
return new TextFile(new File(this.getRootDir(), "nextBuildNumber")); return new TextFile(new File(this.getRootDir(), "nextBuildNumber"));
} }


protected boolean isHoldOffBuildUntilSave() { protected synchronized boolean isHoldOffBuildUntilSave() {
return holdOffBuildUntilSave; return holdOffBuildUntilSave;
} }


Expand Down

0 comments on commit 503c3bd

Please sign in to comment.