Merge pull request #262 from didiez/fix-repeated-SubversionTagAction

[JENKINS-35176] Fixes repeated same "Tag this build" links added to builds.

Author: didiez

src/main/java/hudson/scm/SubversionSCM.java

@@ -161,8 +161,6 @@
 import org.apache.tools.ant.BuildException;
 import org.apache.tools.ant.Project;
 import org.apache.tools.ant.taskdefs.Chmod;
-import org.kohsuke.accmod.Restricted;
-import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.stapler.AncestorInPath;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.QueryParameter;
@@ -192,10 +190,12 @@
 import com.trilead.ssh2.DebugLogger;
 import com.trilead.ssh2.SCPClient;
 import com.trilead.ssh2.crypto.Base64;
+import static java.util.stream.Collectors.toList;
 import javax.annotation.Nonnull;
 import jenkins.MasterToSlaveFileCallable;
 import jenkins.security.Roles;
 import jenkins.security.SlaveToMasterCallable;
+import org.apache.commons.collections.CollectionUtils;
 import org.jenkinsci.remoting.RoleChecker;
 import org.kohsuke.stapler.interceptor.RequirePOST;
 
@@ -882,18 +882,18 @@ public boolean requiresWorkspaceForPolling() {
           }
         }
 
+        List<SvnInfoP> pList = workspace.act(new BuildRevisionMapTask(build, this, listener, externalsForAll, env));
+        List<SvnInfo> revList = pList.stream().map(svnInfoP -> svnInfoP.info).collect(toList());
+
         // write out the revision file
         try (PrintWriter w = new PrintWriter(new FileOutputStream(getRevisionFile(build)))) {
-            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)
+                if (p.pinned) {
                     w.println( p.info.url +'/'+ p.info.revision + "::p");
-                else
+                } else {
                     w.println( p.info.url +'/'+ p.info.revision);
-                revList.add(p.info);
+                }                        
             }
-            build.addAction(new SubversionTagAction(build,revList));
         }
 
         // write out the externals info
@@ -903,8 +903,16 @@ public boolean requiresWorkspaceForPolling() {
             projectExternalsCache.put(build.getParent(), externalsForAll);
         }
 
-        if (changelogFile != null) {
-            calcChangeLog(build, workspace, changelogFile, baseline, listener, externalsMap, env);
+        // check if a SubversionTagAction with the same scm changes has been already added in a previous checkout/update in the build 
+        boolean scmChangesAlreadyProcessed = build.getActions(SubversionTagAction.class).stream()
+                .anyMatch(existingTagAction -> CollectionUtils.isEqualCollection(existingTagAction.getTags().keySet(), revList));
+
+        // Don't add the tag and changelog if no changelog was requested or we've already processed this scm changes before
+        if(changelogFile != null && !scmChangesAlreadyProcessed) {    
+            // add the tag action
+            build.addAction(new SubversionTagAction(build,revList));            
+            // write out the changelog file
+            calcChangeLog(build, workspace, changelogFile, baseline, listener, externalsMap, env);         
         }
     }
 

src/test/java/jenkins/scm/impl/subversion/SubversionStepTest.java

@@ -26,6 +26,7 @@
 import hudson.scm.ChangeLogSet;
 import hudson.scm.SCM;
 import hudson.scm.SubversionSCM;
+import hudson.scm.SubversionTagAction;
 import hudson.triggers.SCMTrigger;
 import java.util.Iterator;
 import java.util.List;
@@ -38,6 +39,7 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.BuildWatcher;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 
 public class SubversionStepTest {
@@ -116,6 +118,44 @@ public void multipleSCMs() throws Exception {
         assertFalse(iterator.hasNext());
     }
 
+    @Issue("JENKINS-38204") 
+    @Test
+    public void identicalSCMs() throws Exception {
+        sampleRepo.init();
+        WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "demo");
+        p.setDefinition(new CpsFlowDefinition(
+            "node {\n" +
+            "    dir('main') {\n" +
+            "        svn(url: '" + sampleRepo.trunkUrl() + "')\n" +
+            "    }\n" +
+            "    dir('other') {\n" +
+            "        svn(url: '" + sampleRepo.trunkUrl() + "')\n" +
+            "    }\n" +
+            "}", true));
+        
+        WorkflowRun b = r.assertBuildStatusSuccess(p.scheduleBuild2(0));
+        assertEquals(1, b.getActions(SubversionTagAction.class).size());
+        assertEquals(0, b.getChangeSets().size());
+        assertEquals(1, p.getSCMs().size());
+
+        // add a file and commit
+        sampleRepo.write("otherfile", "");
+        sampleRepo.svnkit("add", sampleRepo.wc() + "/" + "otherfile");
+        sampleRepo.svnkit("commit", "--message=+otherfile", sampleRepo.wc());
+        
+        WorkflowRun b2 = r.assertBuildStatusSuccess(p.scheduleBuild2(0));
+        
+        // there should by only one tag action and changeset
+        assertEquals(1, b2.getActions(SubversionTagAction.class).size());
+        assertEquals(1, b2.getChangeSets().size());
+        
+        // check items of the only changeset
+        ChangeLogSet changeSet = b2.getChangeSets().get(0);
+        assertFalse(changeSet.isEmptySet());
+        assertEquals(1, changeSet.getItems().length);
+        
+        assertEquals(1, p.getSCMs().size());
+    }
 
     @Test
     public void checkoutDepthAsItIsInfiniteTest() throws Exception {