[FIXED JENKINS-35475] Relax build data present check to allow for dif…

…ferent remote names - Also fix side-panel on BuildData - Also make it possible to access the multiple BuildData actions
jenkinsci · Mar 9, 2017 · b8f94df · b8f94df
1 parent d1ee85b
commit b8f94df
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 7 deletions.
diff --git a/src/main/java/hudson/plugins/git/GitSCM.java b/src/main/java/hudson/plugins/git/GitSCM.java
@@ -1103,7 +1103,17 @@ public void checkout(Run<?, ?> build, Launcher launcher, FilePath workspace, Tas
 
         // Track whether we're trying to add a duplicate BuildData, now that it's been updated with
         // revision info for this build etc. The default assumption is that it's a duplicate.
-        boolean buildDataAlreadyPresent = build.getActions(BuildData.class).contains(buildData);
+        boolean buildDataAlreadyPresent = false;
+        List<BuildData> actions = build.getActions(BuildData.class);
+        for (BuildData d: actions)  {
+            if (d.similarTo(buildData)) {
+                buildDataAlreadyPresent = true;
+                break;
+            }
+        }
+        if (!actions.isEmpty()) {
+            buildData.setIndex(actions.size()+1);
+        }
 
         // If the BuildData is not already attached to this build, add it to the build and mark that
         // it wasn't already present, so that we add the GitTagAction and changelog after the checkout
@@ -1144,7 +1154,11 @@ public void checkout(Run<?, ?> build, Launcher launcher, FilePath workspace, Tas
 
         // Don't add the tag and changelog if we've already processed this BuildData before.
         if (!buildDataAlreadyPresent) {
-            build.addAction(new GitTagAction(build, workspace, revToBuild.revision));
+            if (build.getActions(AbstractScmTagAction.class).isEmpty()) {
+                // only add the tag action if we can be unique as AbstractScmTagAction has a fixed UrlName
+                // so only one of the actions is addressible by users
+                build.addAction(new GitTagAction(build, workspace, revToBuild.revision));
+            }
 
             if (changelogFile != null) {
                 computeChangeLog(git, revToBuild.revision, listener, previousBuildData, new FilePath(changelogFile),

diff --git a/src/main/java/hudson/plugins/git/util/BuildData.java b/src/main/java/hudson/plugins/git/util/BuildData.java
@@ -4,16 +4,25 @@
 import hudson.model.AbstractBuild;
 import hudson.model.Action;
 import hudson.model.Api;
+import hudson.model.Run;
 import hudson.plugins.git.Branch;
 import hudson.plugins.git.Revision;
 import hudson.plugins.git.UserRemoteConfig;
+import java.io.Serializable;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.IdentityHashMap;
+import java.util.Map;
+import java.util.Set;
 import org.eclipse.jgit.lib.ObjectId;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
+import org.kohsuke.stapler.Stapler;
+import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.export.Exported;
 import org.kohsuke.stapler.export.ExportedBean;
 
-import java.io.Serializable;
-import java.util.*;
-
 import static hudson.Util.fixNull;
 /**
  * Captures the Git related information for a build.
@@ -50,6 +59,11 @@ public class BuildData implements Action, Serializable, Cloneable {
      */
     public Set<String> remoteUrls = new HashSet<>();
 
+    /**
+     * Allow disambiguation of the action url when multiple {@link BuildData} actions present.
+     */
+    private Integer index;
+
     public BuildData() {
     }
 
@@ -84,7 +98,25 @@ public String getIconFileName() {
     }
 
     public String getUrlName() {
-        return "git";
+        return index == null ? "git" : "git-"+index;
+    }
+
+    public void setIndex(Integer index) {
+        this.index = index;
+    }
+
+    public Integer getIndex() {
+        return index;
+    }
+
+    @Restricted(NoExternalUse.class) // only used from stapler/jelly
+    @CheckForNull
+    public Run<?,?> getOwningRun() {
+        StaplerRequest req = Stapler.getCurrentRequest();
+        if (req == null) {
+            return null;
+        }
+        return req.findAncestorObject(Run.class);
     }
 
     public Object readResolve() {
@@ -243,6 +275,57 @@ public String toString() {
                 ",lastBuild="+lastBuild+"]";
     }
 
+    /**
+     * Like {@link #equals(Object)} but doesn't check the branch names as strictly  as those can vary depending on the
+     * configured remote name.
+     *
+     * @param that the {@link BuildData} to compare with.
+     * @return {@code true} if the supplied {@link BuildData} is similar to this {@link BuildData}.
+     * @since TODO
+     */
+    public boolean similarTo(BuildData that) {
+        if (this.remoteUrls == null ? that.remoteUrls != null : !this.remoteUrls.equals(that.remoteUrls)) {
+            return false;
+        }
+        if (this.lastBuild == null ? that.lastBuild != null : !this.lastBuild.equals(that.lastBuild)) {
+            return false;
+        }
+        if (this.remoteUrls == null || that.remoteUrls == null) {
+            // if either is null then we do not need the costly comparison
+            return this.remoteUrls == that.remoteUrls;
+        }
+        int thisSize = this.remoteUrls.size();
+        int thatSize = that.remoteUrls.size();
+        if (thisSize != thatSize) {
+            return false;
+        }
+        // assume if there is a prefix/ that the prefix is the origin name and strip it for similarity comparison
+        // now if branch names contain slashes anyway and the user has not configured an origin name
+        // we could have a false positive... but come on, it's the same repo and the same revision on the same build
+        // that's similar enough. If you had configured a remote name we would see these as origin/feature/foobar and
+        // origin/bugfix/foobar but you have not configured a remote name, and both branches are the same revision
+        // anyway... and on the same build
+        Set<String> thisUrls = new HashSet<>(thisSize);
+        for (String url: this.remoteUrls) {
+            int index = url.indexOf('/');
+            if (index == -1 || index + 1 >= url.length()) {
+                thisUrls.add(url);
+            } else {
+                thisUrls.add(url.substring(index + 1));
+            }
+        }
+        Set<String> thatUrls = new HashSet<>(thatSize);
+        for (String url: that.remoteUrls) {
+            int index = url.indexOf('/');
+            if (index == -1 || index + 1 >= url.length()) {
+                thisUrls.add(url);
+            } else {
+                thisUrls.add(url.substring(index + 1));
+            }
+        }
+        return thatUrls.equals(thatUrls);
+    }
+
     @Override
     public boolean equals(Object o) {
         if (!(o instanceof BuildData)) {

diff --git a/src/main/resources/hudson/plugins/git/util/BuildData/index.jelly b/src/main/resources/hudson/plugins/git/util/BuildData/index.jelly
@@ -2,7 +2,10 @@
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
 	<l:layout title="Git">
 
-
+	  <j:set var="build" value="${it.owningRun}"/>
+		<j:if test="${build!=null}">
+			<st:include page="sidepanel" it="${build}" optional="true"/>
+		</j:if>
     <l:main-panel>
 	<h1>${%Git Build Data}</h1>