diff --git a/src/main/java/hudson/scm/SubversionEventHandlerImpl.java b/src/main/java/hudson/scm/SubversionEventHandlerImpl.java index a7f6d4fb6..12e89c666 100644 --- a/src/main/java/hudson/scm/SubversionEventHandlerImpl.java +++ b/src/main/java/hudson/scm/SubversionEventHandlerImpl.java @@ -117,7 +117,7 @@ public void handleEvent(SVNEvent event, double progress) throws SVNException { } } else if (action == SVNEventAction.UPDATE_COMPLETED) { // finished updating - out.println("At revision " + event.getRevision()); + out.println(baseDir+": At revision " + event.getRevision()); return; } else if (action == SVNEventAction.ADD){ out.println("A " + path); diff --git a/src/main/java/hudson/scm/SubversionSCM.java b/src/main/java/hudson/scm/SubversionSCM.java index 4c7e8ec20..bd7b232d0 100755 --- a/src/main/java/hudson/scm/SubversionSCM.java +++ b/src/main/java/hudson/scm/SubversionSCM.java @@ -54,6 +54,7 @@ import com.cloudbees.plugins.credentials.domains.URIRequirementBuilder; import com.cloudbees.plugins.credentials.impl.CertificateCredentialsImpl; import com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsImpl; + import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; import hudson.BulkChange; @@ -94,19 +95,24 @@ import hudson.scm.subversion.WorkspaceUpdater; import hudson.scm.subversion.WorkspaceUpdater.UpdateTask; import hudson.scm.subversion.WorkspaceUpdaterDescriptor; +import hudson.util.AbstractTaskListener; import hudson.util.EditDistance; import hudson.util.FormValidation; +import hudson.util.IOException2; import hudson.util.LogTaskListener; import hudson.util.MultipartFormDataParser; import hudson.util.Scrambler; import hudson.util.Secret; +import hudson.util.StreamTaskListener; import hudson.util.TimeUnit2; import java.io.BufferedOutputStream; import java.io.BufferedReader; import java.io.File; +import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.FileReader; +import java.io.FilterOutputStream; import java.io.IOException; import java.io.InterruptedIOException; import java.io.OutputStream; @@ -128,6 +134,12 @@ import java.util.Set; import java.util.StringTokenizer; import java.util.UUID; +import java.util.concurrent.AbstractExecutorService; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.LogRecord; import java.util.logging.Logger; @@ -176,9 +188,13 @@ import org.tmatesoft.svn.core.wc.SVNWCClient; import org.tmatesoft.svn.core.wc.SVNWCUtil; +import com.google.common.collect.Lists; +import com.google.common.collect.Lists; +import com.google.common.collect.Lists; import com.trilead.ssh2.DebugLogger; import com.trilead.ssh2.SCPClient; import com.trilead.ssh2.crypto.Base64; + import javax.annotation.Nonnull; /** @@ -201,7 +217,21 @@ */ @SuppressWarnings("rawtypes") public class SubversionSCM extends SCM implements Serializable { - /** + + private static final int MAX_CHECKOUT_THREADS; + static { + String prop = System.getProperty("jenkins.subversion-plugin.maxCheckoutThreads", "4"); + int nr; + try { + nr = Integer.parseInt(prop); + } catch (NumberFormatException e) { + nr = 4; + } + + MAX_CHECKOUT_THREADS = nr > 0 ? nr : 1; + } + + /** * the locations field is used to store all configured SVN locations (with * their local and remote part). Direct access to this field should be * avoided and the getLocations() method should be used instead. This is @@ -872,7 +902,7 @@ public boolean requiresWorkspaceForPolling() { * if the operation failed. Otherwise the set of local workspace paths * (relative to the workspace root) that has loaded due to svn:external. */ - private List checkout(Run build, FilePath workspace, TaskListener listener, EnvVars env) throws IOException, InterruptedException { + private List checkout(final Run build, final FilePath workspace, final TaskListener listener, final EnvVars env) throws IOException, InterruptedException { if (repositoryLocationsNoLongerExist(build, listener, env)) { Run lsb = build.getParent().getLastSuccessfulBuild(); if (build instanceof AbstractBuild && lsb != null && build.getNumber()-lsb.getNumber()>10 @@ -889,21 +919,54 @@ private List checkout(Run build, FilePath workspace, TaskListener list } } - List externals = new ArrayList(); + final List externals = Collections.synchronizedList(new ArrayList()); Set unauthenticatedRealms = new LinkedHashSet(); - for (ModuleLocation location : getLocations(env, build)) { - CheckOutTask checkOutTask = - new CheckOutTask(build, this, location, build.getTimestamp().getTime(), listener, env); - externals.addAll(workspace.act(checkOutTask)); - unauthenticatedRealms.addAll(checkOutTask.getUnauthenticatedRealms()); - // olamy: remove null check at it cause test failure - // see https://github.com/jenkinsci/subversion-plugin/commit/de23a2b781b7b86f41319977ce4c11faee75179b#commitcomment-1551273 - /*if ( externalsFound != null ){ - externals.addAll(externalsFound); - } else { - externals.addAll( new ArrayList( 0 ) ); - }*/ + + ModuleLocation[] expandedLocations = getLocations(env, build); + + checkForLocationDuplicatesOrOverlaps(expandedLocations,listener); + + int numberOfExecutors = Math.min(MAX_CHECKOUT_THREADS, expandedLocations.length); + + final ExecutorService service; + + if (numberOfExecutors > 1) { + service = Executors.newFixedThreadPool(numberOfExecutors); + listener.getLogger().println("checking out/updating with " + numberOfExecutors + " parallel threads"); + } else { + service = new CurrentThreadExecutorService(); + } + + @SuppressWarnings("deprecation") + final TaskListener syncedListener = new StreamTaskListener(new SynchronizedPrintStream(listener.getLogger())); + + final Set synchronizedUnauthenticatedRealms = Collections.synchronizedSet(unauthenticatedRealms); + + List> callables = Lists.newArrayListWithExpectedSize(expandedLocations.length); + for (final ModuleLocation location : expandedLocations) { + callables.add(new java.util.concurrent.Callable() { + + public Void call() throws Exception { + CheckOutTask checkOutTask = + new CheckOutTask(build, SubversionSCM.this, location, build.getTimestamp().getTime(), syncedListener, env); + externals.addAll(workspace.act(checkOutTask)); + synchronizedUnauthenticatedRealms.addAll(checkOutTask.getUnauthenticatedRealms()); + return null; + } + }); + } + + List> futures = service.invokeAll(callables); + for (Future future : futures) { + + try { + future.get(); + } catch (ExecutionException e) { + throw new IOException(e); + } } + service.shutdownNow(); + if (additionalCredentials != null) { for (AdditionalCredentials c : additionalCredentials) { unauthenticatedRealms.remove(c.getRealm()); @@ -933,6 +996,127 @@ private List checkout(Run build, FilePath workspace, TaskListener list return externals; } + + /** + * Checks that there a no 2 locations which try to checkout to the same local location, + * as that may cause E200030: BUSY errors. + */ + static boolean checkForLocationDuplicatesOrOverlaps(ModuleLocation[] expandedLocations, + TaskListener listener) { + if (expandedLocations.length < 2) { + return false; + } + + Set localPaths = new HashSet(); + for (ModuleLocation loc : expandedLocations) { + + String locationPath = loc.getLocalDir(); + if (!locationPath.endsWith("/")) { + locationPath += "/"; + } + + for (String path : localPaths) { + if (path.startsWith(locationPath) || locationPath.startsWith(path)) { + listener.getLogger().println("WARN: you have 2 repository locations with overlapping local checkout paths: " + +path+", "+locationPath + +".\n This may cause subsequent errors - e.g. E200030: BUSY"); + return true; + } + } + + localPaths.add(locationPath); + } + + return false; + } + + /** + * {@link PrintStream} which is synchronized on line level. + * + * @author ckutz + */ + private static class SynchronizedPrintStream extends PrintStream { + + public SynchronizedPrintStream(OutputStream out) { + super(out); + } + + @Override + public synchronized void println(boolean x) { + super.println(x); + } + + @Override + public synchronized void println(char x) { + super.println(x); + } + + @Override + public synchronized void println(int x) { + super.println(x); + } + + @Override + public synchronized void println(long x) { + super.println(x); + } + + @Override + public synchronized void println(float x) { + super.println(x); + } + + @Override + public synchronized void println(double x) { + super.println(x); + } + + @Override + public synchronized void println(char[] x) { + super.println(x); + } + + @Override + public synchronized void println(String x) { + super.println(x); + } + + @Override + public synchronized void println(Object x) { + super.println(x); + } + } + + private static class CurrentThreadExecutorService extends AbstractExecutorService { + + private boolean terminated = false; + + public void shutdown() { + terminated = true; + } + + public List shutdownNow() { + terminated = true; + return Collections.emptyList(); + } + + public boolean isShutdown() { + return terminated; + } + + public boolean isTerminated() { + return terminated; + } + + public boolean awaitTermination(long timeout, TimeUnit unit) + throws InterruptedException { + return true; + } + + public void execute(Runnable command) { + command.run(); + } + } private synchronized Map> getProjectExternalsCache() { if (projectExternalsCache == null) { diff --git a/src/main/java/hudson/scm/subversion/CheckoutUpdater.java b/src/main/java/hudson/scm/subversion/CheckoutUpdater.java index 847291b6c..835e998da 100755 --- a/src/main/java/hudson/scm/subversion/CheckoutUpdater.java +++ b/src/main/java/hudson/scm/subversion/CheckoutUpdater.java @@ -31,7 +31,6 @@ import hudson.scm.SubversionSCM.External; import hudson.scm.SubversionWorkspaceSelector; import hudson.util.IOException2; -import hudson.util.StreamCopyThread; import org.apache.commons.lang.time.FastDateFormat; import org.kohsuke.stapler.DataBoundConstructor; @@ -45,8 +44,11 @@ import org.tmatesoft.svn.core.wc2.SvnCheckout; import org.tmatesoft.svn.core.wc2.SvnTarget; +import java.io.BufferedReader; import java.io.File; import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; import java.io.PipedInputStream; import java.io.PipedOutputStream; import java.io.PrintStream; @@ -82,6 +84,7 @@ public List perform() throws IOException, InterruptedException { // buffer the output by a separate thread so that the update operation // won't be blocked by the remoting of the data PipedOutputStream pos = new PipedOutputStream(); + StreamCopyThread sct = new StreamCopyThread("svn log copier", new PipedInputStream(pos), listener.getLogger()); sct.start(); @@ -140,6 +143,43 @@ public List perform() throws IOException, InterruptedException { } }; } + + /** + * {@link Thread} that copies an {@link InputStream} line wise to a {@link PrintStream}. + * + * @author Kohsuke Kawaguchi + * @author Christoph Kutzinski + */ + private static class StreamCopyThread extends Thread { + private final BufferedReader in; + private final PrintStream out; + + public StreamCopyThread(String threadName, InputStream in, PrintStream out) { + super(threadName); + this.in = new BufferedReader(new InputStreamReader(in)); + if (out == null) { + throw new NullPointerException("out is null"); + } + this.out = out; + } + + @Override + public void run() { + try { + try { + String line; + while ((line = in.readLine()) != null) + out.println(line); + } finally { + // it doesn't make sense not to close InputStream that's already EOF-ed, + // so there's no 'closeIn' flag. + in.close(); + } + } catch (IOException e) { + e.printStackTrace(out); + } + } + } @Extension public static class DescriptorImpl extends WorkspaceUpdaterDescriptor { diff --git a/src/test/java/hudson/scm/SubversionSCMUnitTest.java b/src/test/java/hudson/scm/SubversionSCMUnitTest.java index f745a12f0..829eef36e 100644 --- a/src/test/java/hudson/scm/SubversionSCMUnitTest.java +++ b/src/test/java/hudson/scm/SubversionSCMUnitTest.java @@ -12,14 +12,17 @@ import hudson.model.AbstractBuild; import hudson.remoting.VirtualChannel; import hudson.scm.SubversionSCM.ModuleLocation; +import hudson.util.StreamTaskListener; import java.io.IOException; import java.util.HashMap; import java.util.Map; +import org.hamcrest.CoreMatchers; import org.junit.Assert; import org.junit.Test; import org.jvnet.hudson.test.Bug; +import org.jvnet.hudson.test.Url; /** * Unit tests for {@link SubversionSCM}. @@ -105,6 +108,32 @@ public void shouldSetEnvironmentVariablesWithMultipleSvnModules() throws IOExcep assertThat(envVars.get("SVN_REVISION_2"), is("42")); } + @Test + @Url("https://github.com/jenkinsci/subversion-plugin/pull/110#issuecomment-73030817") + public void shouldDetectOverlappingModules() { + ModuleLocation[] locations = new ModuleLocation[2]; + locations[0] = new ModuleLocation("", null, "/trunk", null, false); + + locations[1] = new ModuleLocation("", null, "/trunk/someSubPath/", null, false); + + boolean foundDuplicates = SubversionSCM.checkForLocationDuplicatesOrOverlaps(locations, new StreamTaskListener(System.out)); + + assertThat(foundDuplicates, is(true)); + } + + @Test + @Url("https://github.com/jenkinsci/subversion-plugin/pull/110#issuecomment-73030817") + public void shouldNotBeConfusedByPartialDirectoryNameMatch() { + ModuleLocation[] locations = new ModuleLocation[2]; + locations[0] = new ModuleLocation("", null, "/trunk/someSubPath", null, false); + + locations[1] = new ModuleLocation("", null, "/trunk/someSubPath_and_more/", null, false); + + boolean foundDuplicates = SubversionSCM.checkForLocationDuplicatesOrOverlaps(locations, new StreamTaskListener(System.out)); + + assertThat(foundDuplicates, is(false)); + } + private SubversionSCM mockSCMForBuildEnvVars() { SubversionSCM scm = mock(SubversionSCM.class); doCallRealMethod().when(scm).buildEnvVars(any(AbstractBuild.class), anyMapOf(String.class, String.class));