From 5429d1a0cff593bf28186c38fde7e41798a2c0b8 Mon Sep 17 00:00:00 2001 From: Marcel Trautwein Date: Fri, 23 Feb 2018 07:27:52 +0100 Subject: [PATCH] Make JGit describe behaves same as c-git for lightweight tags JGit now considers lightweight tags only if the --tags option is set i.e. `git.describe().setAllTags(true)` has to be set, else the default is now as in c git: Only annotated tags are evaluated unless you pass true equivalent to --tags (or --all) by the option setAllTags. Hint: This (still) doesn't address any difference between c-git `--all` and `!--all --tags` behavior; perhaps this might be a follow up request Bug: 423206 Change-Id: I9a3699756df0b9c6a7c74a7e8887dea0df17c8e7 Signed-off-by: Marcel Trautwein Signed-off-by: Matthias Sohn --- .../eclipse/jgit/api/DescribeCommandTest.java | 181 ++++++++++++++---- .../org/eclipse/jgit/api/DescribeCommand.java | 44 ++++- 2 files changed, 187 insertions(+), 38 deletions(-) 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 79da2da7ea..a422ef91cb 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 @@ -54,7 +54,6 @@ import java.util.Collection; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.api.errors.RefNotFoundException; -import org.eclipse.jgit.errors.InvalidPatternException; import org.eclipse.jgit.junit.RepositoryTestCase; import org.eclipse.jgit.lib.ObjectId; import org.junit.Test; @@ -68,13 +67,18 @@ public class DescribeCommandTest extends RepositoryTestCase { private Git git; - @Parameter + @Parameter(0) public boolean useAnnotatedTags; - @Parameters + @Parameter(1) + public boolean describeUseAllTags; + + @Parameters(name = "git tag -a {0}?-a: with git describe {1}?--tags:") public static Collection getUseAnnotatedTagsValues() { - return Arrays.asList(new Boolean[][] { { Boolean.TRUE }, - { Boolean.FALSE } }); + return Arrays.asList(new Boolean[][] { { Boolean.TRUE, Boolean.FALSE }, + { Boolean.FALSE, Boolean.FALSE }, + { Boolean.TRUE, Boolean.TRUE }, + { Boolean.FALSE, Boolean.TRUE } }); } @Override @@ -99,35 +103,52 @@ public class DescribeCommandTest extends RepositoryTestCase { tag("bob-t2"); ObjectId c4 = modify("ddd"); + assertNameStartsWith(c4, "3e563c5"); assertNull(describe(c1)); assertNull(describe(c1, true)); assertNull(describe(c1, "a*", "b*", "c*")); - - assertEquals("alice-t1", describe(c2)); - assertEquals("alice-t1", describe(c2, "alice*")); assertNull(describe(c2, "bob*")); assertNull(describe(c2, "?ob*")); - assertEquals("alice-t1", describe(c2, "a*", "b*", "c*")); - assertEquals("bob-t2", describe(c3)); - assertEquals("bob-t2-0-g44579eb", describe(c3, true)); - assertEquals("alice-t1-1-g44579eb", describe(c3, "alice*")); - assertEquals("alice-t1-1-g44579eb", describe(c3, "a??c?-t*")); - assertEquals("bob-t2", describe(c3, "bob*")); - assertEquals("bob-t2", describe(c3, "?ob*")); - assertEquals("bob-t2", describe(c3, "a*", "b*", "c*")); - - assertNameStartsWith(c4, "3e563c5"); - // the value verified with git-describe(1) - assertEquals("bob-t2-1-g3e563c5", describe(c4)); - assertEquals("bob-t2-1-g3e563c5", describe(c4, true)); - assertEquals("alice-t1-2-g3e563c5", describe(c4, "alice*")); - assertEquals("bob-t2-1-g3e563c5", describe(c4, "bob*")); - assertEquals("bob-t2-1-g3e563c5", describe(c4, "a*", "b*", "c*")); + if (useAnnotatedTags || describeUseAllTags) { + assertEquals("alice-t1", describe(c2)); + assertEquals("alice-t1", describe(c2, "alice*")); + assertEquals("alice-t1", describe(c2, "a*", "b*", "c*")); + + assertEquals("bob-t2", describe(c3)); + assertEquals("bob-t2-0-g44579eb", describe(c3, true)); + assertEquals("alice-t1-1-g44579eb", describe(c3, "alice*")); + assertEquals("alice-t1-1-g44579eb", describe(c3, "a??c?-t*")); + assertEquals("bob-t2", describe(c3, "bob*")); + assertEquals("bob-t2", describe(c3, "?ob*")); + assertEquals("bob-t2", describe(c3, "a*", "b*", "c*")); + + // the value verified with git-describe(1) + assertEquals("bob-t2-1-g3e563c5", describe(c4)); + assertEquals("bob-t2-1-g3e563c5", describe(c4, true)); + assertEquals("alice-t1-2-g3e563c5", describe(c4, "alice*")); + assertEquals("bob-t2-1-g3e563c5", describe(c4, "bob*")); + assertEquals("bob-t2-1-g3e563c5", describe(c4, "a*", "b*", "c*")); + } else { + assertEquals(null, describe(c2)); + assertEquals(null, describe(c3)); + assertEquals(null, describe(c4)); + } // test default target - assertEquals("bob-t2-1-g3e563c5", git.describe().call()); + if (useAnnotatedTags) { + assertEquals("bob-t2-1-g3e563c5", git.describe().call()); + assertEquals("bob-t2-1-g3e563c5", + git.describe().setTags(false).call()); + assertEquals("bob-t2-1-g3e563c5", + git.describe().setTags(true).call()); + } else { + assertEquals(null, git.describe().call()); + assertEquals(null, git.describe().setTags(false).call()); + assertEquals("bob-t2-1-g3e563c5", + git.describe().setTags(true).call()); + } } @Test @@ -137,7 +158,14 @@ public class DescribeCommandTest extends RepositoryTestCase { 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 + if (!useAnnotatedTags && !describeUseAllTags) { + assertEquals(null, describe(c1)); + assertEquals(null, describe(c2)); + 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)); @@ -179,7 +207,11 @@ public class DescribeCommandTest extends RepositoryTestCase { ObjectId c4 = merge(c2); assertNameStartsWith(c4, "119892b"); - assertEquals("t-2-g119892b", describe(c4)); // 2 commits: c4 and c3 + if (useAnnotatedTags || describeUseAllTags) { + assertEquals("2 commits: c4 and c3", "t-2-g119892b", describe(c4)); + } else { + assertEquals(null, describe(c4)); + } assertNull(describe(c3)); assertNull(describe(c3, true)); } @@ -211,14 +243,76 @@ public class DescribeCommandTest extends RepositoryTestCase { branch("b", c1); ObjectId c3 = modify("ccc"); + assertNameStartsWith(c3, "0244e7f"); ObjectId c4 = merge(c2); assertNameStartsWith(c4, "119892b"); - assertEquals("t2-2-g119892b", describe(c4)); // 2 commits: c4 and c3 + + if (useAnnotatedTags || describeUseAllTags) { + assertEquals("t2-2-g119892b", describe(c4)); // 2 commits: c4 and c3 + assertEquals("t1-1-g0244e7f", describe(c3)); + } else { + assertEquals(null, describe(c4)); + assertEquals(null, describe(c3)); + } + } + + /** + * When t1 annotated dominates t2 lightweight tag + * + * 
+	 * t1 -+-> t2  -
+	 *     |       |
+	 *     +-> c3 -+-> c4
+	 *
+ * + * @throws Exception + */ + @Test + public void t1AnnotatedDominatesT2lightweight() throws Exception { + ObjectId c1 = modify("aaa"); + tag("t1", useAnnotatedTags); + + ObjectId c2 = modify("bbb"); + tag("t2", false); + + assertNameStartsWith(c2, "3747db3"); + if (useAnnotatedTags && !describeUseAllTags) { + assertEquals( + "only annotated tag t1 expected to be used for describe", + "t1-1-g3747db3", describe(c2)); // 1 commits: t2 overridden + // by t1 + } else if (!useAnnotatedTags && !describeUseAllTags) { + assertEquals("no commits to describe expected", null, describe(c2)); + } else { + assertEquals("lightweight tag t2 expected in describe", "t2", + describe(c2)); + } + + branch("b", c1); + + ObjectId c3 = modify("ccc"); assertNameStartsWith(c3, "0244e7f"); - assertEquals("t1-1-g0244e7f", describe(c3)); + if (useAnnotatedTags || describeUseAllTags) { + assertEquals("t1-1-g0244e7f", describe(c3)); + } + + ObjectId c4 = merge(c2); + + assertNameStartsWith(c4, "119892b"); + if (describeUseAllTags) { + assertEquals( + "2 commits for describe commit increment expected since lightweight tag: c4 and c3", + "t2-2-g119892b", describe(c4)); // 2 commits: c4 and c3 + } else if (!useAnnotatedTags && !describeUseAllTags) { + assertEquals("no matching commits expected", null, describe(c4)); + } else { + assertEquals( + "3 commits for describe commit increment expected since annotated tag: c4 and c3 and c2", + "t1-3-g119892b", describe(c4)); // + } } /** @@ -246,7 +340,11 @@ public class DescribeCommandTest extends RepositoryTestCase { ObjectId c4 = merge(t1); assertNameStartsWith(c4, "bb389a4"); - assertEquals("t1-3-gbb389a4", describe(c4)); + if (useAnnotatedTags || describeUseAllTags) { + assertEquals("t1-3-gbb389a4", describe(c4)); + } else { + assertEquals(null, describe(c4)); + } } /** @@ -275,7 +373,11 @@ public class DescribeCommandTest extends RepositoryTestCase { ObjectId c4 = merge(c2); assertNameStartsWith(c4, "bb389a4"); - assertEquals("t2-4-gbb389a4", describe(c4)); + if (useAnnotatedTags || describeUseAllTags) { + assertEquals("t2-4-gbb389a4", describe(c4)); + } else { + assertEquals(null, describe(c4)); + } } private ObjectId merge(ObjectId c2) throws GitAPIException { @@ -289,10 +391,15 @@ public class DescribeCommandTest extends RepositoryTestCase { } private void tag(String tag) throws GitAPIException { + tag(tag, this.useAnnotatedTags); + } + + private void tag(String tag, boolean annotatedTag) throws GitAPIException { TagCommand tagCommand = git.tag().setName(tag) - .setAnnotated(useAnnotatedTags); - if (useAnnotatedTags) + .setAnnotated(annotatedTag); + if (annotatedTag) { tagCommand.setMessage(tag); + } tagCommand.call(); } @@ -304,15 +411,17 @@ public class DescribeCommandTest extends RepositoryTestCase { private String describe(ObjectId c1, boolean longDesc) throws GitAPIException, IOException { - return git.describe().setTarget(c1).setLong(longDesc).call(); + return git.describe().setTarget(c1).setTags(describeUseAllTags) + .setLong(longDesc).call(); } private String describe(ObjectId c1) throws GitAPIException, IOException { return describe(c1, false); } - private String describe(ObjectId c1, String... patterns) throws GitAPIException, IOException, InvalidPatternException { - return git.describe().setTarget(c1).setMatch(patterns).call(); + private String describe(ObjectId c1, String... patterns) throws Exception { + return git.describe().setTarget(c1).setTags(describeUseAllTags) + .setMatch(patterns).call(); } private static void assertNameStartsWith(ObjectId c4, String prefix) { 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 dc605a91e5..4d5e499571 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/DescribeCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/DescribeCommand.java @@ -103,6 +103,11 @@ public class DescribeCommand extends GitCommand { */ private List matchers = new ArrayList<>(); + /** + * Whether to use all tags (incl. lightweight) or not + */ + private boolean useTags = false; + /** * Constructor for DescribeCommand. * @@ -173,6 +178,22 @@ public class DescribeCommand extends GitCommand { return this; } + /** + * Instead of using only the annotated tags, use any tag found in refs/tags + * namespace. This option enables matching lightweight (non-annotated) tags + * or not. + * + * @param tags + * true enables matching lightweight (non-annotated) + * tags like setting option --tags in c git + * @return {@code this} + * @since 5.0 + */ + public DescribeCommand setTags(boolean tags) { + this.useTags = tags; + return this; + } + private String longDescription(Ref tag, int depth, ObjectId tip) throws IOException { return String.format( @@ -246,13 +267,14 @@ public class DescribeCommand extends GitCommand { public String call() throws GitAPIException { try { checkCallable(); - - if (target == null) + if (target == null) { setTarget(Constants.HEAD); + } Collection tagList = repo.getRefDatabase() .getRefsByPrefix(R_TAGS); Map> tags = tagList.stream() + .filter(this::filterLightweightTags) .collect(Collectors.groupingBy(this::getObjectIdFromRef)); // combined flags of all the candidate instances @@ -376,4 +398,22 @@ public class DescribeCommand extends GitCommand { w.close(); } } + + /** + * Whether we use lightweight tags or not for describe Candidates + * + * @param ref + * reference under inspection + * @return true if it should be used for describe or not regarding + * {@link org.eclipse.jgit.api.DescribeCommand#useTags} + */ + @SuppressWarnings("null") + private boolean filterLightweightTags(Ref ref) { + ObjectId id = ref.getObjectId(); + try { + return this.useTags || (id != null && (w.parseTag(id) != null)); + } catch (IOException e) { + return false; + } + } } -- 2.39.5