[JENKINS-45801] and [JENKINS-49219], when calculate change log of ext…

…ernals, add module location's credentials as default credentials (#212) * when calculate change log of externals add module location's credentials as default credentials, fix [JENKINS-45801] and [JENKINS-49219]
jenkinsci · May 17, 2018 · 9547f74 · 9547f74 · bobbotron · May 22, 2018
1 parent 3c34c83
commit 9547f74
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 27 deletions.
diff --git a/src/main/java/hudson/scm/SubversionChangeLogBuilder.java b/src/main/java/hudson/scm/SubversionChangeLogBuilder.java
@@ -55,8 +55,8 @@
 import java.io.PrintStream;
 import java.io.File;
 import java.io.Serializable;
+import java.util.List;
 import java.util.Map;
-import java.util.Collection;
 import javax.annotation.Nonnull;
 import jenkins.MasterToSlaveFileCallable;
 
@@ -101,7 +101,7 @@ public SubversionChangeLogBuilder(Run<?,?> build, FilePath workspace, @Nonnull S
         this.env = env;
     }
 
-    public boolean run(Collection<SubversionSCM.External> externals, Result changeLog) throws IOException, InterruptedException {
+    public boolean run(@Nonnull Map<String, List<SubversionSCM.External>> externalsMap, Result changeLog) throws IOException, InterruptedException {
         boolean changelogFileCreated = false;
 
         TransformerHandler th = createTransformerHandler();
@@ -122,24 +122,20 @@ public boolean run(Collection<SubversionSCM.External> externals, Result changeLo
                 PathContext context = getUrlForPath(workspace.child(l.getLocalDir()), authProvider);
                 context.moduleWorkspacePath = l.getLocalDir();
                 changelogFileCreated |= buildModule(context, svnlc, logHandler);
+
+                // externals for this module location
+                List<SubversionSCM.External> externals = externalsMap.get(l.remote);
+                if (externals != null) {
+                  for (SubversionSCM.External ext : externals) {
+                    PathContext extContext = getUrlForPath(workspace.child(ext.path), authProvider);
+                    extContext.moduleWorkspacePath = ext.path;
+                    changelogFileCreated |= buildModule(extContext, svnlc, logHandler);
+                  }
+                }
             } finally {
                 manager.dispose();
             }
         }
-        ISVNAuthenticationProvider authProvider =
-                CredentialsSVNAuthenticationProviderImpl
-                        .createAuthenticationProvider(build.getParent(), scm, null, listener);
-        final SVNClientManager manager = SubversionSCM.createClientManager(authProvider).getCore();
-        try {
-            SVNLogClient svnlc = manager.getLogClient();
-            for(SubversionSCM.External ext : externals) {
-                PathContext context = getUrlForPath(workspace.child(ext.path), authProvider);
-                context.moduleWorkspacePath = ext.path;
-                changelogFileCreated |= buildModule(context, svnlc, logHandler);
-            }
-        } finally {
-            manager.dispose();
-        }
 
         if(changelogFileCreated) {
             logHandler.endDocument();

diff --git a/src/main/java/hudson/scm/SubversionSCM.java b/src/main/java/hudson/scm/SubversionSCM.java
@@ -741,7 +741,7 @@ public void buildEnvironment(Run<?, ?> build, Map<String, String> env) {
     /**
      * Called after checkout/update has finished to compute the changelog.
      */
-    private void calcChangeLog(Run<?,?> build, FilePath workspace, File changelogFile, SCMRevisionState baseline, TaskListener listener, List<External> externals, EnvVars env) throws IOException, InterruptedException {
+    private void calcChangeLog(Run<?,?> build, FilePath workspace, File changelogFile, SCMRevisionState baseline, TaskListener listener, Map<String, List<SubversionSCM.External>> externalsMap, EnvVars env) throws IOException, InterruptedException {
         if (baseline == null) {
             // nothing to compare against
             createEmptyChangeLog(changelogFile, listener, "log");
@@ -754,7 +754,7 @@ private void calcChangeLog(Run<?,?> build, FilePath workspace, File changelogFil
         OutputStream os = new BufferedOutputStream(new FileOutputStream(changelogFile));
         boolean created;
         try {
-            created = new SubversionChangeLogBuilder(build, workspace, (SVNRevisionState) baseline, env, listener, this).run(externals, new StreamResult(os));
+            created = new SubversionChangeLogBuilder(build, workspace, (SVNRevisionState) baseline, env, listener, this).run(externalsMap, new StreamResult(os));
         } finally {
             os.close();
         }
@@ -861,13 +861,19 @@ public boolean requiresWorkspaceForPolling() {
             EnvVarsUtils.overrideAll(env, ((AbstractBuild) build).getBuildVariables());
         }
 
-        List<External> externals = null;
-            externals = checkout(build,workspace,listener,env);
+        Map<String, List<External>> externalsMap = checkout(build,workspace,listener,env);
+
+        List<External> externalsForAll = new ArrayList<>();
+        if (externalsMap != null) {
+          for (String moduleLocationRemote : externalsMap.keySet()) {
+            externalsForAll.addAll(externalsMap.get(moduleLocationRemote));
+          }
+        }
 
         // write out the revision file
         PrintWriter w = new PrintWriter(new FileOutputStream(getRevisionFile(build)));
         try {
-            List<SvnInfoP> pList = workspace.act(new BuildRevisionMapTask(build, this, listener, externals, env));
+            List<SvnInfoP> pList = workspace.act(new BuildRevisionMapTask(build, this, listener, externalsForAll, env));
             List<SvnInfo> revList= new ArrayList<SvnInfo>(pList.size());
             for (SvnInfoP p: pList) {
                 if (p.pinned)
@@ -882,14 +888,14 @@ public boolean requiresWorkspaceForPolling() {
         }
 
         // write out the externals info
-        SvnExternalsFileManager.writeExternalsFile(build.getParent(), externals);
+        SvnExternalsFileManager.writeExternalsFile(build.getParent(), externalsForAll);
         Map<Job, List<External>> projectExternalsCache = getProjectExternalsCache();
         synchronized (projectExternalsCache) {
-            projectExternalsCache.put(build.getParent(), externals);
+            projectExternalsCache.put(build.getParent(), externalsForAll);
         }
 
         if (changelogFile != null) {
-            calcChangeLog(build, workspace, changelogFile, baseline, listener, externals, env);
+            calcChangeLog(build, workspace, changelogFile, baseline, listener, externalsMap, env);
         }
     }
 
@@ -904,7 +910,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<External> checkout(Run build, FilePath workspace, TaskListener listener, EnvVars env) throws IOException, InterruptedException {
+    private Map<String, List<External>> checkout(Run build, FilePath workspace, TaskListener listener, 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
@@ -921,12 +927,16 @@ private List<External> checkout(Run build, FilePath workspace, TaskListener list
             return null;
         }
 
-        List<External> externals = new ArrayList<External>();
+        Map<String, List<External>> externalsMap = new HashMap<>();
+
         Set<String> unauthenticatedRealms = new LinkedHashSet<String>();
         for (ModuleLocation location : getLocations(env, build)) {
             CheckOutTask checkOutTask =
                     new CheckOutTask(build, this, location, build.getTimestamp().getTime(), listener, env, quietOperation);
+            List<External> externals = new ArrayList<External>();
             externals.addAll(workspace.act(checkOutTask));
+            // save location <---> externals maps
+            externalsMap.put(location.remote, externals);
             unauthenticatedRealms.addAll(checkOutTask.getUnauthenticatedRealms());
             // olamy: remove null check at it cause test failure
             // see https://github.com/jenkinsci/subversion-plugin/commit/de23a2b781b7b86f41319977ce4c11faee75179b#commitcomment-1551273
@@ -963,7 +973,7 @@ private List<External> checkout(Run build, FilePath workspace, TaskListener list
             }
         }
 
-        return externals;
+        return externalsMap;
     }
 
     private synchronized Map<Job, List<External>> getProjectExternalsCache() {