diff --git a/concurrent-build-branch-note.txt b/concurrent-build-branch-note.txt new file mode 100644 index 0000000000..ae56615945 --- /dev/null +++ b/concurrent-build-branch-note.txt @@ -0,0 +1,40 @@ +TODO: +- think about the implications of this change to workspace deletion + +- replace various "1.XXX" reference to the right revision when we merge this back to trunk. + + +Release Plan +============ + +1. have some interested users try this out +2. merge this to the trunk, but hide the feature. This allows plugins to be updated to use + checkpointing, in preparation of mass launch +3. solicit interested parties to activate this feature +4. soak time for stability +5. ship! + + +Notes +===== +- serialize post build steps, like computing test results, since they depend on the previous result. + we need some way to mark the build steps that need this service, in a way that preserves compatibility. + -> added check pointing + +- serialize the changelog calculation. How do we handle backward compatibility? + -> it turns out that CVSSCM doesn't need checkpointing for this, because it uses a timestamp. + +- think about the implications of this change to polling + -> introduced WorkspaceList to control the lock. + +- figure out what to do with AbstractProject.getWorkspaceResource(). I just removed it from + AbstractProject.getResourceList() but this will break the lock mechanism with SCM polling, tagging, and + other related activities that require a workspace. + -> the way polling is now done should be followed by everyone else who needs to lock the workspace. + +- how to preserve the compatibility with earlier plugins? + (in a way that doesn't carry the burdern forever --- marker interface, abstract method?) + + it should force the build step to wait until the previous build completes. + + -> new method on the interface, which maintains binary compatibility but not source compatibilit. diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java index d738fb05f5..3d16649f10 100644 --- a/core/src/main/java/hudson/FilePath.java +++ b/core/src/main/java/hudson/FilePath.java @@ -784,6 +784,20 @@ public final class FilePath implements Serializable { return r.substring(len+1); } + /** + * Short for {@code getParent().child(rel)}. Useful for getting other files in the same directory. + */ + public FilePath sibling(String rel) { + return getParent().child(rel); + } + + /** + * Returns a {@link FilePath} by adding the given suffix to this path name. + */ + public FilePath withSuffix(String suffix) { + return new FilePath(channel,remote+suffix); + } + /** * The same as {@link FilePath#FilePath(FilePath,String)} but more OO. * @param rel a relative or absolute path diff --git a/core/src/main/java/hudson/FileSystemProvisioner.java b/core/src/main/java/hudson/FileSystemProvisioner.java index e37f81acd1..04b8fcc609 100644 --- a/core/src/main/java/hudson/FileSystemProvisioner.java +++ b/core/src/main/java/hudson/FileSystemProvisioner.java @@ -31,6 +31,7 @@ import hudson.model.Describable; import hudson.model.Job; import hudson.model.TaskListener; import hudson.model.Hudson; +import hudson.model.Executor; import hudson.model.listeners.RunListener; import hudson.scm.SCM; @@ -39,6 +40,7 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.OutputStream; +import java.io.Closeable; /** * Prepares and provisions workspaces for {@link AbstractProject}s. diff --git a/core/src/main/java/hudson/matrix/MatrixConfiguration.java b/core/src/main/java/hudson/matrix/MatrixConfiguration.java index 592c62ab25..9477c1ea83 100644 --- a/core/src/main/java/hudson/matrix/MatrixConfiguration.java +++ b/core/src/main/java/hudson/matrix/MatrixConfiguration.java @@ -23,7 +23,6 @@ */ package hudson.matrix; -import hudson.FilePath; import hudson.util.DescribableList; import hudson.model.AbstractBuild; import hudson.model.Cause; @@ -35,7 +34,6 @@ import hudson.model.Item; import hudson.model.ItemGroup; import hudson.model.JDK; import hudson.model.Label; -import hudson.model.Node; import hudson.model.ParametersAction; import hudson.model.Project; import hudson.model.SCMedItem; @@ -141,18 +139,6 @@ public class MatrixConfiguration extends Project return combination; } - @Override - public FilePath getWorkspace() { - Node node = getLastBuiltOn(); - if(node==null) node = Hudson.getInstance(); - FilePath ws = node.getWorkspaceFor(getParent()); - if(ws==null) return null; - if(useShortWorkspaceName) - return ws.child(digestName); - else - return ws.child(getCombination().toString('/','/')); - } - /** * Since {@link MatrixConfiguration} is always invoked from {@link MatrixRun} * once and just once, there's no point in having a quiet period. @@ -262,7 +248,11 @@ public class MatrixConfiguration extends Project public SCM getScm() { return getParent().getScm(); } - + + /*package*/ String getDigestName() { + return digestName; + } + /** * JDK cannot be set on {@link MatrixConfiguration} because * it's controlled by {@link MatrixProject}. diff --git a/core/src/main/java/hudson/matrix/MatrixProject.java b/core/src/main/java/hudson/matrix/MatrixProject.java index 4bb975a442..d26c821942 100644 --- a/core/src/main/java/hudson/matrix/MatrixProject.java +++ b/core/src/main/java/hudson/matrix/MatrixProject.java @@ -472,13 +472,6 @@ public class MatrixProject extends AbstractProject im return null; } - @Override - public FilePath getWorkspace() { - Node node = getLastBuiltOn(); - if(node==null) node = getParent(); - return node.getWorkspaceFor(this); - } - protected Class getBuildClass() { return MatrixBuild.class; } diff --git a/core/src/main/java/hudson/matrix/MatrixRun.java b/core/src/main/java/hudson/matrix/MatrixRun.java index 1f8e91a810..d3561e9256 100644 --- a/core/src/main/java/hudson/matrix/MatrixRun.java +++ b/core/src/main/java/hudson/matrix/MatrixRun.java @@ -23,18 +23,20 @@ */ package hudson.matrix; +import hudson.FilePath; +import hudson.slaves.WorkspaceList; +import static hudson.matrix.MatrixConfiguration.useShortWorkspaceName; import hudson.model.Build; +import hudson.model.Node; +import org.kohsuke.stapler.Ancestor; +import org.kohsuke.stapler.Stapler; +import org.kohsuke.stapler.StaplerRequest; import java.io.File; import java.io.IOException; -import java.util.Map; -import java.util.HashMap; -import java.util.List; import java.util.Calendar; - -import org.kohsuke.stapler.StaplerRequest; -import org.kohsuke.stapler.Stapler; -import org.kohsuke.stapler.Ancestor; +import java.util.List; +import java.util.Map; /** * Execution of {@link MatrixConfiguration}. @@ -119,4 +121,21 @@ public class MatrixRun extends Build { public MatrixConfiguration getParent() {// don't know why, but javac wants this return super.getParent(); } + + @Override + public void run() { + run(new RunnerImpl()); + } + + protected class RunnerImpl extends Build.RunnerImpl { + @Override + protected FilePath decideWorkspace(Node n, WorkspaceList wsl) { + Node node = getBuiltOn(); + FilePath ws = node.getWorkspaceFor(getParent().getParent()); + if(useShortWorkspaceName) + return wsl.allocate(ws.child(getParent().getDigestName())); + else + return wsl.allocate(ws.child(getParent().getCombination().toString('/','/'))); + } + } } diff --git a/core/src/main/java/hudson/model/AbstractBuild.java b/core/src/main/java/hudson/model/AbstractBuild.java index 63adc697f4..3808b13738 100644 --- a/core/src/main/java/hudson/model/AbstractBuild.java +++ b/core/src/main/java/hudson/model/AbstractBuild.java @@ -27,6 +27,8 @@ import hudson.EnvVars; import hudson.Functions; import hudson.Launcher; import hudson.Util; +import hudson.FilePath; +import hudson.slaves.WorkspaceList; import hudson.matrix.MatrixConfiguration; import hudson.model.Fingerprint.BuildPtr; import hudson.model.Fingerprint.RangeSet; @@ -42,6 +44,7 @@ import hudson.tasks.BuildWrapper; import hudson.tasks.Builder; import hudson.tasks.Fingerprinter.FingerprintAction; import hudson.tasks.Publisher; +import hudson.tasks.BuildStepMonitor; import hudson.tasks.test.AbstractTestResultAction; import hudson.util.AdaptedIterator; import hudson.util.Iterators; @@ -83,6 +86,12 @@ public abstract class AbstractBuild

,R extends Abs */ private String builtOn; + /** + * The file path on the node that performed a build. Kept as a string since {@link FilePath} is not serializable into XML. + * @since 1.XXX + */ + private String workspace; + /** * Version of Hudson that built this. */ @@ -174,6 +183,51 @@ public abstract class AbstractBuild

,R extends Abs return Functions.getNearestAncestorUrl(Stapler.getCurrentRequest(),getParent())+'/'; } + /** + * Gets the directory where this build is being built. + * + *

+ * Note to implementors: to control where the workspace is created, override + * {@link AbstractRunner#decideWorkspace(Node,WorkspaceList)}. + * + * @return + * null if the workspace is on a slave that's not connected. Note that once the build is completed, + * the workspace may be used to build something else, so the value returned from this method may + * no longer show a workspace as it was used for this build. + * @since 1.XXX + */ + public final FilePath getWorkspace() { + if(workspace==null) return null; + Node n = getBuiltOn(); + if(n==null) return null; + return n.createPath(workspace); + } + + /** + * Returns the root directory of the checked-out module. + *

+ * This is usually where pom.xml, build.xml + * and so on exists. + */ + public final FilePath getModuleRoot() { + FilePath ws = getWorkspace(); + if(ws==null) return null; + return getParent().getScm().getModuleRoot(ws); + } + + /** + * Returns the root directories of all checked-out modules. + *

+ * Some SCMs support checking out multiple modules into the same workspace. + * In these cases, the returned array will have a length greater than one. + * @return The roots of all modules checked out from the SCM. + */ + public FilePath[] getModuleRoots() { + FilePath ws = getWorkspace(); + if(ws==null) return null; + return getParent().getScm().getModuleRoots(ws); + } + /** * List of users who committed a change since the last non-broken build till now. * @@ -189,12 +243,15 @@ public abstract class AbstractBuild

,R extends Abs if(culprits==null) { Set r = new HashSet(); R p = getPreviousCompletedBuild(); - if(p !=null && isBuilding() && p.getResult().isWorseThan(Result.UNSTABLE)) { - // we are still building, so this is just the current latest information, - // but we seems to be failing so far, so inherit culprits from the previous build. - // isBuilding() check is to avoid recursion when loading data from old Hudson, which doesn't record - // this information - r.addAll(p.getCulprits()); + if(p !=null && isBuilding()) { + Result pr = p.getResult(); + if(pr!=null && pr.isWorseThan(Result.UNSTABLE)) { + // we are still building, so this is just the current latest information, + // but we seems to be failing so far, so inherit culprits from the previous build. + // isBuilding() check is to avoid recursion when loading data from old Hudson, which doesn't record + // this information + r.addAll(p.getCulprits()); + } } for( Entry e : getChangeSet() ) r.add(e.getAuthor()); @@ -237,7 +294,7 @@ public abstract class AbstractBuild

,R extends Abs return hudsonVersion; } - protected abstract class AbstractRunner implements Runner { + protected abstract class AbstractRunner extends Runner { /** * Since configuration can be changed while a build is in progress, * create a launcher once and stick to it for the entire build duration. @@ -251,6 +308,19 @@ public abstract class AbstractBuild

,R extends Abs return Executor.currentExecutor().getOwner().getNode(); } + /** + * Allocates the workspace from {@link WorkspaceList}. + * + * @param n + * Passed in for the convenience. The node where the build is running. + * @param wsl + * Passed in for the convenience. The returned path must be registered to this object. + */ + protected FilePath decideWorkspace(Node n, WorkspaceList wsl) { + // TODO: this cast is indicative of abstraction problem + return wsl.allocate(n.getWorkspaceFor((TopLevelItem)getProject())); + } + public Result run(BuildListener listener) throws Exception { Node node = getCurrentNode(); assert builtOn==null; @@ -261,32 +331,39 @@ public abstract class AbstractBuild

,R extends Abs if(!Hudson.getInstance().getNodes().isEmpty()) listener.getLogger().println(node instanceof Hudson ? Messages.AbstractBuild_BuildingOnMaster() : Messages.AbstractBuild_BuildingRemotely(builtOn)); - node.getFileSystemProvisioner().prepareWorkspace(AbstractBuild.this,project.getWorkspace(),listener); + final FilePath ws = decideWorkspace(node,Computer.currentComputer().getWorkspaceList()); - checkout(listener); + try { + workspace = ws.getRemote(); + node.getFileSystemProvisioner().prepareWorkspace(AbstractBuild.this,ws,listener); - if(!preBuild(listener,project.getProperties())) - return Result.FAILURE; + checkout(listener); - Result result = doRun(listener); + if(!preBuild(listener,project.getProperties())) + return Result.FAILURE; - // kill run-away processes that are left - // use multiple environment variables so that people can escape this massacre by overriding an environment - // variable for some processes - launcher.kill(getCharacteristicEnvVars()); + Result result = doRun(listener); - // this is ugly, but for historical reason, if non-null value is returned - // it should become the final result. - if(result==null) result = getResult(); - if(result==null) result = Result.SUCCESS; + // kill run-away processes that are left + // use multiple environment variables so that people can escape this massacre by overriding an environment + // variable for some processes + launcher.kill(getCharacteristicEnvVars()); - if(result.isBetterOrEqualTo(Result.UNSTABLE)) - createSymLink(listener,"lastSuccessful"); + // this is ugly, but for historical reason, if non-null value is returned + // it should become the final result. + if(result==null) result = getResult(); + if(result==null) result = Result.SUCCESS; - if(result.isBetterOrEqualTo(Result.SUCCESS)) - createSymLink(listener,"lastStable"); + if(result.isBetterOrEqualTo(Result.UNSTABLE)) + createSymLink(listener,"lastSuccessful"); - return result; + if(result.isBetterOrEqualTo(Result.SUCCESS)) + createSymLink(listener,"lastStable"); + + return result; + } finally { + Computer.currentComputer().getWorkspaceList().release(ws); + } } /** @@ -356,6 +433,7 @@ public abstract class AbstractBuild

,R extends Abs for (User u : getCulprits()) r.add(u.getId()); culprits = r; + CheckPoint.CULPRITS_DETERMINED.report(); } } @@ -376,10 +454,23 @@ public abstract class AbstractBuild

,R extends Abs protected final void performAllBuildStep(BuildListener listener, Iterable buildSteps, boolean phase) throws InterruptedException, IOException { for( BuildStep bs : buildSteps ) { if( (bs instanceof Publisher && ((Publisher)bs).needsToRunAfterFinalized()) ^ phase) - bs.perform(AbstractBuild.this, launcher, listener); + perform(bs,listener); } } + /** + * Calls a build step. + */ + protected final boolean perform(BuildStep bs, BuildListener listener) throws InterruptedException, IOException { + BuildStepMonitor mon; + try { + mon = bs.getRequiredMonitorService(); + } catch (AbstractMethodError e) { + mon = BuildStepMonitor.BUILD; + } + return mon.perform(bs, AbstractBuild.this, launcher, listener); + } + protected final boolean preBuild(BuildListener listener,Map steps) { return preBuild(listener,steps.values()); } @@ -445,7 +536,7 @@ public abstract class AbstractBuild

,R extends Abs @Override public EnvVars getEnvironment(TaskListener log) throws IOException, InterruptedException { EnvVars env = super.getEnvironment(log); - env.put("WORKSPACE", getProject().getWorkspace().getRemote()); + env.put("WORKSPACE", getWorkspace().getRemote()); // servlet container may have set CLASSPATH in its launch script, // so don't let that inherit to the new child process. // see http://www.nabble.com/Run-Job-with-JDK-1.4.2-tf4468601.html diff --git a/core/src/main/java/hudson/model/AbstractProject.java b/core/src/main/java/hudson/model/AbstractProject.java index 9ac065a319..b66341ffb1 100644 --- a/core/src/main/java/hudson/model/AbstractProject.java +++ b/core/src/main/java/hudson/model/AbstractProject.java @@ -28,6 +28,7 @@ import hudson.FeedAdapter; import hudson.FilePath; import hudson.Launcher; import hudson.Util; +import hudson.slaves.WorkspaceList; import hudson.model.Cause.LegacyCodeCause; import hudson.model.Cause.UserCause; import hudson.model.Cause.RemoteCause; @@ -35,6 +36,7 @@ import hudson.model.Descriptor.FormException; import hudson.model.Fingerprint.RangeSet; import hudson.model.RunMap.Constructor; import hudson.model.Queue.WaitingItem; +import hudson.model.Queue.Executable; import hudson.scm.ChangeLogSet; import hudson.scm.ChangeLogSet.Entry; import hudson.scm.NullSCM; @@ -178,6 +180,8 @@ public abstract class AbstractProject

,R extends A */ protected transient /*final*/ List transientActions = new Vector(); + private boolean concurrentBuild; + protected AbstractProject(ItemGroup parent, String name) { super(parent,name); @@ -225,6 +229,19 @@ public abstract class AbstractProject

,R extends A super.performDelete(); } + /** + * Does this project perform concurrent builds? + * @since 1.XXX + */ + public boolean isConcurrentBuild() { + return Hudson.CONCURRENT_BUILD && concurrentBuild; + } + + public void setConcurrentBuild(boolean b) throws IOException { + concurrentBuild = b; + save(); + } + /** * If this project is configured to be always built on this node, * return that {@link Node}. Otherwise null. @@ -280,14 +297,60 @@ public abstract class AbstractProject

,R extends A * * @return * null if the workspace is on a slave that's not connected. + * @deprecated as of 1.XXX + * To support concurrent builds of the same project, this method is moved to {@link AbstractBuild}. + * For backward compatibility, this method returns the right {@link AbstractBuild#getWorkspace()} if called + * from {@link Executor}, and otherwise the workspace of the last build. + * + *

+ * If your are calling this method during a build from an executor, swtich it to {@link AbstractBuild#getWorkspace()}. + * If you are calling this method to serve a file from the workspace, doing a form validation, etc., then + * use {@link #getSomeWorkspace()} */ - public abstract FilePath getWorkspace(); + public final FilePath getWorkspace() { + Executor e = Executor.currentExecutor(); + if(e!=null) { + Executable exe = e.getCurrentExecutable(); + if (exe instanceof AbstractBuild) { + AbstractBuild b = (AbstractBuild) exe; + if(b.getProject()==this) + return b.getWorkspace(); + } + } + R lb = getLastBuild(); + if(lb!=null) return lb.getWorkspace(); + return null; + } + + /** + * Gets a workspace for some build of this project. + * + *

+ * This is useful for obtaining a workspace for the purpose of form field validation, where exactly + * which build the workspace belonged is less important. The implementation makes a cursory effort + * to find some workspace. + * + * @return + * null if there's no available workspace. + * @since 1.XXX + */ + public final FilePath getSomeWorkspace() { + int cnt=0; + for (R b = getLastBuild(); cnt<5 && b!=null; b=b.getPreviousBuild()) { + FilePath ws = b.getWorkspace(); + if (ws!=null) return ws; + } + return null; + } /** * Returns the root directory of the checked-out module. *

* This is usually where pom.xml, build.xml * and so on exists. + * + * @deprecated as of 1.XXX + * See {@link #getWorkspace()} for a migration strategy. */ public FilePath getModuleRoot() { FilePath ws = getWorkspace(); @@ -301,6 +364,9 @@ public abstract class AbstractProject

,R extends A * Some SCMs support checking out multiple modules into the same workspace. * In these cases, the returned array will have a length greater than one. * @return The roots of all modules checked out from the SCM. + * + * @deprecated as of 1.XXX + * See {@link #getWorkspace()} for a migration strategy. */ public FilePath[] getModuleRoots() { return getScm().getModuleRoots(getWorkspace()); @@ -745,7 +811,7 @@ public abstract class AbstractProject

,R extends A * but derived classes can also check other conditions. */ public boolean isBuildBlocked() { - return isBuilding(); + return isBuilding() && !isConcurrentBuild(); } public String getWhyBlocked() { @@ -784,6 +850,15 @@ public abstract class AbstractProject

,R extends A /** * Gets the {@link Resource} that represents the workspace of this project. * Useful for locking and mutual exclusion control. + * + * @deprecated as of 1.XXX + * Projects no longer have a fixed workspace, ands builds will find an available workspace via + * {@link WorkspaceList} for each build (furthermore, that happens after a build is started.) + * So a {@link Resource} representation for a workspace at the project level no longer makes sense. + * + *

+ * If you need to lock a workspace while you do some computation, see the source code of + * {@link #pollSCMChanges(TaskListener)} for how to obtain a lock of a workspace through {@link WorkspaceList}. */ public Resource getWorkspaceResource() { return new Resource(getFullDisplayName()+" workspace"); @@ -801,7 +876,6 @@ public abstract class AbstractProject

,R extends A resourceLists.add(activity.getResourceList()); } } - resourceLists.add(new ResourceList().w(getWorkspaceResource())); return ResourceList.union(resourceLists); } @@ -818,26 +892,14 @@ public abstract class AbstractProject

,R extends A if(scm==null) return true; // no SCM - // Acquire lock for SCMTrigger so poll won't run while we checkout/update - SCMTrigger scmt = getTrigger(SCMTrigger.class); - boolean locked = false; try { - if (scmt!=null) { - scmt.getLock().lockInterruptibly(); - locked = true; - } - - FilePath workspace = getWorkspace(); + FilePath workspace = build.getWorkspace(); workspace.mkdirs(); - return scm.checkout(build, launcher, workspace, listener, changelogFile); } catch (InterruptedException e) { listener.getLogger().println(Messages.AbstractProject_ScmAborted()); LOGGER.log(Level.INFO,build.toString()+" aborted",e); return false; - } finally { - if (locked) - scmt.getLock().unlock(); } } @@ -860,27 +922,42 @@ public abstract class AbstractProject

,R extends A } try { - FilePath workspace = getWorkspace(); - if (scm.requiresWorkspaceForPolling() && (workspace == null || !workspace.exists())) { - // workspace offline. build now, or nothing will ever be built - Label label = getAssignedLabel(); - if (label != null && label.isSelfLabel()) { - // if the build is fixed on a node, then attempting a build will do us - // no good. We should just wait for the slave to come back. - listener.getLogger().println(Messages.AbstractProject_NoWorkspace()); - return false; - } - if (workspace == null) - listener.getLogger().println(Messages.AbstractProject_WorkspaceOffline()); - else - listener.getLogger().println(Messages.AbstractProject_NoWorkspace()); - listener.getLogger().println(Messages.AbstractProject_NewBuildForWorkspace()); - return true; - } + if(scm.requiresWorkspaceForPolling()) { + // lock the workspace of the last build + FilePath ws=null; + R lb = getLastBuild(); + if (lb!=null) ws = lb.getWorkspace(); - Launcher launcher = workspace != null ? workspace.createLauncher(listener) : null; - LOGGER.fine("Polling SCM changes of " + getName()); - return scm.pollChanges(this, launcher, workspace, listener); + if (ws==null || !ws.exists()) { + // workspace offline. build now, or nothing will ever be built + Label label = getAssignedLabel(); + if (label != null && label.isSelfLabel()) { + // if the build is fixed on a node, then attempting a build will do us + // no good. We should just wait for the slave to come back. + listener.getLogger().println(Messages.AbstractProject_NoWorkspace()); + return false; + } + if (ws == null) + listener.getLogger().println(Messages.AbstractProject_WorkspaceOffline()); + else + listener.getLogger().println(Messages.AbstractProject_NoWorkspace()); + listener.getLogger().println(Messages.AbstractProject_NewBuildForWorkspace()); + return true; + } else { + WorkspaceList l = lb.getBuiltOn().toComputer().getWorkspaceList(); + l.acquire(ws); + try { + LOGGER.fine("Polling SCM changes of " + getName()); + return scm.pollChanges(this, ws.createLauncher(listener), ws, listener); + } finally { + l.release(ws); + } + } + } else { + // polling without workspace + LOGGER.fine("Polling SCM changes of " + getName()); + return scm.pollChanges(this, null, null, listener); + } } catch (AbortException e) { listener.fatalError(Messages.AbstractProject_Aborted()); return false; @@ -1197,6 +1274,8 @@ public abstract class AbstractProject

,R extends A assignedNode = null; } + setConcurrentBuild(req.getSubmittedForm().has("concurrentBuild")); + authToken = BuildAuthorizationToken.create(req); setScm(SCMS.parseSCM(req,this)); @@ -1238,7 +1317,7 @@ public abstract class AbstractProject

,R extends A */ public void doWs( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException, InterruptedException { checkPermission(AbstractProject.WORKSPACE); - FilePath ws = getWorkspace(); + FilePath ws = getSomeWorkspace(); if ((ws == null) || (!ws.exists())) { // if there's no workspace, report a nice error message // Would be good if when asked for *plain*, do something else! diff --git a/core/src/main/java/hudson/model/Build.java b/core/src/main/java/hudson/model/Build.java index 5ee349c63c..02ab4ffedd 100644 --- a/core/src/main/java/hudson/model/Build.java +++ b/core/src/main/java/hudson/model/Build.java @@ -29,7 +29,6 @@ import hudson.tasks.BuildStep; import hudson.tasks.BuildTrigger; import hudson.tasks.BuildWrapper; import hudson.tasks.Builder; -import hudson.triggers.SCMTrigger; import java.io.File; import java.io.IOException; @@ -37,7 +36,6 @@ import java.util.ArrayList; import java.util.Calendar; import java.util.Collection; import java.util.List; -import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Logger; /** @@ -48,11 +46,6 @@ import java.util.logging.Logger; public abstract class Build

,B extends Build> extends AbstractBuild { - /** - * If the build required a lock, remember it so that we can release it. - */ - private transient ReentrantLock buildLock; - /** * Creates a new build. */ @@ -71,37 +64,6 @@ public abstract class Build

,B extends Build> super(project,buildDir); } - @Override - protected void onStartBuilding() { - super.onStartBuilding(); - SCMTrigger t = (SCMTrigger)project.getTriggers().get(Hudson.getInstance().getDescriptorByType(SCMTrigger.DescriptorImpl.class)); - if(t!=null) { - // acquire the lock - buildLock = t.getLock(); - synchronized(buildLock) { - try { - if(buildLock.isLocked()) { - long time = System.currentTimeMillis(); - LOGGER.info("Waiting for the polling of "+getParent()+" to complete"); - buildLock.lockInterruptibly(); - LOGGER.info("Polling completed. Waited "+(System.currentTimeMillis()-time)+"ms"); - } else - buildLock.lockInterruptibly(); - } catch (InterruptedException e) { - // handle the interrupt later - Thread.currentThread().interrupt(); - } - } - } - } - - @Override - protected void onEndBuilding() { - super.onEndBuilding(); - if(buildLock!=null) - buildLock.unlock(); - } - // // // actions @@ -192,7 +154,7 @@ public abstract class Build

,B extends Build> private boolean build(BuildListener listener, Collection steps) throws IOException, InterruptedException { for( BuildStep bs : steps ) - if(!bs.perform(Build.this, launcher, listener)) + if(!perform(bs,listener)) return false; return true; } diff --git a/core/src/main/java/hudson/model/CheckPoint.java b/core/src/main/java/hudson/model/CheckPoint.java new file mode 100644 index 0000000000..9e8b98e980 --- /dev/null +++ b/core/src/main/java/hudson/model/CheckPoint.java @@ -0,0 +1,162 @@ +/* + * The MIT License + * + * Copyright (c) 2004-2009, Sun Microsystems, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +package hudson.model; + +import hudson.tasks.BuildStep; +import hudson.tasks.Recorder; +import hudson.tasks.Builder; +import hudson.tasks.junit.JUnitResultArchiver; +import hudson.scm.SCM; + +/** + * Provides a mechanism for synchronizing build executions in the face of concurrent builds. + * + *

+ * At certain points of a build, {@link BuildStep}s and other extension points often need + * to refer to what happened in its earlier build. For example, a {@link SCM} check out + * can run concurrently, but the changelog computation requires that the check out of the + * earlier build has completed. Or if Hudson is sending out an e-mail, he needs to know + * the result of the previous build, so that he can decide an e-mail is necessary or not. + * + *

+ * Check pointing is a primitive mechanism to provide this sort of synchronization. + * These methods can be only invoked from {@link Executor} threads. + * + *

+ * Each {@link CheckPoint} instance represents unique check points. {@link CheckPoint} + * instances are normally created as a static instance, because two builds of the same project + * needs to refer to the same check point instance for synchronization to happen properly. + * + *

+ * This class defines a few well-known check point instances. plugins can define + * their additional check points for their own use. + * + *

Example

+ *

+ * {@link JUnitResultArchiver} provides a good example of how a {@link Recorder} can + * depend on its earlier result. + * + * @author Kohsuke Kawaguchi + * @see BuildStep#getRequiredMonitorService() + * @since 1.XXX + */ +public final class CheckPoint { + private final Object identity; + private final String internalName; + + /** + * For advanced uses. Creates a check point that uses the given object as its identity. + */ + public CheckPoint(String internalName, Object identity) { + this.internalName = internalName; + this.identity = identity; + } + + /** + * @param internalName + * Name of this check point that's used in the logging, stack traces, debug messages, and so on. + * This is not displayed to users. No need for i18n. + */ + public CheckPoint(String internalName) { + this(internalName, new Object()); + } + + @Override + public boolean equals(Object that) { + if (that == null || getClass() != that.getClass()) return false; + return identity== ((CheckPoint) that).identity; + } + + @Override + public int hashCode() { + return identity.hashCode(); + } + + @Override + public String toString() { + return "Check point "+internalName; + } + + /** + * Records that the execution of the build has reached to a check point, idenified + * by the given identifier. + * + *

+ * If the successive builds are {@linkplain #block() waiting for this check point}, + * they'll be released. + * + *

+ * This method can be only called from an {@link Executor} thread. + */ + public void report() { + Run.reportCheckpoint(this); + } + + /** + * Waits until the previous build in progress reaches a check point, identified + * by the given identifier, or until the current executor becomes the youngest build in progress. + * + *

+ * Note that "previous build in progress" should be interpreted as "previous (build in progress)" instead of + * "(previous build) if it's in progress". This makes a difference around builds that are aborted or + * failed very early without reporting the check points. Imagine the following time sequence: + * + *

    + *
  1. Build #1, #2, and #3 happens around the same time + *
  2. Build #3 waits for check point {@link JUnitResultArchiver} + *
  3. Build #2 aborts before getting to that check point + *
  4. Build #1 finally checks in {@link JUnitResultArchiver} + *
+ * + *

+ * Using this method, build #3 correctly waits until the step 4. Because of this behavior, + * the {@link #report()}/{@link #block()} pair can normally + * be used without a try/finally block. + * + *

+ * This method can be only called from an {@link Executor} thread. + * + * @throws InterruptedException + * If the build (represented by the calling executor thread) is aborted while it's waiting. + */ + public void block() throws InterruptedException { + Run.waitForCheckpoint(this); + } + + /** + * {@link CheckPoint} that indicates that {@link AbstractBuild#getCulprits()} is computed. + */ + public static final CheckPoint CULPRITS_DETERMINED = new CheckPoint("CULPRITS_DETERMINED"); + /** + * {@link CheckPoint} that indicates that the build is completed. + * ({@link AbstractBuild#isBuilding()}==false) + */ + public static final CheckPoint COMPLETED = new CheckPoint("COMPLETED"); + /** + * {@link CheckPoint} that indicates that the build has finished executing the "main" portion + * ({@link Builder}s in case of {@link FreeStyleProject}) and now moving on to the post-build + * steps. + */ + public static final CheckPoint MAIN_COMPLETED = new CheckPoint("MAIN_COMPLETED"); +} diff --git a/core/src/main/java/hudson/model/Computer.java b/core/src/main/java/hudson/model/Computer.java index 0c8a69c0e7..81e5f09122 100644 --- a/core/src/main/java/hudson/model/Computer.java +++ b/core/src/main/java/hudson/model/Computer.java @@ -25,6 +25,7 @@ package hudson.model; import hudson.EnvVars; import hudson.Util; +import hudson.FilePath; import hudson.model.Descriptor.FormException; import hudson.node_monitors.NodeMonitor; import hudson.remoting.Channel; @@ -36,6 +37,7 @@ import hudson.security.Permission; import hudson.security.PermissionGroup; import hudson.slaves.ComputerLauncher; import hudson.slaves.RetentionStrategy; +import hudson.slaves.WorkspaceList; import hudson.tasks.BuildWrapper; import hudson.tasks.Publisher; import hudson.util.DaemonThreadFactory; @@ -58,6 +60,9 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Enumeration; +import java.util.Set; +import java.util.HashSet; +import java.util.Collections; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -122,6 +127,8 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces private volatile String cachedHostName; private volatile boolean hostNameCached; + private final WorkspaceList workspaceList = new WorkspaceList(); + public Computer(Node node) { assert node.getNumExecutors()!=0 : "Computer created with 0 executors"; setNode(node); @@ -134,6 +141,13 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces return new File(Hudson.getInstance().getRootDir(),"slave-"+nodeName+".log"); } + /** + * Gets the object that coordinates the workspace allocation on this computer. + */ + public WorkspaceList getWorkspaceList() { + return workspaceList; + } + /** * Gets the string representation of the slave log. */ diff --git a/core/src/main/java/hudson/model/Executor.java b/core/src/main/java/hudson/model/Executor.java index 656d802797..517324c9a1 100644 --- a/core/src/main/java/hudson/model/Executor.java +++ b/core/src/main/java/hudson/model/Executor.java @@ -25,7 +25,9 @@ package hudson.model; import hudson.Util; import hudson.model.Queue.*; +import hudson.FilePath; import hudson.util.TimeUnit2; +import hudson.util.InterceptingProxy; import hudson.security.ACL; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; @@ -37,6 +39,7 @@ import javax.servlet.ServletException; import java.io.IOException; import java.util.logging.Logger; import java.util.logging.Level; +import java.lang.reflect.Method; /** @@ -104,6 +107,8 @@ public class Executor extends Thread implements ModelObject { Queue.Task task = queueItem.task; Throwable problems = null; owner.taskAccepted(this, task); + + final String threadName = getName(); try { try { startTime = System.currentTimeMillis(); @@ -113,6 +118,7 @@ public class Executor extends Thread implements ModelObject { ((Actionable) executable).addAction(action); } } + setName(threadName+" : executing "+executable.toString()); queue.execute(executable, task); } catch (Throwable e) { // for some reason the executor died. this is really @@ -122,6 +128,7 @@ public class Executor extends Thread implements ModelObject { problems = e; } } finally { + setName(threadName); finishTime = System.currentTimeMillis(); if (problems == null) { queueItem.future.set(executable); @@ -164,6 +171,21 @@ public class Executor extends Thread implements ModelObject { return executable; } + /** + * If {@linkplain #getCurrentExecutable() current executable} is {@link AbstractBuild}, + * return the workspace that this executor is using, or null if the build hasn't gotten + * to that point yet. + */ + public FilePath getCurrentWorkspace() { + Executable e = executable; + if(e==null) return null; + if (e instanceof AbstractBuild) { + AbstractBuild ab = (AbstractBuild) e; + return ab.getWorkspace(); + } + return null; + } + /** * Same as {@link #getName()}. */ @@ -340,14 +362,39 @@ public class Executor extends Thread implements ModelObject { return new Api(this); } + /** + * Creates a proxy object that executes the callee in the context that impersonates + * this executor. Useful to export an object to a remote channel. + */ + public T newImpersonatingProxy(Class type, T core) { + return new InterceptingProxy() { + protected Object call(Object o, Method m, Object[] args) throws Throwable { + final Executor old = IMPERSONATION.get(); + IMPERSONATION.set(Executor.this); + try { + return m.invoke(o,args); + } finally { + IMPERSONATION.set(old); + } + } + }.wrap(type,core); + } + /** * Returns the executor of the current thread or null if current thread is not an executor. */ public static Executor currentExecutor() { Thread t = Thread.currentThread(); - return t instanceof Executor ? (Executor)t : null; + if (t instanceof Executor) return (Executor) t; + return IMPERSONATION.get(); } + /** + * Mechanism to allow threads (in particular the channel request handling threads) to + * run on behalf of {@link Executor}. + */ + private static final ThreadLocal IMPERSONATION = new ThreadLocal(); + private static final Logger LOGGER = Logger.getLogger(Executor.class.getName()); } diff --git a/core/src/main/java/hudson/model/FileParameterValue.java b/core/src/main/java/hudson/model/FileParameterValue.java index f1a3adcb9e..49636aff8a 100644 --- a/core/src/main/java/hudson/model/FileParameterValue.java +++ b/core/src/main/java/hudson/model/FileParameterValue.java @@ -66,7 +66,7 @@ public class FileParameterValue extends ParameterValue { return new BuildWrapper() { public Environment setUp(AbstractBuild build, Launcher launcher, BuildListener listener) throws IOException, InterruptedException { listener.getLogger().println("Copying file to "+location); - build.getProject().getWorkspace().child(location).copyFrom(file); + build.getWorkspace().child(location).copyFrom(file); file = null; return new Environment() {}; } diff --git a/core/src/main/java/hudson/model/FreeStyleBuild.java b/core/src/main/java/hudson/model/FreeStyleBuild.java index 1fb3a97cff..03aa91922e 100644 --- a/core/src/main/java/hudson/model/FreeStyleBuild.java +++ b/core/src/main/java/hudson/model/FreeStyleBuild.java @@ -23,6 +23,9 @@ */ package hudson.model; +import hudson.FilePath; +import hudson.slaves.WorkspaceList; + import java.io.IOException; import java.io.File; @@ -37,4 +40,19 @@ public class FreeStyleBuild extends Build { public FreeStyleBuild(FreeStyleProject project, File buildDir) throws IOException { super(project, buildDir); } + + @Override + public void run() { + run(new RunnerImpl()); + } + + protected class RunnerImpl extends Build.RunnerImpl { + @Override + protected FilePath decideWorkspace(Node n, WorkspaceList wsl) { + String customWorkspace = getProject().getCustomWorkspace(); + if (customWorkspace != null) + return n.createPath(customWorkspace); + return super.decideWorkspace(n,wsl); + } + } } diff --git a/core/src/main/java/hudson/model/FreeStyleProject.java b/core/src/main/java/hudson/model/FreeStyleProject.java index 51e33d0108..8ace1c2d31 100644 --- a/core/src/main/java/hudson/model/FreeStyleProject.java +++ b/core/src/main/java/hudson/model/FreeStyleProject.java @@ -74,15 +74,6 @@ public class FreeStyleProject extends Project i return customWorkspace; } - @Override - public FilePath getWorkspace() { - Node node = getLastBuiltOn(); - if(node==null) node = getParent(); - if(customWorkspace!=null) - return node.createPath(customWorkspace); - return node.getWorkspaceFor(this); - } - protected void submit(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException, Descriptor.FormException { if(req.hasParameter("customWorkspace")) customWorkspace = req.getParameter("customWorkspace.directory"); diff --git a/core/src/main/java/hudson/model/Hudson.java b/core/src/main/java/hudson/model/Hudson.java index 294162259d..ae725d0d28 100644 --- a/core/src/main/java/hudson/model/Hudson.java +++ b/core/src/main/java/hudson/model/Hudson.java @@ -3380,6 +3380,14 @@ public final class Hudson extends Node implements ItemGroup, Stapl private static final boolean CONSISTENT_HASH = true; // Boolean.getBoolean(Hudson.class.getName()+".consistentHash"); public static boolean FLYWEIGHT_SUPPORT = Boolean.getBoolean(Hudson.class.getName()+".flyweightSupport"); + /** + * Tentative switch to activate the concurrent build behavior. + * When we merge this back to the trunk, this allows us to keep + * this feature hidden for a while until we iron out the kinks. + * @see AbstractProject#isConcurrentBuild() + */ + public static boolean CONCURRENT_BUILD = true; + private static final Logger LOGGER = Logger.getLogger(Hudson.class.getName()); private static final Pattern ICON_SIZE = Pattern.compile("\\d+x\\d+"); diff --git a/core/src/main/java/hudson/model/JobProperty.java b/core/src/main/java/hudson/model/JobProperty.java index 771da21cda..0c696bd97e 100644 --- a/core/src/main/java/hudson/model/JobProperty.java +++ b/core/src/main/java/hudson/model/JobProperty.java @@ -29,6 +29,7 @@ import hudson.Plugin; import hudson.tasks.BuildStep; import hudson.tasks.Builder; import hudson.tasks.Publisher; +import hudson.tasks.BuildStepMonitor; import java.io.IOException; @@ -134,6 +135,14 @@ public abstract class JobProperty> implements Describable project) { return getJobAction((J)project); } diff --git a/core/src/main/java/hudson/model/Node.java b/core/src/main/java/hudson/model/Node.java index 3b0890eaaf..236380fd55 100644 --- a/core/src/main/java/hudson/model/Node.java +++ b/core/src/main/java/hudson/model/Node.java @@ -173,6 +173,7 @@ public abstract class Node extends AbstractModelObject implements Describable,RunT extends Run,RunT extends Run ID_FORMATTER = new ThreadLocal() { @Override @@ -224,6 +240,7 @@ public abstract class Run ,RunT extends Run,RunT extends Run,RunT extends Run + * We basically follow the existing skip list, and wherever we find a non-optimal pointer, we remember them + * in 'fixUp' and update them later. + */ + public final RunT getPreviousBuildInProgress() { + if(previousBuildInProgress==this) return null; // the most common case + + List fixUp = new ArrayList(); + RunT r = _this(); // 'r' is the source of the pointer (so that we can add it to fix up if we find that the target of the pointer is inefficient.) + RunT answer; + while (true) { + RunT n = r.previousBuildInProgress; + if (n==null) {// no field computed yet. + n=r.getPreviousBuild(); + fixUp.add(r); + } + if (r==n || n==null) { + // this indicates that we know there's no build in progress beyond this point + answer = null; + break; + } + if (n.isBuilding()) { + // we now know 'n' is the target we wanted + answer = n; + break; + } + + fixUp.add(r); // r contains the stale 'previousBuildInProgress' back pointer + r = n; + } + + // fix up so that the next look up will run faster + for (RunT f : fixUp) + f.previousBuildInProgress = answer==null ? f : answer; + return answer; + } + /** * Returns the last build that didn't fail before this build. */ @@ -876,14 +942,84 @@ public abstract class Run ,RunT extends Run checkpoints = new HashSet(); + + private boolean allDone; + + protected synchronized void report(CheckPoint identifier) { + checkpoints.add(identifier); + notifyAll(); + } + + protected synchronized boolean waitForCheckPoint(CheckPoint identifier) throws InterruptedException { + final Thread t = Thread.currentThread(); + final String oldName = t.getName(); + t.setName(oldName+" : waiting for "+identifier+" on "+getFullDisplayName()); + try { + while(!allDone && !checkpoints.contains(identifier)) + wait(); + return checkpoints.contains(identifier); + } finally { + t.setName(oldName); + } + } + + /** + * Notifies that the build is fully completed and all the checkpoint locks be released. + */ + private synchronized void allDone() { + allDone = true; + notifyAll(); + } + } + + private final CheckpointSet checkpoints = new CheckpointSet(); + /** * Performs the main build and returns the status code. * * @throws Exception * exception will be recorded and the build will be considered a failure. */ - Result run( BuildListener listener ) throws Exception, RunnerAbortedException; + public abstract Result run( BuildListener listener ) throws Exception, RunnerAbortedException; /** * Performs the post-build action. @@ -895,7 +1031,7 @@ public abstract class Run ,RunT extends Run,RunT extends Run,RunT extends Run,RunT extends Run,RunT extends Run,RunT extends Run> stack = new WeakHashMap>(); + + synchronized void push(Runner r) { + Executor e = Executor.currentExecutor(); + Stack s = stack.get(e); + if(s==null) stack.put(e,s=new Stack()); + s.push(r); + } + + synchronized void pop() { + Executor e = Executor.currentExecutor(); + Stack s = stack.get(e); + s.pop(); + if(s.isEmpty()) stack.remove(e); + } + + synchronized Runner peek() { + return stack.get(Executor.currentExecutor()).peek(); + } + + static final RunnerStack INSTANCE = new RunnerStack(); +} diff --git a/core/src/main/java/hudson/scm/SCM.java b/core/src/main/java/hudson/scm/SCM.java index cee145b09b..64e8f21376 100644 --- a/core/src/main/java/hudson/scm/SCM.java +++ b/core/src/main/java/hudson/scm/SCM.java @@ -127,6 +127,12 @@ public abstract class SCM implements Describable, ExtensionPoint { * polling is configured, then that would immediately trigger a new build. * *

+ * This flag also affects the mutual exclusion control between builds and polling. + * If this methods returns false, polling will continu asynchronously even + * when a build is in progress, but otherwise the polling activity is blocked + * if a build is currently using a workspace. + * + *

* The default implementation returns true. * *

@@ -191,9 +197,11 @@ public abstract class SCM implements Describable, ExtensionPoint { * @param project * The project to check for updates * @param launcher - * Abstraction of the machine where the polling will take place. + * Abstraction of the machine where the polling will take place. If SCM declares + * that {@linkplain #requiresWorkspaceForPolling() the polling doesn't require a workspace}, this parameter is null. * @param workspace - * The workspace directory that contains baseline files. + * The workspace directory that contains baseline files. If SCM declares + * that {@linkplain #requiresWorkspaceForPolling() the polling doesn't require a workspace}, this parameter is null. * @param listener * Logs during the polling should be sent here. * @@ -340,6 +348,10 @@ public abstract class SCM implements Describable, ExtensionPoint { return (SCMDescriptor)Hudson.getInstance().getDescriptor(getClass()); } +// +// convenience methods +// + protected final boolean createEmptyChangeLog(File changelogFile, BuildListener listener, String rootTag) { try { FileWriter w = new FileWriter(changelogFile); diff --git a/core/src/main/java/hudson/slaves/WorkspaceList.java b/core/src/main/java/hudson/slaves/WorkspaceList.java new file mode 100644 index 0000000000..1f33cbf560 --- /dev/null +++ b/core/src/main/java/hudson/slaves/WorkspaceList.java @@ -0,0 +1,90 @@ +/* + * The MIT License + * + * Copyright (c) 2004-2009, Sun Microsystems, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +package hudson.slaves; + +import hudson.FilePath; +import hudson.model.Computer; + +import java.util.HashSet; +import java.util.Set; + +/** + * Used by {@link Computer} to keep track of workspaces that are actively in use. + * + *

+ * SUBJECT TO CHANGE! Do not use this from plugins directly. + * + * @author Kohsuke Kawaguchi + * @since 1.XXX + * @see Computer#getWorkspaceList() + */ +public final class WorkspaceList { + private final Set inUse = new HashSet(); + + public WorkspaceList() { + } + + /** + * Allocates some workspace by adding some variation to the given base if necessary. + */ + public synchronized FilePath allocate(FilePath base) { + for (int i=1; ; i++) { + FilePath candidate = i==1 ? base : base.withSuffix("@"+i); + if(inUse.contains(candidate)) + continue; + inUse.add(candidate); + return candidate; + } + } + + /** + * Just record that this workspace is being used, without paying any attention to the sycnhronization support. + */ + public synchronized FilePath record(FilePath p) { + inUse.add(p); + return p; + } + + /** + * Releases an allocated or acquired workspace. + */ + public synchronized void release(FilePath p) { + if (!inUse.remove(p)) + throw new AssertionError("Releasing unallocated workspace "+p); + notifyAll(); + } + + /** + * Acquires the given workspace. If necessary, this method blocks until it's made available. + * + * @return + * The same {@link FilePath} as given to this method. + */ + public synchronized FilePath acquire(FilePath p) throws InterruptedException { + while (inUse.contains(p)) + wait(); + inUse.add(p); + return p; + } +} diff --git a/core/src/main/java/hudson/tasks/Ant.java b/core/src/main/java/hudson/tasks/Ant.java index 85f2c78f8a..c2cbb15521 100644 --- a/core/src/main/java/hudson/tasks/Ant.java +++ b/core/src/main/java/hudson/tasks/Ant.java @@ -126,9 +126,6 @@ public class Ant extends Builder { } public boolean perform(AbstractBuild build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException { - AbstractProject proj = build.getProject(); - - ArgumentListBuilder args = new ArgumentListBuilder(); EnvVars env = build.getEnvironment(listener); @@ -152,7 +149,7 @@ public class Ant extends Builder { String buildFile = env.expand(this.buildFile); String targets = Util.replaceMacro(env.expand(this.targets), vr); - FilePath buildFilePath = buildFilePath(proj.getModuleRoot(), buildFile, targets); + FilePath buildFilePath = buildFilePath(build.getModuleRoot(), buildFile, targets); if(!buildFilePath.exists()) { // because of the poor choice of getModuleRoot() with CVS/Subversion, people often get confused @@ -161,7 +158,7 @@ public class Ant extends Builder { // and diagnosing it nicely. See HUDSON-1782 // first check if this appears to be a valid relative path from workspace root - FilePath buildFilePath2 = buildFilePath(proj.getWorkspace(), buildFile, targets); + FilePath buildFilePath2 = buildFilePath(build.getWorkspace(), buildFile, targets); if(buildFilePath2.exists()) { // This must be what the user meant. Let it continue. buildFilePath = buildFilePath2; diff --git a/core/src/main/java/hudson/tasks/ArtifactArchiver.java b/core/src/main/java/hudson/tasks/ArtifactArchiver.java index a55014974b..85c218a12b 100644 --- a/core/src/main/java/hudson/tasks/ArtifactArchiver.java +++ b/core/src/main/java/hudson/tasks/ArtifactArchiver.java @@ -85,8 +85,6 @@ public class ArtifactArchiver extends Recorder { } public boolean perform(AbstractBuild build, Launcher launcher, BuildListener listener) throws InterruptedException { - AbstractProject p = build.getProject(); - if(artifacts.length()==0) { listener.error(Messages.ArtifactArchiver_NoIncludes()); build.setResult(Result.FAILURE); @@ -98,7 +96,7 @@ public class ArtifactArchiver extends Recorder { listener.getLogger().println(Messages.ArtifactArchiver_ARCHIVING_ARTIFACTS()); try { - FilePath ws = p.getWorkspace(); + FilePath ws = build.getWorkspace(); if (ws==null) { // #3330: slave down? return true; } @@ -124,7 +122,6 @@ public class ArtifactArchiver extends Recorder { return true; } - public @Override boolean prebuild(AbstractBuild build, BuildListener listener) { if(latestOnly) { AbstractBuild b = build.getProject().getLastCompletedBuild(); @@ -150,6 +147,10 @@ public class ArtifactArchiver extends Recorder { return true; } + public BuildStepMonitor getRequiredMonitorService() { + return BuildStepMonitor.NONE; + } + /** * @deprecated as of 1.286 * Some plugin depends on this, so this field is left here and points to the last created instance. @@ -171,7 +172,7 @@ public class ArtifactArchiver extends Recorder { * Performs on-the-fly validation on the file mask wildcard. */ public FormValidation doCheckArtifacts(@AncestorInPath AbstractProject project, @QueryParameter String value) throws IOException { - return FilePath.validateFileMask(project.getWorkspace(),value); + return FilePath.validateFileMask(project.getSomeWorkspace(),value); } public ArtifactArchiver newInstance(StaplerRequest req, JSONObject formData) throws FormException { diff --git a/core/src/main/java/hudson/tasks/BuildStep.java b/core/src/main/java/hudson/tasks/BuildStep.java index c893759df2..73f7fd392c 100644 --- a/core/src/main/java/hudson/tasks/BuildStep.java +++ b/core/src/main/java/hudson/tasks/BuildStep.java @@ -35,7 +35,10 @@ import hudson.model.BuildListener; import hudson.model.Descriptor; import hudson.model.Project; import hudson.model.Hudson; +import hudson.model.CheckPoint; +import hudson.model.Run; +import javax.management.monitor.Monitor; import java.io.IOException; import java.util.List; import java.util.AbstractList; @@ -121,6 +124,71 @@ public interface BuildStep { */ Action getProjectAction(AbstractProject project); + + /** + * Declares the scope of the synchronization monitor this {@link BuildStep} expects from outside. + * + *

+ * This method is introduced for preserving compatibility with plugins written for earlier versions of Hudson, + * which never run multiple builds of the same job in parallel. Such plugins often assume that the outcome + * of the previous build is completely available, which is no longer true when we do concurrent builds. + * + *

+ * To minimize the necessary code change for such plugins, {@link BuildStep} implementations can request + * Hudson to externally perform synchronization before executing them. This behavior is as follows: + * + *

+ *
{@link BuildStepMonitor#BUILD} + *
+ * This {@link BuildStep} is only executed after the previous build is fully + * completed (thus fully restoring the earlier semantics of one build at a time.) + * + *
{@link BuildStepMonitor#STEP} + *
+ * This {@link BuildStep} is only executed after the same step in the previous build is completed. + * For build steps that use a weaker assumption and onl rely on the output from the same build step of + * the early builds, this improves the concurrency. + * + *
{@link BuildStepMonitor#NONE} + *
+ * No external synchronization is performed on this build step. This is the most efficient, and thus + * the recommended value for newer plugins. Wherever necessary, you can directly use {@link CheckPoint}s + * to perform necessary synchronizations. + *
+ * + *

Migrating Older Implementation

+ *

+ * If you are migrating {@link BuildStep} implementations written for earlier versions of Hudson, + * here's what you can do: + * + *

    + *
  • + * Just return {@link BuildStepMonitor#BUILD} to demand the backward compatible behavior from Hudson, + * and make no other changes to the code. This will prevent users from reaping the benefits of concurrent + * builds, but at least your plugin will work correctly, and therefore this is a good easy first step. + *
  • + * If your build step doesn't use anything from a previous build (for example, if you don't even call + * {@link Run#getPreviousBuild()}), then you can return {@link BuildStepMonitor#NONE} without making further + * code changes and you are done with migration. + *
  • + * If your build step only depends on {@link Action}s that you added in the previous build by yourself, + * then you only need {@link BuildStepMonitor#STEP} scope synchronization. Return it from this method + * ,and you are done with migration without any further code changes. + *
  • + * If your build step makes more complex assumptions, return {@link BuildStepMonitor#NONE} and use + * {@link CheckPoint}s directly in your code. The general idea is to call {@link CheckPoint#block()} before + * you try to access the state from the previous build. + *
+ * + *

Note to caller

+ *

+ * For plugins written against earlier versions of Hudson, calling this method results in + * {@link AbstractMethodError}. + * + * @since 1.XXX + */ + BuildStepMonitor getRequiredMonitorService(); + /** * List of all installed builders. * diff --git a/core/src/main/java/hudson/tasks/BuildStepCompatibilityLayer.java b/core/src/main/java/hudson/tasks/BuildStepCompatibilityLayer.java index a975a3b298..79cb8cfde3 100644 --- a/core/src/main/java/hudson/tasks/BuildStepCompatibilityLayer.java +++ b/core/src/main/java/hudson/tasks/BuildStepCompatibilityLayer.java @@ -29,6 +29,8 @@ import hudson.model.Action; import hudson.model.Project; import hudson.model.AbstractBuild; import hudson.model.AbstractProject; +import hudson.model.Run; +import hudson.model.CheckPoint; import hudson.Launcher; import java.io.IOException; diff --git a/core/src/main/java/hudson/tasks/BuildStepMonitor.java b/core/src/main/java/hudson/tasks/BuildStepMonitor.java new file mode 100644 index 0000000000..8834a1766b --- /dev/null +++ b/core/src/main/java/hudson/tasks/BuildStepMonitor.java @@ -0,0 +1,44 @@ +package hudson.tasks; + +import hudson.model.AbstractBuild; +import hudson.model.BuildListener; +import hudson.model.CheckPoint; +import hudson.Launcher; + +import java.io.IOException; + +/** + * Used by {@link BuildStep#getRequiredMonitorService()}. + * + * @author Kohsuke Kawaguchi + * @since 1.XXX + */ +public enum BuildStepMonitor { + NONE { + public boolean perform(BuildStep bs, AbstractBuild build, Launcher launcher, BuildListener listener) throws IOException, InterruptedException { + return bs.perform(build,launcher,listener); + } + }, + STEP { + public boolean perform(BuildStep bs, AbstractBuild build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException { + CheckPoint cp = new CheckPoint(bs.getClass().getName(),bs.getClass()); + cp.block(); + try { + return bs.perform(build,launcher,listener); + } finally { + cp.report(); + } + } + }, + BUILD { + public boolean perform(BuildStep bs, AbstractBuild build, Launcher launcher, BuildListener listener) throws IOException, InterruptedException { + CheckPoint.COMPLETED.block(); + return bs.perform(build,launcher,listener); + } + }; + + /** + * Calls {@link BuildStep#perform(AbstractBuild, Launcher, BuildListener)} with the proper synchronization. + */ + public abstract boolean perform(BuildStep bs, AbstractBuild build, Launcher launcher, BuildListener listener) throws IOException, InterruptedException; +} diff --git a/core/src/main/java/hudson/tasks/BuildTrigger.java b/core/src/main/java/hudson/tasks/BuildTrigger.java index 38721bfcd5..617262885e 100644 --- a/core/src/main/java/hudson/tasks/BuildTrigger.java +++ b/core/src/main/java/hudson/tasks/BuildTrigger.java @@ -122,6 +122,10 @@ public class BuildTrigger extends Recorder implements DependecyDeclarer, MatrixA return Items.fromNameList(childProjects,AbstractProject.class); } + public BuildStepMonitor getRequiredMonitorService() { + return BuildStepMonitor.NONE; + } + /** * Checks if this trigger has the exact same set of children as the given list. */ diff --git a/core/src/main/java/hudson/tasks/Builder.java b/core/src/main/java/hudson/tasks/Builder.java index 0f06d04397..752fbbc2e9 100644 --- a/core/src/main/java/hudson/tasks/Builder.java +++ b/core/src/main/java/hudson/tasks/Builder.java @@ -63,6 +63,14 @@ public abstract class Builder extends BuildStepCompatibilityLayer implements Bui return null; } + /** + * Returns {@link BuildStepMonitor#NONE} by default, as {@link Builder}s normally don't depend + * on its previous result. + */ + public BuildStepMonitor getRequiredMonitorService() { + return BuildStepMonitor.NONE; + } + public Descriptor getDescriptor() { return Hudson.getInstance().getDescriptor(getClass()); } diff --git a/core/src/main/java/hudson/tasks/CommandInterpreter.java b/core/src/main/java/hudson/tasks/CommandInterpreter.java index cbb800bd19..7fdca706c8 100644 --- a/core/src/main/java/hudson/tasks/CommandInterpreter.java +++ b/core/src/main/java/hudson/tasks/CommandInterpreter.java @@ -59,8 +59,7 @@ public abstract class CommandInterpreter extends Builder { } public boolean perform(AbstractBuild build, Launcher launcher, TaskListener listener) throws InterruptedException { - AbstractProject proj = build.getProject(); - FilePath ws = proj.getWorkspace(); + FilePath ws = build.getWorkspace(); FilePath script=null; try { try { diff --git a/core/src/main/java/hudson/tasks/Fingerprinter.java b/core/src/main/java/hudson/tasks/Fingerprinter.java index 8203807eac..95e519dfb0 100644 --- a/core/src/main/java/hudson/tasks/Fingerprinter.java +++ b/core/src/main/java/hudson/tasks/Fingerprinter.java @@ -101,8 +101,8 @@ public class Fingerprinter extends Recorder implements Serializable { if(targets.length()!=0) record(build, listener, record, targets); - if(recordBuildArtifacts && build instanceof Build) { - ArtifactArchiver aa = ((Build)build).getProject().getPublishersList().get(ArtifactArchiver.class); + if(recordBuildArtifacts) { + ArtifactArchiver aa = build.getProject().getPublishersList().get(ArtifactArchiver.class); if(aa==null) { // configuration error listener.error(Messages.Fingerprinter_NoArchiving()); @@ -123,6 +123,10 @@ public class Fingerprinter extends Recorder implements Serializable { return true; } + public BuildStepMonitor getRequiredMonitorService() { + return BuildStepMonitor.NONE; + } + private void record(AbstractBuild build, BuildListener listener, Map record, final String targets) throws IOException, InterruptedException { final class Record implements Serializable { final boolean produced; @@ -145,10 +149,9 @@ public class Fingerprinter extends Recorder implements Serializable { private static final long serialVersionUID = 1L; } - AbstractProject p = build.getProject(); final long buildTimestamp = build.getTimestamp().getTimeInMillis(); - FilePath ws = p.getWorkspace(); + FilePath ws = build.getWorkspace(); if(ws==null) { listener.error(Messages.Fingerprinter_NoWorkspace()); build.setResult(Result.FAILURE); @@ -208,7 +211,7 @@ public class Fingerprinter extends Recorder implements Serializable { * Performs on-the-fly validation on the file mask wildcard. */ public FormValidation doCheck(@AncestorInPath AbstractProject project, @QueryParameter String value) throws IOException { - return FilePath.validateFileMask(project.getWorkspace(),value); + return FilePath.validateFileMask(project.getSomeWorkspace(),value); } public Publisher newInstance(StaplerRequest req) { diff --git a/core/src/main/java/hudson/tasks/JavadocArchiver.java b/core/src/main/java/hudson/tasks/JavadocArchiver.java index 79d84cf9a7..a4eb95dbdf 100644 --- a/core/src/main/java/hudson/tasks/JavadocArchiver.java +++ b/core/src/main/java/hudson/tasks/JavadocArchiver.java @@ -86,7 +86,7 @@ public class JavadocArchiver extends Recorder { public boolean perform(AbstractBuild build, Launcher launcher, BuildListener listener) throws InterruptedException { listener.getLogger().println(Messages.JavadocArchiver_Publishing()); - FilePath javadoc = build.getParent().getWorkspace().child(javadocDir); + FilePath javadoc = build.getWorkspace().child(javadocDir); FilePath target = new FilePath(keepAll ? getJavadocDir(build) : getJavadocDir(build.getProject())); try { @@ -117,6 +117,10 @@ public class JavadocArchiver extends Recorder { return new JavadocAction(project); } + public BuildStepMonitor getRequiredMonitorService() { + return BuildStepMonitor.NONE; + } + protected static abstract class BaseJavadocAction implements Action { public String getUrlName() { return "javadoc"; @@ -205,7 +209,7 @@ public class JavadocArchiver extends Recorder { * Performs on-the-fly validation on the file mask wildcard. */ public FormValidation doCheck(@AncestorInPath AbstractProject project, @QueryParameter String value) throws IOException, ServletException { - FilePath ws = project.getWorkspace(); + FilePath ws = project.getSomeWorkspace(); return ws != null ? ws.validateRelativeDirectory(value) : FormValidation.ok(); } diff --git a/core/src/main/java/hudson/tasks/MailSender.java b/core/src/main/java/hudson/tasks/MailSender.java index b551497b9e..7aa9c02daf 100644 --- a/core/src/main/java/hudson/tasks/MailSender.java +++ b/core/src/main/java/hudson/tasks/MailSender.java @@ -103,6 +103,8 @@ public class MailSender { } } catch (MessagingException e) { e.printStackTrace(listener.error(e.getMessage())); + } finally { + CHECKPOINT.report(); } return true; @@ -111,10 +113,14 @@ public class MailSender { /** * To correctly compute the state change from the previous build to this build, * we need to ignore aborted builds. - * * See http://www.nabble.com/Losing-build-state-after-aborts--td24335949.html + * + *

+ * And since we are consulting the earlier result, we need to wait for the previous build + * to pass the check point. */ - private Result findPreviousBuildResult(AbstractBuild b) { + private Result findPreviousBuildResult(AbstractBuild b) throws InterruptedException { + CHECKPOINT.block(); do { b=b.getPreviousBuild(); if(b==null) return null; @@ -229,7 +235,7 @@ public class MailSender { // URL which has already been corrected in a subsequent build. To fix, archive. workspaceUrl = baseUrl + Util.encode(build.getProject().getUrl()) + "ws/"; artifactUrl = baseUrl + Util.encode(build.getUrl()) + "artifact/"; - FilePath ws = build.getProject().getWorkspace(); + FilePath ws = build.getWorkspace(); // Match either file or URL patterns, i.e. either // c:\hudson\workdir\jobs\foo\workspace\src\Foo.java // file:/c:/hudson/workdir/jobs/foo/workspace/src/Foo.java @@ -360,4 +366,9 @@ public class MailSender { public static boolean debug = false; private static final int MAX_LOG_LINES = Integer.getInteger(MailSender.class.getName()+".maxLogLines",250); + + /** + * Sometimes the outcome of the previous build affects the e-mail we send, hence this checkpoint. + */ + private static final CheckPoint CHECKPOINT = new CheckPoint("mail sent"); } diff --git a/core/src/main/java/hudson/tasks/Mailer.java b/core/src/main/java/hudson/tasks/Mailer.java index a365f8eb77..41c1e29ab4 100644 --- a/core/src/main/java/hudson/tasks/Mailer.java +++ b/core/src/main/java/hudson/tasks/Mailer.java @@ -118,6 +118,13 @@ public class Mailer extends Notifier { }.execute(build,listener); } + /** + * This class does explicit check pointing. + */ + public BuildStepMonitor getRequiredMonitorService() { + return BuildStepMonitor.NONE; + } + /** * @deprecated as of 1.286 * Use {@link #descriptor()} to obtain the current instance. diff --git a/core/src/main/java/hudson/tasks/Maven.java b/core/src/main/java/hudson/tasks/Maven.java index 5bfb86a412..bfbe981143 100644 --- a/core/src/main/java/hudson/tasks/Maven.java +++ b/core/src/main/java/hudson/tasks/Maven.java @@ -179,8 +179,6 @@ public class Maven extends Builder { } public boolean perform(AbstractBuild build, Launcher launcher, BuildListener listener) throws IOException, InterruptedException { - AbstractProject proj = build.getProject(); - VariableResolver vr = build.getBuildVariableResolver(); EnvVars env = build.getEnvironment(listener); @@ -207,7 +205,7 @@ public class Maven extends Builder { ArgumentListBuilder args = new ArgumentListBuilder(); MavenInstallation mi = getMaven(); if(mi==null) { - String execName = proj.getWorkspace().act(new DecideDefaultMavenCommand(normalizedTarget)); + String execName = build.getWorkspace().act(new DecideDefaultMavenCommand(normalizedTarget)); args.add(execName); } else { mi = mi.forNode(Computer.currentComputer().getNode(), listener); @@ -244,7 +242,7 @@ public class Maven extends Builder { env.put("MAVEN_OPTS",jvmOptions); try { - int r = launcher.launch().cmds(args).envs(env).stdout(listener).pwd(proj.getModuleRoot()).join(); + int r = launcher.launch().cmds(args).envs(env).stdout(listener).pwd(build.getModuleRoot()).join(); if (0 != r) { return false; } diff --git a/core/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java b/core/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java index eba26698ca..38c3a0327e 100644 --- a/core/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java +++ b/core/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java @@ -37,10 +37,12 @@ import hudson.model.AbstractProject; import hudson.model.Action; import hudson.model.BuildListener; import hudson.model.Result; +import hudson.model.CheckPoint; import hudson.remoting.VirtualChannel; import hudson.tasks.BuildStepDescriptor; import hudson.tasks.Publisher; import hudson.tasks.Recorder; +import hudson.tasks.BuildStepMonitor; import hudson.tasks.test.TestResultAggregator; import hudson.tasks.test.TestResultProjectAction; import hudson.util.FormValidation; @@ -79,7 +81,7 @@ public class JUnitResultArchiver extends Recorder implements Serializable, Matri final long buildTime = build.getTimestamp().getTimeInMillis(); final long nowMaster = System.currentTimeMillis(); - TestResult result = build.getProject().getWorkspace().act(new FileCallable() { + TestResult result = build.getWorkspace().act(new FileCallable() { public TestResult invoke(File ws, VirtualChannel channel) throws IOException { final long nowSlave = System.currentTimeMillis(); @@ -96,6 +98,7 @@ public class JUnitResultArchiver extends Recorder implements Serializable, Matri } }); + CHECKPOINT.block(); action = new TestResultAction(build, result, listener); if(result.getPassCount()==0 && result.getFailCount()==0) throw new AbortException(Messages.JUnitResultArchiver_ResultIsEmpty()); @@ -114,8 +117,8 @@ public class JUnitResultArchiver extends Recorder implements Serializable, Matri return true; } - build.getActions().add(action); + CHECKPOINT.report(); if(action.getResult().getFailCount()>0) build.setResult(Result.UNSTABLE); @@ -127,6 +130,13 @@ public class JUnitResultArchiver extends Recorder implements Serializable, Matri return new TestResult(buildTime, ds); } + /** + * This class does explicit checkpointing. + */ + public BuildStepMonitor getRequiredMonitorService() { + return BuildStepMonitor.NONE; + } + public String getTestResults() { return testResults; } @@ -140,6 +150,10 @@ public class JUnitResultArchiver extends Recorder implements Serializable, Matri public MatrixAggregator createAggregator(MatrixBuild build, Launcher launcher, BuildListener listener) { return new TestResultAggregator(build,launcher,listener); } + /** + * Test result tracks the diff from the previous run, hence the checkpoint. + */ + private static final CheckPoint CHECKPOINT = new CheckPoint("JUnit result archiving"); private static final long serialVersionUID = 1L; @@ -157,7 +171,7 @@ public class JUnitResultArchiver extends Recorder implements Serializable, Matri * Performs on-the-fly validation on the file mask wildcard. */ public FormValidation doCheckTestResults(@AncestorInPath AbstractProject project, @QueryParameter String value) throws IOException { - return FilePath.validateFileMask(project.getWorkspace(),value); + return FilePath.validateFileMask(project.getSomeWorkspace(),value); } public boolean isApplicable(Class jobType) { diff --git a/core/src/main/java/hudson/tasks/junit/TestResult.java b/core/src/main/java/hudson/tasks/junit/TestResult.java index 7b656f8a18..4b3f061dab 100644 --- a/core/src/main/java/hudson/tasks/junit/TestResult.java +++ b/core/src/main/java/hudson/tasks/junit/TestResult.java @@ -108,7 +108,7 @@ public final class TestResult extends MetaTabulatedResult { for (String value : includedFiles) { File reportFile = new File(baseDir, value); // only count files that were actually updated during this build - if(buildTime-1000/*error margin*/ <= reportFile.lastModified()) { + if(buildTime-3000/*error margin*/ <= reportFile.lastModified()) { if(reportFile.length()==0) { // this is a typical problem when JVM quits abnormally, like OutOfMemoryError during a test. SuiteResult sr = new SuiteResult(reportFile.getName(), "", ""); diff --git a/core/src/main/java/hudson/tasks/test/AggregatedTestResultPublisher.java b/core/src/main/java/hudson/tasks/test/AggregatedTestResultPublisher.java index 33faf0f85c..80e750eae0 100644 --- a/core/src/main/java/hudson/tasks/test/AggregatedTestResultPublisher.java +++ b/core/src/main/java/hudson/tasks/test/AggregatedTestResultPublisher.java @@ -40,6 +40,7 @@ import hudson.model.listeners.RunListener; import hudson.tasks.BuildStepDescriptor; import hudson.tasks.Publisher; import hudson.tasks.Recorder; +import hudson.tasks.BuildStepMonitor; import hudson.tasks.Fingerprinter.FingerprintAction; import hudson.util.FormValidation; import net.sf.json.JSONObject; @@ -77,6 +78,10 @@ public class AggregatedTestResultPublisher extends Recorder { return true; } + public BuildStepMonitor getRequiredMonitorService() { + return BuildStepMonitor.NONE; + } + /** * Action that serves the aggregated record. * diff --git a/core/src/main/java/hudson/triggers/SCMTrigger.java b/core/src/main/java/hudson/triggers/SCMTrigger.java index 32c796506b..b8f0140961 100644 --- a/core/src/main/java/hudson/triggers/SCMTrigger.java +++ b/core/src/main/java/hudson/triggers/SCMTrigger.java @@ -36,24 +36,18 @@ import hudson.model.SCMedItem; import hudson.model.AdministrativeMonitor; import hudson.util.StreamTaskListener; import hudson.util.TimeUnit2; +import hudson.util.SequentialExecutionQueue; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.DataBoundConstructor; import java.io.File; import java.io.IOException; -import java.io.ObjectStreamException; import java.io.PrintStream; -import java.util.Arrays; -import java.util.Collections; import java.util.Date; -import java.util.HashSet; import java.util.List; -import java.util.Set; import java.util.ArrayList; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; -import java.util.concurrent.ThreadPoolExecutor; -import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Level; import java.util.logging.Logger; import java.text.DateFormat; @@ -66,46 +60,18 @@ import net.sf.json.JSONObject; * @author Kohsuke Kawaguchi */ public class SCMTrigger extends Trigger { - /** - * If we'd like to run another polling run, this is set to true. - * - *

- * To avoid submitting more than one polling jobs (which could flood the queue), - * we first use the boolean flag. - * - * @guardedBy this - */ - private transient volatile boolean pollingScheduled; - - /** - * This lock is used to control the mutual exclusion of the SCM activity, - * so that the build and polling don't happen at the same time. - */ - private transient ReentrantLock lock; - @DataBoundConstructor public SCMTrigger(String scmpoll_spec) throws ANTLRException { super(scmpoll_spec); - lock = new ReentrantLock(); - } - - public ReentrantLock getLock() { - return lock; - } - - protected Object readResolve() throws ObjectStreamException { - lock = new ReentrantLock(); - return super.readResolve(); } public void run() { - if(pollingScheduled || Hudson.getInstance().isQuietingDown()) + if(Hudson.getInstance().isQuietingDown()) return; // noop - pollingScheduled = true; - LOGGER.fine("Scheduling a polling for "+job); DescriptorImpl d = getDescriptor(); + LOGGER.fine("Scheduling a polling for "+job); if (d.synchronousPolling) { LOGGER.fine("Running the trigger directly without threading, " + "as it's already taken care of by Trigger.Cron"); @@ -115,7 +81,7 @@ public class SCMTrigger extends Trigger { // even if we end up submitting this too many times, that's OK. // the real exclusion control happens inside Runner. LOGGER.fine("scheduling the trigger to (asynchronously) run"); - d.getExecutor().submit(new Runner()); + d.queue.execute(new Runner()); d.clogCheck(); } } @@ -139,8 +105,13 @@ public class SCMTrigger extends Trigger { public static class DescriptorImpl extends TriggerDescriptor { /** * Used to control the execution of the polling tasks. + *

+ * This executor implementation has a semantics suitable for polling. Namely, no two threads will try to poll the same project + * at once, and multiple polling requests to the same job will be combined into one. Note that because executor isn't aware + * of a potential workspace lock between a build and a polling, we may end up using executor threads unwisely --- they + * may block. */ - transient volatile ThreadPoolExecutor executor; + private transient final SequentialExecutionQueue queue = new SequentialExecutionQueue(Executors.newSingleThreadExecutor()); /** * Whether the projects should be polled all in one go in the order of dependencies. The default behavior is @@ -148,11 +119,6 @@ public class SCMTrigger extends Trigger { */ public boolean synchronousPolling = false; - /** - * Jobs that are being polled. The value is useful for trouble-shooting. - */ - final transient Set items = Collections.synchronizedSet(new HashSet()); - /** * Max number of threads for SCM polling. * 0 for unbounded. @@ -161,10 +127,6 @@ public class SCMTrigger extends Trigger { public DescriptorImpl() { load(); - /* - * Need to resize the thread pool here in case there is no existing configuration file for SCMTrigger as - * setPollingThreadCount() is not called in this case - */ resizeThreadPool(); } @@ -173,7 +135,7 @@ public class SCMTrigger extends Trigger { } public ExecutorService getExecutor() { - return executor; + return queue.getExecutors(); } /** @@ -181,14 +143,7 @@ public class SCMTrigger extends Trigger { * than it can handle. */ public boolean isClogged() { - for( Runnable r : executor.getQueue() ) { - if (r instanceof Runner) {// this should be always true, but let's be defensive. - Runner rr = (Runner) r; - if(rr.isStarving()) - return true; - } - } - return false; + return queue.isStarving(STARVATION_THRESHOLD); } /** @@ -203,21 +158,17 @@ public class SCMTrigger extends Trigger { * Gets the snapshot of {@link Runner}s that are performing polling. */ public List getRunners() { - synchronized (items) { - return Arrays.asList(items.toArray(new Runner[items.size()])); - } + return Util.filter(queue.getInProgress(),Runner.class); } /** * Gets the snapshot of {@link SCMedItem}s that are being polled at this very moment. */ public List getItemsBeingPolled() { - synchronized (items) { - List r = new ArrayList(); - for (Runner i : items) - r.add(i.getTarget()); - return r; - } + List r = new ArrayList(); + for (Runner i : getRunners()) + r.add(i.getTarget()); + return r; } public String getDisplayName() { @@ -256,13 +207,8 @@ public class SCMTrigger extends Trigger { * Update the {@link ExecutorService} instance. */ /*package*/ synchronized void resizeThreadPool() { - // swap to a new one, and shut down the old one gradually - ThreadPoolExecutor newExec = (ThreadPoolExecutor) - (maximumThreads==0 ? Executors.newCachedThreadPool() : Executors.newFixedThreadPool(maximumThreads)); - ExecutorService old = executor; - executor = newExec; - if(old!=null) - old.shutdown(); + queue.setExecutors( + (maximumThreads==0 ? Executors.newCachedThreadPool() : Executors.newFixedThreadPool(maximumThreads))); } public boolean configure(StaplerRequest req, JSONObject json) throws FormException { @@ -325,14 +271,6 @@ public class SCMTrigger extends Trigger { */ private volatile long startTime; - /** - * When was this object submitted to {@link DescriptorImpl#getExecutor()}? - * - *

- * This field is used to check if the queue is clogged. - */ - public final long submissionTime = System.currentTimeMillis(); - /** * Where the log file is written. */ @@ -361,17 +299,6 @@ public class SCMTrigger extends Trigger { return Util.getTimeSpanString(System.currentTimeMillis()-startTime); } - /** - * Returns true if too much time is spent since this item is put into the queue. - * - *

- * This property makes sense only when called before the task actually starts, - * as the {@link #run()} method may take a long time to execute. - */ - public boolean isStarving() { - return System.currentTimeMillis()-submissionTime > STARVATION_THRESHOLD; - } - private boolean runPolling() { try { // to make sure that the log file contains up-to-date text, @@ -408,39 +335,33 @@ public class SCMTrigger extends Trigger { String threadName = Thread.currentThread().getName(); Thread.currentThread().setName("SCM polling for "+job); try { - while(pollingScheduled) { - getLock().lockInterruptibly(); - boolean foundChanges=false; - try { - if(pollingScheduled) { - pollingScheduled = false; - startTime = System.currentTimeMillis(); - getDescriptor().items.add(this); - try { - foundChanges = runPolling(); - } finally { - getDescriptor().items.remove(this); - } - } - } finally { - getLock().unlock(); - } - - if(foundChanges) { - String name = " #"+job.asProject().getNextBuildNumber(); - if(job.scheduleBuild(new SCMTriggerCause())) { - LOGGER.info("SCM changes detected in "+ job.getName()+". Triggering "+name); - } else { - LOGGER.info("SCM changes detected in "+ job.getName()+". Job is already in the queue"); - } + startTime = System.currentTimeMillis(); + if(runPolling()) { + String name = " #"+job.asProject().getNextBuildNumber(); + if(job.scheduleBuild(new SCMTriggerCause())) { + LOGGER.info("SCM changes detected in "+ job.getName()+". Triggering "+name); + } else { + LOGGER.info("SCM changes detected in "+ job.getName()+". Job is already in the queue"); } } - } catch (InterruptedException e) { - LOGGER.info("Aborted"); } finally { Thread.currentThread().setName(threadName); } } + + private SCMedItem job() { + return job; + } + + // as per the requirement of SequentialExecutionQueue, value equality is necessary + public boolean equals(Object that) { + return job()==((Runner)that).job(); + } + + @Override + public int hashCode() { + return job.hashCode(); + } } public static class SCMTriggerCause extends Cause { diff --git a/core/src/main/java/hudson/util/FormFieldValidator.java b/core/src/main/java/hudson/util/FormFieldValidator.java index 7d6a37eb06..b628456f62 100644 --- a/core/src/main/java/hudson/util/FormFieldValidator.java +++ b/core/src/main/java/hudson/util/FormFieldValidator.java @@ -389,7 +389,7 @@ public abstract class FormFieldValidator { * The base directory from which the path name is resolved. */ protected FilePath getBaseDirectory(AbstractProject p) { - return p.getWorkspace(); + return p.getSomeWorkspace(); } } @@ -481,7 +481,7 @@ public abstract class FormFieldValidator { * The base directory from which the path name is resolved. */ protected FilePath getBaseDirectory(AbstractProject p) { - return p.getWorkspace(); + return p.getSomeWorkspace(); } } diff --git a/core/src/main/java/hudson/util/InterceptingProxy.java b/core/src/main/java/hudson/util/InterceptingProxy.java new file mode 100644 index 0000000000..1d08611d90 --- /dev/null +++ b/core/src/main/java/hudson/util/InterceptingProxy.java @@ -0,0 +1,30 @@ +package hudson.util; + +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; + +/** + * Creates a proxy that traps every method call. + * + * @author Kohsuke Kawaguchi + */ +public abstract class InterceptingProxy { + /** + * Intercepts every method call. + */ + protected abstract Object call(Object o, Method m, Object[] args) throws Throwable; + + public final T wrap(Class type, final T object) { + return type.cast(Proxy.newProxyInstance(type.getClassLoader(), new Class[]{type}, new InvocationHandler() { + public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { + try { + return call(object,method,args); + } catch (InvocationTargetException e) { + throw e.getTargetException(); + } + } + })); + } +} diff --git a/core/src/main/java/hudson/util/SequentialExecutionQueue.java b/core/src/main/java/hudson/util/SequentialExecutionQueue.java new file mode 100644 index 0000000000..7b805a6c6f --- /dev/null +++ b/core/src/main/java/hudson/util/SequentialExecutionQueue.java @@ -0,0 +1,127 @@ +package hudson.util; + +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import java.util.HashSet; +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; + +/** + * {@link Executor} that collapses two equal {@link Runnable}s into one, + * and makes sure no two equal {@link Runnable}s get executed simultaneously. + * + *

+ * That is, if a {@link Runnable} is executing and another one gets submitted, + * the 2nd one waits for the completion of the 1st one. + * + * {@link Object#equals(Object)} is used on {@link Runnable} to identify + * two equal {@link Runnable}s. + * + * @author Kohsuke Kawaguchi + */ +public class SequentialExecutionQueue implements Executor { + /** + * Access is sycnhronized by {@code Queue.this} + */ + private final Map entries = new HashMap(); + private ExecutorService executors; + + /** + * {@link Runnable}s that are currently executing. Useful for trouble-shooting. + */ + private final Set inProgress = new HashSet(); + + public SequentialExecutionQueue(ExecutorService executors) { + this.executors = executors; + } + + /** + * Gets the base underlying executors., + */ + public synchronized ExecutorService getExecutors() { + return executors; + } + + /** + * Starts using a new {@link ExecutorService} to carry out executions. + * + *

+ * The older {@link ExecutorService} will be shut down (but it's still expected to + * complete whatever they are doing and scheduled.) + */ + public synchronized void setExecutors(ExecutorService svc) { + ExecutorService old = this.executors; + this.executors = svc; + // gradually executions will be taken over by a new pool + old.shutdown(); + } + + + public synchronized void execute(Runnable item) { + QueueEntry e = entries.get(item); + if(e==null) { + e = new QueueEntry(item); + entries.put(item,e); + e.submit(); + } else { + e.queued = true; + } + } + + /** + * Returns true if too much time is spent since some {@link Runnable} is submitted into the queue + * until they get executed. + */ + public synchronized boolean isStarving(long threshold) { + long now = System.currentTimeMillis(); + for (QueueEntry e : entries.values()) + if (now-e.submissionTime > threshold) + return true; + return false; + } + + /** + * Gets {@link Runnable}s that are currently executed by a live thread. + */ + public synchronized Set getInProgress() { + return new HashSet(inProgress); + } + + private final class QueueEntry implements Runnable { + private final Runnable item; + private boolean queued; + private long submissionTime; + + private QueueEntry(Runnable item) { + this.item = item; + this.queued = true; + } + + // Caller must have a lock + private void submit() { + submissionTime = System.currentTimeMillis(); + executors.submit(this); + } + + public void run() { + try { + synchronized (SequentialExecutionQueue.this) { + assert queued; + queued = false; + inProgress.add(this); + } + item.run(); + } finally { + synchronized (SequentialExecutionQueue.this) { + if(queued) + // another polling for this job is requested while we were doing the polling. do it again. + submit(); + else + entries.remove(item); + inProgress.remove(this); + } + } + } + } +} diff --git a/core/src/main/java/hudson/widgets/HistoryWidget.java b/core/src/main/java/hudson/widgets/HistoryWidget.java index 9dfa7f8c56..0a98326e62 100644 --- a/core/src/main/java/hudson/widgets/HistoryWidget.java +++ b/core/src/main/java/hudson/widgets/HistoryWidget.java @@ -48,7 +48,7 @@ import java.util.List; */ public class HistoryWidget extends Widget { /** - * The given data model of records. + * The given data model of records. Newer ones first. */ public Iterable baseList; @@ -68,6 +68,11 @@ public class HistoryWidget extends Widget { public final Adapter adapter; + /** + * First transient build record. Everything >= this will be discarded when AJAX call is made. + */ + private String firstTransientBuildKey; + /** * @param owner * The parent model object that owns this widget. @@ -90,6 +95,19 @@ public class HistoryWidget extends Widget { return "buildHistory"; } + public String getFirstTransientBuildKey() { + return firstTransientBuildKey; + } + + private Iterable updateFirstTransientBuildKey(Iterable source) { + String key=null; + for (T t : source) + if(adapter.isBuilding(t)) + key = adapter.getKey(t); + firstTransientBuildKey = key; + return source; + } + /** * The records to be rendered this time. */ @@ -101,17 +119,17 @@ public class HistoryWidget extends Widget { if(lst.size()>THRESHOLD) return lst.subList(0,THRESHOLD); trimmed=false; - return lst; + return updateFirstTransientBuildKey(lst); } else { lst = new ArrayList(THRESHOLD); Iterator itr = baseList.iterator(); while(lst.size()<=THRESHOLD && itr.hasNext()) lst.add(itr.next()); - trimmed = itr.hasNext(); - return lst; + trimmed = itr.hasNext(); // if we don't have enough items in the base list, setting this to false will optimize the next getRenderList() invocation. + return updateFirstTransientBuildKey(lst); } } else - return baseList; + return updateFirstTransientBuildKey(baseList); } public boolean isTrimmed() { @@ -137,22 +155,32 @@ public class HistoryWidget extends Widget { // pick up builds to send back List items = new ArrayList(); + String nn=null; // we'll compute next n here + + // list up all builds >=n. for (T t : baseList) { - if(adapter.compare(t,n)>=0) + if(adapter.compare(t,n)>=0) { items.add(t); - else + if(adapter.isBuilding(t)) + nn = adapter.getKey(t); // the next fetch should start from youngest build in progress + } else break; } - baseList = items; - if(!items.isEmpty()) { - T b = items.get(0); - n = adapter.getKey(b); - if(!adapter.isBuilding(b)) - n = adapter.getNextKey(n); + if (nn==null) { + if (items.isEmpty()) { + // nothing to report back. next fetch should retry the same 'n' + nn=n; + } else { + // every record fetched this time is frozen. next fetch should start from the next build + nn=adapter.getNextKey(adapter.getKey(items.get(0))); + } } - rsp.setHeader("n",n); + baseList = items; + + rsp.setHeader("n",nn); + firstTransientBuildKey = nn; // all builds >= nn should be marked transient req.getView(this,"ajaxBuildHistory.jelly").forward(req,rsp); } diff --git a/core/src/main/resources/hudson/model/AbstractItem/configure-common.jelly b/core/src/main/resources/hudson/model/AbstractItem/configure-common.jelly index c5392e8f56..f7535fa514 100644 --- a/core/src/main/resources/hudson/model/AbstractItem/configure-common.jelly +++ b/core/src/main/resources/hudson/model/AbstractItem/configure-common.jelly @@ -27,6 +27,9 @@ THE SOFTWARE. --> + + + @@ -36,7 +39,7 @@ THE SOFTWARE. diff --git a/core/src/main/resources/hudson/model/AbstractItem/help-concurrentBuild.html b/core/src/main/resources/hudson/model/AbstractItem/help-concurrentBuild.html new file mode 100644 index 0000000000..92307897c7 --- /dev/null +++ b/core/src/main/resources/hudson/model/AbstractItem/help-concurrentBuild.html @@ -0,0 +1,16 @@ +

+ If this option is checked, Hudson will schedule and execute multiple builds concurrently (provided + that you have sufficient executors and incoming build requests.) This is useful on builds and test jobs that + take a long time, as each run build will only contain a smaller number of changes, and the total turn-around + time decreases due to the shorter time a build request spends waiting for the previous build to complete. + It is also very useful with parameterized builds, whose individual executions are independent from each other. + +

+ For other kinds of jobs, allowing concurrent executions of multiple builds may be problematic, + for example if it assumes a monopoly on a certain resource, like database, or for jobs where you use Hudson + as a cron replacement. + +

+ Even when concurrently run, builds are isolated from each other. If they are scheduled on the same node, + they use different workspaces. +

\ No newline at end of file diff --git a/core/src/main/resources/hudson/model/Job/configure.jelly b/core/src/main/resources/hudson/model/Job/configure.jelly index 1fe55cf0b1..d040c8d843 100644 --- a/core/src/main/resources/hudson/model/Job/configure.jelly +++ b/core/src/main/resources/hudson/model/Job/configure.jelly @@ -30,6 +30,9 @@ THE SOFTWARE. + + + @@ -51,12 +54,14 @@ THE SOFTWARE. - - + + + - - - + + + + diff --git a/core/src/main/resources/hudson/widgets/HistoryWidget/entry.jelly b/core/src/main/resources/hudson/widgets/HistoryWidget/entry.jelly index 25102e03c1..d4ee32ddd6 100644 --- a/core/src/main/resources/hudson/widgets/HistoryWidget/entry.jelly +++ b/core/src/main/resources/hudson/widgets/HistoryWidget/entry.jelly @@ -27,7 +27,8 @@ THE SOFTWARE. --> - + + ${build.iconColor.description} #${build.number} @@ -61,7 +62,7 @@ THE SOFTWARE. - + ${build.truncatedDescription} diff --git a/core/src/main/resources/lib/form/checkbox.jelly b/core/src/main/resources/lib/form/checkbox.jelly index e8f95a7a4c..fb392b3115 100644 --- a/core/src/main/resources/lib/form/checkbox.jelly +++ b/core/src/main/resources/lib/form/checkbox.jelly @@ -36,6 +36,7 @@ THE SOFTWARE. + Used for databinding. TBD. diff --git a/core/src/main/resources/lib/form/optionalBlock.jelly b/core/src/main/resources/lib/form/optionalBlock.jelly index b6795265de..8422bf9635 100644 --- a/core/src/main/resources/lib/form/optionalBlock.jelly +++ b/core/src/main/resources/lib/form/optionalBlock.jelly @@ -26,17 +26,20 @@ THE SOFTWARE. Foldable block that can be expanded to show more controls by checking the checkbox. - + Name of the checkbox. Can be used by the server to determine if the block is collapsed or expanded at the time of submission. Note that when the block is collapsed, none of its child controls will send the values to the server (unlike <f:advanced>) - + Human readable text that follows the checkbox. - + + Used for databinding. TBD. Either this or @name/@title combo is required. + + initial checkbox status. true/false. @@ -47,17 +50,22 @@ THE SOFTWARE. if present, the foldable section expands when the checkbox is unchecked. + + + + - + - Help for feature: ${title} + Help for feature: ${title} diff --git a/maven-plugin/src/main/java/hudson/maven/MavenBuild.java b/maven-plugin/src/main/java/hudson/maven/MavenBuild.java index ddce2bc94f..95506d3cd7 100644 --- a/maven-plugin/src/main/java/hudson/maven/MavenBuild.java +++ b/maven-plugin/src/main/java/hudson/maven/MavenBuild.java @@ -26,6 +26,7 @@ package hudson.maven; import hudson.FilePath; import hudson.Util; import hudson.EnvVars; +import hudson.slaves.WorkspaceList; import hudson.maven.agent.AbortException; import hudson.model.AbstractBuild; import hudson.model.AbstractProject; @@ -37,6 +38,8 @@ import hudson.model.Run; import hudson.model.Cause.UpstreamCause; import hudson.model.Environment; import hudson.model.TaskListener; +import hudson.model.Node; +import hudson.model.Executor; import hudson.remoting.Channel; import hudson.scm.ChangeLogSet; import hudson.scm.ChangeLogSet.Entry; @@ -464,15 +467,20 @@ public class MavenBuild extends AbstractMavenBuild { } private Object writeReplace() { - return Channel.current().export(MavenBuildProxy2.class,this); + // when called from remote, methods need to be executed in the proper Executor's context. + return Channel.current().export(MavenBuildProxy2.class, + Executor.currentExecutor().newImpersonatingProxy(MavenBuildProxy2.class,this)); } - - } private class RunnerImpl extends AbstractRunner { private List reporters; + @Override + protected FilePath decideWorkspace(Node n, WorkspaceList wsl) { + return wsl.allocate(getParentBuild().getModuleRoot().child(getProject().getRelativePath())); + } + protected Result doRun(BuildListener listener) throws Exception { // pick up a list of reporters to run reporters = getProject().createReporters(); @@ -498,8 +506,8 @@ public class MavenBuild extends AbstractMavenBuild { if(mms.usesPrivateRepository()) // use the per-project repository. should it be per-module? But that would cost too much in terms of disk // the workspace must be on this node, so getRemote() is safe. - margs.add("-Dmaven.repo.local="+mms.getWorkspace().child(".repository").getRemote()); - margs.add("-f",getProject().getModuleRoot().child("pom.xml").getRemote()); + margs.add("-Dmaven.repo.local="+getWorkspace().child(".repository").getRemote()); + margs.add("-f",getModuleRoot().child("pom.xml").getRemote()); margs.addTokenized(getProject().getGoals()); Map systemProps = new HashMap(envVars); @@ -510,7 +518,7 @@ public class MavenBuild extends AbstractMavenBuild { try { Result r = process.channel.call(new Builder( listener,new ProxyImpl(), - reporters.toArray(new MavenReporter[0]), margs.toList(), systemProps)); + reporters.toArray(new MavenReporter[reporters.size()]), margs.toList(), systemProps)); normalExit = true; return r; } finally { diff --git a/maven-plugin/src/main/java/hudson/maven/MavenBuildProxy2.java b/maven-plugin/src/main/java/hudson/maven/MavenBuildProxy2.java index 2cda6bb276..0f32d0c659 100644 --- a/maven-plugin/src/main/java/hudson/maven/MavenBuildProxy2.java +++ b/maven-plugin/src/main/java/hudson/maven/MavenBuildProxy2.java @@ -30,7 +30,7 @@ package hudson.maven; * * @author Kohsuke Kawaguchi */ -interface MavenBuildProxy2 extends MavenBuildProxy { +public interface MavenBuildProxy2 extends MavenBuildProxy { /** * Notifies that the build has entered a module. */ diff --git a/maven-plugin/src/main/java/hudson/maven/MavenModule.java b/maven-plugin/src/main/java/hudson/maven/MavenModule.java index e3058cc7ba..57b524c245 100644 --- a/maven-plugin/src/main/java/hudson/maven/MavenModule.java +++ b/maven-plugin/src/main/java/hudson/maven/MavenModule.java @@ -216,7 +216,7 @@ public final class MavenModule extends AbstractMavenProject> getPublishersList() { // TODO return new DescribableList>(this); diff --git a/maven-plugin/src/main/java/hudson/maven/MavenModuleSet.java b/maven-plugin/src/main/java/hudson/maven/MavenModuleSet.java index 34379bef94..ed70a8b3a0 100644 --- a/maven-plugin/src/main/java/hudson/maven/MavenModuleSet.java +++ b/maven-plugin/src/main/java/hudson/maven/MavenModuleSet.java @@ -323,15 +323,6 @@ public final class MavenModuleSet extends AbstractMavenProject getBuildClass() { return MavenModuleSetBuild.class; diff --git a/maven-plugin/src/main/java/hudson/maven/MavenModuleSetBuild.java b/maven-plugin/src/main/java/hudson/maven/MavenModuleSetBuild.java index 804eed4dab..dd395e4416 100644 --- a/maven-plugin/src/main/java/hudson/maven/MavenModuleSetBuild.java +++ b/maven-plugin/src/main/java/hudson/maven/MavenModuleSetBuild.java @@ -438,8 +438,8 @@ public class MavenModuleSetBuild extends AbstractMavenBuild poms; try { - poms = project.getModuleRoot().act(new PomParser(listener, mvn, project)); + poms = getModuleRoot().act(new PomParser(listener, mvn, project)); } catch (IOException e) { if (e.getCause() instanceof AbortException) throw (AbortException) e.getCause(); diff --git a/maven-plugin/src/main/java/hudson/maven/MavenRedeployer.java b/maven-plugin/src/main/java/hudson/maven/MavenRedeployer.java index 9469e783ec..128468b7a5 100644 --- a/maven-plugin/src/main/java/hudson/maven/MavenRedeployer.java +++ b/maven-plugin/src/main/java/hudson/maven/MavenRedeployer.java @@ -29,6 +29,7 @@ import hudson.model.BuildListener; import hudson.model.Result; import hudson.tasks.BuildStepDescriptor; import hudson.tasks.Publisher; +import hudson.tasks.BuildStepMonitor; import hudson.Launcher; import hudson.maven.reporters.MavenArtifactRecord; @@ -62,6 +63,10 @@ public class MavenRedeployer extends Publisher { return true; } + public BuildStepMonitor getRequiredMonitorService() { + return BuildStepMonitor.NONE; + } + public BuildStepDescriptor getDescriptor() { return DESCRIPTOR; } diff --git a/maven-plugin/src/main/java/hudson/maven/RedeployPublisher.java b/maven-plugin/src/main/java/hudson/maven/RedeployPublisher.java index 1ccbd540ae..af6b6c5919 100644 --- a/maven-plugin/src/main/java/hudson/maven/RedeployPublisher.java +++ b/maven-plugin/src/main/java/hudson/maven/RedeployPublisher.java @@ -33,6 +33,7 @@ import hudson.model.Result; import hudson.tasks.BuildStepDescriptor; import hudson.tasks.Publisher; import hudson.tasks.Recorder; +import hudson.tasks.BuildStepMonitor; import net.sf.json.JSONObject; import org.apache.maven.artifact.deployer.ArtifactDeploymentException; import org.apache.maven.artifact.repository.ArtifactRepository; @@ -117,6 +118,10 @@ public class RedeployPublisher extends Recorder { return build.getAction(MavenAbstractArtifactRecord.class); } + public BuildStepMonitor getRequiredMonitorService() { + return BuildStepMonitor.NONE; + } + @Extension public static class DescriptorImpl extends BuildStepDescriptor { public DescriptorImpl() { diff --git a/test/src/test/java/hudson/model/AbstractProjectTest.java b/test/src/test/java/hudson/model/AbstractProjectTest.java index 76d4f3c325..820ee0f381 100644 --- a/test/src/test/java/hudson/model/AbstractProjectTest.java +++ b/test/src/test/java/hudson/model/AbstractProjectTest.java @@ -44,10 +44,10 @@ public class AbstractProjectTest extends HudsonTestCase { FreeStyleProject project = createFreeStyleProject(); project.getBuildersList().add(new Shell("echo hello")); - project.scheduleBuild2(0).get(); + FreeStyleBuild b = project.scheduleBuild2(0).get(); assertTrue("Workspace should exist by now", - project.getWorkspace().exists()); + b.getWorkspace().exists()); // emulate the user behavior WebClient webClient = new WebClient(); @@ -58,7 +58,7 @@ public class AbstractProjectTest extends HudsonTestCase { page = (HtmlPage)((HtmlForm)page.getElementById("confirmation")).submit(null); assertFalse("Workspace should be gone by now", - project.getWorkspace().exists()); + b.getWorkspace().exists()); } /** @@ -69,10 +69,9 @@ public class AbstractProjectTest extends HudsonTestCase { FreeStyleProject project = createFreeStyleProject(); project.getBuildersList().add(new Shell("echo hello")); - project.scheduleBuild2(0).get(); + FreeStyleBuild b = project.scheduleBuild2(0).get(); - assertTrue("Workspace should exist by now", - project.getWorkspace().exists()); + assertTrue("Workspace should exist by now",b.getWorkspace().exists()); // make sure that the action link is protected try { @@ -106,4 +105,16 @@ public class AbstractProjectTest extends HudsonTestCase { // OK } } + + /** + * Tests the <optionalBlock @field> round trip behavior by using {@link AbstractProject#concurrentBuild} + */ + public void testOptionalBlockDataBindingRoundtrip() throws Exception { + FreeStyleProject p = createFreeStyleProject(); + for( boolean b : new boolean[]{true,false}) { + p.setConcurrentBuild(b); + submit(new WebClient().getPage(p,"configure").getFormByName("config")); + assertEquals(b,p.isConcurrentBuild()); + } + } } diff --git a/test/src/test/java/hudson/tasks/ArtifactArchiverTest.java b/test/src/test/java/hudson/tasks/ArtifactArchiverTest.java index 0822603850..17d9a43e7e 100644 --- a/test/src/test/java/hudson/tasks/ArtifactArchiverTest.java +++ b/test/src/test/java/hudson/tasks/ArtifactArchiverTest.java @@ -127,7 +127,7 @@ public class ArtifactArchiverTest extends HudsonTestCase { project.getPublishersList().replaceBy(Collections.singleton(artifactArchiver)); project.getBuildersList().replaceBy(Collections.singleton(new TestBuilder() { public boolean perform(AbstractBuild build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException { - FilePath dir = build.getProject().getWorkspace().child("dir"); + FilePath dir = build.getWorkspace().child("dir"); dir.child("subdir1").mkdirs(); FilePath subdir2 = dir.child("subdir2"); subdir2.mkdirs(); @@ -150,7 +150,7 @@ public class ArtifactArchiverTest extends HudsonTestCase { private static class CreateArtifact extends TestBuilder { public boolean perform(AbstractBuild build, Launcher launcher, BuildListener listener) throws IOException, InterruptedException { - build.getProject().getWorkspace().child("f").write("content", "UTF-8"); + build.getWorkspace().child("f").write("content", "UTF-8"); return true; } } diff --git a/test/src/test/java/hudson/tasks/LogRotatorTest.java b/test/src/test/java/hudson/tasks/LogRotatorTest.java index a4047b3dd0..f3b24f2d37 100644 --- a/test/src/test/java/hudson/tasks/LogRotatorTest.java +++ b/test/src/test/java/hudson/tasks/LogRotatorTest.java @@ -87,6 +87,11 @@ public class LogRotatorTest extends HudsonTestCase { build.setResult(Result.UNSTABLE); return true; } + + public BuildStepMonitor getRequiredMonitorService() { + return BuildStepMonitor.NONE; + } + public Descriptor getDescriptor() { return new Descriptor(TestsFail.class) { public String getDisplayName() { diff --git a/test/src/test/java/hudson/tasks/junit/CaseResultTest.java b/test/src/test/java/hudson/tasks/junit/CaseResultTest.java index 47c50042ab..80270d0725 100644 --- a/test/src/test/java/hudson/tasks/junit/CaseResultTest.java +++ b/test/src/test/java/hudson/tasks/junit/CaseResultTest.java @@ -65,7 +65,7 @@ public class CaseResultTest extends HudsonTestCase { FreeStyleProject p = createFreeStyleProject(); p.getBuildersList().add(new TestBuilder() { public boolean perform(AbstractBuild build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException { - build.getProject().getWorkspace().child("junit.xml").copyFrom( + build.getWorkspace().child("junit.xml").copyFrom( getClass().getResource("junit-report-20090516.xml")); return true; } diff --git a/test/src/test/java/hudson/util/ProcessTreeKillerTest.java b/test/src/test/java/hudson/util/ProcessTreeKillerTest.java index 0984b4a339..108fe81e38 100644 --- a/test/src/test/java/hudson/util/ProcessTreeKillerTest.java +++ b/test/src/test/java/hudson/util/ProcessTreeKillerTest.java @@ -1,6 +1,7 @@ package hudson.util; import hudson.model.FreeStyleProject; +import hudson.model.FreeStyleBuild; import hudson.tasks.Maven; import org.easymock.EasyMock; @@ -24,14 +25,15 @@ public class ProcessTreeKillerTest extends HudsonTestCase { project.scheduleBuild(0); Thread.sleep(2000); - project.getLastBuild().doStop( + FreeStyleBuild b = project.getLastBuild(); + b.doStop( EasyMock.createNiceMock(StaplerRequest.class), EasyMock.createNiceMock(StaplerResponse.class)); Thread.sleep(1000); // will fail (at least on windows) if test process is still running - project.getWorkspace().deleteRecursive(); + b.getWorkspace().deleteRecursive(); }