{"thread":{"id":"46771","subject":"[PATCH] describe: teach --match to handle branches and remotes","startedAt":"2017-09-17T14:32:20Z","lastAt":"2017-09-20T01:10:58Z","messageCount":5,"participants":["Max Kirillov","Junio C Hamano","Jacob Keller"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"328264","messageId":"20170917142416.30685-1-max@max630.net","threadId":"46771","inReplyTo":null,"subject":"[PATCH] describe: teach --match to handle branches and remotes","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2017-09-17T14:24:16Z","receivedAt":"2017-09-17T14:32:20Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"When `git describe` uses `--match`, it matches only tags, basically\nignoring the `--all` argument even when it is specified.\n\nFix it by also matching branch name and $remote_name/$remote_branch_name,\nfor remote-tracking references, with the specified patterns. Update\ndocumentation accordingly and add tests.\n\nSigned-off-by: Max Kirillov <max@max630.net>\n---\nRequires https://public-inbox.org/git/20170916055344.31866-1-max@max630.net/\n\nThis extends --match to branches and remote-tracking references. It is in some respect\nregression, if anybody have used --all and --match together this would find another\nreference, but since that combination did not make sense anyway probably it is not\na big issue.\n\nThere are ambiguity with this approach if --match=foo matches tag \"foo\", or branch \"foo\".\nProbably to resolve it there should appear some --match-full option, so that --match would mean\nfull reference name, with prefix. It could be a room for further improvement.\n\nFrom documentation I removed the usage example part, mainly to not expand the size too much, but\nprobably they do not really belong there.\n Documentation/git-describe.txt | 24 ++++++++++++++----------\n builtin/describe.c             | 26 ++++++++++++++++++++++----\n t/t6120-describe.sh            | 18 ++++++++++++++++++\n 3 files changed, 54 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt\nindex 26f19d3b07..c924c945ba 100644\n--- a/Documentation/git-describe.txt\n+++ b/Documentation/git-describe.txt\n@@ -87,19 +87,23 @@ OPTIONS\n \n --match <pattern>::\n \tOnly consider tags matching the given `glob(7)` pattern,\n-\texcluding the \"refs/tags/\" prefix.  This can be used to avoid\n-\tleaking private tags from the repository. If given multiple times, a\n-\tlist of patterns will be accumulated, and tags matching any of the\n-\tpatterns will be considered. Use `--no-match` to clear and reset the\n-\tlist of patterns.\n+\texcluding the \"refs/tags/\" prefix. If used with `--all`, it also\n+\tconsiders local branches and remote-tracking references matching the\n+\tpattern, excluding respectively \"refs/heads/\" and \"refs/remotes/\"\n+\tprefix; references of other types are never considered. If given\n+\tmultiple times, a list of patterns will be accumulated, and tags\n+\tmatching any of the patterns will be considered.  Use `--no-match` to\n+\tclear and reset the list of patterns.\n \n --exclude <pattern>::\n \tDo not consider tags matching the given `glob(7)` pattern, excluding\n-\tthe \"refs/tags/\" prefix. This can be used to narrow the tag space and\n-\tfind only tags matching some meaningful criteria. If given multiple\n-\ttimes, a list of patterns will be accumulated and tags matching any\n-\tof the patterns will be excluded. When combined with --match a tag will\n-\tbe considered when it matches at least one --match pattern and does not\n+\tthe \"refs/tags/\" prefix. If used with `--all`, it also does not consider\n+\tlocal branches and remote-tracking references matching the pattern,\n+\texcluding respectively \"refs/heads/\" and \"refs/remotes/\" prefix;\n+\treferences of other types are never considered. If given multiple times,\n+\ta list of patterns will be accumulated and tags matching any of the\n+\tpatterns will be excluded. When combined with --match a tag will be\n+\tconsidered when it matches at least one --match pattern and does not\n \tmatch any of the --exclude patterns. Use `--no-exclude` to clear and\n \treset the list of patterns.\n \ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 94ff2fba0b..2a2e998063 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -124,6 +124,22 @@ static void add_to_known_names(const char *path,\n \t}\n }\n \n+/* Drops prefix. Returns NULL if the path is not expected with current settings. */\n+static const char *get_path_to_match(int is_tag, int all, const char *path)\n+{\n+\tif (is_tag)\n+\t\treturn path + 10;\n+\telse if (all) {\n+\t\tif (starts_with(path, \"refs/heads/\"))\n+\t\t\treturn path + 11; /* \"refs/heads/...\" */\n+\t\telse if (starts_with(path, \"refs/remotes/\"))\n+\t\t\treturn path + 13; /* \"refs/remotes/...\" */\n+\t\telse\n+\t\t\treturn 0;\n+\t} else\n+\t\treturn NULL;\n+}\n+\n static int get_name(const char *path, const struct object_id *oid, int flag, void *cb_data)\n {\n \tint is_tag = starts_with(path, \"refs/tags/\");\n@@ -140,12 +156,13 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi\n \t */\n \tif (exclude_patterns.nr) {\n \t\tstruct string_list_item *item;\n+\t\tconst char *path_to_match = get_path_to_match(is_tag, all, path);\n \n-\t\tif (!is_tag)\n+\t\tif (!path_to_match)\n \t\t\treturn 0;\n \n \t\tfor_each_string_list_item(item, &exclude_patterns) {\n-\t\t\tif (!wildmatch(item->string, path + 10, 0))\n+\t\t\tif (!wildmatch(item->string, path_to_match, 0))\n \t\t\t\treturn 0;\n \t\t}\n \t}\n@@ -156,13 +173,14 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi\n \t */\n \tif (patterns.nr) {\n \t\tint found = 0;\n+\t\tconst char *path_to_match = get_path_to_match(is_tag, all, path);\n \t\tstruct string_list_item *item;\n \n-\t\tif (!is_tag)\n+\t\tif (!path_to_match)\n \t\t\treturn 0;\n \n \t\tfor_each_string_list_item(item, &patterns) {\n-\t\t\tif (!wildmatch(item->string, path + 10, 0)) {\n+\t\t\tif (!wildmatch(item->string, path_to_match, 0)) {\n \t\t\t\tfound = 1;\n \t\t\t\tbreak;\n \t\t\t}\ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex 25110ea55d..fac52bd9dc 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -190,6 +190,24 @@ check_describe \"test1-lightweight-*\" --long --tags --match=\"test1-*\" --match=\"te\n \n check_describe \"test1-lightweight-*\" --long --tags --match=\"test3-*\" --match=\"test1-*\" HEAD\n \n+test_expect_success 'set-up branches' '\n+\tgit branch branch_A A &&\n+\tgit branch branch_c c &&\n+\tgit update-ref refs/remotes/origin/remote_branch_A \"A^{commit}\" &&\n+\tgit update-ref refs/remotes/origin/remote_branch_c \"c^{commit}\" &&\n+\tgit update-ref refs/original/original_branch_A test-annotated~2\n+'\n+\n+check_describe \"heads/branch_A*\" --all --match=\"branch_*\" --exclude=\"branch_c\" HEAD\n+\n+check_describe \"remotes/origin/remote_branch_A*\" --all --match=\"origin/remote_branch_*\" --exclude=\"origin/remote_branch_c\" HEAD\n+\n+check_describe \"original/original_branch_A*\" --all test-annotated~1\n+\n+test_expect_success '--match does not work for other types' '\n+\ttest_must_fail git describe --all --match=\"*original_branch_*\" test-annotated~1\n+'\n+\n test_expect_success 'name-rev with exact tags' '\n \techo A >expect &&\n \ttag_object=$(git rev-parse refs/tags/A) &&\n-- \n2.11.0.1122.gc3fec58.dirty\n\n"},{"id":"328323","messageId":"xmqqzi9rsgxz.fsf@gitster.mtv.corp.google.com","threadId":"46771","inReplyTo":"20170917142416.30685-1-max@max630.net","subject":"Re: [PATCH] describe: teach --match to handle branches and remotes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-09-18T23:52:24Z","receivedAt":"2017-09-18T23:52:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Kirillov <max@max630.net> writes:\n\n>  --match <pattern>::\n>  \tOnly consider tags matching the given `glob(7)` pattern,\n> -\texcluding the \"refs/tags/\" prefix.  This can be used to avoid\n> -\tleaking private tags from the repository. If given multiple times, a\n> -\tlist of patterns will be accumulated, and tags matching any of the\n> -\tpatterns will be considered. Use `--no-match` to clear and reset the\n> -\tlist of patterns.\n> +\texcluding the \"refs/tags/\" prefix. If used with `--all`, it also\n> +\tconsiders local branches and remote-tracking references matching the\n> +\tpattern, excluding respectively \"refs/heads/\" and \"refs/remotes/\"\n> +\tprefix; references of other types are never considered. If given\n> +\tmultiple times, a list of patterns will be accumulated, and tags\n> +\tmatching any of the patterns will be considered.  Use `--no-match` to\n> +\tclear and reset the list of patterns.\n>  \n>  --exclude <pattern>::\n>  \tDo not consider tags matching the given `glob(7)` pattern, excluding\n> -\tthe \"refs/tags/\" prefix. This can be used to narrow the tag space and\n> -\tfind only tags matching some meaningful criteria. If given multiple\n> -\ttimes, a list of patterns will be accumulated and tags matching any\n> -\tof the patterns will be excluded. When combined with --match a tag will\n> -\tbe considered when it matches at least one --match pattern and does not\n> +\tthe \"refs/tags/\" prefix. If used with `--all`, it also does not consider\n> +\tlocal branches and remote-tracking references matching the pattern,\n> +\texcluding respectively \"refs/heads/\" and \"refs/remotes/\" prefix;\n> +\treferences of other types are never considered. If given multiple times,\n> +\ta list of patterns will be accumulated and tags matching any of the\n> +\tpatterns will be excluded. When combined with --match a tag will be\n> +\tconsidered when it matches at least one --match pattern and does not\n>  \tmatch any of the --exclude patterns. Use `--no-exclude` to clear and\n>  \treset the list of patterns.\n\nOK, I find this written clearly enough.\n\n> diff --git a/builtin/describe.c b/builtin/describe.c\n> index 94ff2fba0b..2a2e998063 100644\n> --- a/builtin/describe.c\n> +++ b/builtin/describe.c\n> @@ -124,6 +124,22 @@ static void add_to_known_names(const char *path,\n>  \t}\n>  }\n>  \n> +/* Drops prefix. Returns NULL if the path is not expected with current settings. */\n> +static const char *get_path_to_match(int is_tag, int all, const char *path)\n> +{\n> +\tif (is_tag)\n> +\t\treturn path + 10;\n\nThis is a faithful conversion of the existing code that wants to\nbehave the same as original, but a bit more on this later.\n\n> +\telse if (all) {\n> +\t\tif (starts_with(path, \"refs/heads/\"))\n> +\t\t\treturn path + 11; /* \"refs/heads/...\" */\n> +\t\telse if (starts_with(path, \"refs/remotes/\"))\n> +\t\t\treturn path + 13; /* \"refs/remotes/...\" */\n> +\t\telse\n> +\t\t\treturn 0;\n\nI think you can use skip_prefix() to avoid counting the length of\nthe prefix yourself, i.e.\n\n\telse if all {\n\t\tconst char *body;\n\n                if (skip_prefix(path, \"refs/heads/\", &body))\n\t\t\treturn body;\n\t\telse if (skip_prefix(path, \"refs/remotes/\", &body))\n\t\t\t...\n\t}\n\nWhether you do the above or not, the last one that returns 0 should\nreturn NULL (to the language it is the same thing, but to humans, we\nwrite NULL when it is the null pointer, not the number 0).\n\n> +\t} else\n> +\t\treturn NULL;\n> +}\n\nPerhaps the whole thing may want to be a bit more simplified, like:\n\n        static const *skip_ref_prefix(const char *path, int all)\n        {\n                const char *prefix[] = {\n                        \"refs/tags/\", \"refs/heads/\", \"refs/remotes/\"\n                };\n                const char *body;\n                int cnt;\n                int bound = all ? ARRAY_SIZE(prefix) : 1;\n\n                for (cnt = 0; cnt < bound; cnt++)\n                        if (skip_prefix(path, prefix[cnt], &body);\n                                return body;\n                return NULL;\n        }\n\nThe hardcoded +10 for \"is_tag\" case assumes that anything other than\n\"refs/tags/something\" would ever be used to call into this function\nwhen is_tag is true, and that may well be true in the current code\nand have been so ever since the original code was written, but it\nstill smells like an invitation for future bugs.\n\nI dunno.\n\n> +\n>  static int get_name(const char *path, const struct object_id *oid, int flag, void *cb_data)\n>  {\n>  \tint is_tag = starts_with(path, \"refs/tags/\");\n> @@ -140,12 +156,13 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi\n>  \t */\n>  \tif (exclude_patterns.nr) {\n>  \t\tstruct string_list_item *item;\n> +\t\tconst char *path_to_match = get_path_to_match(is_tag, all, path);\n\n> +test_expect_success 'set-up branches' '\n> +\tgit branch branch_A A &&\n> +\tgit branch branch_c c &&\n\nWas there a reason why A and c are in different cases?  Are we\nworried about case insensitive filesystems or something?\n\n> +\tgit update-ref refs/remotes/origin/remote_branch_A \"A^{commit}\" &&\n> +\tgit update-ref refs/remotes/origin/remote_branch_c \"c^{commit}\" &&\n> +\tgit update-ref refs/original/original_branch_A test-annotated~2\n> +'\n\nThanks.\n"},{"id":"328326","messageId":"CA+P7+xqTTXFoUnkD2Z0q3e9G8ByoCmAN1ZuXtZquRX7ofKewCA@mail.gmail.com","threadId":"46771","inReplyTo":"xmqqzi9rsgxz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] describe: teach --match to handle branches and remotes","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-09-19T00:45:00Z","receivedAt":"2017-09-19T00:45:27Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Mon, Sep 18, 2017 at 4:52 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Max Kirillov <max@max630.net> writes:\n>\n>>  --match <pattern>::\n>>       Only consider tags matching the given `glob(7)` pattern,\n>> -     excluding the \"refs/tags/\" prefix.  This can be used to avoid\n>> -     leaking private tags from the repository. If given multiple times, a\n>> -     list of patterns will be accumulated, and tags matching any of the\n>> -     patterns will be considered. Use `--no-match` to clear and reset the\n>> -     list of patterns.\n>> +     excluding the \"refs/tags/\" prefix. If used with `--all`, it also\n>> +     considers local branches and remote-tracking references matching the\n>> +     pattern, excluding respectively \"refs/heads/\" and \"refs/remotes/\"\n>> +     prefix; references of other types are never considered. If given\n>> +     multiple times, a list of patterns will be accumulated, and tags\n>> +     matching any of the patterns will be considered.  Use `--no-match` to\n>> +     clear and reset the list of patterns.\n>>\n>>  --exclude <pattern>::\n>>       Do not consider tags matching the given `glob(7)` pattern, excluding\n>> -     the \"refs/tags/\" prefix. This can be used to narrow the tag space and\n>> -     find only tags matching some meaningful criteria. If given multiple\n>> -     times, a list of patterns will be accumulated and tags matching any\n>> -     of the patterns will be excluded. When combined with --match a tag will\n>> -     be considered when it matches at least one --match pattern and does not\n>> +     the \"refs/tags/\" prefix. If used with `--all`, it also does not consider\n>> +     local branches and remote-tracking references matching the pattern,\n>> +     excluding respectively \"refs/heads/\" and \"refs/remotes/\" prefix;\n>> +     references of other types are never considered. If given multiple times,\n>> +     a list of patterns will be accumulated and tags matching any of the\n>> +     patterns will be excluded. When combined with --match a tag will be\n>> +     considered when it matches at least one --match pattern and does not\n>>       match any of the --exclude patterns. Use `--no-exclude` to clear and\n>>       reset the list of patterns.\n>\n> OK, I find this written clearly enough.\n>\n>> diff --git a/builtin/describe.c b/builtin/describe.c\n>> index 94ff2fba0b..2a2e998063 100644\n>> --- a/builtin/describe.c\n>> +++ b/builtin/describe.c\n>> @@ -124,6 +124,22 @@ static void add_to_known_names(const char *path,\n>>       }\n>>  }\n>>\n>> +/* Drops prefix. Returns NULL if the path is not expected with current settings. */\n>> +static const char *get_path_to_match(int is_tag, int all, const char *path)\n>> +{\n>> +     if (is_tag)\n>> +             return path + 10;\n>\n> This is a faithful conversion of the existing code that wants to\n> behave the same as original, but a bit more on this later.\n>\n>> +     else if (all) {\n>> +             if (starts_with(path, \"refs/heads/\"))\n>> +                     return path + 11; /* \"refs/heads/...\" */\n>> +             else if (starts_with(path, \"refs/remotes/\"))\n>> +                     return path + 13; /* \"refs/remotes/...\" */\n>> +             else\n>> +                     return 0;\n>\n> I think you can use skip_prefix() to avoid counting the length of\n> the prefix yourself, i.e.\n>\n>         else if all {\n>                 const char *body;\n>\n>                 if (skip_prefix(path, \"refs/heads/\", &body))\n>                         return body;\n>                 else if (skip_prefix(path, \"refs/remotes/\", &body))\n>                         ...\n>         }\n>\n> Whether you do the above or not, the last one that returns 0 should\n> return NULL (to the language it is the same thing, but to humans, we\n> write NULL when it is the null pointer, not the number 0).\n>\n>> +     } else\n>> +             return NULL;\n>> +}\n>\n> Perhaps the whole thing may want to be a bit more simplified, like:\n>\n>         static const *skip_ref_prefix(const char *path, int all)\n>         {\n>                 const char *prefix[] = {\n>                         \"refs/tags/\", \"refs/heads/\", \"refs/remotes/\"\n>                 };\n>                 const char *body;\n>                 int cnt;\n>                 int bound = all ? ARRAY_SIZE(prefix) : 1;\n>\n\nI found the implicit use of \"bound = 1\" means \"we only care about\ntags\" to be a bit weird here. I guess it's not really that big a deal\noverall, and this is definitely cleaner than the original\nimplementation.\n\n>                 for (cnt = 0; cnt < bound; cnt++)\n>                         if (skip_prefix(path, prefix[cnt], &body);\n>                                 return body;\n>                 return NULL;\n>         }\n>\n> The hardcoded +10 for \"is_tag\" case assumes that anything other than\n> \"refs/tags/something\" would ever be used to call into this function\n> when is_tag is true, and that may well be true in the current code\n> and have been so ever since the original code was written, but it\n> still smells like an invitation for future bugs.\n>\n> I dunno.\n>\n>> +\n>>  static int get_name(const char *path, const struct object_id *oid, int flag, void *cb_data)\n>>  {\n>>       int is_tag = starts_with(path, \"refs/tags/\");\n>> @@ -140,12 +156,13 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi\n>>        */\n>>       if (exclude_patterns.nr) {\n>>               struct string_list_item *item;\n>> +             const char *path_to_match = get_path_to_match(is_tag, all, path);\n>\n>> +test_expect_success 'set-up branches' '\n>> +     git branch branch_A A &&\n>> +     git branch branch_c c &&\n>\n> Was there a reason why A and c are in different cases?  Are we\n> worried about case insensitive filesystems or something?\n>\n>> +     git update-ref refs/remotes/origin/remote_branch_A \"A^{commit}\" &&\n>> +     git update-ref refs/remotes/origin/remote_branch_c \"c^{commit}\" &&\n>> +     git update-ref refs/original/original_branch_A test-annotated~2\n>> +'\n>\n> Thanks.\n"},{"id":"328432","messageId":"20170920010719.GA12408@jessie.local","threadId":"46771","inReplyTo":"xmqqzi9rsgxz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] describe: teach --match to handle branches and remotes","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2017-09-20T01:07:20Z","receivedAt":"2017-09-20T01:08:01Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"On Tue, Sep 19, 2017 at 08:52:24AM +0900, Junio C Hamano wrote:\n> I think you can use skip_prefix() to avoid counting the length of\n> the prefix yourself, i.e.\n\nThanks, will use it.\n\n> The hardcoded +10 for \"is_tag\" case assumes that anything other than\n> \"refs/tags/something\" would ever be used to call into this function\n> when is_tag is true, and that may well be true in the current code\n> and have been so ever since the original code was written, but it\n> still smells like an invitation for future bugs.\n\nis_tag is used later. I'll chance it so that it does not\nrely on it to match, but it still has to produce it.\n\n> Was there a reason why A and c are in different cases?  Are we\n> worried about case insensitive filesystems or something?\n\nThe tags have been there of different case already. I don't\nknow why. I'll change the branch names but I'm reluctant to\ntouch existing tests.\n\n-- \nMax\n"},{"id":"328433","messageId":"20170920011010.10399-1-max@max630.net","threadId":"46771","inReplyTo":"20170920010719.GA12408@jessie.local","subject":"[PATCH v2] describe: teach --match to handle branches and remotes","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2017-09-20T01:10:10Z","receivedAt":"2017-09-20T01:10:58Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"When `git describe` uses `--match`, it matches only tags, basically\nignoring the `--all` argument even when it is specified.\n\nFix it by also matching branch name and $remote_name/$remote_branch_name,\nfor remote-tracking references, with the specified patterns. Update\ndocumentation accordingly and add tests.\n\nSigned-off-by: Max Kirillov <max@max630.net>\n---\nChanged to use skip_prefix(). Calculate path_to_match only once.\n\nAdd case of discarding unknown type with exclude\n Documentation/git-describe.txt | 24 ++++++++++++++----------\n builtin/describe.c             | 29 +++++++++++++++++------------\n t/t6120-describe.sh            | 27 +++++++++++++++++++++++++++\n 3 files changed, 58 insertions(+), 22 deletions(-)\n\ndiff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt\nindex 26f19d3b07..c924c945ba 100644\n--- a/Documentation/git-describe.txt\n+++ b/Documentation/git-describe.txt\n@@ -87,19 +87,23 @@ OPTIONS\n \n --match <pattern>::\n \tOnly consider tags matching the given `glob(7)` pattern,\n-\texcluding the \"refs/tags/\" prefix.  This can be used to avoid\n-\tleaking private tags from the repository. If given multiple times, a\n-\tlist of patterns will be accumulated, and tags matching any of the\n-\tpatterns will be considered. Use `--no-match` to clear and reset the\n-\tlist of patterns.\n+\texcluding the \"refs/tags/\" prefix. If used with `--all`, it also\n+\tconsiders local branches and remote-tracking references matching the\n+\tpattern, excluding respectively \"refs/heads/\" and \"refs/remotes/\"\n+\tprefix; references of other types are never considered. If given\n+\tmultiple times, a list of patterns will be accumulated, and tags\n+\tmatching any of the patterns will be considered.  Use `--no-match` to\n+\tclear and reset the list of patterns.\n \n --exclude <pattern>::\n \tDo not consider tags matching the given `glob(7)` pattern, excluding\n-\tthe \"refs/tags/\" prefix. This can be used to narrow the tag space and\n-\tfind only tags matching some meaningful criteria. If given multiple\n-\ttimes, a list of patterns will be accumulated and tags matching any\n-\tof the patterns will be excluded. When combined with --match a tag will\n-\tbe considered when it matches at least one --match pattern and does not\n+\tthe \"refs/tags/\" prefix. If used with `--all`, it also does not consider\n+\tlocal branches and remote-tracking references matching the pattern,\n+\texcluding respectively \"refs/heads/\" and \"refs/remotes/\" prefix;\n+\treferences of other types are never considered. If given multiple times,\n+\ta list of patterns will be accumulated and tags matching any of the\n+\tpatterns will be excluded. When combined with --match a tag will be\n+\tconsidered when it matches at least one --match pattern and does not\n \tmatch any of the --exclude patterns. Use `--no-exclude` to clear and\n \treset the list of patterns.\n \ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 42afa1e244..f15b6e531d 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -129,13 +129,24 @@ static void add_to_known_names(const char *path,\n \n static int get_name(const char *path, const struct object_id *oid, int flag, void *cb_data)\n {\n-\tint is_tag = starts_with(path, \"refs/tags/\");\n+\tint is_tag = 0;\n \tstruct object_id peeled;\n \tint is_annotated, prio;\n-\n-\t/* Reject anything outside refs/tags/ unless --all */\n-\tif (!all && !is_tag)\n+\tconst char *path_to_match = NULL;\n+\n+\tif (skip_prefix(path, \"refs/tags/\", &path_to_match)) {\n+\t\tis_tag = 1;\n+\t} else if (all) {\n+\t\tif ((exclude_patterns.nr || patterns.nr) &&\n+\t\t    !skip_prefix(path, \"refs/heads/\", &path_to_match) &&\n+\t\t    !skip_prefix(path, \"refs/remotes/\", &path_to_match)) {\n+\t\t\t/* Only accept reference of known type if there are match/exclude patterns */\n+\t\t\treturn 0;\n+\t\t}\n+\t} else {\n+\t\t/* Reject anything outside refs/tags/ unless --all */\n \t\treturn 0;\n+\t}\n \n \t/*\n \t * If we're given exclude patterns, first exclude any tag which match\n@@ -144,11 +155,8 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi\n \tif (exclude_patterns.nr) {\n \t\tstruct string_list_item *item;\n \n-\t\tif (!is_tag)\n-\t\t\treturn 0;\n-\n \t\tfor_each_string_list_item(item, &exclude_patterns) {\n-\t\t\tif (!wildmatch(item->string, path + 10, 0))\n+\t\t\tif (!wildmatch(item->string, path_to_match, 0))\n \t\t\t\treturn 0;\n \t\t}\n \t}\n@@ -161,11 +169,8 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi\n \t\tint found = 0;\n \t\tstruct string_list_item *item;\n \n-\t\tif (!is_tag)\n-\t\t\treturn 0;\n-\n \t\tfor_each_string_list_item(item, &patterns) {\n-\t\t\tif (!wildmatch(item->string, path + 10, 0)) {\n+\t\t\tif (!wildmatch(item->string, path_to_match, 0)) {\n \t\t\t\tfound = 1;\n \t\t\t\tbreak;\n \t\t\t}\ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex 25110ea55d..0a8f754100 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -190,6 +190,33 @@ check_describe \"test1-lightweight-*\" --long --tags --match=\"test1-*\" --match=\"te\n \n check_describe \"test1-lightweight-*\" --long --tags --match=\"test3-*\" --match=\"test1-*\" HEAD\n \n+test_expect_success 'set-up branches' '\n+\tgit branch branch_A A &&\n+\tgit branch branch_C c &&\n+\tgit update-ref refs/remotes/origin/remote_branch_A \"A^{commit}\" &&\n+\tgit update-ref refs/remotes/origin/remote_branch_C \"c^{commit}\" &&\n+\tgit update-ref refs/original/original_branch_A test-annotated~2\n+'\n+\n+check_describe \"heads/branch_A*\" --all --match=\"branch_*\" --exclude=\"branch_C\" HEAD\n+\n+check_describe \"remotes/origin/remote_branch_A*\" --all --match=\"origin/remote_branch_*\" --exclude=\"origin/remote_branch_C\" HEAD\n+\n+check_describe \"original/original_branch_A*\" --all test-annotated~1\n+\n+test_expect_success '--match does not work for other types' '\n+\ttest_must_fail git describe --all --match=\"*original_branch_*\" test-annotated~1\n+'\n+\n+test_expect_success '--exclude does not work for other types' '\n+\tR=$(git describe --all --exclude=\"any_pattern_even_not_matching\" test-annotated~1) &&\n+\tcase \"$R\" in\n+\t*original_branch_A*) echo \"fail: Found unknown reference $R with --exclude\"\n+\t\tfalse;;\n+\t*) echo ok: Found some known type;;\n+\tesac\n+'\n+\n test_expect_success 'name-rev with exact tags' '\n \techo A >expect &&\n \ttag_object=$(git rev-parse refs/tags/A) &&\n-- \n2.11.0.1122.gc3fec58.dirty\n\n"}]}