diff --git a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java index bad590804c..d6add1dd53 100644 --- a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java +++ b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java @@ -870,6 +870,7 @@ protected SCMRevision retrieve(@NonNull final String revision, @NonNull final Ta Set fullHashMatches = new TreeSet<>(); String fullHashMatch = null; GitRefSCMRevision candidateOtherRef = null; + Boolean shortRevisionAmbiguity = false; for (Map.Entry entry: remoteReferences.entrySet()) { String name = entry.getKey(); String rev = entry.getValue().name(); @@ -923,18 +924,25 @@ protected SCMRevision retrieve(@NonNull final String revision, @NonNull final Ta break; } } - if (rev.toLowerCase(Locale.ENGLISH).startsWith(revision.toLowerCase(Locale.ENGLISH))) { + if (!revision.isEmpty() && rev.toLowerCase(Locale.ENGLISH).startsWith(revision.toLowerCase(Locale.ENGLISH))) { shortNameMatches.add(name); + listener.getLogger().printf("Candidate partial match: %s revision %s%n", name, rev); if (shortHashMatch == null) { - listener.getLogger().printf("Candidate partial match: %s revision %s%n", name, rev); shortHashMatch = rev; } else { - listener.getLogger().printf("Candidate partial match: %s revision %s%n", name, rev); listener.getLogger().printf("Cannot resolve ambiguous short revision %s%n", revision); - return null; + if (fullTagMatches.isEmpty() && fullHashMatches.isEmpty() && fullHashMatch == null && candidateOtherRef == null) { + // We haven't found any matches, and we have ambiguous matches, cannot determine + shortRevisionAmbiguity = true; + } } } } + if (fullHashMatch != null) { + // JENKINS-62592, for predictability, if an exact match is found, that should be returned + //since this would have been skipped if this was a head or a tag we can just return whatever + return new GitRefSCMRevision(new GitRefSCMHead(fullHashMatch, fullHashMatches.iterator().next()), fullHashMatch); + } if (!fullTagMatches.isEmpty()) { // we just want a tag so we can do a minimal fetch String name = StringUtils.removeStart(fullTagMatches.iterator().next(), Constants.R_TAGS); @@ -944,34 +952,6 @@ protected SCMRevision retrieve(@NonNull final String revision, @NonNull final Ta context.wantTags(true); context.withoutRefSpecs(); } - if (fullHashMatch != null) { - //since this would have been skipped if this was a head or a tag we can just return whatever - return new GitRefSCMRevision(new GitRefSCMHead(fullHashMatch, fullHashMatches.iterator().next()), fullHashMatch); - } - if (shortHashMatch != null) { - // woot this seems unambiguous - for (String name: shortNameMatches) { - if (name.startsWith(Constants.R_HEADS)) { - listener.getLogger().printf("Selected match: %s revision %s%n", name, shortHashMatch); - // WIN it's also a branch - return new GitBranchSCMRevision(new GitBranchSCMHead(StringUtils.removeStart(name, Constants.R_HEADS)), - shortHashMatch); - } else if (name.startsWith(Constants.R_TAGS)) { - tagName = StringUtils.removeStart(name, Constants.R_TAGS); - context.wantBranches(false); - context.wantTags(true); - context.withoutRefSpecs(); - } - } - if (tagName != null) { - listener.getLogger().printf("Selected match: %s revision %s%n", tagName, shortHashMatch); - } else { - return new GitRefSCMRevision(new GitRefSCMHead(shortHashMatch, shortNameMatches.iterator().next()), shortHashMatch); - } - } - if (candidateOtherRef != null) { - return candidateOtherRef; - } //if PruneStaleBranches it should take affect on the following retrievals boolean pruneRefs = context.pruneRefs(); if (tagName != null) { @@ -998,6 +978,34 @@ public SCMRevision run(GitClient client, String remoteName) throws IOException, context, listener, pruneRefs, retrieveContext); } + if (shortHashMatch != null) { + // woot this seems unambiguous + for (String name: shortNameMatches) { + if (name.startsWith(Constants.R_HEADS)) { + listener.getLogger().printf("Selected match: %s revision %s%n", name, shortHashMatch); + // WIN it's also a branch + return new GitBranchSCMRevision(new GitBranchSCMHead(StringUtils.removeStart(name, Constants.R_HEADS)), + shortHashMatch); + } else if (name.startsWith(Constants.R_TAGS)) { + tagName = StringUtils.removeStart(name, Constants.R_TAGS); + context.wantBranches(false); + context.wantTags(true); + context.withoutRefSpecs(); + } + } + if (tagName != null) { + listener.getLogger().printf("Selected match: %s revision %s%n", tagName, shortHashMatch); + } else { + return new GitRefSCMRevision(new GitRefSCMHead(shortHashMatch, shortNameMatches.iterator().next()), shortHashMatch); + } + } + if (candidateOtherRef != null) { + return candidateOtherRef; + } + if (shortRevisionAmbiguity) { + // Check if we have any other matches first, if not, return null as we cannot determine a match + return null; + } // Pokémon!... Got to catch them all listener.getLogger().printf("Could not find %s in remote references. " + "Pulling heads to local for deep search...%n", revision); @@ -1015,7 +1023,6 @@ public SCMRevision run(GitClient client, String remoteName) throws IOException, //just to be safe listener.error("Could not resolve %s", revision); return null; - } hash = objectId.name(); String candidatePrefix = Constants.R_REMOTES.substring(Constants.R_REFS.length()) diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index e6b8ccee19..3f678d0d0b 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -8,6 +8,7 @@ import com.cloudbees.plugins.credentials.common.StandardCredentials; import com.cloudbees.plugins.credentials.domains.Domain; import com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsImpl; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.EnvVars; import hudson.FilePath; import hudson.Launcher; @@ -25,6 +26,7 @@ import hudson.plugins.git.extensions.impl.LocalBranch; import hudson.util.StreamTaskListener; import java.io.File; +import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -38,7 +40,6 @@ import jenkins.plugins.git.traits.IgnoreOnPushNotificationTrait; import jenkins.plugins.git.traits.PruneStaleBranchTrait; import jenkins.plugins.git.traits.TagDiscoveryTrait; - import jenkins.scm.api.SCMHead; import jenkins.scm.api.SCMHeadObserver; import jenkins.scm.api.SCMRevision; @@ -53,6 +54,7 @@ import jenkins.scm.api.SCMSourceCriteria; import jenkins.scm.api.SCMSourceOwner; import jenkins.scm.api.metadata.PrimaryInstanceMetadataAction; +import jenkins.scm.api.trait.SCMSourceTrait; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.transport.RefSpec; @@ -425,7 +427,7 @@ public void retrieveByName() throws Exception { listener.getLogger().printf("%n=== fetch('%s') ===%n%n", devHash); rev = source.fetch(devHash, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(devHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(devHash)); assertThat(rev.getHead().getName(), is("dev")); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", devHash.substring(0, 10)); @@ -536,7 +538,7 @@ private void retrievePrimaryHead(boolean duplicatePrimary) throws Exception { } PrimaryInstanceMetadataAction primary = null; - for (Action a: actions) { + for (Action a : actions) { if (a instanceof PrimaryInstanceMetadataAction) { primary = (PrimaryInstanceMetadataAction) a; break; @@ -835,7 +837,7 @@ public void fetchOtherRef() throws Exception { StreamTaskListener listener = StreamTaskListener.fromStderr(); final SCMHeadObserver.Collector collector = - source.fetch((SCMSourceCriteria) (probe, listener1) -> true, new SCMHeadObserver.Collector(), listener); + source.fetch((SCMSourceCriteria) (probe, listener1) -> true, new SCMHeadObserver.Collector(), listener); final Map result = collector.result(); assertThat(result.entrySet(), hasSize(4)); @@ -1157,6 +1159,115 @@ public void when_commits_added_during_discovery_we_do_not_crash() throws Excepti sharedSampleRepo = null; } } + + @Issue("JENKINS-62592") + @Test + public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception { + sampleRepo.init(); + String masterHash = sampleRepo.head(); + sampleRepo.git("checkout", "-b", "dev"); + sampleRepo.write("file", "modified"); + sampleRepo.git("commit", "--all", "--message=dev"); + sampleRepo.git("tag", "v1"); + String v1Hash = sampleRepo.head(); + sampleRepo.write("file", "modified2"); + sampleRepo.git("commit", "--all", "--message=dev2"); + sampleRepo.git("tag", "-a", "v2", "-m", "annotated"); + String v2Hash = sampleRepo.head(); + sampleRepo.write("file", "modified3"); + sampleRepo.git("commit", "--all", "--message=dev3"); + // Grab the devHash, but lets try and generate a hash that we know will cause an issue in our test + ArrayList hashFirstLetter = new ArrayList<>( + Arrays.asList(masterHash.substring(0, 1), v1Hash.substring(0, 1), v2Hash.substring(0, 1)) + ); + sampleRepo.git("tag", "devTag"); + String devTagHash = sampleRepo.head(); + int devTagIteration = 4; // In order to name new files and create new commits + String previousNewHash = null; + String newHash = devTagHash; + while (!hashFirstLetter.contains(devTagHash.substring(0, 1))) { + // Generate a new commit and try again + sampleRepo.git("tag", "devTag" + devTagIteration); + sampleRepo.write("file", "modified" + devTagIteration); + sampleRepo.git("commit", "--all", "--message=dev" + (devTagIteration++)); + previousNewHash = newHash; + newHash = sampleRepo.head(); + hashFirstLetter.add(newHash.substring(0, 1)); + } + GitSCMSource source = new GitSCMSource(sampleRepo.toString()); + source.setTraits(new ArrayList<>()); + + TaskListener listener = StreamTaskListener.fromStderr(); + listener.getLogger().printf("ArrayList of first hash chars: %s", hashFirstLetter); + + // Test existing functionality with additional revisions + listener.getLogger().println("\n=== fetch('master') ===\n"); + SCMRevision rev = source.fetch("master", listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(masterHash)); + listener.getLogger().println("\n=== fetch('v1') ===\n"); + rev = source.fetch("v1", listener, null); + assertThat(rev, instanceOf(GitTagSCMRevision.class)); + assertThat(((GitTagSCMRevision)rev).getHash(), is(v1Hash)); + listener.getLogger().println("\n=== fetch('v2') ===\n"); + rev = source.fetch("v2", listener, null); + assertThat(rev, instanceOf(GitTagSCMRevision.class)); + assertThat(((GitTagSCMRevision)rev).getHash(), is(v2Hash)); + + listener.getLogger().printf("%n=== fetch('%s') ===%n%n", masterHash); + rev = source.fetch(masterHash, listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(masterHash)); + assertThat(rev.getHead().getName(), is("master")); + + // Test new functionality and verify that we are able to grab what we expect (full match versus hazy short hash) + if (!devTagHash.equals(newHash)) { + listener.getLogger().printf("%n=== fetch('%s') ===%n%n", newHash); + rev = source.fetch(newHash, listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(newHash)); + assertThat(rev.getHead().getName(), is("dev")); + + listener.getLogger().printf("%n=== fetch('%s') short hash ===%n%n", newHash.substring(0, 6)); + rev = source.fetch(newHash.substring(0, 6), listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(newHash)); + assertThat(rev.getHead().getName(), is("dev")); + + + int lastDevTag = devTagIteration == 4 ? 4 : devTagIteration - 1; + String headHash = devTagIteration == 4 ? devTagHash : previousNewHash; + listener.getLogger().printf("%n=== fetch(devTag%d) ===%n%n", lastDevTag); + rev = source.fetch("devTag" + lastDevTag, listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(rev.getHead().getName(), is("devTag" + lastDevTag)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(headHash)); + + String ambiguousTag = hashFirstLetter.get(hashFirstLetter.size() - 1); + String ambiguousHash = sampleRepo.head(); + sampleRepo.git("tag", ambiguousTag); + sampleRepo.write("file", "modified and ambiguous"); + sampleRepo.git("commit", "--all", "--message=ambiguousTagCommit"); + listener.getLogger().printf("%n=== fetch(%s) ===%n%n", ambiguousTag); + rev = source.fetch(ambiguousTag, listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(rev.getHead().getName(), is(ambiguousTag)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(ambiguousHash)); + + // Remove ambiguous tag from the repo and thne create a branch with the ambiguous search + sampleRepo.git("tag", "-d", ambiguousTag); + sampleRepo.git("checkout", "-b", ambiguousTag); + sampleRepo.write("file", "modified and ambiguous but a branch this time!"); + sampleRepo.git("commit", "--all", "--message=ambiguousBranchCommit"); + listener.getLogger().printf("%n=== fetch(%s) branch ===%n%n", ambiguousTag); + String ambiguousBranchHash = sampleRepo.head(); + rev = source.fetch(ambiguousTag, listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(rev.getHead().getName(), is(ambiguousTag)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(ambiguousBranchHash)); + } + } + //Ugly but MockGitClient needs to be static and no good way to pass it on static GitSampleRepoRule sharedSampleRepo;