{"thread":{"id":"44636","subject":"[PATCH 2/2] describe: add support for multiple match patterns","startedAt":"2016-12-07T02:34:25Z","lastAt":"2016-12-08T01:12:57Z","messageCount":6,"participants":["Jacob Keller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"307095","messageId":"20161207023259.29355-2-jacob.e.keller@intel.com","threadId":"44636","inReplyTo":"20161207023259.29355-1-jacob.e.keller@intel.com","subject":"[PATCH 2/2] describe: add support for multiple match patterns","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2016-12-07T02:32:59Z","receivedAt":"2016-12-07T02:34:25Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nConvert \"--match\" into a string list that accumulates patterns. If any\npatterns are given, then include all tags that match at least one\npattern. This allows the user to construct multiple small patterns and\ncompose them. If desired, a future patch could expand functionality by\nadding a new option to determine the style of combining multiple\npatterns.\n\nThe primary use of this feature can be seen when trying to find which\nrelease tag a given commit made it into. Suppose that you version all\nyour official releases such as \"v1.2\", \"v1.3\", \"v1.4\", \"v2.1\" and so on.\nNow, you also have other tags which represent -rc releases and other\nsuch tags. If you want to find the first major release that contains\na given commit you might try\n\ngit describe --contains --match=\"v?.?\" \n\nThis will work as long as you have only single digits. But if you start\nadding multiple digits, the pattern becomes not enough to match all the\ntags you wanted while excluding the ones you didn't.\n\nBy implementing multiple --match invocations, this can be avoided by\nsimply passing multiple match patterns to the command:\n\ngit describe --contains --match=\"v[0-9].[0-9]\" --match=\"v[0-9].[0-9][0-9]\" ...\n\nThe end result is the ability to easily search tag space for which\ntag included a given commit, without including -rc and other tags which\naren't interesting to you.\n\nSigned-off-by: Jacob Keller \n---\n Documentation/git-describe.txt | 6 +++++-\n builtin/describe.c | 31 ++++++++++++++++++++++++-------\n t/t6120-describe.sh | 19 +++++++++++++++++++\n 3 files changed, 48 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt\nindex e4ac448ff565..c85f2811ce28 100644\n--- a/Documentation/git-describe.txt\n+++ b/Documentation/git-describe.txt\n@@ -83,7 +83,11 @@ OPTIONS\n --match ::\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.\n+\tleaking private tags from the repository, or to shrink the scope of\n+\tsearched tags to avoid -rc tags or similar. If given multiple a list\n+\tof patterns will be accumulated, and tags matching any of the patterns\n+\twill be considered. Use `--no-match` to clear and reset the list of\n+\tpatterns.\n \n --always::\n \tShow uniquely abbreviated commit object as fallback.\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 01490a157efc..e3ceab65e273 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -28,7 +28,7 @@ static int abbrev = -1; /* unspecified */\n static int max_candidates = 10;\n static struct hashmap names;\n static int have_util;\n-static const char *pattern;\n+static struct string_list patterns = STRING_LIST_INIT_NODUP;\n static int always;\n static const char *dirty;\n \n@@ -129,9 +129,25 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi\n \tif (!all && !is_tag)\n \t\treturn 0;\n \n-\t/* Accept only tags that match the pattern, if given */\n-\tif (pattern && (!is_tag || wildmatch(pattern, path + 10, 0, NULL)))\n-\t\treturn 0;\n+\t/*\n+\t * If we're given patterns, accept only tags which match at least one\n+\t * pattern.\n+\t */\n+\tif (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, &patterns) {\n+\t\t\tif (!wildmatch(item->string, path + 10, 0, NULL)) {\n+\t\t\t\tbreak;\n+\t\t\t}\n+\n+\t\t\t/* If we get here, no pattern matched. */\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n \n \t/* Is it annotated? */\n \tif (!peel_ref(path, peeled.hash)) {\n@@ -404,7 +420,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"only output exact matches\"), 0),\n \t\tOPT_INTEGER(0, \"candidates\", &max_candidates,\n \t\t\t N_(\"consider most recent tags (default: 10)\")),\n-\t\tOPT_STRING(0, \"match\", &pattern, N_(\"pattern\"),\n+\t\tOPT_STRING_LIST(0, \"match\", &patterns, N_(\"pattern\"),\n \t\t\t N_(\"only consider tags matching \")),\n \t\tOPT_BOOL(0, \"always\", &always,\n \t\t\tN_(\"show abbreviated commit object as fallback\")),\n@@ -430,6 +446,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"--long is incompatible with --abbrev=0\"));\n \n \tif (contains) {\n+\t\tstruct string_list_item *item;\n \t\tstruct argv_array args;\n \n \t\targv_array_init(&args);\n@@ -440,8 +457,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \t\t\targv_array_push(&args, \"--always\");\n \t\tif (!all) {\n \t\t\targv_array_push(&args, \"--tags\");\n-\t\t\tif (pattern)\n-\t\t\t\targv_array_pushf(&args, \"--refs=refs/tags/%s\", pattern);\n+\t\t\tfor_each_string_list_item(item, &patterns)\n+\t\t\t\targv_array_pushf(&args, \"--refs=refs/tags/%s\", item->string);\n \t\t}\n \t\tif (argc)\n \t\t\targv_array_pushv(&args, argv);\ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex 85f269411cb3..9e5db9b87a1f 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -182,6 +182,10 @@ check_describe \"test2-lightweight-*\" --tags --match=\"test2-*\"\n \n check_describe \"test2-lightweight-*\" --long --tags --match=\"test2-*\" HEAD^\n \n+check_describe \"test1-lightweight-*\" --long --tags --match=\"test1-*\" --match=\"test2-*\" HEAD^\n+\n+check_describe \"test2-lightweight-*\" --long --tags --match=\"test1-*\" --no-match --match=\"test2-*\" HEAD^\n+\n test_expect_success 'name-rev with exact tags' '\n \techo A >expect &&\n \ttag_object=$(git rev-parse refs/tags/A) &&\n@@ -206,4 +210,19 @@ test_expect_success 'describe --contains with the exact tags' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'describe --contains and --match' '\n+\techo \"A^0\" >expect &&\n+\ttagged_commit=$(git rev-parse \"refs/tags/A^0\") &&\n+\ttest_must_fail git describe --contains --match=\"B\" $tagged_commit &&\n+\tgit describe --contains --match=\"B\" --match=\"A\" $tagged_commit >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'describe --contains and --no-match' '\n+\techo \"A^0\" >expect &&\n+\ttagged_commit=$(git rev-parse \"refs/tags/A^0\") &&\n+\tgit describe --contains --match=\"B\" --no-match $tagged_commit >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.11.0.rc2.152.g4d04e67\n\n"},{"id":"307096","messageId":"20161207023259.29355-1-jacob.e.keller@intel.com","threadId":"44636","inReplyTo":null,"subject":"[PATCH 1/2] name-rev: allow passing multiple patterns using --refs","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2016-12-07T02:32:58Z","receivedAt":"2016-12-07T02:36:15Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nAllow git name-rev to take a string list of patterns instead of only\na single pattern. All patterns are matched inclusively, meaning that\na ref only needs to match one pattern to be included.\n\nAdd tests and documentation for this change.\n\nSigned-off-by: Jacob Keller \n---\n Documentation/git-name-rev.txt | 4 +++-\n builtin/name-rev.c | 41 +++++++++++++++++++++++++-----------\n t/t6007-rev-list-cherry-pick-file.sh | 30 ++++++++++++++++++++++++++\n 3 files changed, 62 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-name-rev.txt b/Documentation/git-name-rev.txt\nindex ca28fb8e2a07..7433627db12d 100644\n--- a/Documentation/git-name-rev.txt\n+++ b/Documentation/git-name-rev.txt\n@@ -26,7 +26,9 @@ OPTIONS\n \n --refs=::\n \tOnly use refs whose names match a given shell pattern. The pattern\n-\tcan be one of branch name, tag name or fully qualified ref name.\n+\tcan be one of branch name, tag name or fully qualified ref name. If\n+\tgiven multiple times, use refs whose names match any of the given shell\n+\tpatterns. Use `--no-refs` to clear any previous ref patterns given.\n \n --all::\n \tList all commits reachable from all refs\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex cd89d48b65e8..000a2a700ed3 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -108,7 +108,7 @@ static const char *name_ref_abbrev(const char *refname, int shorten_unambiguous)\n struct name_ref_data {\n \tint tags_only;\n \tint name_only;\n-\tconst char *ref_filter;\n+\tstruct string_list ref_filters;\n };\n \n static struct tip_table {\n@@ -150,16 +150,33 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo\n \tif (data->tags_only && !starts_with(path, \"refs/tags/\"))\n \t\treturn 0;\n \n-\tif (data->ref_filter) {\n-\t\tswitch (subpath_matches(path, data->ref_filter)) {\n-\t\tcase -1: /* did not match */\n-\t\t\treturn 0;\n-\t\tcase 0: /* matched fully */\n-\t\t\tbreak;\n-\t\tdefault: /* matched subpath */\n-\t\t\tcan_abbreviate_output = 1;\n-\t\t\tbreak;\n+\tif (data->ref_filters.nr) {\n+\t\tstruct string_list_item *item;\n+\t\tint matched = 0;\n+\n+\t\t/* See if any of the patterns match. */\n+\t\tfor_each_string_list_item(item, &data->ref_filters) {\n+\t\t\t/*\n+\t\t\t * We want to check every pattern even if we already\n+\t\t\t * found a match, just in case one of the later\n+\t\t\t * patterns could abbreviate the output.\n+\t\t\t */\n+\t\t\tswitch (subpath_matches(path, item->string)) {\n+\t\t\tcase -1: /* did not match */\n+\t\t\t\tbreak;\n+\t\t\tcase 0: /* matched fully */\n+\t\t\t\tmatched = 1;\n+\t\t\t\tbreak;\n+\t\t\tdefault: /* matched subpath */\n+\t\t\t\tmatched = 1;\n+\t\t\t\tcan_abbreviate_output = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n \t\t}\n+\n+\t\t/* If none of the patterns matched, stop now */\n+\t\tif (!matched)\n+\t\t\treturn 0;\n \t}\n \n \tadd_to_tip_table(oid->hash, path, can_abbreviate_output);\n@@ -306,11 +323,11 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n {\n \tstruct object_array revs = OBJECT_ARRAY_INIT;\n \tint all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;\n-\tstruct name_ref_data data = { 0, 0, NULL };\n+\tstruct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP };\n \tstruct option opts[] = {\n \t\tOPT_BOOL(0, \"name-only\", &data.name_only, N_(\"print only names (no SHA-1)\")),\n \t\tOPT_BOOL(0, \"tags\", &data.tags_only, N_(\"only use tags to name the commits\")),\n-\t\tOPT_STRING(0, \"refs\", &data.ref_filter, N_(\"pattern\"),\n+\t\tOPT_STRING_LIST(0, \"refs\", &data.ref_filters, N_(\"pattern\"),\n \t\t\t\t N_(\"only use refs matching \")),\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BOOL(0, \"all\", &all, N_(\"list all commits reachable from all refs\")),\ndiff --git a/t/t6007-rev-list-cherry-pick-file.sh b/t/t6007-rev-list-cherry-pick-file.sh\nindex 1408b608eb03..d072ec43b016 100755\n--- a/t/t6007-rev-list-cherry-pick-file.sh\n+++ b/t/t6007-rev-list-cherry-pick-file.sh\n@@ -99,6 +99,36 @@ test_expect_success '--cherry-pick bar does not come up empty (II)' '\n \ttest_cmp actual.named expect\n '\n \n+test_expect_success 'name-rev multiple --refs combine inclusive' '\n+\tgit rev-list --left-right --cherry-pick F...E -- bar > actual &&\n+\tgit name-rev --stdin --name-only --refs=\"*tags/F\" --refs=\"*tags/E\" \\\n+\t\t< actual > actual.named &&\n+\ttest_cmp actual.named expect\n+'\n+\n+cat >expect < actual &&\n+\tgit name-rev --stdin --name-only --refs=\"*tags/F\" \\\n+\t\t< actual > actual.named &&\n+\ttest_cmp actual.named expect\n+'\n+\n+cat >expect < actual &&\n+\tgit name-rev --stdin --name-only --refs=\"*tags/F\" --refs=\"*tags/E\" --no-refs --refs=\"*tags/G\" \\\n+\t\t< actual > actual.named &&\n+\ttest_cmp actual.named expect\n+'\n+\n cat >expect < writes:\n\n> ... Suppose that you version all\n> your official releases such as \"v1.2\", \"v1.3\", \"v1.4\", \"v2.1\" and so on.\n> Now, you also have other tags which represent -rc releases and other\n> such tags. If you want to find the first major release that contains\n> a given commit you might try\n>\n> git describe --contains --match=\"v?.?\" \n>\n> This will work as long as you have only single digits. But if you start\n> adding multiple digits, the pattern becomes not enough to match all the\n> tags you wanted while excluding the ones you didn't.\n\nIsn't what you really want for the use case a negative pattern,\ni.e. \"I want ones that match v* but not the ones that match *-rc*\",\nthough?\n"},{"id":"307199","messageId":"CA+P7+xrPivwMzGhzKxu30jns+YvSQGXBKUc4JDmfbenTy27tZg@mail.gmail.com","threadId":"44636","inReplyTo":"xmqqa8c7wfxu.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] describe: add support for multiple match patterns","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-12-07T23:49:22Z","receivedAt":"2016-12-07T23:49:47Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Wed, Dec 7, 2016 at 2:08 PM, Junio C Hamano wrote:\n> Jacob Keller writes:\n>\n>> ... Suppose that you version all\n>> your official releases such as \"v1.2\", \"v1.3\", \"v1.4\", \"v2.1\" and so on.\n>> Now, you also have other tags which represent -rc releases and other\n>> such tags. If you want to find the first major release that contains\n>> a given commit you might try\n>>\n>> git describe --contains --match=\"v?.?\" \n>>\n>> This will work as long as you have only single digits. But if you start\n>> adding multiple digits, the pattern becomes not enough to match all the\n>> tags you wanted while excluding the ones you didn't.\n>\n> Isn't what you really want for the use case a negative pattern,\n> i.e. \"I want ones that match v* but not the ones that match *-rc*\",\n> though?\n\nThat's another way of implementing it. I just went with straight\nforward patterns that I was already using in sequence.\n\nBasically, this started as a script to try each pattern in sequence,\nbut this is slow, cumbersome and easy to mess up.\n\nYou're suggesting just add a single second pattern that we will do\nmatches and discard any tag that matches that first?\n\nI think I can implement that pretty easily, and it should have simpler\nsemantics. We can discard first, and then match what remains easily.\n\nThanks,\nJake\n"},{"id":"307203","messageId":"xmqq60mvuv8v.fsf@gitster.mtv.corp.google.com","threadId":"44636","inReplyTo":"CA+P7+xrPivwMzGhzKxu30jns+YvSQGXBKUc4JDmfbenTy27tZg@mail.gmail.com","subject":"Re: [PATCH 2/2] describe: add support for multiple match patterns","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-08T00:20:32Z","receivedAt":"2016-12-08T00:20:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller writes:\n\n> Basically, this started as a script to try each pattern in sequence,\n> but this is slow, cumbersome and easy to mess up.\n>\n> You're suggesting just add a single second pattern that we will do\n> matches and discard any tag that matches that first?\n\nI am not suggesting anything. I was just trying to see how well what\nwas designed and implemented supports the use case that motivated\nthe feature. Think of it as a sanity check and review of the design.\n\n> I think I can implement that pretty easily, and it should have simpler\n> semantics. We can discard first, and then match what remains easily.\n\nI actually think \"multiple\" and \"negative\" are orthogonal and both\nare good things. If we are enhancing the filtering by refname\npatterns to allow multiple patterns (i.e. your patch), that is good,\nand it would be ideal if we can also have support for negative ones.\n"},{"id":"307207","messageId":"CA+P7+xrWUS9BuwoOjGv+VtC5gR9vWrf7ANnks87fqvoDst9dAg@mail.gmail.com","threadId":"44636","inReplyTo":"xmqq60mvuv8v.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] describe: add support for multiple match patterns","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-12-08T01:04:44Z","receivedAt":"2016-12-08T01:12:57Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Wed, Dec 7, 2016 at 4:20 PM, Junio C Hamano wrote:\n> Jacob Keller writes:\n>\n>> Basically, this started as a script to try each pattern in sequence,\n>> but this is slow, cumbersome and easy to mess up.\n>>\n>> You're suggesting just add a single second pattern that we will do\n>> matches and discard any tag that matches that first?\n>\n> I am not suggesting anything. I was just trying to see how well what\n> was designed and implemented supports the use case that motivated\n> the feature. Think of it as a sanity check and review of the design.\n>\n\nMakes sense.\n\n>> I think I can implement that pretty easily, and it should have simpler\n>> semantics. We can discard first, and then match what remains easily.\n>\n> I actually think \"multiple\" and \"negative\" are orthogonal and both\n> are good things. If we are enhancing the filtering by refname\n> patterns to allow multiple patterns (i.e. your patch), that is good,\n> and it would be ideal if we can also have support for negative ones.\n\nI can add support for negative matches pretty easily. I personally\ndon't see the value of \"logical and\" filters, ie to match only tags\nthat match all the given filters, though that does allow some other\nforms of expression.\n\nI do like the idea of negative filters, and I'll go ahead and work on\nadding that as another extension.\n\nThanks,\nJake\n"}]}