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 79da2da7e..a2d8e8969 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; @@ -134,24 +135,48 @@ 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"); - // 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.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 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 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*")); + // Ensure that ordering of match precedence is preserved as per Git behaviour + assertEquals("v1.0.1", describe(c1, "v1.0*", "v1.1*")); + assertEquals("v1.1.1", describe(c1, "v1.1*", "v1.0*")); + assertEquals("v1.0.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)); + + 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 01fe4aa9e..18e8a7f4f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/DescribeCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/DescribeCommand.java @@ -50,6 +50,7 @@ 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; @@ -71,6 +72,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; /** @@ -203,11 +205,29 @@ 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 @@ -215,6 +235,7 @@ private Optional getBestMatch(List tags) { Optional match = tags.stream() .filter(tag -> matcher.matches(tag.getName(), false, false)) + .sorted(TAG_TIE_BREAKER) .findFirst(); if (match.isPresent()) { return match;