diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/DescribeCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/DescribeCommandTest.java index a422ef91c..f2093e394 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/DescribeCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/DescribeCommandTest.java @@ -44,6 +44,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import java.io.File; @@ -155,6 +156,11 @@ public void testDescribe() throws Exception { public void testDescribeMultiMatch() throws Exception { ObjectId c1 = modify("aaa"); tag("v1.0.0"); + tick(); + tag("v1.0.1"); + tick(); + tag("v1.1.0"); + tick(); tag("v1.1.1"); ObjectId c2 = modify("bbb"); @@ -164,22 +170,39 @@ public void testDescribeMultiMatch() throws Exception { return; } - // Ensure that if we're interested in any tags, we get the first match - // as per Git behaviour - assertEquals("v1.0.0", describe(c1)); - assertEquals("v1.0.0-1-g3747db3", describe(c2)); + // Ensure that if we're interested in any tags, we get the most recent tag + // as per Git behaviour since 1.7.1.1 + if (useAnnotatedTags) { + assertEquals("v1.1.1", describe(c1)); + assertEquals("v1.1.1-1-gb89dead", describe(c2)); + // Ensure that if we're only interested in one of multiple tags, we get the right match + assertEquals("v1.0.1", describe(c1, "v1.0*")); + assertEquals("v1.1.1", describe(c1, "v1.1*")); + assertEquals("v1.0.1-1-gb89dead", describe(c2, "v1.0*")); + assertEquals("v1.1.1-1-gb89dead", describe(c2, "v1.1*")); - // Ensure that if we're only interested in one of multiple tags, we get the right match - assertEquals("v1.0.0", describe(c1, "v1.0*")); - assertEquals("v1.1.1", describe(c1, "v1.1*")); - assertEquals("v1.0.0-1-g3747db3", describe(c2, "v1.0*")); - assertEquals("v1.1.1-1-g3747db3", describe(c2, "v1.1*")); + // Ensure that ordering of match precedence is preserved as per Git behaviour + assertEquals("v1.1.1", describe(c1, "v1.0*", "v1.1*")); + assertEquals("v1.1.1", describe(c1, "v1.1*", "v1.0*")); + assertEquals("v1.1.1-1-gb89dead", describe(c2, "v1.0*", "v1.1*")); + assertEquals("v1.1.1-1-gb89dead", describe(c2, "v1.1*", "v1.0*")); + } else { + // no timestamps so no guarantees on which tag is chosen + assertNotNull(describe(c1)); + assertNotNull(describe(c2)); - // Ensure that ordering of match precedence is preserved as per Git behaviour - assertEquals("v1.0.0", describe(c1, "v1.0*", "v1.1*")); - assertEquals("v1.1.1", describe(c1, "v1.1*", "v1.0*")); - assertEquals("v1.0.0-1-g3747db3", describe(c2, "v1.0*", "v1.1*")); - assertEquals("v1.1.1-1-g3747db3", describe(c2, "v1.1*", "v1.0*")); + assertNotNull(describe(c1, "v1.0*")); + assertNotNull(describe(c1, "v1.1*")); + assertNotNull(describe(c2, "v1.0*")); + assertNotNull(describe(c2, "v1.1*")); + + // Ensure that ordering of match precedence is preserved as per Git behaviour + assertNotNull(describe(c1, "v1.0*", "v1.1*")); + assertNotNull(describe(c1, "v1.1*", "v1.0*")); + assertNotNull(describe(c2, "v1.0*", "v1.1*")); + assertNotNull(describe(c2, "v1.1*", "v1.0*")); + + } } /** diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/DescribeCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/DescribeCommand.java index 4d5e49957..a484742e0 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/DescribeCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/DescribeCommand.java @@ -50,10 +50,12 @@ import java.util.Collection; import java.util.Collections; import java.util.Comparator; +import java.util.Date; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.api.errors.JGitInternalException; @@ -71,6 +73,7 @@ import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevFlag; import org.eclipse.jgit.revwalk.RevFlagSet; +import org.eclipse.jgit.revwalk.RevTag; import org.eclipse.jgit.revwalk.RevWalk; /** @@ -224,24 +227,40 @@ public DescribeCommand setMatch(String... patterns) throws InvalidPatternExcepti return this; } + private final Comparator TAG_TIE_BREAKER = new Comparator() { + + @Override + public int compare(Ref o1, Ref o2) { + try { + return tagDate(o2).compareTo(tagDate(o1)); + } catch (IOException e) { + return 0; + } + } + + private Date tagDate(Ref tag) throws IOException { + RevTag t = w.parseTag(tag.getObjectId()); + w.parseBody(t); + return t.getTaggerIdent().getWhen(); + } + }; + private Optional getBestMatch(List tags) { if (tags == null || tags.size() == 0) { return Optional.empty(); } else if (matchers.size() == 0) { - // No matchers, simply return the first tag entry + Collections.sort(tags, TAG_TIE_BREAKER); return Optional.of(tags.get(0)); } else { - // Find the first tag that matches one of the matchers; precedence according to matcher definition order + // Find the first tag that matches in the stream of all tags + // filtered by matchers ordered by tie break order + Stream matchingTags = Stream.empty(); for (IMatcher matcher : matchers) { - Optional match = tags.stream() - .filter(tag -> matcher.matches(tag.getName(), false, - false)) - .findFirst(); - if (match.isPresent()) { - return match; - } + Stream m = tags.stream().filter( + tag -> matcher.matches(tag.getName(), false, false)); + matchingTags = Stream.of(matchingTags, m).flatMap(i -> i); } - return Optional.empty(); + return matchingTags.sorted(TAG_TIE_BREAKER).findFirst(); } }