describe: fix --exclude, --match with --contains and --all

git describe --contains acts as a wrapper around git name-rev. When
operating with --contains and --all, the --match and --exclude patterns
are not properly forwarded to name-rev as --exclude and --refs options.

This results in the command silently discarding match and exclude
requests from the user when operating in --all mode.

We could check and die() if the user provides --contains, --all, and
--match/--exclude. However, its also straight forward to just pass the
filters down to git name-rev.

Notice that the documentation for --match and --exclude mention the
--all mode. It explains that they operate on refs with the prefix
refs/tags, and additionally refs/heads and refs/remotes when using
--all.

Fix the describe logic to pass the patterns down with the appropriate
prefixes when --all is provided. This fixes the support to match the
documented behavior.

Add tests to check that this works as expected.

Reported-by: Tuomas Ahola <taahol@utu.fi>
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/describe.c b/builtin/describe.c
index bffeed13a3cb14d7ce738597460c29556fe8aa2a..62800ef15ed915742f03ddee07918b0bfa40cfc4 100644 (file)
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -712,13 +712,25 @@ int cmd_describe(int argc,
                             NULL);
                if (always)
                        strvec_push(&args, "--always");
-               if (!all) {
+               if (!all)
                        strvec_push(&args, "--tags");
+
+               for_each_string_list_item(item, &patterns)
+                       strvec_pushf(&args, "--refs=refs/tags/%s", item->string);
+               for_each_string_list_item(item, &exclude_patterns)
+                       strvec_pushf(&args, "--exclude=refs/tags/%s", item->string);
+
+               if (all) {
                        for_each_string_list_item(item, &patterns)
-                               strvec_pushf(&args, "--refs=refs/tags/%s", item->string);
+                               strvec_pushf(&args, "--refs=refs/heads/%s", item->string);
                        for_each_string_list_item(item, &exclude_patterns)
-                               strvec_pushf(&args, "--exclude=refs/tags/%s", item->string);
+                               strvec_pushf(&args, "--exclude=refs/heads/%s", item->string);
+                       for_each_string_list_item(item, &patterns)
+                               strvec_pushf(&args, "--refs=refs/remotes/%s", item->string);
+                       for_each_string_list_item(item, &exclude_patterns)
+                               strvec_pushf(&args, "--exclude=refs/remotes/%s", item->string);
                }
+
                if (argc)
                        strvec_pushv(&args, argv);
                else

diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 2c70cc561ad5f64ae9b900aacc68d231491839ae..e5bcf537602a21a4586bbb29575a05b817680e07 100755 (executable)
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -345,6 +345,28 @@ test_expect_success 'describe --contains and --no-match' '
        test_cmp expect actual
 '
 
+test_expect_success 'describe --contains --all --match no matching commit' '
+       echo "tags/A^0" >expect &&
+       tagged_commit=$(git rev-parse "refs/tags/A^0") &&
+       test_must_fail git describe --contains --all --match="B" $tagged_commit
+'
+
+check_describe "tags/A^0" --contains --all --match="A" $(git rev-parse "refs/tags/A^0")
+
+check_describe "branch_A" --contains --all --match="branch*" $(git rev-parse "refs/tags/A^0")
+
+check_describe "branch_C~1" --contains --all --match="branch*" --exclude="branch_A" $(git rev-parse "refs/tags/A^0")
+
+check_describe "branch_A" --contains --all \
+       --exclude="A" --exclude="c" --exclude="test*" --exclude="origin/remote_branch_A" \
+       $(git rev-parse "refs/tags/A^0")
+
+check_describe "remotes/origin/remote_branch_A" --contains --all --match="origin/remote*" $(git rev-parse "refs/tags/A^0")
+
+check_describe "remotes/origin/remote_branch_C~1" --contains --all \
+       --match="origin/remote*" --exclude="origin/remote_branch_A" \
+       $(git rev-parse "refs/tags/A^0")
+
 test_expect_success 'setup and absorb a submodule' '
        test_create_repo sub1 &&
        test_commit -C sub1 initial &&