{"thread":{"id":"45321","subject":"[PATCH] branch & tag: Add a --no-contains option","startedAt":"2017-03-08T20:29:33Z","lastAt":"2017-03-12T17:49:43Z","messageCount":28,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano","Jeff King","Christian Couder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"313541","messageId":"20170308202025.17900-1-avarab@gmail.com","threadId":"45321","inReplyTo":null,"subject":"[PATCH] branch & tag: Add a --no-contains option","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-03-08T20:20:25Z","receivedAt":"2017-03-08T20:29:33Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the branch & tag commands to have a --no-contains option in\naddition to their longstanding --contains options.\n\nThe use-case I have for this is mainly to find the last-good rollout\ntag given a known-bad . Right given a hypothetically bad\ncommit v2.10.1-3-gcf5c7253e0 now you can find that with this hacky\none-liner:\n\n (./git tag -l 'v[0-9]*'; ./git tag -l 'v[0-9]*' --contains v2.10.1-3-gcf5c7253e0)|sort|uniq -c|grep -E '^ *1 '|awk '{print $2}'\n\nBut with the --no-contains option you can now get the exact same\noutput with:\n\n ./git tag -l 'v[0-9]*' --no-contains v2.10.1-3-gcf5c7253e0 | sort\n\nOnce I'd implemented this for \"tag\" it was easy enough to add it for\n\"branch\". I haven't added it to \"for-each-ref\" but that would be\ntrivial if anyone cares, but that use-case would be even more obscure\nthan adding it to \"branch\", so I haven't bothered. The \"describe\"\ncommand also has a --contains option, but its semantics are unrelated\nto what tag/branch/for-each-ref use --contains for, and I don't see\nhow a --no-contains option for it would make any sense.\n\nMore notes about this patch:\n\n * I'm not really happy with the \"special attention\" documentation\n example in git-branch.txt, but it follows logically from the\n description for --contains just above it which I think is overly\n specific as well. IMO that entire NOTES section in git-branch.txt\n could just be removed.\n\n * I'm adding a --without option as an alias for --no-contains for\n consistency with --with and --contains. Since we don't even\n document --with anymore (or test it) perhaps we shouldn't be adding\n --without.\n\n * Where I'm changing existing documentation lines I'm mainly word\n wrapping at 75 columns to be consistent with the existing style.\n\n * Ditto the minor change to git-completion.bash.\n\n * Perhaps we should just copy/paste commit_contains() into\n commit_no_contains() and skip the ternary struct assignment. It's a\n hot loop, but I have faith in modern compilers.\n\n * All of the test changes I've made are just doing the inverse of the\n existing --contains tests, with this --no-contains for both tag &\n branch should be just as tested as the existing --contains option.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/git-branch.txt | 24 +++--\n Documentation/git-tag.txt | 6 +-\n builtin/branch.c | 4 +-\n builtin/tag.c | 2 +\n contrib/completion/git-completion.bash | 9 +-\n parse-options.h | 4 +-\n ref-filter.c | 17 ++--\n ref-filter.h | 1 +\n t/t3201-branch-contains.sh | 40 +++++++-\n t/t7004-tag.sh | 163 ++++++++++++++++++++++++++++++++-\n 10 files changed, 245 insertions(+), 25 deletions(-)\n\ndiff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\nindex 092f1bcf9f..316ec5b2d4 100644\n--- a/Documentation/git-branch.txt\n+++ b/Documentation/git-branch.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n 'git branch' [--color[=] | --no-color] [-r | -a]\n \t[--list] [-v [--abbrev= | --no-abbrev]]\n \t[--column[=] | --no-column]\n-\t[(--merged | --no-merged | --contains) []] [--sort=]\n+\t[(--merged | --no-merged | --contains | --no-contains) []] [--sort=]\n \t[--points-at ] [--format=] [...]\n 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] []\n 'git branch' (--set-upstream-to= | -u ) []\n@@ -35,11 +35,12 @@ as branch creation.\n \n With `--contains`, shows only the branches that contain the named commit\n (in other words, the branches whose tip commits are descendants of the\n-named commit). With `--merged`, only branches merged into the named\n-commit (i.e. the branches whose tip commits are reachable from the named\n-commit) will be listed. With `--no-merged` only branches not merged into\n-the named commit will be listed. If the argument is missing it\n-defaults to `HEAD` (i.e. the tip of the current branch).\n+named commit), `--no-contains` inverts it. With `--merged`, only branches\n+merged into the named commit (i.e. the branches whose tip commits are\n+reachable from the named commit) will be listed. With `--no-merged` only\n+branches not merged into the named commit will be listed. If the \n+argument is missing it defaults to `HEAD` (i.e. the tip of the current\n+branch).\n \n The command's second form creates a new branch head named \n which points to the current `HEAD`, or if given.\n@@ -213,6 +214,10 @@ start-point is either a local or remote-tracking branch.\n \tOnly list branches which contain the specified commit (HEAD\n \tif not specified). Implies `--list`.\n \n+--no-contains []::\n+\tOnly list branches which don't contain the specified commit\n+\t(HEAD if not specified). Implies `--list`.\n+\n --merged []::\n \tOnly list branches whose tips are reachable from the\n \tspecified commit (HEAD if not specified). Implies `--list`.\n@@ -296,13 +301,16 @@ If you are creating a branch that you want to checkout immediately, it is\n easier to use the git checkout command with its `-b` option to create\n a branch and check it out with a single command.\n \n-The options `--contains`, `--merged` and `--no-merged` serve three related\n-but different purposes:\n+The options `--contains`, `--no-contains`, `--merged` and `--no-merged`\n+serve three related but different purposes:\n \n - `--contains ` is used to find all branches which will need\n special attention if were to be rebased or amended, since those\n branches contain the specified .\n \n+- `--no-contains ` is used to find those branches which won't need\n+ that special attention.\n+\n - `--merged` is used to find all branches which can be safely deleted,\n since those branches are fully contained by HEAD.\n \ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex 525737a5d8..4938496194 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -12,7 +12,7 @@ SYNOPSIS\n 'git tag' [-a | -s | -u ] [-f] [-m | -F ]\n \t [ | ]\n 'git tag' -d ...\n-'git tag' [-n[]] -l [--contains ] [--points-at ]\n+'git tag' [-n[]] -l [--[no-]contains ] [--points-at ]\n \t[--column[=] | --no-column] [--create-reflog] [--sort=]\n \t[--format=] [--[no-]merged []] [...]\n 'git tag' -v [--format=] ...\n@@ -124,6 +124,10 @@ This option is only applicable when listing tags without annotation lines.\n \tOnly list tags which contain the specified commit (HEAD if not\n \tspecified).\n \n+--no-contains []::\n+\tOnly list tags which don't contain the specified commit (HEAD if\n+\tnot secified).\n+\n --points-at ::\n \tOnly list tags of the given object.\n \ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 94f7de7fa5..e8d534604c 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -548,7 +548,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT_SET_INT('r', \"remotes\", &filter.kind, N_(\"act on remote-tracking branches\"),\n \t\t\tFILTER_REFS_REMOTES),\n \t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only branches that contain the commit\")),\n+\t\tOPT_NO_CONTAINS(&filter.no_commit, N_(\"print only branches that don't contain the commit\")),\n \t\tOPT_WITH(&filter.with_commit, N_(\"print only branches that contain the commit\")),\n+\t\tOPT_WITHOUT(&filter.with_commit, N_(\"print only branches that don't contain the commit\")),\n \t\tOPT__ABBREV(&filter.abbrev),\n \n \t\tOPT_GROUP(N_(\"Specific git-branch actions:\")),\n@@ -604,7 +606,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \tif (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)\n \t\tlist = 1;\n \n-\tif (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)\n+\tif (filter.with_commit || filter.no_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)\n \t\tlist = 1;\n \n \tif (!!delete + !!rename + !!new_upstream +\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex ad29be6923..737a83028a 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -424,7 +424,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tOPT_GROUP(N_(\"Tag listing options\")),\n \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"show tag list in columns\")),\n \t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n+\t\tOPT_NO_CONTAINS(&filter.no_commit, N_(\"print only tags that don't contain the commit\")),\n \t\tOPT_WITH(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n+\t\tOPT_WITHOUT(&filter.no_commit, N_(\"print only tags that don't contain the commit\")),\n \t\tOPT_MERGED(&filter, N_(\"print only tags that are merged\")),\n \t\tOPT_NO_MERGED(&filter, N_(\"print only tags that are not merged\")),\n \t\tOPT_CALLBACK(0 , \"sort\", sorting_tail, N_(\"key\"),\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex fc32286a43..fa3da49478 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1093,9 +1093,9 @@ _git_branch ()\n \t--*)\n \t\t__gitcomp \"\n \t\t\t--color --no-color --verbose --abbrev= --no-abbrev\n-\t\t\t--track --no-track --contains --merged --no-merged\n-\t\t\t--set-upstream-to= --edit-description --list\n-\t\t\t--unset-upstream --delete --move --remotes\n+\t\t\t--track --no-track --contains --no-contains --merged\n+\t\t\t--no-merged --set-upstream-to= --edit-description\n+\t\t\t--list --unset-upstream --delete --move --remotes\n \t\t\t--column --no-column --sort= --points-at\n \t\t\t\"\n \t\t;;\n@@ -2862,7 +2862,8 @@ _git_tag ()\n \t\t__gitcomp \"\n \t\t\t--list --delete --verify --annotate --message --file\n \t\t\t--sign --cleanup --local-user --force --column --sort=\n-\t\t\t--contains --points-at --merged --no-merged --create-reflog\n+\t\t\t--contains --no-contains --points-at --merged\n+\t\t\t--no-merged --create-reflog\n \t\t\t\"\n \t\t;;\n \tesac\ndiff --git a/parse-options.h b/parse-options.h\nindex dcd8a0926c..0eac90b510 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -258,7 +258,9 @@ extern int parse_opt_passthru_argv(const struct option *, const char *, int);\n \t PARSE_OPT_LASTARG_DEFAULT | flag, \\\n \t parse_opt_commits, (intptr_t) \"HEAD\" \\\n \t}\n-#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"contains\", v, h, 0)\n+#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"contains\", v, h, PARSE_OPT_NONEG)\n+#define OPT_NO_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"no-contains\", v, h, PARSE_OPT_NONEG)\n #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH(\"with\", v, h, PARSE_OPT_HIDDEN)\n+#define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH(\"without\", v, h, PARSE_OPT_HIDDEN)\n \n #endif\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 1ec0fb8391..6a7ca1cdac 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1571,11 +1571,12 @@ static enum contains_result contains_tag_algo(struct commit *candidate,\n \treturn contains_test(candidate, want);\n }\n \n-static int commit_contains(struct ref_filter *filter, struct commit *commit)\n+static int commit_contains(struct ref_filter *filter, struct commit *commit, const int with_commit)\n {\n+\tstruct commit_list *tmp = with_commit ? filter->with_commit : filter->no_commit;\n \tif (filter->with_commit_tag_algo)\n-\t\treturn contains_tag_algo(commit, filter->with_commit);\n-\treturn is_descendant_of(commit, filter->with_commit);\n+\t\treturn contains_tag_algo(commit, tmp);\n+\treturn is_descendant_of(commit, tmp);\n }\n \n /*\n@@ -1765,13 +1766,17 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,\n \t * obtain the commit using the 'oid' available and discard all\n \t * non-commits early. The actual filtering is done later.\n \t */\n-\tif (filter->merge_commit || filter->with_commit || filter->verbose) {\n+\tif (filter->merge_commit || filter->with_commit || filter->no_commit || filter->verbose) {\n \t\tcommit = lookup_commit_reference_gently(oid->hash, 1);\n \t\tif (!commit)\n \t\t\treturn 0;\n-\t\t/* We perform the filtering for the '--contains' option */\n+\t\t/* We perform the filtering for the '--contains' option... */\n \t\tif (filter->with_commit &&\n-\t\t !commit_contains(filter, commit))\n+\t\t !commit_contains(filter, commit, 1))\n+\t\t\treturn 0;\n+\t\t/* ...or for the `--no-contains' option */\n+\t\tif (filter->no_commit &&\n+\t\t commit_contains(filter, commit, 0))\n \t\t\treturn 0;\n \t}\n \ndiff --git a/ref-filter.h b/ref-filter.h\nindex 154e24c405..af85eb4592 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -53,6 +53,7 @@ struct ref_filter {\n \tconst char **name_patterns;\n \tstruct sha1_array points_at;\n \tstruct commit_list *with_commit;\n+\tstruct commit_list *no_commit;\n \n \tenum {\n \t\tREF_FILTER_MERGED_NONE = 0,\ndiff --git a/t/t3201-branch-contains.sh b/t/t3201-branch-contains.sh\nindex 7f3ec47241..9fb79e66f0 100755\n--- a/t/t3201-branch-contains.sh\n+++ b/t/t3201-branch-contains.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-test_description='branch --contains , --merged, and --no-merged'\n+test_description='branch --contains , --no-contains --merged, and --no-merged'\n \n . ./test-lib.sh\n \n@@ -45,6 +45,22 @@ test_expect_success 'branch --contains master' '\n \n '\n \n+test_expect_success 'branch --no-contains=master' '\n+\n+\tgit branch --no-contains=master >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+\n+'\n+\n+test_expect_success 'branch --no-contains master' '\n+\n+\tgit branch --no-contains master >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+\n+'\n+\n test_expect_success 'branch --contains=side' '\n \n \tgit branch --contains=side >actual &&\n@@ -55,6 +71,16 @@ test_expect_success 'branch --contains=side' '\n \n '\n \n+test_expect_success 'branch --no-contains=side' '\n+\n+\tgit branch --no-contains=side >actual &&\n+\t{\n+\t\techo \" master\"\n+\t} >expect &&\n+\ttest_cmp expect actual\n+\n+'\n+\n test_expect_success 'branch --contains with pattern implies --list' '\n \n \tgit branch --contains=master master >actual &&\n@@ -65,6 +91,14 @@ test_expect_success 'branch --contains with pattern implies --list' '\n \n '\n \n+test_expect_success 'branch --no-contains with pattern implies --list' '\n+\n+\tgit branch --no-contains=master master >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+\n+'\n+\n test_expect_success 'side: branch --merged' '\n \n \tgit branch --merged >actual &&\n@@ -126,7 +160,9 @@ test_expect_success 'branch --no-merged with pattern implies --list' '\n test_expect_success 'implicit --list conflicts with modification options' '\n \n \ttest_must_fail git branch --contains=master -d &&\n-\ttest_must_fail git branch --contains=master -m foo\n+\ttest_must_fail git branch --contains=master -m foo &&\n+\ttest_must_fail git branch --no-contains=master -d &&\n+\ttest_must_fail git branch --no-contains=master -m foo\n \n '\n \ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex b4698ab5f5..f01bcafea4 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1385,6 +1385,23 @@ test_expect_success 'checking that first commit is in all tags (relative)' \"\n \ttest_cmp expected actual\n \"\n \n+# All the --contains tests above, but with --no-contains\n+test_expect_success 'checking that first commit is not listed in any tag with --no-contains (hash)' \"\n+\t>expected &&\n+\tgit tag -l --no-contains $hash1 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n+test_expect_success 'checking that first commit is in all tags (tag)' \"\n+\tgit tag -l --no-contains v1.0 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n+test_expect_success 'checking that first commit is in all tags (relative)' \"\n+\tgit tag -l --no-contains HEAD~2 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n cat > expected < expected <actual &&\n+\ttest_cmp expected actual\n+\"\n \n cat > expected < expected <actual &&\n+\ttest_cmp expected actual\n+\"\n+\n # how about a simple merge?\n \n test_expect_success 'creating simple branch' '\n@@ -1424,6 +1465,19 @@ test_expect_success 'checking that branch head only has one tag' \"\n \ttest_cmp expected actual\n \"\n \n+cat > expected <actual &&\n+\ttest_cmp expected actual\n+\"\n+\n test_expect_success 'merging original branch into this branch' '\n \tgit merge --strategy=ours master &&\n git tag v4.0\n@@ -1445,6 +1499,20 @@ v1.0.1\n v1.1.3\n v2.0\n v3.0\n+EOF\n+\n+test_expect_success 'checking that original branch head with --no-contains lists all but one tag now' \"\n+\tgit tag -l --no-contains $hash3 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n+cat > expected <expected &&\n+\tgit tag -l --no-contains $hash1 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n # mixing modes and options:\n \n test_expect_success 'mixing incompatibles modes and options is forbidden' '\n@@ -1708,8 +1782,91 @@ run_with_limited_stack () {\n \n test_lazy_prereq ULIMIT_STACK_SIZE 'run_with_limited_stack true'\n \n+# These are all the tags we've created above\n+cat >expect.no-contains <expect &&\n \ti=1 &&\n \twhile test $i -lt 8000\n@@ -1725,7 +1882,9 @@ EOF\"\n \tgit checkout master &&\n \tgit tag far-far-away HEAD^ &&\n \trun_with_limited_stack git tag --contains HEAD >actual &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect actual &&\n+\trun_with_limited_stack git tag --no-contains HEAD >actual &&\n+\ttest_cmp expect.no-contains actual\n '\n \n test_expect_success '--format should list tags as per format given' '\n-- \n2.11.0\n\n"},{"id":"313570","messageId":"xmqqefy71iej.fsf@gitster.mtv.corp.google.com","threadId":"45321","inReplyTo":"20170308202025.17900-1-avarab@gmail.com","subject":"Re: [PATCH] branch & tag: Add a --no-contains option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-08T23:02:44Z","receivedAt":"2017-03-08T23:02:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> More notes about this patch:\n>\n> * I'm not really happy with the \"special attention\" documentation\n> example in git-branch.txt, but it follows logically from the\n> description for --contains just above it which I think is overly\n> specific as well. IMO that entire NOTES section in git-branch.txt\n> could just be removed.\n\nThe first paragraph of the section is unrelated to the topic and I\ndo not think anybody would miss it if it goes, but I always feel\nuncomfortable between --contains and --merged. I do not expect\nanybody needs lengthy explanation to tell --merged and --no-merged\n(similarly --contains and --no-contains) apart, but perhaps because\nI often use --with (which is a hidden synonym to --contains) and\nalmost never --merged, and as we are adding the fourth, I find it a\nvery good idea to extend the description to tell users what they\nwant to use \"contains\" for (i.e. find the set of containers given a\ncommit) and what they want to use \"merged\" for (i.e. find the set of\ncontainees given a commit).\n\n> * I'm adding a --without option as an alias for --no-contains for\n> consistency with --with and --contains. Since we don't even\n> document --with anymore (or test it) perhaps we shouldn't be adding\n> --without.\n\nI do not think anybody other than me uses \"--with\" to begin with, so\nI do not care too deeply about it. If it makes the patch simpler\nnot to support \"--without\", I'd be supportive if you want to drop it.\n\nI'll review the body of the patch later.\n\nThanks.\n"},{"id":"313626","messageId":"20170309100910.z4h7bwqzxw2xynyu@sigill.intra.peff.net","threadId":"45321","inReplyTo":"20170308202025.17900-1-avarab@gmail.com","subject":"Re: [PATCH] branch & tag: Add a --no-contains option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-09T10:09:10Z","receivedAt":"2017-03-09T10:16:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 08, 2017 at 08:20:25PM +0000, Ævar Arnfjörð Bjarmason wrote:\n\n> Change the branch & tag commands to have a --no-contains option in\n> addition to their longstanding --contains options.\n> \n> The use-case I have for this is mainly to find the last-good rollout\n> tag given a known-bad . Right given a hypothetically bad\n> commit v2.10.1-3-gcf5c7253e0 now you can find that with this hacky\n> one-liner:\n> \n> (./git tag -l 'v[0-9]*'; ./git tag -l 'v[0-9]*' --contains v2.10.1-3-gcf5c7253e0)|sort|uniq -c|grep -E '^ *1 '|awk '{print $2}'\n> \n> But with the --no-contains option you can now get the exact same\n> output with:\n> \n> ./git tag -l 'v[0-9]*' --no-contains v2.10.1-3-gcf5c7253e0 | sort\n\nI think that's a good goal.\n\nI'm not sure about the name. I would have expected \"--no-contains\" to\nreset the list of \"--contains\" commits to the empty set. That's an\noption convention we've been slowly moving towards (e.g., with\nOPT_STRING_LIST).\n\nWhat you've added here _does_ match \"--no-merged\", though. I'm not sure\nof the best way forward. At the very least, \"--no-contains\" is currently\nan error, so you would not be changing existing behavior.\n\n> Once I'd implemented this for \"tag\" it was easy enough to add it for\n> \"branch\". I haven't added it to \"for-each-ref\" but that would be\n> trivial if anyone cares, but that use-case would be even more obscure\n> than adding it to \"branch\", so I haven't bothered.\n\nI'd prefer to have it consistently in all three. We should be able to\ntell people to use for-each-ref in their scripts, and that's harder if\nit is missing features.\n\n> The \"describe\" command also has a --contains option, but its semantics\n> are unrelated to what tag/branch/for-each-ref use --contains for, and\n> I don't see how a --no-contains option for it would make any sense.\n\nYeah, I think that feature is orthogonal.\n\n> -static int commit_contains(struct ref_filter *filter, struct commit *commit)\n> +static int commit_contains(struct ref_filter *filter, struct commit *commit, const int with_commit)\n> {\n> +\tstruct commit_list *tmp = with_commit ? filter->with_commit : filter->no_commit;\n> \tif (filter->with_commit_tag_algo)\n> -\t\treturn contains_tag_algo(commit, filter->with_commit);\n> -\treturn is_descendant_of(commit, filter->with_commit);\n> +\t\treturn contains_tag_algo(commit, tmp);\n> +\treturn is_descendant_of(commit, tmp);\n> }\n\nPerhaps it would be simpler if the caller just passed the right\ncommit_list rather than a flag. We unfortunately do still need to pass\nthe \"filter\" (for the algorithm field), but the caller is then:\n\n if (filter->with_commit &&\n !commit_contains(filter, filter->with_commit, commit))\n return 0;\n if (filter->no_commit &&\n commit_contains(filter, filter->no_commit, commit))\n return 0;\n\nwhich avoids the 0/1 flag whose meaning is not immediately apparent at\nthe callsite. One day we can hopefully unify the two algorithms and\nditch the extra filter parameter.\n\nI almost suggested that there simply be an option to invert the match\n(like --invert-contains or something). But what you have here is more\nflexible, if somebody ever wanted to do:\n\n git tag --contains X --no-contains Y\n\nThat could be useful if a feature was introduced in X and then got buggy\nin Y.\n\n> @@ -1708,8 +1782,91 @@ run_with_limited_stack () {\n> \n> test_lazy_prereq ULIMIT_STACK_SIZE 'run_with_limited_stack true'\n> \n> +# These are all the tags we've created above\n> +cat >expect.no-contains < [...80 tags...]\n> +EOF\n\nThat's a lot of tags, and I'd worry it makes the test a little brittle.\nCan we limit the set of tags with a name-match? It shouldn't affect the\npurpose of the test (the deep stack comes from traversing the commits,\nnot the number of tags).\n\n-Peff\n"},{"id":"313628","messageId":"CACBZZX53rMiB5-cA_7-SeU2Dt7d_Cr7_GgyC0rjQQPPf4qyCqw@mail.gmail.com","threadId":"45321","inReplyTo":"20170309100910.z4h7bwqzxw2xynyu@sigill.intra.peff.net","subject":"Re: [PATCH] branch & tag: Add a --no-contains option","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-03-09T10:41:59Z","receivedAt":"2017-03-09T10:51:13Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Mar 9, 2017 at 11:09 AM, Jeff King wrote:\n> On Wed, Mar 08, 2017 at 08:20:25PM +0000, Ævar Arnfjörð Bjarmason wrote:\n>\n>> Change the branch & tag commands to have a --no-contains option in\n>> addition to their longstanding --contains options.\n>>\n>> The use-case I have for this is mainly to find the last-good rollout\n>> tag given a known-bad . Right given a hypothetically bad\n>> commit v2.10.1-3-gcf5c7253e0 now you can find that with this hacky\n>> one-liner:\n>>\n>> (./git tag -l 'v[0-9]*'; ./git tag -l 'v[0-9]*' --contains v2.10.1-3-gcf5c7253e0)|sort|uniq -c|grep -E '^ *1 '|awk '{print $2}'\n>>\n>> But with the --no-contains option you can now get the exact same\n>> output with:\n>>\n>> ./git tag -l 'v[0-9]*' --no-contains v2.10.1-3-gcf5c7253e0 | sort\n>\n> I think that's a good goal.\n>\n> I'm not sure about the name. I would have expected \"--no-contains\" to\n> reset the list of \"--contains\" commits to the empty set. That's an\n> option convention we've been slowly moving towards (e.g., with\n> OPT_STRING_LIST).\n>\n> What you've added here _does_ match \"--no-merged\", though. I'm not sure\n> of the best way forward. At the very least, \"--no-contains\" is currently\n> an error, so you would not be changing existing behavior.\n\nI initially started hacking this up as --not-contains, but after\nbriefly chatting with Christian about it off-list he suggested --no-*.\nSince as you point out it's consistent with --no-merge. I have no\nstrong view on it, I just want the feature whatever the flag is\ncalled.\n\n>> Once I'd implemented this for \"tag\" it was easy enough to add it for\n>> \"branch\". I haven't added it to \"for-each-ref\" but that would be\n>> trivial if anyone cares, but that use-case would be even more obscure\n>> than adding it to \"branch\", so I haven't bothered.\n>\n> I'd prefer to have it consistently in all three. We should be able to\n> tell people to use for-each-ref in their scripts, and that's harder if\n> it is missing features.\n\nAgreed. I'd already hacked that up this morning for a v2. It works &\nhas tests at https://github.com/avar/git/tree/avar/no-contains-2\n\n>> The \"describe\" command also has a --contains option, but its semantics\n>> are unrelated to what tag/branch/for-each-ref use --contains for, and\n>> I don't see how a --no-contains option for it would make any sense.\n>\n> Yeah, I think that feature is orthogonal.\n\n*Nod* just adding a note about it in case anyone's puzzled about why\ndescribe doesn't have --no-contains, elaborated & clarified this a bit\nin my WIP v2.\n\n>> -static int commit_contains(struct ref_filter *filter, struct commit *commit)\n>> +static int commit_contains(struct ref_filter *filter, struct commit *commit, const int with_commit)\n>> {\n>> + struct commit_list *tmp = with_commit ? filter->with_commit : filter->no_commit;\n>> if (filter->with_commit_tag_algo)\n>> - return contains_tag_algo(commit, filter->with_commit);\n>> - return is_descendant_of(commit, filter->with_commit);\n>> + return contains_tag_algo(commit, tmp);\n>> + return is_descendant_of(commit, tmp);\n>> }\n>\n> Perhaps it would be simpler if the caller just passed the right\n> commit_list rather than a flag. We unfortunately do still need to pass\n> the \"filter\" (for the algorithm field), but the caller is then:\n>\n> if (filter->with_commit &&\n> !commit_contains(filter, filter->with_commit, commit))\n> return 0;\n> if (filter->no_commit &&\n> commit_contains(filter, filter->no_commit, commit))\n> return 0;\n>\n> which avoids the 0/1 flag whose meaning is not immediately apparent at\n> the callsite. One day we can hopefully unify the two algorithms and\n> ditch the extra filter parameter.\n\nMy C rustyness is showing. Yeah that's much better, thanks, changed it\nto that in my WIP v2.\n\n> I almost suggested that there simply be an option to invert the match\n> (like --invert-contains or something). But what you have here is more\n> flexible, if somebody ever wanted to do:\n>\n> git tag --contains X --no-contains Y\n\nYeah that's really useful. E.g. this shows the branches I branched off\n(or have locally) from 2.6..2.8:\n\n $ ./git branch --contains v2.6.0 --no-contains v2.8.0\n avar/monkeypatch-untracked-cache-disabled\n avar/uc-notifs21\n dturner/pclouds-watchman-noshm\n\nBut I'd expect this to show all the tags between the two:\n\n $ ./git tag --contains v2.6.0 --no-contains v2.8.0\n $\n\nBut it just returns an empty list. Manually disabling the\ncontains_tag_algo() path (i.e. effectively locally reverting your\nffc4b8012d) makes it \"work\", but of course it's much slower now. I\nhaven't dug into why it's not working yet.\n\nAlso I wonder if this should be an error:\n\n $ ./git [tag|branch|for-each-ref] --contains A --no-contains A\n\nI.e. when you give the same argument to both, this can never return\nanything for obvious reasons.\n\n>> @@ -1708,8 +1782,91 @@ run_with_limited_stack () {\n>>\n>> test_lazy_prereq ULIMIT_STACK_SIZE 'run_with_limited_stack true'\n>>\n>> +# These are all the tags we've created above\n>> +cat >expect.no-contains <> [...80 tags...]\n>> +EOF\n>\n> That's a lot of tags, and I'd worry it makes the test a little brittle.\n> Can we limit the set of tags with a name-match? It shouldn't affect the\n> purpose of the test (the deep stack comes from traversing the commits,\n> not the number of tags).\n\nI'll make this less sucky in v2 somehow. I did it this way because no\nexisting test was checking all the tags we'd created at the end, so\nthis does that by proxy now, but I agree it's too verbose. Will fix\nit.\n"},{"id":"313629","messageId":"20170309104657.7pwreyozxo2tdhk4@sigill.intra.peff.net","threadId":"45321","inReplyTo":"CACBZZX53rMiB5-cA_7-SeU2Dt7d_Cr7_GgyC0rjQQPPf4qyCqw@mail.gmail.com","subject":"Re: [PATCH] branch & tag: Add a --no-contains option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-09T10:46:57Z","receivedAt":"2017-03-09T10:53:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 09, 2017 at 11:41:59AM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> > I almost suggested that there simply be an option to invert the match\n> > (like --invert-contains or something). But what you have here is more\n> > flexible, if somebody ever wanted to do:\n> >\n> > git tag --contains X --no-contains Y\n> \n> Yeah that's really useful. E.g. this shows the branches I branched off\n> (or have locally) from 2.6..2.8:\n> \n> $ ./git branch --contains v2.6.0 --no-contains v2.8.0\n> avar/monkeypatch-untracked-cache-disabled\n> avar/uc-notifs21\n> dturner/pclouds-watchman-noshm\n\nOh, that's a clever application.\n\n> But I'd expect this to show all the tags between the two:\n> \n> $ ./git tag --contains v2.6.0 --no-contains v2.8.0\n> $\n> \n> But it just returns an empty list. Manually disabling the\n> contains_tag_algo() path (i.e. effectively locally reverting your\n> ffc4b8012d) makes it \"work\", but of course it's much slower now. I\n> haven't dug into why it's not working yet.\n\nI'm almost certain this is because the contains_tag_algo one doesn't\nclean up the flag bits it sets on the commit objects. So running it\ntwice in the same process is going to give you nonsense results.\n\nCoincidentally, I've been looking into resurrecting the cleaner approach\nthat I sent long ago:\n\n http://public-inbox.org/git/20140625233429.GA20457@sigill.intra.peff.net/\n\nBut it's sufficiently complex that it's probably worth fixing the\nexisting algorithm to clean up its bits in the meantime.\n\n> Also I wonder if this should be an error:\n> \n> $ ./git [tag|branch|for-each-ref] --contains A --no-contains A\n> \n> I.e. when you give the same argument to both, this can never return\n> anything for obvious reasons.\n\nIt's clearly nonsense, but I don't think there's any need for it to be\nan error. GIGO.\n\n-Peff\n"},{"id":"313635","messageId":"CACBZZX5i+8bQLhLB4DnUAaUw1vA_KQjhtNBvm7drLepPAFFbAQ@mail.gmail.com","threadId":"45321","inReplyTo":"20170309104657.7pwreyozxo2tdhk4@sigill.intra.peff.net","subject":"Re: [PATCH] branch & tag: Add a --no-contains option","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-03-09T12:12:08Z","receivedAt":"2017-03-09T12:14:19Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Mar 9, 2017 at 11:46 AM, Jeff King wrote:\n> On Thu, Mar 09, 2017 at 11:41:59AM +0100, Ævar Arnfjörð Bjarmason wrote:\n>\n>> > I almost suggested that there simply be an option to invert the match\n>> > (like --invert-contains or something). But what you have here is more\n>> > flexible, if somebody ever wanted to do:\n>> >\n>> > git tag --contains X --no-contains Y\n>>\n>> Yeah that's really useful. E.g. this shows the branches I branched off\n>> (or have locally) from 2.6..2.8:\n>>\n>> $ ./git branch --contains v2.6.0 --no-contains v2.8.0\n>> avar/monkeypatch-untracked-cache-disabled\n>> avar/uc-notifs21\n>> dturner/pclouds-watchman-noshm\n>\n> Oh, that's a clever application.\n>\n>> But I'd expect this to show all the tags between the two:\n>>\n>> $ ./git tag --contains v2.6.0 --no-contains v2.8.0\n>> $\n>>\n>> But it just returns an empty list. Manually disabling the\n>> contains_tag_algo() path (i.e. effectively locally reverting your\n>> ffc4b8012d) makes it \"work\", but of course it's much slower now. I\n>> haven't dug into why it's not working yet.\n>\n> I'm almost certain this is because the contains_tag_algo one doesn't\n> clean up the flag bits it sets on the commit objects. So running it\n> twice in the same process is going to give you nonsense results.\n\nYeah indeed.\n\nI tried to hack something up to avoid this, but the\nlookup_commit_reference_gently() we call will return the same\nobject.parent pointer for two invocations, i.e. the underlying\n{commit,object}.c API has a cache of objects it returns, couldn't find\nsome way to quickly make it burst that cache.\n\nThe other approach of making contains_tag_algo() itself detect that\nit's been called before (or us passing a flag) and going around\nsetting commit.object.flags on everything to 0 seemed even more\nbrittle, particularly since I think between filter->with_commit &\nfilter->no_commit we might end up visiting different commits, so it's\nnot easy to just clear it.\n\nI'm happy to hack on it given some pointers, will visit it again, but\nfor now unless I'm missing something obvious / you can point out some\nway to hack this up I'll just submit v2 with the combination of\n--contains & --no-contains dying with a TODO message.\n\nThe patch without that functionality is still really useful, and we\ncan implement that later.\n\n> Coincidentally, I've been looking into resurrecting the cleaner approach\n> that I sent long ago:\n>\n> http://public-inbox.org/git/20140625233429.GA20457@sigill.intra.peff.net/\n>\n> But it's sufficiently complex that it's probably worth fixing the\n> existing algorithm to clean up its bits in the meantime.\n>\n>> Also I wonder if this should be an error:\n>>\n>> $ ./git [tag|branch|for-each-ref] --contains A --no-contains A\n>>\n>> I.e. when you give the same argument to both, this can never return\n>> anything for obvious reasons.\n>\n> It's clearly nonsense, but I don't think there's any need for it to be\n> an error. GIGO.\n\nYeah, make sense.\n"},{"id":"313639","messageId":"20170309125132.tubwxtneffok4nrc@sigill.intra.peff.net","threadId":"45321","inReplyTo":"CACBZZX5i+8bQLhLB4DnUAaUw1vA_KQjhtNBvm7drLepPAFFbAQ@mail.gmail.com","subject":"Re: [PATCH] branch & tag: Add a --no-contains option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-09T12:51:32Z","receivedAt":"2017-03-09T12:51:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 09, 2017 at 01:12:08PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> > I'm almost certain this is because the contains_tag_algo one doesn't\n> > clean up the flag bits it sets on the commit objects. So running it\n> > twice in the same process is going to give you nonsense results.\n> \n> Yeah indeed.\n> \n> I tried to hack something up to avoid this, but the\n> lookup_commit_reference_gently() we call will return the same\n> object.parent pointer for two invocations, i.e. the underlying\n> {commit,object}.c API has a cache of objects it returns, couldn't find\n> some way to quickly make it burst that cache.\n\nYeah, you'll always get the same struct for a given sha1.\n\n> The other approach of making contains_tag_algo() itself detect that\n> it's been called before (or us passing a flag) and going around\n> setting commit.object.flags on everything to 0 seemed even more\n> brittle, particularly since I think between filter->with_commit &\n> filter->no_commit we might end up visiting different commits, so it's\n> not easy to just clear it.\n\nYou can clear the marks with clear_object_flags(). But I don't think\nthat type of solution will quite help us here. We consider each ref\nindependently and ask \"does it match --contains\" and \"does it match\n--no-contains?\". So there is no moment where we are done with all of the\n--contains marks, and can move on to the --no-contains ones. The lookups\nare interleaved.\n\nWe could move to doing them in chunks (the way filter->merge_commit\nworks), and then clearing in between. Or we could use a separate bitset.\nThe patch below does that.\n\n> I'm happy to hack on it given some pointers, will visit it again, but\n> for now unless I'm missing something obvious / you can point out some\n> way to hack this up I'll just submit v2 with the combination of\n> --contains & --no-contains dying with a TODO message.\n> \n> The patch without that functionality is still really useful, and we\n> can implement that later.\n\nI'm not opposed to that, though see what you think of the patch below.\nIt's a bit noisy but it's conceptually pretty straightforward. It should\nhopefully be obvious how you'd add in a separate contains_cache for the\n\"without\" case.\n\nLooking at this, I'm pretty sure that using \"--contains\" with \"--merged\"\nhas similar problems, as they both use the UNINTERESTING bit. So even\nwithout your patch, there is a lurking bug.\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 3820b21cc..42b1bc463 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -14,6 +14,7 @@\n #include \"git-compat-util.h\"\n #include \"version.h\"\n #include \"trailer.h\"\n+#include \"commit-slab.h\"\n \n typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;\n \n@@ -1137,10 +1138,22 @@ static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom\n \t*v = &ref->value[atom];\n }\n \n+/*\n+ * Unknown has to be \"0\" here, because that's what unfilled entries in our slab\n+ * will return.\n+ */\n enum contains_result {\n-\tCONTAINS_UNKNOWN = -1,\n-\tCONTAINS_NO = 0,\n-\tCONTAINS_YES = 1\n+\tCONTAINS_UNKNOWN = 0,\n+\tCONTAINS_NO = 1,\n+\tCONTAINS_YES = 2,\n+};\n+\n+define_commit_slab(contains_cache, enum contains_result);\n+\n+struct ref_filter_cbdata {\n+\tstruct ref_array *array;\n+\tstruct ref_filter *filter;\n+\tstruct contains_cache contains_cache;\n };\n \n /*\n@@ -1171,24 +1184,25 @@ static int in_commit_list(const struct commit_list *want, struct commit *c)\n * Do not recurse to find out, though, but return -1 if inconclusive.\n */\n static enum contains_result contains_test(struct commit *candidate,\n-\t\t\t const struct commit_list *want)\n+\t\t\t const struct commit_list *want,\n+\t\t\t struct contains_cache *cache)\n {\n-\t/* was it previously marked as containing a want commit? */\n-\tif (candidate->object.flags & TMP_MARK)\n-\t\treturn 1;\n-\t/* or marked as not possibly containing a want commit? */\n-\tif (candidate->object.flags & UNINTERESTING)\n-\t\treturn 0;\n+\tenum contains_result *cached = contains_cache_at(cache, candidate);\n+\n+\t/* if we already found the answer, return it without traversing */\n+\tif (*cached)\n+\t\treturn *cached;\n+\n \t/* or are we it? */\n \tif (in_commit_list(want, candidate)) {\n-\t\tcandidate->object.flags |= TMP_MARK;\n-\t\treturn 1;\n+\t\t*cached = CONTAINS_YES;\n+\t\treturn *cached;\n \t}\n \n \tif (parse_commit(candidate) < 0)\n-\t\treturn 0;\n+\t\treturn CONTAINS_NO;\n \n-\treturn -1;\n+\treturn CONTAINS_UNKNOWN;\n }\n \n static void push_to_contains_stack(struct commit *candidate, struct contains_stack *contains_stack)\n@@ -1199,10 +1213,11 @@ static void push_to_contains_stack(struct commit *candidate, struct contains_sta\n }\n \n static enum contains_result contains_tag_algo(struct commit *candidate,\n-\t\tconst struct commit_list *want)\n+\t\t\t\t\t const struct commit_list *want,\n+\t\t\t\t\t struct contains_cache *cache)\n {\n \tstruct contains_stack contains_stack = { 0, 0, NULL };\n-\tint result = contains_test(candidate, want);\n+\tenum contains_result result = contains_test(candidate, want, cache);\n \n \tif (result != CONTAINS_UNKNOWN)\n \t\treturn result;\n@@ -1214,16 +1229,16 @@ static enum contains_result contains_tag_algo(struct commit *candidate,\n \t\tstruct commit_list *parents = entry->parents;\n \n \t\tif (!parents) {\n-\t\t\tcommit->object.flags |= UNINTERESTING;\n+\t\t\t*contains_cache_at(cache, commit) = CONTAINS_NO;\n \t\t\tcontains_stack.nr--;\n \t\t}\n \t\t/*\n \t\t * If we just popped the stack, parents->item has been marked,\n-\t\t * therefore contains_test will return a meaningful 0 or 1.\n+\t\t * therefore contains_test will return a meaningful yes/no.\n \t\t */\n-\t\telse switch (contains_test(parents->item, want)) {\n+\t\telse switch (contains_test(parents->item, want, cache)) {\n \t\tcase CONTAINS_YES:\n-\t\t\tcommit->object.flags |= TMP_MARK;\n+\t\t\t*contains_cache_at(cache, commit) = CONTAINS_YES;\n \t\t\tcontains_stack.nr--;\n \t\t\tbreak;\n \t\tcase CONTAINS_NO:\n@@ -1235,13 +1250,14 @@ static enum contains_result contains_tag_algo(struct commit *candidate,\n \t\t}\n \t}\n \tfree(contains_stack.contains_stack);\n-\treturn contains_test(candidate, want);\n+\treturn contains_test(candidate, want, cache);\n }\n \n-static int commit_contains(struct ref_filter *filter, struct commit *commit)\n+static int commit_contains(struct ref_filter *filter, struct commit *commit,\n+\t\t\t struct contains_cache *cache)\n {\n \tif (filter->with_commit_tag_algo)\n-\t\treturn contains_tag_algo(commit, filter->with_commit);\n+\t\treturn contains_tag_algo(commit, filter->with_commit, cache) == CONTAINS_YES;\n \treturn is_descendant_of(commit, filter->with_commit);\n }\n \n@@ -1438,7 +1454,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,\n \t\t\treturn 0;\n \t\t/* We perform the filtering for the '--contains' option */\n \t\tif (filter->with_commit &&\n-\t\t !commit_contains(filter, commit))\n+\t\t !commit_contains(filter, commit, &ref_cbdata->contains_cache))\n \t\t\treturn 0;\n \t}\n \n@@ -1538,6 +1554,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int\n \t\tbroken = 1;\n \tfilter->kind = type & FILTER_REFS_KIND_MASK;\n \n+\tinit_contains_cache(&ref_cbdata.contains_cache);\n+\n \t/* Simple per-ref filtering */\n \tif (!filter->kind)\n \t\tdie(\"filter_refs: invalid type\");\n@@ -1560,6 +1578,7 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int\n \t\t\thead_ref(ref_filter_handler, &ref_cbdata);\n \t}\n \n+\tclear_contains_cache(&ref_cbdata.contains_cache);\n \n \t/* Filters that need revision walking */\n \tif (filter->merge_commit)\ndiff --git a/ref-filter.h b/ref-filter.h\nindex 7b05592ba..89af9f451 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -71,11 +71,6 @@ struct ref_filter {\n \t\tverbose;\n };\n \n-struct ref_filter_cbdata {\n-\tstruct ref_array *array;\n-\tstruct ref_filter *filter;\n-};\n-\n /* Macros for checking --merged and --no-merged options */\n #define _OPT_MERGED_NO_MERGED(option, filter, h) \\\n \t{ OPTION_CALLBACK, 0, option, (filter), N_(\"commit\"), (h), \\\n"},{"id":"313642","messageId":"20170309132728.c57ltzel746l366a@sigill.intra.peff.net","threadId":"45321","inReplyTo":"20170309125132.tubwxtneffok4nrc@sigill.intra.peff.net","subject":"[PATCH 0/4] fix object flag pollution in \"tag --contains\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-09T13:27:28Z","receivedAt":"2017-03-09T13:27:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 09, 2017 at 07:51:32AM -0500, Jeff King wrote:\n\n> Looking at this, I'm pretty sure that using \"--contains\" with \"--merged\"\n> has similar problems, as they both use the UNINTERESTING bit. So even\n> without your patch, there is a lurking bug.\n\nI wasn't able to come up with a simple case that actually demonstrates\nthe bug. But I feel like it has to be triggerable with the right\nsequence of history.\n\nEven without that, though, I feel like moving away from this flag usage\nis a good cleanup. Here's a cleaned-up series. What do you think of\nbuilding your patch on top?\n\nWe can do it the other way around if you prefer.\n\n [1/4]: ref-filter: move ref_cbdata definition into ref-filter.c\n [2/4]: ref-filter: use contains_result enum consistently\n [3/4]: ref-filter: die on parse_commit errors\n [4/4]: ref-filter: use separate cache for contains_tag_algo\n\n ref-filter.c | 70 ++++++++++++++++++++++++++++++++++++++----------------------\n ref-filter.h | 5 -----\n 2 files changed, 44 insertions(+), 31 deletions(-)\n\n-Peff\n"},{"id":"313643","messageId":"20170309132755.x4zp3wtzpjmeyjpb@sigill.intra.peff.net","threadId":"45321","inReplyTo":"20170309132728.c57ltzel746l366a@sigill.intra.peff.net","subject":"[PATCH 1/4] ref-filter: move ref_cbdata definition into ref-filter.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-09T13:27:55Z","receivedAt":"2017-03-09T13:28:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This is an implementation detail of how filter_refs() works,\nand does not need to be exposed to the outside world. This\nwill become more important in future patches as we add new\nprivate data types to it.\n\nSigned-off-by: Jeff King \n---\n ref-filter.c | 5 +++++\n ref-filter.h | 5 -----\n 2 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 1ec0fb839..6546dba73 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1476,6 +1476,11 @@ enum contains_result {\n \tCONTAINS_YES = 1\n };\n \n+struct ref_filter_cbdata {\n+\tstruct ref_array *array;\n+\tstruct ref_filter *filter;\n+};\n+\n /*\n * Mimicking the real stack, this stack lives on the heap, avoiding stack\n * overflows.\ndiff --git a/ref-filter.h b/ref-filter.h\nindex 154e24c40..e738c5dfd 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -71,11 +71,6 @@ struct ref_filter {\n \t\tverbose;\n };\n \n-struct ref_filter_cbdata {\n-\tstruct ref_array *array;\n-\tstruct ref_filter *filter;\n-};\n-\n /* Macros for checking --merged and --no-merged options */\n #define _OPT_MERGED_NO_MERGED(option, filter, h) \\\n \t{ OPTION_CALLBACK, 0, option, (filter), N_(\"commit\"), (h), \\\n-- \n2.12.0.445.g818af77e0\n\n"},{"id":"313644","messageId":"20170309132847.ptysbio377cy765j@sigill.intra.peff.net","threadId":"45321","inReplyTo":"20170309132728.c57ltzel746l366a@sigill.intra.peff.net","subject":"[PATCH 2/4] ref-filter: use contains_result enum consistently","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-09T13:28:48Z","receivedAt":"2017-03-09T13:29:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Commit cbc60b672 (git tag --contains: avoid stack overflow,\n2014-04-24) adapted the -1/0/1 contains status into a\ntri-state enum. However, some of the code still used the\nnumeric values, or assumed that no/yes correspond to C's\nboolean true/false.\n\nLet's switch to using the symbolic values everywhere, which\nwill make it easier to change them.\n\nSigned-off-by: Jeff King \n---\n ref-filter.c | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 6546dba73..631978a4f 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1513,20 +1513,20 @@ static enum contains_result contains_test(struct commit *candidate,\n {\n \t/* was it previously marked as containing a want commit? */\n \tif (candidate->object.flags & TMP_MARK)\n-\t\treturn 1;\n+\t\treturn CONTAINS_YES;\n \t/* or marked as not possibly containing a want commit? */\n \tif (candidate->object.flags & UNINTERESTING)\n-\t\treturn 0;\n+\t\treturn CONTAINS_NO;\n \t/* or are we it? */\n \tif (in_commit_list(want, candidate)) {\n \t\tcandidate->object.flags |= TMP_MARK;\n-\t\treturn 1;\n+\t\treturn CONTAINS_YES;\n \t}\n \n \tif (parse_commit(candidate) < 0)\n-\t\treturn 0;\n+\t\treturn CONTAINS_NO;\n \n-\treturn -1;\n+\treturn CONTAINS_UNKNOWN;\n }\n \n static void push_to_contains_stack(struct commit *candidate, struct contains_stack *contains_stack)\n@@ -1540,7 +1540,7 @@ static enum contains_result contains_tag_algo(struct commit *candidate,\n \t\tconst struct commit_list *want)\n {\n \tstruct contains_stack contains_stack = { 0, 0, NULL };\n-\tint result = contains_test(candidate, want);\n+\tenum contains_result result = contains_test(candidate, want);\n \n \tif (result != CONTAINS_UNKNOWN)\n \t\treturn result;\n@@ -1557,7 +1557,7 @@ static enum contains_result contains_tag_algo(struct commit *candidate,\n \t\t}\n \t\t/*\n \t\t * If we just popped the stack, parents->item has been marked,\n-\t\t * therefore contains_test will return a meaningful 0 or 1.\n+\t\t * therefore contains_test will return a meaningful yes/no.\n \t\t */\n \t\telse switch (contains_test(parents->item, want)) {\n \t\tcase CONTAINS_YES:\n@@ -1579,7 +1579,7 @@ static enum contains_result contains_tag_algo(struct commit *candidate,\n static int commit_contains(struct ref_filter *filter, struct commit *commit)\n {\n \tif (filter->with_commit_tag_algo)\n-\t\treturn contains_tag_algo(commit, filter->with_commit);\n+\t\treturn contains_tag_algo(commit, filter->with_commit) == CONTAINS_YES;\n \treturn is_descendant_of(commit, filter->with_commit);\n }\n \n-- \n2.12.0.445.g818af77e0\n\n"},{"id":"313645","messageId":"20170309132903.om7b2uvn6vsvjiwt@sigill.intra.peff.net","threadId":"45321","inReplyTo":"20170309132728.c57ltzel746l366a@sigill.intra.peff.net","subject":"[PATCH 3/4] ref-filter: die on parse_commit errors","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-09T13:29:04Z","receivedAt":"2017-03-09T13:29:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The tag-contains algorithm quietly returns \"does not\ncontain\" when parse_commit() fails. But a parse failure is\nan indication that the repository is corrupt. We should die\nloudly rather than producing a bogus result.\n\nSigned-off-by: Jeff King \n---\n ref-filter.c | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 631978a4f..5cb49b7c2 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1523,9 +1523,7 @@ static enum contains_result contains_test(struct commit *candidate,\n \t\treturn CONTAINS_YES;\n \t}\n \n-\tif (parse_commit(candidate) < 0)\n-\t\treturn CONTAINS_NO;\n-\n+\tparse_commit_or_die(candidate);\n \treturn CONTAINS_UNKNOWN;\n }\n \n-- \n2.12.0.445.g818af77e0\n\n"},{"id":"313646","messageId":"20170309132949.shyc4gmdrhazykev@sigill.intra.peff.net","threadId":"45321","inReplyTo":"20170309132728.c57ltzel746l366a@sigill.intra.peff.net","subject":"[PATCH 4/4] ref-filter: use separate cache for contains_tag_algo","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-09T13:29:49Z","receivedAt":"2017-03-09T13:37:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The algorithm which powers \"tag --contains\" uses the\nTMP_MARK and UNINTERESTING bits, but never cleans up after\nitself. As a result, stale UNINTERESTING bits may impact\nlater traversals (like \"--merged\").\n\nWe could fix this by clearing the bits after we're done with\nthe --contains traversal. That would be enough to fix the\nexisting problem, but it leaves future developers in a bad\nspot: they cannot add other traversals that operate\nsimultaneously with --contains (e.g., if you wanted to add\n\"--no-contains\" and use both filters at the same time).\n\nInstead, we can use a commit slab to store our cached\nresults, which will store the bits outside of the commit\nstructs entirely. This adds an extra level of indirection,\nbut in my tests (running \"git tag --contains HEAD\" on\nlinux.git), there was no measurable slowdown.\n\nSigned-off-by: Jeff King \n---\n ref-filter.c | 55 +++++++++++++++++++++++++++++++++++--------------------\n 1 file changed, 35 insertions(+), 20 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 5cb49b7c2..7eeecc608 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -15,6 +15,7 @@\n #include \"version.h\"\n #include \"trailer.h\"\n #include \"wt-status.h\"\n+#include \"commit-slab.h\"\n \n static struct ref_msg {\n \tconst char *gone;\n@@ -1470,15 +1471,22 @@ static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom\n \t*v = &ref->value[atom];\n }\n \n+/*\n+ * Unknown has to be \"0\" here, because that's the default value for\n+ * contains_cache slab entries that have not yet been assigned.\n+ */\n enum contains_result {\n-\tCONTAINS_UNKNOWN = -1,\n-\tCONTAINS_NO = 0,\n-\tCONTAINS_YES = 1\n+\tCONTAINS_UNKNOWN = 0,\n+\tCONTAINS_NO,\n+\tCONTAINS_YES\n };\n \n+define_commit_slab(contains_cache, enum contains_result);\n+\n struct ref_filter_cbdata {\n \tstruct ref_array *array;\n \tstruct ref_filter *filter;\n+\tstruct contains_cache contains_cache;\n };\n \n /*\n@@ -1509,20 +1517,22 @@ static int in_commit_list(const struct commit_list *want, struct commit *c)\n * Do not recurse to find out, though, but return -1 if inconclusive.\n */\n static enum contains_result contains_test(struct commit *candidate,\n-\t\t\t const struct commit_list *want)\n+\t\t\t\t\t const struct commit_list *want,\n+\t\t\t\t\t struct contains_cache *cache)\n {\n-\t/* was it previously marked as containing a want commit? */\n-\tif (candidate->object.flags & TMP_MARK)\n-\t\treturn CONTAINS_YES;\n-\t/* or marked as not possibly containing a want commit? */\n-\tif (candidate->object.flags & UNINTERESTING)\n-\t\treturn CONTAINS_NO;\n+\tenum contains_result *cached = contains_cache_at(cache, candidate);\n+\n+\t/* If we already have the answer cached, return that. */\n+\tif (*cached)\n+\t\treturn *cached;\n+\n \t/* or are we it? */\n \tif (in_commit_list(want, candidate)) {\n-\t\tcandidate->object.flags |= TMP_MARK;\n+\t\t*cached = CONTAINS_YES;\n \t\treturn CONTAINS_YES;\n \t}\n \n+\t/* Otherwise, we don't know; prepare to recurse */\n \tparse_commit_or_die(candidate);\n \treturn CONTAINS_UNKNOWN;\n }\n@@ -1535,10 +1545,11 @@ static void push_to_contains_stack(struct commit *candidate, struct contains_sta\n }\n \n static enum contains_result contains_tag_algo(struct commit *candidate,\n-\t\tconst struct commit_list *want)\n+\t\t\t\t\t const struct commit_list *want,\n+\t\t\t\t\t struct contains_cache *cache)\n {\n \tstruct contains_stack contains_stack = { 0, 0, NULL };\n-\tenum contains_result result = contains_test(candidate, want);\n+\tenum contains_result result = contains_test(candidate, want, cache);\n \n \tif (result != CONTAINS_UNKNOWN)\n \t\treturn result;\n@@ -1550,16 +1561,16 @@ static enum contains_result contains_tag_algo(struct commit *candidate,\n \t\tstruct commit_list *parents = entry->parents;\n \n \t\tif (!parents) {\n-\t\t\tcommit->object.flags |= UNINTERESTING;\n+\t\t\t*contains_cache_at(cache, commit) = CONTAINS_NO;\n \t\t\tcontains_stack.nr--;\n \t\t}\n \t\t/*\n \t\t * If we just popped the stack, parents->item has been marked,\n \t\t * therefore contains_test will return a meaningful yes/no.\n \t\t */\n-\t\telse switch (contains_test(parents->item, want)) {\n+\t\telse switch (contains_test(parents->item, want, cache)) {\n \t\tcase CONTAINS_YES:\n-\t\t\tcommit->object.flags |= TMP_MARK;\n+\t\t\t*contains_cache_at(cache, commit) = CONTAINS_YES;\n \t\t\tcontains_stack.nr--;\n \t\t\tbreak;\n \t\tcase CONTAINS_NO:\n@@ -1571,13 +1582,14 @@ static enum contains_result contains_tag_algo(struct commit *candidate,\n \t\t}\n \t}\n \tfree(contains_stack.contains_stack);\n-\treturn contains_test(candidate, want);\n+\treturn contains_test(candidate, want, cache);\n }\n \n-static int commit_contains(struct ref_filter *filter, struct commit *commit)\n+static int commit_contains(struct ref_filter *filter, struct commit *commit,\n+\t\t\t struct contains_cache *cache)\n {\n \tif (filter->with_commit_tag_algo)\n-\t\treturn contains_tag_algo(commit, filter->with_commit) == CONTAINS_YES;\n+\t\treturn contains_tag_algo(commit, filter->with_commit, cache) == CONTAINS_YES;\n \treturn is_descendant_of(commit, filter->with_commit);\n }\n \n@@ -1774,7 +1786,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,\n \t\t\treturn 0;\n \t\t/* We perform the filtering for the '--contains' option */\n \t\tif (filter->with_commit &&\n-\t\t !commit_contains(filter, commit))\n+\t\t !commit_contains(filter, commit, &ref_cbdata->contains_cache))\n \t\t\treturn 0;\n \t}\n \n@@ -1874,6 +1886,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int\n \t\tbroken = 1;\n \tfilter->kind = type & FILTER_REFS_KIND_MASK;\n \n+\tinit_contains_cache(&ref_cbdata.contains_cache);\n+\n \t/* Simple per-ref filtering */\n \tif (!filter->kind)\n \t\tdie(\"filter_refs: invalid type\");\n@@ -1896,6 +1910,7 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int\n \t\t\thead_ref(ref_filter_handler, &ref_cbdata);\n \t}\n \n+\tclear_contains_cache(&ref_cbdata.contains_cache);\n \n \t/* Filters that need revision walking */\n \tif (filter->merge_commit)\n-- \n2.12.0.445.g818af77e0\n"},{"id":"313647","messageId":"CACBZZX4W7brFe5ecTvQPMmf4X5_zH+dw3cB5xeVg+2hWYps0Ug@mail.gmail.com","threadId":"45321","inReplyTo":"20170309125132.tubwxtneffok4nrc@sigill.intra.peff.net","subject":"Re: [PATCH] branch & tag: Add a --no-contains option","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-03-09T14:52:09Z","receivedAt":"2017-03-09T14:52:43Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Mar 9, 2017 at 1:51 PM, Jeff King wrote:\n> On Thu, Mar 09, 2017 at 01:12:08PM +0100, Ævar Arnfjörð Bjarmason wrote:\n>\n>> > I'm almost certain this is because the contains_tag_algo one doesn't\n>> > clean up the flag bits it sets on the commit objects. So running it\n>> > twice in the same process is going to give you nonsense results.\n>>\n>> Yeah indeed.\n>>\n>> I tried to hack something up to avoid this, but the\n>> lookup_commit_reference_gently() we call will return the same\n>> object.parent pointer for two invocations, i.e. the underlying\n>> {commit,object}.c API has a cache of objects it returns, couldn't find\n>> some way to quickly make it burst that cache.\n>\n> Yeah, you'll always get the same struct for a given sha1.\n>\n>> The other approach of making contains_tag_algo() itself detect that\n>> it's been called before (or us passing a flag) and going around\n>> setting commit.object.flags on everything to 0 seemed even more\n>> brittle, particularly since I think between filter->with_commit &\n>> filter->no_commit we might end up visiting different commits, so it's\n>> not easy to just clear it.\n>\n> You can clear the marks with clear_object_flags(). But I don't think\n> that type of solution will quite help us here. We consider each ref\n> independently and ask \"does it match --contains\" and \"does it match\n> --no-contains?\". So there is no moment where we are done with all of the\n> --contains marks, and can move on to the --no-contains ones. The lookups\n> are interleaved.\n>\n> We could move to doing them in chunks (the way filter->merge_commit\n> works), and then clearing in between. Or we could use a separate bitset.\n> The patch below does that.\n>\n>> I'm happy to hack on it given some pointers, will visit it again, but\n>> for now unless I'm missing something obvious / you can point out some\n>> way to hack this up I'll just submit v2 with the combination of\n>> --contains & --no-contains dying with a TODO message.\n>>\n>> The patch without that functionality is still really useful, and we\n>> can implement that later.\n>\n> I'm not opposed to that, though see what you think of the patch below.\n> It's a bit noisy but it's conceptually pretty straightforward. It should\n> hopefully be obvious how you'd add in a separate contains_cache for the\n> \"without\" case.\n\nI wonder how worthwhile making the combination of options case fast &\nadding more complexity is, as opposed to just using the slower path\nfor those cases via:\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 737a83028a..d90e8675a8 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -53,7 +53,10 @@ static int list_tags(struct ref_filter *filter,\nstruct ref_sorting *sorting, con\n }\n\n verify_ref_format(format);\n- filter->with_commit_tag_algo = 1;\n+ if ((filter->merge_commit + filter->with_commit +\nfilter->no_commit) > 1)\n+ filter->with_commit_tag_algo = 0;\n+ else\n+ filter->with_commit_tag_algo = 1;\n filter_refs(&array, filter, FILTER_REFS_TAGS);\n ref_array_sort(sorting, &array);\n\nI think I'll amend the tip of my WIP v2 to have something like that,\nand then we can follow-up with making these cases where you supply\nmultiple options faster.\n\n> Looking at this, I'm pretty sure that using \"--contains\" with \"--merged\"\n> has similar problems, as they both use the UNINTERESTING bit. So even\n> without your patch, there is a lurking bug.\n\nI'm currently running this:\nhttps://gist.github.com/avar/45cf288ce7cdc43e7395c6cbf9a98d68\n\nwith this patch to builtin/tag.c:\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 737a83028a..b93c5e1754 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -56,2 +56,4 @@ static int list_tags(struct ref_filter *filter,\nstruct ref_sorting *sorting, con\n filter->with_commit_tag_algo = 1;\n+ if (getenv(\"GIT_NO_TAG_ALGO\"))\n+ filter->with_commit_tag_algo = 0;\n\nTo smoke out any combinations of commits in git.git where with &\nwithout the tag algo we already return different results, nothing so\nfar, but it's slow going.\n\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 3820b21cc..42b1bc463 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -14,6 +14,7 @@\n> #include \"git-compat-util.h\"\n> #include \"version.h\"\n> #include \"trailer.h\"\n> +#include \"commit-slab.h\"\n>\n> typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;\n>\n> @@ -1137,10 +1138,22 @@ static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom\n> *v = &ref->value[atom];\n> }\n>\n> +/*\n> + * Unknown has to be \"0\" here, because that's what unfilled entries in our slab\n> + * will return.\n> + */\n> enum contains_result {\n> - CONTAINS_UNKNOWN = -1,\n> - CONTAINS_NO = 0,\n> - CONTAINS_YES = 1\n> + CONTAINS_UNKNOWN = 0,\n> + CONTAINS_NO = 1,\n> + CONTAINS_YES = 2,\n> +};\n> +\n> +define_commit_slab(contains_cache, enum contains_result);\n> +\n> +struct ref_filter_cbdata {\n> + struct ref_array *array;\n> + struct ref_filter *filter;\n> + struct contains_cache contains_cache;\n> };\n>\n> /*\n> @@ -1171,24 +1184,25 @@ static int in_commit_list(const struct commit_list *want, struct commit *c)\n> * Do not recurse to find out, though, but return -1 if inconclusive.\n> */\n> static enum contains_result contains_test(struct commit *candidate,\n> - const struct commit_list *want)\n> + const struct commit_list *want,\n> + struct contains_cache *cache)\n> {\n> - /* was it previously marked as containing a want commit? */\n> - if (candidate->object.flags & TMP_MARK)\n> - return 1;\n> - /* or marked as not possibly containing a want commit? */\n> - if (candidate->object.flags & UNINTERESTING)\n> - return 0;\n> + enum contains_result *cached = contains_cache_at(cache, candidate);\n> +\n> + /* if we already found the answer, return it without traversing */\n> + if (*cached)\n> + return *cached;\n> +\n> /* or are we it? */\n> if (in_commit_list(want, candidate)) {\n> - candidate->object.flags |= TMP_MARK;\n> - return 1;\n> + *cached = CONTAINS_YES;\n> + return *cached;\n> }\n>\n> if (parse_commit(candidate) < 0)\n> - return 0;\n> + return CONTAINS_NO;\n>\n> - return -1;\n> + return CONTAINS_UNKNOWN;\n> }\n>\n> static void push_to_contains_stack(struct commit *candidate, struct contains_stack *contains_stack)\n> @@ -1199,10 +1213,11 @@ static void push_to_contains_stack(struct commit *candidate, struct contains_sta\n> }\n>\n> static enum contains_result contains_tag_algo(struct commit *candidate,\n> - const struct commit_list *want)\n> + const struct commit_list *want,\n> + struct contains_cache *cache)\n> {\n> struct contains_stack contains_stack = { 0, 0, NULL };\n> - int result = contains_test(candidate, want);\n> + enum contains_result result = contains_test(candidate, want, cache);\n>\n> if (result != CONTAINS_UNKNOWN)\n> return result;\n> @@ -1214,16 +1229,16 @@ static enum contains_result contains_tag_algo(struct commit *candidate,\n> struct commit_list *parents = entry->parents;\n>\n> if (!parents) {\n> - commit->object.flags |= UNINTERESTING;\n> + *contains_cache_at(cache, commit) = CONTAINS_NO;\n> contains_stack.nr--;\n> }\n> /*\n> * If we just popped the stack, parents->item has been marked,\n> - * therefore contains_test will return a meaningful 0 or 1.\n> + * therefore contains_test will return a meaningful yes/no.\n> */\n> - else switch (contains_test(parents->item, want)) {\n> + else switch (contains_test(parents->item, want, cache)) {\n> case CONTAINS_YES:\n> - commit->object.flags |= TMP_MARK;\n> + *contains_cache_at(cache, commit) = CONTAINS_YES;\n> contains_stack.nr--;\n> break;\n> case CONTAINS_NO:\n> @@ -1235,13 +1250,14 @@ static enum contains_result contains_tag_algo(struct commit *candidate,\n> }\n> }\n> free(contains_stack.contains_stack);\n> - return contains_test(candidate, want);\n> + return contains_test(candidate, want, cache);\n> }\n>\n> -static int commit_contains(struct ref_filter *filter, struct commit *commit)\n> +static int commit_contains(struct ref_filter *filter, struct commit *commit,\n> + struct contains_cache *cache)\n> {\n> if (filter->with_commit_tag_algo)\n> - return contains_tag_algo(commit, filter->with_commit);\n> + return contains_tag_algo(commit, filter->with_commit, cache) == CONTAINS_YES;\n> return is_descendant_of(commit, filter->with_commit);\n> }\n>\n> @@ -1438,7 +1454,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,\n> return 0;\n> /* We perform the filtering for the '--contains' option */\n> if (filter->with_commit &&\n> - !commit_contains(filter, commit))\n> + !commit_contains(filter, commit, &ref_cbdata->contains_cache))\n> return 0;\n> }\n>\n> @@ -1538,6 +1554,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int\n> broken = 1;\n> filter->kind = type & FILTER_REFS_KIND_MASK;\n>\n> + init_contains_cache(&ref_cbdata.contains_cache);\n> +\n> /* Simple per-ref filtering */\n> if (!filter->kind)\n> die(\"filter_refs: invalid type\");\n> @@ -1560,6 +1578,7 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int\n> head_ref(ref_filter_handler, &ref_cbdata);\n> }\n>\n> + clear_contains_cache(&ref_cbdata.contains_cache);\n>\n> /* Filters that need revision walking */\n> if (filter->merge_commit)\n> diff --git a/ref-filter.h b/ref-filter.h\n> index 7b05592ba..89af9f451 100644\n> --- a/ref-filter.h\n> +++ b/ref-filter.h\n> @@ -71,11 +71,6 @@ struct ref_filter {\n> verbose;\n> };\n>\n> -struct ref_filter_cbdata {\n> - struct ref_array *array;\n> - struct ref_filter *filter;\n> -};\n> -\n> /* Macros for checking --merged and --no-merged options */\n> #define _OPT_MERGED_NO_MERGED(option, filter, h) \\\n> { OPTION_CALLBACK, 0, option, (filter), N_(\"commit\"), (h), \\\n"},{"id":"313648","messageId":"20170309145555.kzrttqya3bghdhe4@sigill.intra.peff.net","threadId":"45321","inReplyTo":"CACBZZX4W7brFe5ecTvQPMmf4X5_zH+dw3cB5xeVg+2hWYps0Ug@mail.gmail.com","subject":"Re: [PATCH] branch & tag: Add a --no-contains option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-09T14:55:55Z","receivedAt":"2017-03-09T15:02:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 09, 2017 at 03:52:09PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> - filter->with_commit_tag_algo = 1;\n> + if ((filter->merge_commit + filter->with_commit +\n> filter->no_commit) > 1)\n> + filter->with_commit_tag_algo = 0;\n> + else\n> + filter->with_commit_tag_algo = 1;\n> filter_refs(&array, filter, FILTER_REFS_TAGS);\n> ref_array_sort(sorting, &array);\n> \n> I think I'll amend the tip of my WIP v2 to have something like that,\n> and then we can follow-up with making these cases where you supply\n> multiple options faster.\n\nYeah, that's another option. I think you may find that it's unbearably\nslow if you have a lot of tags.\n\n> > Looking at this, I'm pretty sure that using \"--contains\" with \"--merged\"\n> > has similar problems, as they both use the UNINTERESTING bit. So even\n> > without your patch, there is a lurking bug.\n> \n> I'm currently running this:\n> https://gist.github.com/avar/45cf288ce7cdc43e7395c6cbf9a98d68\n\nCute. I'll be curious if it turns up anything.\n\n-Peff\n"},{"id":"313672","messageId":"20170309200243.2203-1-avarab@gmail.com","threadId":"45321","inReplyTo":"20170309125132.tubwxtneffok4nrc@sigill.intra.peff.net","subject":"[PATCH v2] ref-filter: Add --no-contains option to tag/branch/for-each-ref","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-03-09T20:02:43Z","receivedAt":"2017-03-09T20:03:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the tag, branch & for-each-ref commands to have a --no-contains\noption in addition to their longstanding --contains options.\n\nThe use-case I have for this is to find the last-good rollout tag\ngiven a known-bad . Right now, given a hypothetically bad\ncommit v2.10.1-3-gcf5c7253e0, you can find which git version to revert\nto with this hacky two-liner:\n\n (./git tag -l 'v[0-9]*'; ./git tag -l 'v[0-9]*' --contains v2.10.1-3-gcf5c7253e0) \\\n |sort|uniq -c|grep -E '^ *1 '|awk '{print $2}' | tail -n 10\n\nBut with the --no-contains option you can now get the exact same\noutput with:\n\n ./git tag -l 'v[0-9]*' --no-contains v2.10.1-3-gcf5c7253e0|sort|tail -n 10\n\nThe filtering machinery is generic between the tag, branch &\nfor-each-ref commands, so once I'd implemented it for tag it was\ntrivial to add support for this to the other two.\n\nA practical use for this with \"branch\" is e.g. finding branches which\ndiverged between 2.8 & 2.10:\n\n ./git branch --contains v2.8.0 --no-contains v2.10.0\n\nThe \"describe\" command also has a --contains option, but its semantics\nare unrelated to what tag/branch/for-each-ref use --contains for. I\ndon't see how a --no-contains option for \"describe\" would make any\nsense, other than being exactly equivalent to not supplying --contains\nat all, which would be confusing at best.\n\nI'm adding a --without option to \"tag\" as an alias for --no-contains\nfor consistency with --with and --contains. Since we don't even\ndocument --with anymore (or test it). The --with option is\nundocumented, and possibly the only user of it is Junio[1]. But it's\ntrivial to support, so let's do that.\n\nWhere I'm changing existing documentation lines I'm mainly word\nwrapping at 75 columns to be consistent with the existing style. The\nchanges to Documentation/ are much smaller with: git show --word-diff,\nsame for the minor change to git-completion.bash.\n\nMost of the test changes I've made are just doing the inverse of the\nexisting --contains tests, with this change --no-contains for tag,\nbranch & for-each-ref is just as well tested as the existing\n--contains option.\n\nIn addition to those tests I've added tests for --contains in\ncombination with --no-contains for all three commands, and a test for\n\"tag\" which asserts that --no-contains won't find tree/blob tags,\nwhich is slightly unintuitive, but consistent with how --contains\nworks.\n\nWhen --contains and --no-contains are provided to \"tag\" we disable the\noptimized code to find tags added in v1.7.2-rc1-1-gffc4b8012d. That\ncode changes flags on commit objects as an optimization, and thus\ncan't be called twice.\n\nJeff King has a WIP patch to fix that[2], but rather than try to\nincorporate that into this already big patch I'm just disabling the\noptimized codepath. Using both --contains and --no-contains will most\nlikely be rare, and we can get an initial correct version working &\noptimize it later.\n\nIt's possible that the --merge & --no-merge codepaths for \"tag\" have a\nsimilar bug, but as of writing I can't produce any evidence of that\nvia a brute-force test script[3].\n\n1. \n2. <20170309125132.tubwxtneffok4nrc@sigill.intra.peff.net>\n3. \n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nChanges since v1:\n\n * Now git-for-each-ref has --no-contains\n\n * Doc fixes, happy with the NOTES section in git-branch.txt now.\n\n * Now tag --contains works with --no-contains, but falls back on\n the much slower pre-1.8 era code pending efforts to make\n commit_contains() re-callable.\n\n * Now `git tag --no-contains HEAD hi` dies, just like `git tag\n --contains HEAD hi` does.\n\n * Fixed up my shitty C in ref-filter.c as Peff suggested.\n\n * Tests for --contains combined with --no-contains for all the\n commands.\n\n * Rewrote the very verbose t7004-tag.sh tag test Peff complied\n about.\n\n Documentation/git-branch.txt | 24 ++++---\n Documentation/git-for-each-ref.txt | 6 +-\n Documentation/git-tag.txt | 6 +-\n builtin/branch.c | 4 +-\n builtin/for-each-ref.c | 1 +\n builtin/tag.c | 15 +++-\n contrib/completion/git-completion.bash | 9 +--\n parse-options.h | 4 +-\n ref-filter.c | 16 +++--\n ref-filter.h | 1 +\n t/t3201-branch-contains.sh | 51 +++++++++++++-\n t/t6302-for-each-ref-filter.sh | 16 +++++\n t/t7004-tag.sh | 123 ++++++++++++++++++++++++++++++++-\n 13 files changed, 249 insertions(+), 27 deletions(-)\n\ndiff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\nindex 092f1bcf9f..28a36a8a0a 100644\n--- a/Documentation/git-branch.txt\n+++ b/Documentation/git-branch.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n 'git branch' [--color[=] | --no-color] [-r | -a]\n \t[--list] [-v [--abbrev= | --no-abbrev]]\n \t[--column[=] | --no-column]\n-\t[(--merged | --no-merged | --contains) []] [--sort=]\n+\t[(--merged | --no-merged | --contains | --no-contains) []] [--sort=]\n \t[--points-at ] [--format=] [...]\n 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] []\n 'git branch' (--set-upstream-to= | -u ) []\n@@ -35,11 +35,12 @@ as branch creation.\n \n With `--contains`, shows only the branches that contain the named commit\n (in other words, the branches whose tip commits are descendants of the\n-named commit). With `--merged`, only branches merged into the named\n-commit (i.e. the branches whose tip commits are reachable from the named\n-commit) will be listed. With `--no-merged` only branches not merged into\n-the named commit will be listed. If the argument is missing it\n-defaults to `HEAD` (i.e. the tip of the current branch).\n+named commit), `--no-contains` inverts it. With `--merged`, only branches\n+merged into the named commit (i.e. the branches whose tip commits are\n+reachable from the named commit) will be listed. With `--no-merged` only\n+branches not merged into the named commit will be listed. If the \n+argument is missing it defaults to `HEAD` (i.e. the tip of the current\n+branch).\n \n The command's second form creates a new branch head named \n which points to the current `HEAD`, or if given.\n@@ -213,6 +214,10 @@ start-point is either a local or remote-tracking branch.\n \tOnly list branches which contain the specified commit (HEAD\n \tif not specified). Implies `--list`.\n \n+--no-contains []::\n+\tOnly list branches which don't contain the specified commit\n+\t(HEAD if not specified). Implies `--list`.\n+\n --merged []::\n \tOnly list branches whose tips are reachable from the\n \tspecified commit (HEAD if not specified). Implies `--list`.\n@@ -296,13 +301,16 @@ If you are creating a branch that you want to checkout immediately, it is\n easier to use the git checkout command with its `-b` option to create\n a branch and check it out with a single command.\n \n-The options `--contains`, `--merged` and `--no-merged` serve three related\n-but different purposes:\n+The options `--contains`, `--no-contains`, `--merged` and `--no-merged`\n+serve four related but different purposes:\n \n - `--contains ` is used to find all branches which will need\n special attention if were to be rebased or amended, since those\n branches contain the specified .\n \n+- `--no-contains ` is the inverse of that, i.e. branches that don't\n+ contain the specified .\n+\n - `--merged` is used to find all branches which can be safely deleted,\n since those branches are fully contained by HEAD.\n \ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 111e1be6f5..83b93c75a8 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n 'git for-each-ref' [--count=] [--shell|--perl|--python|--tcl]\n \t\t [(--sort=)...] [--format=] [...]\n \t\t [--points-at ] [(--merged | --no-merged) []]\n-\t\t [--contains []]\n+\t\t [(--contains | --no-contains) []]\n \n DESCRIPTION\n -----------\n@@ -79,6 +79,10 @@ OPTIONS\n \tOnly list refs which contain the specified commit (HEAD if not\n \tspecified).\n \n+--no-contains []::\n+\tOnly list refs which don't contain the specified commit (HEAD\n+\tif not specified).\n+\n --ignore-case::\n \tSorting and filtering refs are case insensitive.\n \ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex 525737a5d8..4938496194 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -12,7 +12,7 @@ SYNOPSIS\n 'git tag' [-a | -s | -u ] [-f] [-m | -F ]\n \t [ | ]\n 'git tag' -d ...\n-'git tag' [-n[]] -l [--contains ] [--points-at ]\n+'git tag' [-n[]] -l [--[no-]contains ] [--points-at ]\n \t[--column[=] | --no-column] [--create-reflog] [--sort=]\n \t[--format=] [--[no-]merged []] [...]\n 'git tag' -v [--format=] ...\n@@ -124,6 +124,10 @@ This option is only applicable when listing tags without annotation lines.\n \tOnly list tags which contain the specified commit (HEAD if not\n \tspecified).\n \n+--no-contains []::\n+\tOnly list tags which don't contain the specified commit (HEAD if\n+\tnot secified).\n+\n --points-at ::\n \tOnly list tags of the given object.\n \ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 94f7de7fa5..e8d534604c 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -548,7 +548,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT_SET_INT('r', \"remotes\", &filter.kind, N_(\"act on remote-tracking branches\"),\n \t\t\tFILTER_REFS_REMOTES),\n \t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only branches that contain the commit\")),\n+\t\tOPT_NO_CONTAINS(&filter.no_commit, N_(\"print only branches that don't contain the commit\")),\n \t\tOPT_WITH(&filter.with_commit, N_(\"print only branches that contain the commit\")),\n+\t\tOPT_WITHOUT(&filter.with_commit, N_(\"print only branches that don't contain the commit\")),\n \t\tOPT__ABBREV(&filter.abbrev),\n \n \t\tOPT_GROUP(N_(\"Specific git-branch actions:\")),\n@@ -604,7 +606,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \tif (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)\n \t\tlist = 1;\n \n-\tif (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)\n+\tif (filter.with_commit || filter.no_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)\n \t\tlist = 1;\n \n \tif (!!delete + !!rename + !!new_upstream +\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex df41fa0350..b1ae2388e6 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -43,6 +43,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \t\tOPT_MERGED(&filter, N_(\"print only refs that are merged\")),\n \t\tOPT_NO_MERGED(&filter, N_(\"print only refs that are not merged\")),\n \t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only refs which contain the commit\")),\n+\t\tOPT_NO_CONTAINS(&filter.no_commit, N_(\"print only refs which don't contain the commit\")),\n \t\tOPT_BOOL(0, \"ignore-case\", &icase, N_(\"sorting and filtering are case insensitive\")),\n \t\tOPT_END(),\n \t};\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex ad29be6923..d83674e3e6 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -53,7 +53,16 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con\n \t}\n \n \tverify_ref_format(format);\n-\tfilter->with_commit_tag_algo = 1;\n+\tif (filter->with_commit && filter->no_commit)\n+\t\t/* Due to the way the contains_tag_algo() function\n+\t\t touches object.flags we can only call it once\n+\t\t per-process.\n+\n+\t\t For now sacrifice performance for correctness when\n+\t\t both --contains and --no-contains are provided */\n+\t\tfilter->with_commit_tag_algo = 0;\n+\telse\n+\t\tfilter->with_commit_tag_algo = 1;\n \tfilter_refs(&array, filter, FILTER_REFS_TAGS);\n \tref_array_sort(sorting, &array);\n \n@@ -424,7 +433,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tOPT_GROUP(N_(\"Tag listing options\")),\n \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"show tag list in columns\")),\n \t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n+\t\tOPT_NO_CONTAINS(&filter.no_commit, N_(\"print only tags that don't contain the commit\")),\n \t\tOPT_WITH(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n+\t\tOPT_WITHOUT(&filter.no_commit, N_(\"print only tags that don't contain the commit\")),\n \t\tOPT_MERGED(&filter, N_(\"print only tags that are merged\")),\n \t\tOPT_NO_MERGED(&filter, N_(\"print only tags that are not merged\")),\n \t\tOPT_CALLBACK(0 , \"sort\", sorting_tail, N_(\"key\"),\n@@ -488,6 +499,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"-n option is only allowed with -l.\"));\n \tif (filter.with_commit)\n \t\tdie(_(\"--contains option is only allowed with -l.\"));\n+\tif (filter.no_commit)\n+\t\tdie(_(\"--no-contains option is only allowed with -l.\"));\n \tif (filter.points_at.nr)\n \t\tdie(_(\"--points-at option is only allowed with -l.\"));\n \tif (filter.merge_commit)\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex fc32286a43..fa3da49478 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1093,9 +1093,9 @@ _git_branch ()\n \t--*)\n \t\t__gitcomp \"\n \t\t\t--color --no-color --verbose --abbrev= --no-abbrev\n-\t\t\t--track --no-track --contains --merged --no-merged\n-\t\t\t--set-upstream-to= --edit-description --list\n-\t\t\t--unset-upstream --delete --move --remotes\n+\t\t\t--track --no-track --contains --no-contains --merged\n+\t\t\t--no-merged --set-upstream-to= --edit-description\n+\t\t\t--list --unset-upstream --delete --move --remotes\n \t\t\t--column --no-column --sort= --points-at\n \t\t\t\"\n \t\t;;\n@@ -2862,7 +2862,8 @@ _git_tag ()\n \t\t__gitcomp \"\n \t\t\t--list --delete --verify --annotate --message --file\n \t\t\t--sign --cleanup --local-user --force --column --sort=\n-\t\t\t--contains --points-at --merged --no-merged --create-reflog\n+\t\t\t--contains --no-contains --points-at --merged\n+\t\t\t--no-merged --create-reflog\n \t\t\t\"\n \t\t;;\n \tesac\ndiff --git a/parse-options.h b/parse-options.h\nindex dcd8a0926c..0eac90b510 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -258,7 +258,9 @@ extern int parse_opt_passthru_argv(const struct option *, const char *, int);\n \t PARSE_OPT_LASTARG_DEFAULT | flag, \\\n \t parse_opt_commits, (intptr_t) \"HEAD\" \\\n \t}\n-#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"contains\", v, h, 0)\n+#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"contains\", v, h, PARSE_OPT_NONEG)\n+#define OPT_NO_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"no-contains\", v, h, PARSE_OPT_NONEG)\n #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH(\"with\", v, h, PARSE_OPT_HIDDEN)\n+#define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH(\"without\", v, h, PARSE_OPT_HIDDEN)\n \n #endif\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 1ec0fb8391..b6dab75729 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1571,11 +1571,11 @@ static enum contains_result contains_tag_algo(struct commit *candidate,\n \treturn contains_test(candidate, want);\n }\n \n-static int commit_contains(struct ref_filter *filter, struct commit *commit)\n+static int commit_contains(struct ref_filter *filter, struct commit_list *list, struct commit *commit)\n {\n \tif (filter->with_commit_tag_algo)\n-\t\treturn contains_tag_algo(commit, filter->with_commit);\n-\treturn is_descendant_of(commit, filter->with_commit);\n+\t\treturn contains_tag_algo(commit, list);\n+\treturn is_descendant_of(commit, list);\n }\n \n /*\n@@ -1765,13 +1765,17 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,\n \t * obtain the commit using the 'oid' available and discard all\n \t * non-commits early. The actual filtering is done later.\n \t */\n-\tif (filter->merge_commit || filter->with_commit || filter->verbose) {\n+\tif (filter->merge_commit || filter->with_commit || filter->no_commit || filter->verbose) {\n \t\tcommit = lookup_commit_reference_gently(oid->hash, 1);\n \t\tif (!commit)\n \t\t\treturn 0;\n-\t\t/* We perform the filtering for the '--contains' option */\n+\t\t/* We perform the filtering for the '--contains' option... */\n \t\tif (filter->with_commit &&\n-\t\t !commit_contains(filter, commit))\n+\t\t !commit_contains(filter, filter->with_commit, commit))\n+\t\t\treturn 0;\n+\t\t/* ...or for the `--no-contains' option */\n+\t\tif (filter->no_commit &&\n+\t\t commit_contains(filter, filter->no_commit, commit))\n \t\t\treturn 0;\n \t}\n \ndiff --git a/ref-filter.h b/ref-filter.h\nindex 154e24c405..af85eb4592 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -53,6 +53,7 @@ struct ref_filter {\n \tconst char **name_patterns;\n \tstruct sha1_array points_at;\n \tstruct commit_list *with_commit;\n+\tstruct commit_list *no_commit;\n \n \tenum {\n \t\tREF_FILTER_MERGED_NONE = 0,\ndiff --git a/t/t3201-branch-contains.sh b/t/t3201-branch-contains.sh\nindex 7f3ec47241..506415dbd3 100755\n--- a/t/t3201-branch-contains.sh\n+++ b/t/t3201-branch-contains.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-test_description='branch --contains , --merged, and --no-merged'\n+test_description='branch --contains , --no-contains --merged, and --no-merged'\n \n . ./test-lib.sh\n \n@@ -45,6 +45,22 @@ test_expect_success 'branch --contains master' '\n \n '\n \n+test_expect_success 'branch --no-contains=master' '\n+\n+\tgit branch --no-contains=master >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+\n+'\n+\n+test_expect_success 'branch --no-contains master' '\n+\n+\tgit branch --no-contains master >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+\n+'\n+\n test_expect_success 'branch --contains=side' '\n \n \tgit branch --contains=side >actual &&\n@@ -55,6 +71,16 @@ test_expect_success 'branch --contains=side' '\n \n '\n \n+test_expect_success 'branch --no-contains=side' '\n+\n+\tgit branch --no-contains=side >actual &&\n+\t{\n+\t\techo \" master\"\n+\t} >expect &&\n+\ttest_cmp expect actual\n+\n+'\n+\n test_expect_success 'branch --contains with pattern implies --list' '\n \n \tgit branch --contains=master master >actual &&\n@@ -65,6 +91,14 @@ test_expect_success 'branch --contains with pattern implies --list' '\n \n '\n \n+test_expect_success 'branch --no-contains with pattern implies --list' '\n+\n+\tgit branch --no-contains=master master >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+\n+'\n+\n test_expect_success 'side: branch --merged' '\n \n \tgit branch --merged >actual &&\n@@ -126,7 +160,9 @@ test_expect_success 'branch --no-merged with pattern implies --list' '\n test_expect_success 'implicit --list conflicts with modification options' '\n \n \ttest_must_fail git branch --contains=master -d &&\n-\ttest_must_fail git branch --contains=master -m foo\n+\ttest_must_fail git branch --contains=master -m foo &&\n+\ttest_must_fail git branch --no-contains=master -d &&\n+\ttest_must_fail git branch --no-contains=master -m foo\n \n '\n \n@@ -159,4 +195,15 @@ test_expect_success 'branch --merged with --verbose' '\n \ttest_i18ncmp expect actual\n '\n \n+test_expect_success 'branch --contains combined with --no-contains' '\n+\tgit branch --contains zzz --no-contains topic >actual &&\n+\tcat >expect <<-\\EOF &&\n+\t master\n+\t side\n+\t zzz\n+\tEOF\n+\ttest_cmp expect actual\n+\n+'\n+\n test_done\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex a09a1a46ef..4902ba5f16 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -93,6 +93,22 @@ test_expect_success 'filtering with --contains' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'filtering with --no-contains' '\n+\tcat >expect <<-\\EOF &&\n+\trefs/tags/one\n+\tEOF\n+\tgit for-each-ref --format=\"%(refname)\" --no-contains=two >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'filtering with --contains and --no-contains' '\n+\tcat >expect <<-\\EOF &&\n+\trefs/tags/two\n+\tEOF\n+\tgit for-each-ref --format=\"%(refname)\" --contains=two --no-contains=three >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '%(color) must fail' '\n \ttest_must_fail git for-each-ref --format=\"%(color)%(refname)\"\n '\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex b4698ab5f5..8ad5719962 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1385,6 +1385,23 @@ test_expect_success 'checking that first commit is in all tags (relative)' \"\n \ttest_cmp expected actual\n \"\n \n+# All the --contains tests above, but with --no-contains\n+test_expect_success 'checking that first commit is not listed in any tag with --no-contains (hash)' \"\n+\t>expected &&\n+\tgit tag -l --no-contains $hash1 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n+test_expect_success 'checking that first commit is in all tags (tag)' \"\n+\tgit tag -l --no-contains v1.0 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n+test_expect_success 'checking that first commit is in all tags (relative)' \"\n+\tgit tag -l --no-contains HEAD~2 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n cat > expected < expected <actual &&\n+\ttest_cmp expected actual\n+\"\n \n cat > expected < expected <actual &&\n+\ttest_cmp expected actual\n+\"\n+\n # how about a simple merge?\n \n test_expect_success 'creating simple branch' '\n@@ -1424,6 +1465,19 @@ test_expect_success 'checking that branch head only has one tag' \"\n \ttest_cmp expected actual\n \"\n \n+cat > expected <actual &&\n+\ttest_cmp expected actual\n+\"\n+\n test_expect_success 'merging original branch into this branch' '\n \tgit merge --strategy=ours master &&\n git tag v4.0\n@@ -1445,6 +1499,20 @@ v1.0.1\n v1.1.3\n v2.0\n v3.0\n+EOF\n+\n+test_expect_success 'checking that original branch head with --no-contains lists all but one tag now' \"\n+\tgit tag -l --no-contains $hash3 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n+cat > expected <expected &&\n+\tgit tag -l --no-contains $hash1 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n # mixing modes and options:\n \n test_expect_success 'mixing incompatibles modes and options is forbidden' '\n@@ -1709,7 +1783,7 @@ run_with_limited_stack () {\n test_lazy_prereq ULIMIT_STACK_SIZE 'run_with_limited_stack true'\n \n # we require ulimit, this excludes Windows\n-test_expect_success ULIMIT_STACK_SIZE '--contains works in a deep repo' '\n+test_expect_success ULIMIT_STACK_SIZE '--contains and --no-contains work in a deep repo' '\n \t>expect &&\n \ti=1 &&\n \twhile test $i -lt 8000\n@@ -1725,7 +1799,9 @@ EOF\"\n \tgit checkout master &&\n \tgit tag far-far-away HEAD^ &&\n \trun_with_limited_stack git tag --contains HEAD >actual &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect actual &&\n+\trun_with_limited_stack git tag --no-contains HEAD >actual &&\n+\ttest_line_count \">\" 70 actual\n '\n \n test_expect_success '--format should list tags as per format given' '\n@@ -1773,4 +1849,47 @@ test_expect_success 'ambiguous branch/tags not marked' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--contains combined with --no-contains' '\n+\t(\n+\t\tgit init no-contains &&\n+\t\tcd no-contains &&\n+\t\ttest_commit v0.1 &&\n+\t\ttest_commit v0.2 &&\n+\t\ttest_commit v0.3 &&\n+\t\ttest_commit v0.4 &&\n+\t\ttest_commit v0.5 &&\n+\t\tcat >expected <<-\\EOF &&\n+\t\tv0.2\n+\t\tv0.3\n+\t\tv0.4\n+\t\tEOF\n+\t\tgit tag --contains v0.2 --no-contains v0.5 >actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n+# As the docs say, list tags which contain a specified *commit*. We\n+# don't recurse down to tags for trees or blobs pointed to by *those*\n+# commits.\n+test_expect_success 'Does --[no-]contains stop at commits? Yes!' '\n+\tcd no-contains &&\n+\tblob=$(git rev-parse v0.3:v0.3.t) &&\n+\ttree=$(git rev-parse v0.3^{tree}) &&\n+\tgit tag tag-blob $blob &&\n+\tgit tag tag-tree $tree &&\n+\tgit tag --contains v0.3 >actual &&\n+\tcat >expected <<-\\EOF &&\n+\tv0.3\n+\tv0.4\n+\tv0.5\n+\tEOF\n+\ttest_cmp expected actual &&\n+\tgit tag --no-contains v0.3 >actual &&\n+\tcat >expected <<-\\EOF &&\n+\tv0.1\n+\tv0.2\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.11.0\n\n"},{"id":"313677","messageId":"CAP8UFD3zGTO5RBpX-WE+8KfTa0pv5NcNDtBRVdRVEdGomL_Wfg@mail.gmail.com","threadId":"45321","inReplyTo":"20170309200243.2203-1-avarab@gmail.com","subject":"Re: [PATCH v2] ref-filter: Add --no-contains option to tag/branch/for-each-ref","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-03-09T20:31:51Z","receivedAt":"2017-03-09T20:32:36Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Mar 9, 2017 at 9:02 PM, Ævar Arnfjörð Bjarmason\n wrote:\n>\n> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\n> index 525737a5d8..4938496194 100644\n> --- a/Documentation/git-tag.txt\n> +++ b/Documentation/git-tag.txt\n> @@ -12,7 +12,7 @@ SYNOPSIS\n> 'git tag' [-a | -s | -u ] [-f] [-m | -F ]\n> [ | ]\n> 'git tag' -d ...\n> -'git tag' [-n[]] -l [--contains ] [--points-at ]\n> +'git tag' [-n[]] -l [--[no-]contains ] [--points-at ]\n> [--column[=] | --no-column] [--create-reflog] [--sort=]\n> [--format=] [--[no-]merged []] [...]\n> 'git tag' -v [--format=] ...\n> @@ -124,6 +124,10 @@ This option is only applicable when listing tags without annotation lines.\n> Only list tags which contain the specified commit (HEAD if not\n> specified).\n>\n> +--no-contains []::\n> + Only list tags which don't contain the specified commit (HEAD if\n> + not secified).\n\ns/secified/specified/\n\n> +\n> --points-at ::\n> Only list tags of the given object.\n>\n> diff --git a/builtin/branch.c b/builtin/branch.c\n> index 94f7de7fa5..e8d534604c 100644\n> --- a/builtin/branch.c\n> +++ b/builtin/branch.c\n> @@ -548,7 +548,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n> OPT_SET_INT('r', \"remotes\", &filter.kind, N_(\"act on remote-tracking branches\"),\n> FILTER_REFS_REMOTES),\n> OPT_CONTAINS(&filter.with_commit, N_(\"print only branches that contain the commit\")),\n> + OPT_NO_CONTAINS(&filter.no_commit, N_(\"print only branches that don't contain the commit\")),\n> OPT_WITH(&filter.with_commit, N_(\"print only branches that contain the commit\")),\n> + OPT_WITHOUT(&filter.with_commit, N_(\"print only branches that don't contain the commit\")),\n\ns/with_commit/no_commit/\n"},{"id":"313744","messageId":"CACBZZX7-OBUX5BiMq04HEV+RSPP-WeOmjzbXBi0z7Gf6=fFefw@mail.gmail.com","threadId":"45321","inReplyTo":"20170309145555.kzrttqya3bghdhe4@sigill.intra.peff.net","subject":"Re: [PATCH] branch & tag: Add a --no-contains option","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-03-10T11:31:35Z","receivedAt":"2017-03-10T11:32:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Mar 9, 2017 at 3:55 PM, Jeff King wrote:\n> On Thu, Mar 09, 2017 at 03:52:09PM +0100, Ævar Arnfjörð Bjarmason wrote:\n>\n>> - filter->with_commit_tag_algo = 1;\n>> + if ((filter->merge_commit + filter->with_commit +\n>> filter->no_commit) > 1)\n>> + filter->with_commit_tag_algo = 0;\n>> + else\n>> + filter->with_commit_tag_algo = 1;\n>> filter_refs(&array, filter, FILTER_REFS_TAGS);\n>> ref_array_sort(sorting, &array);\n>>\n>> I think I'll amend the tip of my WIP v2 to have something like that,\n>> and then we can follow-up with making these cases where you supply\n>> multiple options faster.\n>\n> Yeah, that's another option. I think you may find that it's unbearably\n> slow if you have a lot of tags.\n\nIt's what I'm going for in my v2 currently. I think it's a rare enough\n(and new) use-case that it's OK to slow it down for now & investigate\nyour line of patching separately.\n\n>> > Looking at this, I'm pretty sure that using \"--contains\" with \"--merged\"\n>> > has similar problems, as they both use the UNINTERESTING bit. So even\n>> > without your patch, there is a lurking bug.\n>>\n>> I'm currently running this:\n>> https://gist.github.com/avar/45cf288ce7cdc43e7395c6cbf9a98d68\n>\n> Cute. I'll be curious if it turns up anything.\n\nCurrently at ~500 attempts out of 5K with no bad results.\n"},{"id":"313745","messageId":"CACBZZX74fpEzic3Qs0kG5i5pb-up+Ct0cp71beDXaj+BoDcKRQ@mail.gmail.com","threadId":"45321","inReplyTo":"CAP8UFD3zGTO5RBpX-WE+8KfTa0pv5NcNDtBRVdRVEdGomL_Wfg@mail.gmail.com","subject":"Re: [PATCH v2] ref-filter: Add --no-contains option to tag/branch/for-each-ref","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-03-10T11:46:24Z","receivedAt":"2017-03-10T11:46:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Mar 9, 2017 at 9:31 PM, Christian Couder\n wrote:\n> On Thu, Mar 9, 2017 at 9:02 PM, Ævar Arnfjörð Bjarmason\n> wrote:\n>>\n>> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\n>> index 525737a5d8..4938496194 100644\n>> --- a/Documentation/git-tag.txt\n>> +++ b/Documentation/git-tag.txt\n>> @@ -12,7 +12,7 @@ SYNOPSIS\n>> 'git tag' [-a | -s | -u ] [-f] [-m | -F ]\n>> [ | ]\n>> 'git tag' -d ...\n>> -'git tag' [-n[]] -l [--contains ] [--points-at ]\n>> +'git tag' [-n[]] -l [--[no-]contains ] [--points-at ]\n>> [--column[=] | --no-column] [--create-reflog] [--sort=]\n>> [--format=] [--[no-]merged []] [...]\n>> 'git tag' -v [--format=] ...\n>> @@ -124,6 +124,10 @@ This option is only applicable when listing tags without annotation lines.\n>> Only list tags which contain the specified commit (HEAD if not\n>> specified).\n>>\n>> +--no-contains []::\n>> + Only list tags which don't contain the specified commit (HEAD if\n>> + not secified).\n>\n> s/secified/specified/\n>\n>> +\n>> --points-at ::\n>> Only list tags of the given object.\n>>\n>> diff --git a/builtin/branch.c b/builtin/branch.c\n>> index 94f7de7fa5..e8d534604c 100644\n>> --- a/builtin/branch.c\n>> +++ b/builtin/branch.c\n>> @@ -548,7 +548,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n>> OPT_SET_INT('r', \"remotes\", &filter.kind, N_(\"act on remote-tracking branches\"),\n>> FILTER_REFS_REMOTES),\n>> OPT_CONTAINS(&filter.with_commit, N_(\"print only branches that contain the commit\")),\n>> + OPT_NO_CONTAINS(&filter.no_commit, N_(\"print only branches that don't contain the commit\")),\n>> OPT_WITH(&filter.with_commit, N_(\"print only branches that contain the commit\")),\n>> + OPT_WITHOUT(&filter.with_commit, N_(\"print only branches that don't contain the commit\")),\n>\n> s/with_commit/no_commit/\n\nThanks!\n\nFWIW this is the current status of my WIP v3. I noticed a couple of\nother issues where --contains was mentioned without --no-contains.\n\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex 4938496194..d9243bf5e4 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -129 +129 @@ This option is only applicable when listing tags\nwithout annotation lines.\n- not secified).\n+ not specified).\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex e8d534604c..dd96319726 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -553 +553 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n- OPT_WITHOUT(&filter.with_commit, N_(\"print only\nbranches that don't contain the commit\")),\n+ OPT_WITHOUT(&filter.no_commit, N_(\"print only branches\nthat don't contain the commit\")),\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex b1ae2388e6..a11542c4fd 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -12 +12 @@ static char const * const for_each_ref_usage[] = {\n- N_(\"git for-each-ref [--contains []]\"),\n+ N_(\"git for-each-ref [(--contains | --no-contains) []]\"),\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex d83674e3e6..57289132a9 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -25 +25 @@ static const char * const git_tag_usage[] = {\n- N_(\"git tag -l [-n[]] [--contains ] [--points-at ]\"\n+ N_(\"git tag -l [-n[]] [--[no-]contains ]\n[--points-at ]\"\n\n\nThese last two hunks are going to bust the i18n files for most\nlanguages. Junio/Jiang, in cases like these where I could fix those up\nwith a search/replace on po/* without knowing the languages in\nquestion (since it's purely changing e.g. --contains to\n--[no-]contains), what do you prefer to do, have that as part of this\npatch, or do it after the fact through the normal i18n maintenance\nprocess?\n"},{"id":"313746","messageId":"CACBZZX7EW013ZhDtUWome5nOqLjha34B9gJCRQgc0J0qkvWBuA@mail.gmail.com","threadId":"45321","inReplyTo":"CACBZZX74fpEzic3Qs0kG5i5pb-up+Ct0cp71beDXaj+BoDcKRQ@mail.gmail.com","subject":"Re: [PATCH v2] ref-filter: Add --no-contains option to tag/branch/for-each-ref","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-03-10T12:09:30Z","receivedAt":"2017-03-10T12:10:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Fri, Mar 10, 2017 at 12:46 PM, Ævar Arnfjörð Bjarmason\n wrote:\n> On Thu, Mar 9, 2017 at 9:31 PM, Christian Couder\n> wrote:\n>> On Thu, Mar 9, 2017 at 9:02 PM, Ævar Arnfjörð Bjarmason\n>> wrote:\n>>>\n>>> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\n>>> index 525737a5d8..4938496194 100644\n>>> --- a/Documentation/git-tag.txt\n>>> +++ b/Documentation/git-tag.txt\n>>> @@ -12,7 +12,7 @@ SYNOPSIS\n>>> 'git tag' [-a | -s | -u ] [-f] [-m | -F ]\n>>> [ | ]\n>>> 'git tag' -d ...\n>>> -'git tag' [-n[]] -l [--contains ] [--points-at ]\n>>> +'git tag' [-n[]] -l [--[no-]contains ] [--points-at ]\n>>> [--column[=] | --no-column] [--create-reflog] [--sort=]\n>>> [--format=] [--[no-]merged []] [...]\n>>> 'git tag' -v [--format=] ...\n>>> @@ -124,6 +124,10 @@ This option is only applicable when listing tags without annotation lines.\n>>> Only list tags which contain the specified commit (HEAD if not\n>>> specified).\n>>>\n>>> +--no-contains []::\n>>> + Only list tags which don't contain the specified commit (HEAD if\n>>> + not secified).\n>>\n>> s/secified/specified/\n>>\n>>> +\n>>> --points-at ::\n>>> Only list tags of the given object.\n>>>\n>>> diff --git a/builtin/branch.c b/builtin/branch.c\n>>> index 94f7de7fa5..e8d534604c 100644\n>>> --- a/builtin/branch.c\n>>> +++ b/builtin/branch.c\n>>> @@ -548,7 +548,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n>>> OPT_SET_INT('r', \"remotes\", &filter.kind, N_(\"act on remote-tracking branches\"),\n>>> FILTER_REFS_REMOTES),\n>>> OPT_CONTAINS(&filter.with_commit, N_(\"print only branches that contain the commit\")),\n>>> + OPT_NO_CONTAINS(&filter.no_commit, N_(\"print only branches that don't contain the commit\")),\n>>> OPT_WITH(&filter.with_commit, N_(\"print only branches that contain the commit\")),\n>>> + OPT_WITHOUT(&filter.with_commit, N_(\"print only branches that don't contain the commit\")),\n>>\n>> s/with_commit/no_commit/\n>\n> Thanks!\n>\n> FWIW this is the current status of my WIP v3. I noticed a couple of\n> other issues where --contains was mentioned without --no-contains.\n>\n> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\n> index 4938496194..d9243bf5e4 100644\n> --- a/Documentation/git-tag.txt\n> +++ b/Documentation/git-tag.txt\n> @@ -129 +129 @@ This option is only applicable when listing tags\n> without annotation lines.\n> - not secified).\n> + not specified).\n> diff --git a/builtin/branch.c b/builtin/branch.c\n> index e8d534604c..dd96319726 100644\n> --- a/builtin/branch.c\n> +++ b/builtin/branch.c\n> @@ -553 +553 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n> - OPT_WITHOUT(&filter.with_commit, N_(\"print only\n> branches that don't contain the commit\")),\n> + OPT_WITHOUT(&filter.no_commit, N_(\"print only branches\n> that don't contain the commit\")),\n> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\n> index b1ae2388e6..a11542c4fd 100644\n> --- a/builtin/for-each-ref.c\n> +++ b/builtin/for-each-ref.c\n> @@ -12 +12 @@ static char const * const for_each_ref_usage[] = {\n> - N_(\"git for-each-ref [--contains []]\"),\n> + N_(\"git for-each-ref [(--contains | --no-contains) []]\"),\n> diff --git a/builtin/tag.c b/builtin/tag.c\n> index d83674e3e6..57289132a9 100644\n> --- a/builtin/tag.c\n> +++ b/builtin/tag.c\n> @@ -25 +25 @@ static const char * const git_tag_usage[] = {\n> - N_(\"git tag -l [-n[]] [--contains ] [--points-at ]\"\n> + N_(\"git tag -l [-n[]] [--[no-]contains ]\n> [--points-at ]\"\n\nAdd to that:\n\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 8ad5719962..bec3d0fb42 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1804 +1804 @@ EOF\"\n- test_line_count \">\" 70 actual\n+ test_line_count \">\" 10 actual\n\nThe tests currently fail with v2 if gpg isn't installed, which'll\ncause the number to dip below 70, just setting it to a much more\nconservative 10, but maybe this should just be \"test -s actual\" ...\n\n>\n> These last two hunks are going to bust the i18n files for most\n> languages. Junio/Jiang, in cases like these where I could fix those up\n> with a search/replace on po/* without knowing the languages in\n> question (since it's purely changing e.g. --contains to\n> --[no-]contains), what do you prefer to do, have that as part of this\n> patch, or do it after the fact through the normal i18n maintenance\n> process?\n"},{"id":"313794","messageId":"20170310203348.675-1-avarab@gmail.com","threadId":"45321","inReplyTo":"20170309200243.2203-1-avarab@gmail.com","subject":"[PATCH v3] ref-filter: Add --no-contains option to tag/branch/for-each-ref","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-03-10T20:33:48Z","receivedAt":"2017-03-10T20:34:09Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the tag, branch & for-each-ref commands to have a --no-contains\noption in addition to their longstanding --contains options.\n\nThe use-case I have for this is to find the last-good rollout tag\ngiven a known-bad . Right now, given a hypothetically bad\ncommit v2.10.1-3-gcf5c7253e0, you can find which git version to revert\nto with this hacky two-liner:\n\n (./git tag -l 'v[0-9]*'; ./git tag -l 'v[0-9]*' --contains v2.10.1-3-gcf5c7253e0) \\\n |sort|uniq -c|grep -E '^ *1 '|awk '{print $2}' | tail -n 10\n\nBut with the --no-contains option you can now get the exact same\noutput with:\n\n ./git tag -l 'v[0-9]*' --no-contains v2.10.1-3-gcf5c7253e0|sort|tail -n 10\n\nThe filtering machinery is generic between the tag, branch &\nfor-each-ref commands, so once I'd implemented it for tag it was\ntrivial to add support for this to the other two.\n\nA practical use for this with \"branch\" is e.g. finding branches which\ndiverged between 2.8 & 2.10:\n\n ./git branch --contains v2.8.0 --no-contains v2.10.0\n\nThe \"describe\" command also has a --contains option, but its semantics\nare unrelated to what tag/branch/for-each-ref use --contains for. I\ndon't see how a --no-contains option for \"describe\" would make any\nsense, other than being exactly equivalent to not supplying --contains\nat all, which would be confusing at best.\n\nI'm adding a --without option to \"tag\" as an alias for --no-contains\nfor consistency with --with and --contains. Since we don't even\ndocument --with anymore (or test it). The --with option is\nundocumented, and possibly the only user of it is Junio[1]. But it's\ntrivial to support, so let's do that.\n\nWhere I'm changing existing documentation lines I'm mainly word\nwrapping at 75 columns to be consistent with the existing style. The\nchanges to Documentation/ are much smaller with: git show --word-diff,\nsame for the minor change to git-completion.bash.\n\nMost of the test changes I've made are just doing the inverse of the\nexisting --contains tests, with this change --no-contains for tag,\nbranch & for-each-ref is just as well tested as the existing\n--contains option.\n\nIn addition to those tests I've added tests for --contains in\ncombination with --no-contains for all three commands, and a test for\n\"tag\" which asserts that --no-contains won't find tree/blob tags,\nwhich is slightly unintuitive, but consistent with how --contains\nworks.\n\nWhen --contains and --no-contains are provided to \"tag\" we disable the\noptimized code to find tags added in v1.7.2-rc1-1-gffc4b8012d. That\ncode changes flags on commit objects as an optimization, and thus\ncan't be called twice.\n\nJeff King has a WIP patch to fix that[2], but rather than try to\nincorporate that into this already big patch I'm just disabling the\noptimized codepath. Using both --contains and --no-contains will most\nlikely be rare, and we can get an initial correct version working &\noptimize it later.\n\nIt's possible that the --merge & --no-merge codepaths for \"tag\" have a\nsimilar bug, but as of writing I can't produce any evidence of that\nvia a brute-force test script[3].\n\n1. \n2. <20170309125132.tubwxtneffok4nrc@sigill.intra.peff.net>\n3. \n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nWith the changes noted in my\n\n&&\n.\n\n Documentation/git-branch.txt | 24 ++++---\n Documentation/git-for-each-ref.txt | 6 +-\n Documentation/git-tag.txt | 6 +-\n builtin/branch.c | 4 +-\n builtin/for-each-ref.c | 3 +-\n builtin/tag.c | 17 ++++-\n contrib/completion/git-completion.bash | 9 +--\n parse-options.h | 4 +-\n ref-filter.c | 16 +++--\n ref-filter.h | 1 +\n t/t3201-branch-contains.sh | 51 +++++++++++++-\n t/t6302-for-each-ref-filter.sh | 16 +++++\n t/t7004-tag.sh | 123 ++++++++++++++++++++++++++++++++-\n 13 files changed, 251 insertions(+), 29 deletions(-)\n\ndiff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\nindex 092f1bcf9f..28a36a8a0a 100644\n--- a/Documentation/git-branch.txt\n+++ b/Documentation/git-branch.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n 'git branch' [--color[=] | --no-color] [-r | -a]\n \t[--list] [-v [--abbrev= | --no-abbrev]]\n \t[--column[=] | --no-column]\n-\t[(--merged | --no-merged | --contains) []] [--sort=]\n+\t[(--merged | --no-merged | --contains | --no-contains) []] [--sort=]\n \t[--points-at ] [--format=] [...]\n 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] []\n 'git branch' (--set-upstream-to= | -u ) []\n@@ -35,11 +35,12 @@ as branch creation.\n \n With `--contains`, shows only the branches that contain the named commit\n (in other words, the branches whose tip commits are descendants of the\n-named commit). With `--merged`, only branches merged into the named\n-commit (i.e. the branches whose tip commits are reachable from the named\n-commit) will be listed. With `--no-merged` only branches not merged into\n-the named commit will be listed. If the argument is missing it\n-defaults to `HEAD` (i.e. the tip of the current branch).\n+named commit), `--no-contains` inverts it. With `--merged`, only branches\n+merged into the named commit (i.e. the branches whose tip commits are\n+reachable from the named commit) will be listed. With `--no-merged` only\n+branches not merged into the named commit will be listed. If the \n+argument is missing it defaults to `HEAD` (i.e. the tip of the current\n+branch).\n \n The command's second form creates a new branch head named \n which points to the current `HEAD`, or if given.\n@@ -213,6 +214,10 @@ start-point is either a local or remote-tracking branch.\n \tOnly list branches which contain the specified commit (HEAD\n \tif not specified). Implies `--list`.\n \n+--no-contains []::\n+\tOnly list branches which don't contain the specified commit\n+\t(HEAD if not specified). Implies `--list`.\n+\n --merged []::\n \tOnly list branches whose tips are reachable from the\n \tspecified commit (HEAD if not specified). Implies `--list`.\n@@ -296,13 +301,16 @@ If you are creating a branch that you want to checkout immediately, it is\n easier to use the git checkout command with its `-b` option to create\n a branch and check it out with a single command.\n \n-The options `--contains`, `--merged` and `--no-merged` serve three related\n-but different purposes:\n+The options `--contains`, `--no-contains`, `--merged` and `--no-merged`\n+serve four related but different purposes:\n \n - `--contains ` is used to find all branches which will need\n special attention if were to be rebased or amended, since those\n branches contain the specified .\n \n+- `--no-contains ` is the inverse of that, i.e. branches that don't\n+ contain the specified .\n+\n - `--merged` is used to find all branches which can be safely deleted,\n since those branches are fully contained by HEAD.\n \ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 111e1be6f5..83b93c75a8 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n 'git for-each-ref' [--count=] [--shell|--perl|--python|--tcl]\n \t\t [(--sort=)...] [--format=] [...]\n \t\t [--points-at ] [(--merged | --no-merged) []]\n-\t\t [--contains []]\n+\t\t [(--contains | --no-contains) []]\n \n DESCRIPTION\n -----------\n@@ -79,6 +79,10 @@ OPTIONS\n \tOnly list refs which contain the specified commit (HEAD if not\n \tspecified).\n \n+--no-contains []::\n+\tOnly list refs which don't contain the specified commit (HEAD\n+\tif not specified).\n+\n --ignore-case::\n \tSorting and filtering refs are case insensitive.\n \ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex 525737a5d8..d9243bf5e4 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -12,7 +12,7 @@ SYNOPSIS\n 'git tag' [-a | -s | -u ] [-f] [-m | -F ]\n \t [ | ]\n 'git tag' -d ...\n-'git tag' [-n[]] -l [--contains ] [--points-at ]\n+'git tag' [-n[]] -l [--[no-]contains ] [--points-at ]\n \t[--column[=] | --no-column] [--create-reflog] [--sort=]\n \t[--format=] [--[no-]merged []] [...]\n 'git tag' -v [--format=] ...\n@@ -124,6 +124,10 @@ This option is only applicable when listing tags without annotation lines.\n \tOnly list tags which contain the specified commit (HEAD if not\n \tspecified).\n \n+--no-contains []::\n+\tOnly list tags which don't contain the specified commit (HEAD if\n+\tnot specified).\n+\n --points-at ::\n \tOnly list tags of the given object.\n \ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 94f7de7fa5..dd96319726 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -548,7 +548,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT_SET_INT('r', \"remotes\", &filter.kind, N_(\"act on remote-tracking branches\"),\n \t\t\tFILTER_REFS_REMOTES),\n \t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only branches that contain the commit\")),\n+\t\tOPT_NO_CONTAINS(&filter.no_commit, N_(\"print only branches that don't contain the commit\")),\n \t\tOPT_WITH(&filter.with_commit, N_(\"print only branches that contain the commit\")),\n+\t\tOPT_WITHOUT(&filter.no_commit, N_(\"print only branches that don't contain the commit\")),\n \t\tOPT__ABBREV(&filter.abbrev),\n \n \t\tOPT_GROUP(N_(\"Specific git-branch actions:\")),\n@@ -604,7 +606,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \tif (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)\n \t\tlist = 1;\n \n-\tif (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)\n+\tif (filter.with_commit || filter.no_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)\n \t\tlist = 1;\n \n \tif (!!delete + !!rename + !!new_upstream +\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex df41fa0350..a11542c4fd 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -9,7 +9,7 @@ static char const * const for_each_ref_usage[] = {\n \tN_(\"git for-each-ref [] []\"),\n \tN_(\"git for-each-ref [--points-at ]\"),\n \tN_(\"git for-each-ref [(--merged | --no-merged) []]\"),\n-\tN_(\"git for-each-ref [--contains []]\"),\n+\tN_(\"git for-each-ref [(--contains | --no-contains) []]\"),\n \tNULL\n };\n \n@@ -43,6 +43,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \t\tOPT_MERGED(&filter, N_(\"print only refs that are merged\")),\n \t\tOPT_NO_MERGED(&filter, N_(\"print only refs that are not merged\")),\n \t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only refs which contain the commit\")),\n+\t\tOPT_NO_CONTAINS(&filter.no_commit, N_(\"print only refs which don't contain the commit\")),\n \t\tOPT_BOOL(0, \"ignore-case\", &icase, N_(\"sorting and filtering are case insensitive\")),\n \t\tOPT_END(),\n \t};\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex ad29be6923..57289132a9 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -22,7 +22,7 @@\n static const char * const git_tag_usage[] = {\n \tN_(\"git tag [-a | -s | -u ] [-f] [-m | -F ] []\"),\n \tN_(\"git tag -d ...\"),\n-\tN_(\"git tag -l [-n[]] [--contains ] [--points-at ]\"\n+\tN_(\"git tag -l [-n[]] [--[no-]contains ] [--points-at ]\"\n \t\t\"\\n\\t\\t[--format=] [--[no-]merged []] [...]\"),\n \tN_(\"git tag -v [--format=] ...\"),\n \tNULL\n@@ -53,7 +53,16 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con\n \t}\n \n \tverify_ref_format(format);\n-\tfilter->with_commit_tag_algo = 1;\n+\tif (filter->with_commit && filter->no_commit)\n+\t\t/* Due to the way the contains_tag_algo() function\n+\t\t touches object.flags we can only call it once\n+\t\t per-process.\n+\n+\t\t For now sacrifice performance for correctness when\n+\t\t both --contains and --no-contains are provided */\n+\t\tfilter->with_commit_tag_algo = 0;\n+\telse\n+\t\tfilter->with_commit_tag_algo = 1;\n \tfilter_refs(&array, filter, FILTER_REFS_TAGS);\n \tref_array_sort(sorting, &array);\n \n@@ -424,7 +433,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tOPT_GROUP(N_(\"Tag listing options\")),\n \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"show tag list in columns\")),\n \t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n+\t\tOPT_NO_CONTAINS(&filter.no_commit, N_(\"print only tags that don't contain the commit\")),\n \t\tOPT_WITH(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n+\t\tOPT_WITHOUT(&filter.no_commit, N_(\"print only tags that don't contain the commit\")),\n \t\tOPT_MERGED(&filter, N_(\"print only tags that are merged\")),\n \t\tOPT_NO_MERGED(&filter, N_(\"print only tags that are not merged\")),\n \t\tOPT_CALLBACK(0 , \"sort\", sorting_tail, N_(\"key\"),\n@@ -488,6 +499,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"-n option is only allowed with -l.\"));\n \tif (filter.with_commit)\n \t\tdie(_(\"--contains option is only allowed with -l.\"));\n+\tif (filter.no_commit)\n+\t\tdie(_(\"--no-contains option is only allowed with -l.\"));\n \tif (filter.points_at.nr)\n \t\tdie(_(\"--points-at option is only allowed with -l.\"));\n \tif (filter.merge_commit)\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex fc32286a43..fa3da49478 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1093,9 +1093,9 @@ _git_branch ()\n \t--*)\n \t\t__gitcomp \"\n \t\t\t--color --no-color --verbose --abbrev= --no-abbrev\n-\t\t\t--track --no-track --contains --merged --no-merged\n-\t\t\t--set-upstream-to= --edit-description --list\n-\t\t\t--unset-upstream --delete --move --remotes\n+\t\t\t--track --no-track --contains --no-contains --merged\n+\t\t\t--no-merged --set-upstream-to= --edit-description\n+\t\t\t--list --unset-upstream --delete --move --remotes\n \t\t\t--column --no-column --sort= --points-at\n \t\t\t\"\n \t\t;;\n@@ -2862,7 +2862,8 @@ _git_tag ()\n \t\t__gitcomp \"\n \t\t\t--list --delete --verify --annotate --message --file\n \t\t\t--sign --cleanup --local-user --force --column --sort=\n-\t\t\t--contains --points-at --merged --no-merged --create-reflog\n+\t\t\t--contains --no-contains --points-at --merged\n+\t\t\t--no-merged --create-reflog\n \t\t\t\"\n \t\t;;\n \tesac\ndiff --git a/parse-options.h b/parse-options.h\nindex dcd8a0926c..0eac90b510 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -258,7 +258,9 @@ extern int parse_opt_passthru_argv(const struct option *, const char *, int);\n \t PARSE_OPT_LASTARG_DEFAULT | flag, \\\n \t parse_opt_commits, (intptr_t) \"HEAD\" \\\n \t}\n-#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"contains\", v, h, 0)\n+#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"contains\", v, h, PARSE_OPT_NONEG)\n+#define OPT_NO_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"no-contains\", v, h, PARSE_OPT_NONEG)\n #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH(\"with\", v, h, PARSE_OPT_HIDDEN)\n+#define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH(\"without\", v, h, PARSE_OPT_HIDDEN)\n \n #endif\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 1ec0fb8391..b6dab75729 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1571,11 +1571,11 @@ static enum contains_result contains_tag_algo(struct commit *candidate,\n \treturn contains_test(candidate, want);\n }\n \n-static int commit_contains(struct ref_filter *filter, struct commit *commit)\n+static int commit_contains(struct ref_filter *filter, struct commit_list *list, struct commit *commit)\n {\n \tif (filter->with_commit_tag_algo)\n-\t\treturn contains_tag_algo(commit, filter->with_commit);\n-\treturn is_descendant_of(commit, filter->with_commit);\n+\t\treturn contains_tag_algo(commit, list);\n+\treturn is_descendant_of(commit, list);\n }\n \n /*\n@@ -1765,13 +1765,17 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,\n \t * obtain the commit using the 'oid' available and discard all\n \t * non-commits early. The actual filtering is done later.\n \t */\n-\tif (filter->merge_commit || filter->with_commit || filter->verbose) {\n+\tif (filter->merge_commit || filter->with_commit || filter->no_commit || filter->verbose) {\n \t\tcommit = lookup_commit_reference_gently(oid->hash, 1);\n \t\tif (!commit)\n \t\t\treturn 0;\n-\t\t/* We perform the filtering for the '--contains' option */\n+\t\t/* We perform the filtering for the '--contains' option... */\n \t\tif (filter->with_commit &&\n-\t\t !commit_contains(filter, commit))\n+\t\t !commit_contains(filter, filter->with_commit, commit))\n+\t\t\treturn 0;\n+\t\t/* ...or for the `--no-contains' option */\n+\t\tif (filter->no_commit &&\n+\t\t commit_contains(filter, filter->no_commit, commit))\n \t\t\treturn 0;\n \t}\n \ndiff --git a/ref-filter.h b/ref-filter.h\nindex 154e24c405..af85eb4592 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -53,6 +53,7 @@ struct ref_filter {\n \tconst char **name_patterns;\n \tstruct sha1_array points_at;\n \tstruct commit_list *with_commit;\n+\tstruct commit_list *no_commit;\n \n \tenum {\n \t\tREF_FILTER_MERGED_NONE = 0,\ndiff --git a/t/t3201-branch-contains.sh b/t/t3201-branch-contains.sh\nindex 7f3ec47241..506415dbd3 100755\n--- a/t/t3201-branch-contains.sh\n+++ b/t/t3201-branch-contains.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-test_description='branch --contains , --merged, and --no-merged'\n+test_description='branch --contains , --no-contains --merged, and --no-merged'\n \n . ./test-lib.sh\n \n@@ -45,6 +45,22 @@ test_expect_success 'branch --contains master' '\n \n '\n \n+test_expect_success 'branch --no-contains=master' '\n+\n+\tgit branch --no-contains=master >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+\n+'\n+\n+test_expect_success 'branch --no-contains master' '\n+\n+\tgit branch --no-contains master >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+\n+'\n+\n test_expect_success 'branch --contains=side' '\n \n \tgit branch --contains=side >actual &&\n@@ -55,6 +71,16 @@ test_expect_success 'branch --contains=side' '\n \n '\n \n+test_expect_success 'branch --no-contains=side' '\n+\n+\tgit branch --no-contains=side >actual &&\n+\t{\n+\t\techo \" master\"\n+\t} >expect &&\n+\ttest_cmp expect actual\n+\n+'\n+\n test_expect_success 'branch --contains with pattern implies --list' '\n \n \tgit branch --contains=master master >actual &&\n@@ -65,6 +91,14 @@ test_expect_success 'branch --contains with pattern implies --list' '\n \n '\n \n+test_expect_success 'branch --no-contains with pattern implies --list' '\n+\n+\tgit branch --no-contains=master master >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+\n+'\n+\n test_expect_success 'side: branch --merged' '\n \n \tgit branch --merged >actual &&\n@@ -126,7 +160,9 @@ test_expect_success 'branch --no-merged with pattern implies --list' '\n test_expect_success 'implicit --list conflicts with modification options' '\n \n \ttest_must_fail git branch --contains=master -d &&\n-\ttest_must_fail git branch --contains=master -m foo\n+\ttest_must_fail git branch --contains=master -m foo &&\n+\ttest_must_fail git branch --no-contains=master -d &&\n+\ttest_must_fail git branch --no-contains=master -m foo\n \n '\n \n@@ -159,4 +195,15 @@ test_expect_success 'branch --merged with --verbose' '\n \ttest_i18ncmp expect actual\n '\n \n+test_expect_success 'branch --contains combined with --no-contains' '\n+\tgit branch --contains zzz --no-contains topic >actual &&\n+\tcat >expect <<-\\EOF &&\n+\t master\n+\t side\n+\t zzz\n+\tEOF\n+\ttest_cmp expect actual\n+\n+'\n+\n test_done\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex a09a1a46ef..4902ba5f16 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -93,6 +93,22 @@ test_expect_success 'filtering with --contains' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'filtering with --no-contains' '\n+\tcat >expect <<-\\EOF &&\n+\trefs/tags/one\n+\tEOF\n+\tgit for-each-ref --format=\"%(refname)\" --no-contains=two >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'filtering with --contains and --no-contains' '\n+\tcat >expect <<-\\EOF &&\n+\trefs/tags/two\n+\tEOF\n+\tgit for-each-ref --format=\"%(refname)\" --contains=two --no-contains=three >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '%(color) must fail' '\n \ttest_must_fail git for-each-ref --format=\"%(color)%(refname)\"\n '\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex b4698ab5f5..bec3d0fb42 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1385,6 +1385,23 @@ test_expect_success 'checking that first commit is in all tags (relative)' \"\n \ttest_cmp expected actual\n \"\n \n+# All the --contains tests above, but with --no-contains\n+test_expect_success 'checking that first commit is not listed in any tag with --no-contains (hash)' \"\n+\t>expected &&\n+\tgit tag -l --no-contains $hash1 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n+test_expect_success 'checking that first commit is in all tags (tag)' \"\n+\tgit tag -l --no-contains v1.0 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n+test_expect_success 'checking that first commit is in all tags (relative)' \"\n+\tgit tag -l --no-contains HEAD~2 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n cat > expected < expected <actual &&\n+\ttest_cmp expected actual\n+\"\n \n cat > expected < expected <actual &&\n+\ttest_cmp expected actual\n+\"\n+\n # how about a simple merge?\n \n test_expect_success 'creating simple branch' '\n@@ -1424,6 +1465,19 @@ test_expect_success 'checking that branch head only has one tag' \"\n \ttest_cmp expected actual\n \"\n \n+cat > expected <actual &&\n+\ttest_cmp expected actual\n+\"\n+\n test_expect_success 'merging original branch into this branch' '\n \tgit merge --strategy=ours master &&\n git tag v4.0\n@@ -1445,6 +1499,20 @@ v1.0.1\n v1.1.3\n v2.0\n v3.0\n+EOF\n+\n+test_expect_success 'checking that original branch head with --no-contains lists all but one tag now' \"\n+\tgit tag -l --no-contains $hash3 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n+cat > expected <expected &&\n+\tgit tag -l --no-contains $hash1 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n # mixing modes and options:\n \n test_expect_success 'mixing incompatibles modes and options is forbidden' '\n@@ -1709,7 +1783,7 @@ run_with_limited_stack () {\n test_lazy_prereq ULIMIT_STACK_SIZE 'run_with_limited_stack true'\n \n # we require ulimit, this excludes Windows\n-test_expect_success ULIMIT_STACK_SIZE '--contains works in a deep repo' '\n+test_expect_success ULIMIT_STACK_SIZE '--contains and --no-contains work in a deep repo' '\n \t>expect &&\n \ti=1 &&\n \twhile test $i -lt 8000\n@@ -1725,7 +1799,9 @@ EOF\"\n \tgit checkout master &&\n \tgit tag far-far-away HEAD^ &&\n \trun_with_limited_stack git tag --contains HEAD >actual &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect actual &&\n+\trun_with_limited_stack git tag --no-contains HEAD >actual &&\n+\ttest_line_count \">\" 10 actual\n '\n \n test_expect_success '--format should list tags as per format given' '\n@@ -1773,4 +1849,47 @@ test_expect_success 'ambiguous branch/tags not marked' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--contains combined with --no-contains' '\n+\t(\n+\t\tgit init no-contains &&\n+\t\tcd no-contains &&\n+\t\ttest_commit v0.1 &&\n+\t\ttest_commit v0.2 &&\n+\t\ttest_commit v0.3 &&\n+\t\ttest_commit v0.4 &&\n+\t\ttest_commit v0.5 &&\n+\t\tcat >expected <<-\\EOF &&\n+\t\tv0.2\n+\t\tv0.3\n+\t\tv0.4\n+\t\tEOF\n+\t\tgit tag --contains v0.2 --no-contains v0.5 >actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n+# As the docs say, list tags which contain a specified *commit*. We\n+# don't recurse down to tags for trees or blobs pointed to by *those*\n+# commits.\n+test_expect_success 'Does --[no-]contains stop at commits? Yes!' '\n+\tcd no-contains &&\n+\tblob=$(git rev-parse v0.3:v0.3.t) &&\n+\ttree=$(git rev-parse v0.3^{tree}) &&\n+\tgit tag tag-blob $blob &&\n+\tgit tag tag-tree $tree &&\n+\tgit tag --contains v0.3 >actual &&\n+\tcat >expected <<-\\EOF &&\n+\tv0.3\n+\tv0.4\n+\tv0.5\n+\tEOF\n+\ttest_cmp expected actual &&\n+\tgit tag --no-contains v0.3 >actual &&\n+\tcat >expected <<-\\EOF &&\n+\tv0.1\n+\tv0.2\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.11.0\n\n"},{"id":"313826","messageId":"CACBZZX5rgomtp3E6JkZdyJPY_iw=j+Sg3-nYw-05jW0raRvK0w@mail.gmail.com","threadId":"45321","inReplyTo":"20170309132728.c57ltzel746l366a@sigill.intra.peff.net","subject":"Re: [PATCH 0/4] fix object flag pollution in \"tag --contains\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-03-11T13:06:55Z","receivedAt":"2017-03-11T13:07:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Mar 9, 2017 at 2:27 PM, Jeff King wrote:\n> On Thu, Mar 09, 2017 at 07:51:32AM -0500, Jeff King wrote:\n>\n>> Looking at this, I'm pretty sure that using \"--contains\" with \"--merged\"\n>> has similar problems, as they both use the UNINTERESTING bit. So even\n>> without your patch, there is a lurking bug.\n\nSorry about the late reply. Been a while since I was active on the git\nML, and my broken list search was searching for cc:me, not to:me. So I\nsent my v3 in <20170310203348.675-1-avarab@gmail.com> without reading\nthis.\n\n> I wasn't able to come up with a simple case that actually demonstrates\n> the bug. But I feel like it has to be triggerable with the right\n> sequence of history.\n\nThe tag brute force script I hacked up\n(https://gist.github.com/avar/45cf288ce7cdc43e7395c6cbf9a98d68) is now\nat >1k iterations without finding anything. But of course it may be\nbroken / this may not be producible on git.git\n\n> Even without that, though, I feel like moving away from this flag usage\n> is a good cleanup. Here's a cleaned-up series. What do you think of\n> building your patch on top?\n>\n> We can do it the other way around if you prefer.\n\nGetting this in master first sounds good. I already have a working v4\non top of this, which is of course much faster for the --contains\ncombined with --no-contains case. Gotta run now, but will clean up\nthat patch & submit it to the list soon.\n\n> [1/4]: ref-filter: move ref_cbdata definition into ref-filter.c\n> [2/4]: ref-filter: use contains_result enum consistently\n> [3/4]: ref-filter: die on parse_commit errors\n> [4/4]: ref-filter: use separate cache for contains_tag_algo\n>\n> ref-filter.c | 70 ++++++++++++++++++++++++++++++++++++++----------------------\n> ref-filter.h | 5 -----\n> 2 files changed, 44 insertions(+), 31 deletions(-)\n>\n> -Peff\n"},{"id":"313831","messageId":"CACBZZX5A46-ua9y_HxqrqycQviR0uGnStnaPwH9jukzPVak6ug@mail.gmail.com","threadId":"45321","inReplyTo":"20170309132949.shyc4gmdrhazykev@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] ref-filter: use separate cache for contains_tag_algo","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-03-11T20:01:25Z","receivedAt":"2017-03-11T20:02:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Mar 9, 2017 at 2:29 PM, Jeff King wrote:\n> [...]\n> @@ -1874,6 +1886,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int\n> broken = 1;\n> filter->kind = type & FILTER_REFS_KIND_MASK;\n>\n> + init_contains_cache(&ref_cbdata.contains_cache);\n> +\n> /* Simple per-ref filtering */\n> [...]\n>\n> + clear_contains_cache(&ref_cbdata.contains_cache);\n>\n> /* Filters that need revision walking */\n> if (filter->merge_commit)\n\nShouldn't both of those be guarded by a \"if (filter->with_commit)\" test?\n\nThat init/clear codepath is rather light, but it seems to me that we\ncan avoid it entirely if filter->with_commit isn't defined. I've\ntested this locally and it still passes all tests.\n"},{"id":"313832","messageId":"20170311201858.27555-1-avarab@gmail.com","threadId":"45321","inReplyTo":"20170309132728.c57ltzel746l366a@sigill.intra.peff.net","subject":"[PATCH v4] ref-filter: Add --no-contains option to tag/branch/for-each-ref","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-03-11T20:18:58Z","receivedAt":"2017-03-11T20:22:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the tag, branch & for-each-ref commands to have a --no-contains\noption in addition to their longstanding --contains options.\n\nThe use-case I have for this is to find the last-good rollout tag\ngiven a known-bad . Right now, given a hypothetically bad\ncommit v2.10.1-3-gcf5c7253e0, you can find which git version to revert\nto with this hacky two-liner:\n\n (./git tag -l 'v[0-9]*'; ./git tag -l 'v[0-9]*' --contains v2.10.1-3-gcf5c7253e0) \\\n |sort|uniq -c|grep -E '^ *1 '|awk '{print $2}' | tail -n 10\n\nBut with the --no-contains option you can now get the exact same\noutput with:\n\n ./git tag -l 'v[0-9]*' --no-contains v2.10.1-3-gcf5c7253e0|sort|tail -n 10\n\nThe filtering machinery is generic between the tag, branch &\nfor-each-ref commands, so once I'd implemented it for tag it was\ntrivial to add support for this to the other two.\n\nA practical use for this with \"branch\" is e.g. finding branches which\ndiverged between 2.8 & 2.10:\n\n ./git branch --contains v2.8.0 --no-contains v2.10.0\n\nThe \"describe\" command also has a --contains option, but its semantics\nare unrelated to what tag/branch/for-each-ref use --contains for. I\ndon't see how a --no-contains option for \"describe\" would make any\nsense, other than being exactly equivalent to not supplying --contains\nat all, which would be confusing at best.\n\nI'm adding a --without option to \"tag\" as an alias for --no-contains\nfor consistency with --with and --contains. Since we don't even\ndocument --with anymore (or test it). The --with option is\nundocumented, and possibly the only user of it is Junio[1]. But it's\ntrivial to support, so let's do that.\n\nWhere I'm changing existing documentation lines I'm mainly word\nwrapping at 75 columns to be consistent with the existing style. The\nchanges to Documentation/ are much smaller with: git show --word-diff,\nsame for the minor change to git-completion.bash.\n\nMost of the test changes I've made are just doing the inverse of the\nexisting --contains tests, with this change --no-contains for tag,\nbranch & for-each-ref is just as well tested as the existing\n--contains option.\n\nIn addition to those tests I've added tests for --contains in\ncombination with --no-contains for all three commands, and a test for\n\"tag\" which asserts that --no-contains won't find tree/blob tags,\nwhich is slightly unintuitive, but consistent with how --contains\nworks.\n\nSince we don't need to init/clear the contains_cache unless we're\nactually using this filter, I skip that unless filter->no_commit is\ntrue, and while I'm at it amend the code added in Jeff King's\n\"ref-filter: use separate cache for contains_tag_algo\" patch to do\nthat for filter->with_commit as well.\n\n1. \n2. <20170309125132.tubwxtneffok4nrc@sigill.intra.peff.net>\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nThis is now based on top of pu, which has Jeff King's \"fix object flag\npollution in \"tag --contains\" series. Thus the whole hack to disable\nthe optimized codepath when both --contains and --no-contains are\nprovided is gone, since we can use the optimized codepath now.\n\n Documentation/git-branch.txt | 24 ++++---\n Documentation/git-for-each-ref.txt | 6 +-\n Documentation/git-tag.txt | 6 +-\n builtin/branch.c | 4 +-\n builtin/for-each-ref.c | 3 +-\n builtin/tag.c | 6 +-\n contrib/completion/git-completion.bash | 9 +--\n parse-options.h | 4 +-\n ref-filter.c | 27 +++++---\n ref-filter.h | 1 +\n t/t3201-branch-contains.sh | 51 +++++++++++++-\n t/t6302-for-each-ref-filter.sh | 16 +++++\n t/t7004-tag.sh | 123 ++++++++++++++++++++++++++++++++-\n 13 files changed, 250 insertions(+), 30 deletions(-)\n\ndiff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\nindex 092f1bcf9f..28a36a8a0a 100644\n--- a/Documentation/git-branch.txt\n+++ b/Documentation/git-branch.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n 'git branch' [--color[=] | --no-color] [-r | -a]\n \t[--list] [-v [--abbrev= | --no-abbrev]]\n \t[--column[=] | --no-column]\n-\t[(--merged | --no-merged | --contains) []] [--sort=]\n+\t[(--merged | --no-merged | --contains | --no-contains) []] [--sort=]\n \t[--points-at ] [--format=] [...]\n 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] []\n 'git branch' (--set-upstream-to= | -u ) []\n@@ -35,11 +35,12 @@ as branch creation.\n \n With `--contains`, shows only the branches that contain the named commit\n (in other words, the branches whose tip commits are descendants of the\n-named commit). With `--merged`, only branches merged into the named\n-commit (i.e. the branches whose tip commits are reachable from the named\n-commit) will be listed. With `--no-merged` only branches not merged into\n-the named commit will be listed. If the argument is missing it\n-defaults to `HEAD` (i.e. the tip of the current branch).\n+named commit), `--no-contains` inverts it. With `--merged`, only branches\n+merged into the named commit (i.e. the branches whose tip commits are\n+reachable from the named commit) will be listed. With `--no-merged` only\n+branches not merged into the named commit will be listed. If the \n+argument is missing it defaults to `HEAD` (i.e. the tip of the current\n+branch).\n \n The command's second form creates a new branch head named \n which points to the current `HEAD`, or if given.\n@@ -213,6 +214,10 @@ start-point is either a local or remote-tracking branch.\n \tOnly list branches which contain the specified commit (HEAD\n \tif not specified). Implies `--list`.\n \n+--no-contains []::\n+\tOnly list branches which don't contain the specified commit\n+\t(HEAD if not specified). Implies `--list`.\n+\n --merged []::\n \tOnly list branches whose tips are reachable from the\n \tspecified commit (HEAD if not specified). Implies `--list`.\n@@ -296,13 +301,16 @@ If you are creating a branch that you want to checkout immediately, it is\n easier to use the git checkout command with its `-b` option to create\n a branch and check it out with a single command.\n \n-The options `--contains`, `--merged` and `--no-merged` serve three related\n-but different purposes:\n+The options `--contains`, `--no-contains`, `--merged` and `--no-merged`\n+serve four related but different purposes:\n \n - `--contains ` is used to find all branches which will need\n special attention if were to be rebased or amended, since those\n branches contain the specified .\n \n+- `--no-contains ` is the inverse of that, i.e. branches that don't\n+ contain the specified .\n+\n - `--merged` is used to find all branches which can be safely deleted,\n since those branches are fully contained by HEAD.\n \ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 111e1be6f5..83b93c75a8 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n 'git for-each-ref' [--count=] [--shell|--perl|--python|--tcl]\n \t\t [(--sort=)...] [--format=] [...]\n \t\t [--points-at ] [(--merged | --no-merged) []]\n-\t\t [--contains []]\n+\t\t [(--contains | --no-contains) []]\n \n DESCRIPTION\n -----------\n@@ -79,6 +79,10 @@ OPTIONS\n \tOnly list refs which contain the specified commit (HEAD if not\n \tspecified).\n \n+--no-contains []::\n+\tOnly list refs which don't contain the specified commit (HEAD\n+\tif not specified).\n+\n --ignore-case::\n \tSorting and filtering refs are case insensitive.\n \ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex 525737a5d8..d9243bf5e4 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -12,7 +12,7 @@ SYNOPSIS\n 'git tag' [-a | -s | -u ] [-f] [-m | -F ]\n \t [ | ]\n 'git tag' -d ...\n-'git tag' [-n[]] -l [--contains ] [--points-at ]\n+'git tag' [-n[]] -l [--[no-]contains ] [--points-at ]\n \t[--column[=] | --no-column] [--create-reflog] [--sort=]\n \t[--format=] [--[no-]merged []] [...]\n 'git tag' -v [--format=] ...\n@@ -124,6 +124,10 @@ This option is only applicable when listing tags without annotation lines.\n \tOnly list tags which contain the specified commit (HEAD if not\n \tspecified).\n \n+--no-contains []::\n+\tOnly list tags which don't contain the specified commit (HEAD if\n+\tnot specified).\n+\n --points-at ::\n \tOnly list tags of the given object.\n \ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 52688f2e1b..b4871db9cb 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -562,7 +562,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT_SET_INT('r', \"remotes\", &filter.kind, N_(\"act on remote-tracking branches\"),\n \t\t\tFILTER_REFS_REMOTES),\n \t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only branches that contain the commit\")),\n+\t\tOPT_NO_CONTAINS(&filter.no_commit, N_(\"print only branches that don't contain the commit\")),\n \t\tOPT_WITH(&filter.with_commit, N_(\"print only branches that contain the commit\")),\n+\t\tOPT_WITHOUT(&filter.no_commit, N_(\"print only branches that don't contain the commit\")),\n \t\tOPT__ABBREV(&filter.abbrev),\n \n \t\tOPT_GROUP(N_(\"Specific git-branch actions:\")),\n@@ -618,7 +620,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \tif (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)\n \t\tlist = 1;\n \n-\tif (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)\n+\tif (filter.with_commit || filter.no_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)\n \t\tlist = 1;\n \n \tif (!!delete + !!rename + !!new_upstream +\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex df41fa0350..a11542c4fd 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -9,7 +9,7 @@ static char const * const for_each_ref_usage[] = {\n \tN_(\"git for-each-ref [] []\"),\n \tN_(\"git for-each-ref [--points-at ]\"),\n \tN_(\"git for-each-ref [(--merged | --no-merged) []]\"),\n-\tN_(\"git for-each-ref [--contains []]\"),\n+\tN_(\"git for-each-ref [(--contains | --no-contains) []]\"),\n \tNULL\n };\n \n@@ -43,6 +43,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \t\tOPT_MERGED(&filter, N_(\"print only refs that are merged\")),\n \t\tOPT_NO_MERGED(&filter, N_(\"print only refs that are not merged\")),\n \t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only refs which contain the commit\")),\n+\t\tOPT_NO_CONTAINS(&filter.no_commit, N_(\"print only refs which don't contain the commit\")),\n \t\tOPT_BOOL(0, \"ignore-case\", &icase, N_(\"sorting and filtering are case insensitive\")),\n \t\tOPT_END(),\n \t};\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex ad29be6923..da1fca3677 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -22,7 +22,7 @@\n static const char * const git_tag_usage[] = {\n \tN_(\"git tag [-a | -s | -u ] [-f] [-m | -F ] []\"),\n \tN_(\"git tag -d ...\"),\n-\tN_(\"git tag -l [-n[]] [--contains ] [--points-at ]\"\n+\tN_(\"git tag -l [-n[]] [--[no-]contains ] [--points-at ]\"\n \t\t\"\\n\\t\\t[--format=] [--[no-]merged []] [...]\"),\n \tN_(\"git tag -v [--format=] ...\"),\n \tNULL\n@@ -424,7 +424,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tOPT_GROUP(N_(\"Tag listing options\")),\n \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"show tag list in columns\")),\n \t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n+\t\tOPT_NO_CONTAINS(&filter.no_commit, N_(\"print only tags that don't contain the commit\")),\n \t\tOPT_WITH(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n+\t\tOPT_WITHOUT(&filter.no_commit, N_(\"print only tags that don't contain the commit\")),\n \t\tOPT_MERGED(&filter, N_(\"print only tags that are merged\")),\n \t\tOPT_NO_MERGED(&filter, N_(\"print only tags that are not merged\")),\n \t\tOPT_CALLBACK(0 , \"sort\", sorting_tail, N_(\"key\"),\n@@ -488,6 +490,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"-n option is only allowed with -l.\"));\n \tif (filter.with_commit)\n \t\tdie(_(\"--contains option is only allowed with -l.\"));\n+\tif (filter.no_commit)\n+\t\tdie(_(\"--no-contains option is only allowed with -l.\"));\n \tif (filter.points_at.nr)\n \t\tdie(_(\"--points-at option is only allowed with -l.\"));\n \tif (filter.merge_commit)\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 2121211cb4..d121c45709 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1159,9 +1159,9 @@ _git_branch ()\n \t--*)\n \t\t__gitcomp \"\n \t\t\t--color --no-color --verbose --abbrev= --no-abbrev\n-\t\t\t--track --no-track --contains --merged --no-merged\n-\t\t\t--set-upstream-to= --edit-description --list\n-\t\t\t--unset-upstream --delete --move --remotes\n+\t\t\t--track --no-track --contains --no-contains --merged\n+\t\t\t--no-merged --set-upstream-to= --edit-description\n+\t\t\t--list --unset-upstream --delete --move --remotes\n \t\t\t--column --no-column --sort= --points-at\n \t\t\t\"\n \t\t;;\n@@ -2928,7 +2928,8 @@ _git_tag ()\n \t\t__gitcomp \"\n \t\t\t--list --delete --verify --annotate --message --file\n \t\t\t--sign --cleanup --local-user --force --column --sort=\n-\t\t\t--contains --points-at --merged --no-merged --create-reflog\n+\t\t\t--contains --no-contains --points-at --merged\n+\t\t\t--no-merged --create-reflog\n \t\t\t\"\n \t\t;;\n \tesac\ndiff --git a/parse-options.h b/parse-options.h\nindex dcd8a0926c..0eac90b510 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -258,7 +258,9 @@ extern int parse_opt_passthru_argv(const struct option *, const char *, int);\n \t PARSE_OPT_LASTARG_DEFAULT | flag, \\\n \t parse_opt_commits, (intptr_t) \"HEAD\" \\\n \t}\n-#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"contains\", v, h, 0)\n+#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"contains\", v, h, PARSE_OPT_NONEG)\n+#define OPT_NO_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"no-contains\", v, h, PARSE_OPT_NONEG)\n #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH(\"with\", v, h, PARSE_OPT_HIDDEN)\n+#define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH(\"without\", v, h, PARSE_OPT_HIDDEN)\n \n #endif\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 9c82b5b9d6..6f68ae84d4 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1487,6 +1487,7 @@ struct ref_filter_cbdata {\n \tstruct ref_array *array;\n \tstruct ref_filter *filter;\n \tstruct contains_cache contains_cache;\n+\tstruct contains_cache no_contains_cache;\n };\n \n /*\n@@ -1586,11 +1587,11 @@ static enum contains_result contains_tag_algo(struct commit *candidate,\n }\n \n static int commit_contains(struct ref_filter *filter, struct commit *commit,\n-\t\t\t struct contains_cache *cache)\n+\t\t\t struct commit_list *list, struct contains_cache *cache)\n {\n \tif (filter->with_commit_tag_algo)\n-\t\treturn contains_tag_algo(commit, filter->with_commit, cache) == CONTAINS_YES;\n-\treturn is_descendant_of(commit, filter->with_commit);\n+\t\treturn contains_tag_algo(commit, list, cache) == CONTAINS_YES;\n+\treturn is_descendant_of(commit, list);\n }\n \n /*\n@@ -1780,13 +1781,17 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,\n \t * obtain the commit using the 'oid' available and discard all\n \t * non-commits early. The actual filtering is done later.\n \t */\n-\tif (filter->merge_commit || filter->with_commit || filter->verbose) {\n+\tif (filter->merge_commit || filter->with_commit || filter->no_commit || filter->verbose) {\n \t\tcommit = lookup_commit_reference_gently(oid->hash, 1);\n \t\tif (!commit)\n \t\t\treturn 0;\n-\t\t/* We perform the filtering for the '--contains' option */\n+\t\t/* We perform the filtering for the '--contains' option... */\n \t\tif (filter->with_commit &&\n-\t\t !commit_contains(filter, commit, &ref_cbdata->contains_cache))\n+\t\t !commit_contains(filter, commit, filter->with_commit, &ref_cbdata->contains_cache))\n+\t\t\treturn 0;\n+\t\t/* ...or for the `--no-contains' option */\n+\t\tif (filter->no_commit &&\n+\t\t commit_contains(filter, commit, filter->no_commit, &ref_cbdata->no_contains_cache))\n \t\t\treturn 0;\n \t}\n \n@@ -1886,7 +1891,10 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int\n \t\tbroken = 1;\n \tfilter->kind = type & FILTER_REFS_KIND_MASK;\n \n-\tinit_contains_cache(&ref_cbdata.contains_cache);\n+\tif (filter->with_commit)\n+\t\tinit_contains_cache(&ref_cbdata.contains_cache);\n+\tif (filter->no_commit)\n+\t\tinit_contains_cache(&ref_cbdata.no_contains_cache);\n \n \t/* Simple per-ref filtering */\n \tif (!filter->kind)\n@@ -1910,7 +1918,10 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int\n \t\t\thead_ref(ref_filter_handler, &ref_cbdata);\n \t}\n \n-\tclear_contains_cache(&ref_cbdata.contains_cache);\n+\tif (filter->with_commit)\n+\t\tclear_contains_cache(&ref_cbdata.contains_cache);\n+\tif (filter->no_commit)\n+\t\tclear_contains_cache(&ref_cbdata.no_contains_cache);\n \n \t/* Filters that need revision walking */\n \tif (filter->merge_commit)\ndiff --git a/ref-filter.h b/ref-filter.h\nindex e738c5dfd3..dde40f6849 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -53,6 +53,7 @@ struct ref_filter {\n \tconst char **name_patterns;\n \tstruct sha1_array points_at;\n \tstruct commit_list *with_commit;\n+\tstruct commit_list *no_commit;\n \n \tenum {\n \t\tREF_FILTER_MERGED_NONE = 0,\ndiff --git a/t/t3201-branch-contains.sh b/t/t3201-branch-contains.sh\nindex 7f3ec47241..506415dbd3 100755\n--- a/t/t3201-branch-contains.sh\n+++ b/t/t3201-branch-contains.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-test_description='branch --contains , --merged, and --no-merged'\n+test_description='branch --contains , --no-contains --merged, and --no-merged'\n \n . ./test-lib.sh\n \n@@ -45,6 +45,22 @@ test_expect_success 'branch --contains master' '\n \n '\n \n+test_expect_success 'branch --no-contains=master' '\n+\n+\tgit branch --no-contains=master >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+\n+'\n+\n+test_expect_success 'branch --no-contains master' '\n+\n+\tgit branch --no-contains master >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+\n+'\n+\n test_expect_success 'branch --contains=side' '\n \n \tgit branch --contains=side >actual &&\n@@ -55,6 +71,16 @@ test_expect_success 'branch --contains=side' '\n \n '\n \n+test_expect_success 'branch --no-contains=side' '\n+\n+\tgit branch --no-contains=side >actual &&\n+\t{\n+\t\techo \" master\"\n+\t} >expect &&\n+\ttest_cmp expect actual\n+\n+'\n+\n test_expect_success 'branch --contains with pattern implies --list' '\n \n \tgit branch --contains=master master >actual &&\n@@ -65,6 +91,14 @@ test_expect_success 'branch --contains with pattern implies --list' '\n \n '\n \n+test_expect_success 'branch --no-contains with pattern implies --list' '\n+\n+\tgit branch --no-contains=master master >actual &&\n+\t>expect &&\n+\ttest_cmp expect actual\n+\n+'\n+\n test_expect_success 'side: branch --merged' '\n \n \tgit branch --merged >actual &&\n@@ -126,7 +160,9 @@ test_expect_success 'branch --no-merged with pattern implies --list' '\n test_expect_success 'implicit --list conflicts with modification options' '\n \n \ttest_must_fail git branch --contains=master -d &&\n-\ttest_must_fail git branch --contains=master -m foo\n+\ttest_must_fail git branch --contains=master -m foo &&\n+\ttest_must_fail git branch --no-contains=master -d &&\n+\ttest_must_fail git branch --no-contains=master -m foo\n \n '\n \n@@ -159,4 +195,15 @@ test_expect_success 'branch --merged with --verbose' '\n \ttest_i18ncmp expect actual\n '\n \n+test_expect_success 'branch --contains combined with --no-contains' '\n+\tgit branch --contains zzz --no-contains topic >actual &&\n+\tcat >expect <<-\\EOF &&\n+\t master\n+\t side\n+\t zzz\n+\tEOF\n+\ttest_cmp expect actual\n+\n+'\n+\n test_done\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex a09a1a46ef..4902ba5f16 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -93,6 +93,22 @@ test_expect_success 'filtering with --contains' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'filtering with --no-contains' '\n+\tcat >expect <<-\\EOF &&\n+\trefs/tags/one\n+\tEOF\n+\tgit for-each-ref --format=\"%(refname)\" --no-contains=two >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'filtering with --contains and --no-contains' '\n+\tcat >expect <<-\\EOF &&\n+\trefs/tags/two\n+\tEOF\n+\tgit for-each-ref --format=\"%(refname)\" --contains=two --no-contains=three >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '%(color) must fail' '\n \ttest_must_fail git for-each-ref --format=\"%(color)%(refname)\"\n '\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex b4698ab5f5..bec3d0fb42 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1385,6 +1385,23 @@ test_expect_success 'checking that first commit is in all tags (relative)' \"\n \ttest_cmp expected actual\n \"\n \n+# All the --contains tests above, but with --no-contains\n+test_expect_success 'checking that first commit is not listed in any tag with --no-contains (hash)' \"\n+\t>expected &&\n+\tgit tag -l --no-contains $hash1 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n+test_expect_success 'checking that first commit is in all tags (tag)' \"\n+\tgit tag -l --no-contains v1.0 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n+test_expect_success 'checking that first commit is in all tags (relative)' \"\n+\tgit tag -l --no-contains HEAD~2 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n cat > expected < expected <actual &&\n+\ttest_cmp expected actual\n+\"\n \n cat > expected < expected <actual &&\n+\ttest_cmp expected actual\n+\"\n+\n # how about a simple merge?\n \n test_expect_success 'creating simple branch' '\n@@ -1424,6 +1465,19 @@ test_expect_success 'checking that branch head only has one tag' \"\n \ttest_cmp expected actual\n \"\n \n+cat > expected <actual &&\n+\ttest_cmp expected actual\n+\"\n+\n test_expect_success 'merging original branch into this branch' '\n \tgit merge --strategy=ours master &&\n git tag v4.0\n@@ -1445,6 +1499,20 @@ v1.0.1\n v1.1.3\n v2.0\n v3.0\n+EOF\n+\n+test_expect_success 'checking that original branch head with --no-contains lists all but one tag now' \"\n+\tgit tag -l --no-contains $hash3 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n+cat > expected <expected &&\n+\tgit tag -l --no-contains $hash1 v* >actual &&\n+\ttest_cmp expected actual\n+\"\n+\n # mixing modes and options:\n \n test_expect_success 'mixing incompatibles modes and options is forbidden' '\n@@ -1709,7 +1783,7 @@ run_with_limited_stack () {\n test_lazy_prereq ULIMIT_STACK_SIZE 'run_with_limited_stack true'\n \n # we require ulimit, this excludes Windows\n-test_expect_success ULIMIT_STACK_SIZE '--contains works in a deep repo' '\n+test_expect_success ULIMIT_STACK_SIZE '--contains and --no-contains work in a deep repo' '\n \t>expect &&\n \ti=1 &&\n \twhile test $i -lt 8000\n@@ -1725,7 +1799,9 @@ EOF\"\n \tgit checkout master &&\n \tgit tag far-far-away HEAD^ &&\n \trun_with_limited_stack git tag --contains HEAD >actual &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect actual &&\n+\trun_with_limited_stack git tag --no-contains HEAD >actual &&\n+\ttest_line_count \">\" 10 actual\n '\n \n test_expect_success '--format should list tags as per format given' '\n@@ -1773,4 +1849,47 @@ test_expect_success 'ambiguous branch/tags not marked' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--contains combined with --no-contains' '\n+\t(\n+\t\tgit init no-contains &&\n+\t\tcd no-contains &&\n+\t\ttest_commit v0.1 &&\n+\t\ttest_commit v0.2 &&\n+\t\ttest_commit v0.3 &&\n+\t\ttest_commit v0.4 &&\n+\t\ttest_commit v0.5 &&\n+\t\tcat >expected <<-\\EOF &&\n+\t\tv0.2\n+\t\tv0.3\n+\t\tv0.4\n+\t\tEOF\n+\t\tgit tag --contains v0.2 --no-contains v0.5 >actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n+# As the docs say, list tags which contain a specified *commit*. We\n+# don't recurse down to tags for trees or blobs pointed to by *those*\n+# commits.\n+test_expect_success 'Does --[no-]contains stop at commits? Yes!' '\n+\tcd no-contains &&\n+\tblob=$(git rev-parse v0.3:v0.3.t) &&\n+\ttree=$(git rev-parse v0.3^{tree}) &&\n+\tgit tag tag-blob $blob &&\n+\tgit tag tag-tree $tree &&\n+\tgit tag --contains v0.3 >actual &&\n+\tcat >expected <<-\\EOF &&\n+\tv0.3\n+\tv0.4\n+\tv0.5\n+\tEOF\n+\ttest_cmp expected actual &&\n+\tgit tag --no-contains v0.3 >actual &&\n+\tcat >expected <<-\\EOF &&\n+\tv0.1\n+\tv0.2\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.11.0\n\n"},{"id":"313833","messageId":"CACBZZX7M9-4Vn4SWRNNkp4ek-D8GwtLKp3pr11SAr6jrk5fEXA@mail.gmail.com","threadId":"45321","inReplyTo":"CACBZZX5A46-ua9y_HxqrqycQviR0uGnStnaPwH9jukzPVak6ug@mail.gmail.com","subject":"Re: [PATCH 4/4] ref-filter: use separate cache for contains_tag_algo","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-03-11T20:21:35Z","receivedAt":"2017-03-11T20:22:05Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sat, Mar 11, 2017 at 9:01 PM, Ævar Arnfjörð Bjarmason\n wrote:\n> On Thu, Mar 9, 2017 at 2:29 PM, Jeff King wrote:\n>> [...]\n>> @@ -1874,6 +1886,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int\n>> broken = 1;\n>> filter->kind = type & FILTER_REFS_KIND_MASK;\n>>\n>> + init_contains_cache(&ref_cbdata.contains_cache);\n>> +\n>> /* Simple per-ref filtering */\n>> [...]\n>>\n>> + clear_contains_cache(&ref_cbdata.contains_cache);\n>>\n>> /* Filters that need revision walking */\n>> if (filter->merge_commit)\n>\n> Shouldn't both of those be guarded by a \"if (filter->with_commit)\" test?\n>\n> That init/clear codepath is rather light, but it seems to me that we\n> can avoid it entirely if filter->with_commit isn't defined. I've\n> tested this locally and it still passes all tests.\n\nSince that seems to work perfectly and we're doing less work I've\nincorporated that into the v4 of my series at\n<20170311201858.27555-1-avarab@gmail.com>.\n"},{"id":"313842","messageId":"xmqqwpbvumrk.fsf@gitster.mtv.corp.google.com","threadId":"45321","inReplyTo":"20170311201858.27555-1-avarab@gmail.com","subject":"Re: [PATCH v4] ref-filter: Add --no-contains option to tag/branch/for-each-ref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-12T04:44:47Z","receivedAt":"2017-03-12T04:44:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Change the tag, branch & for-each-ref commands to have a --no-contains\n> option in addition to their longstanding --contains options.\n>\n> The use-case I have for this is to find the last-good rollout tag\n> given a known-bad . Right now, given a hypothetically bad\n> commit v2.10.1-3-gcf5c7253e0, you can find which git version to revert\n> to with this hacky two-liner:\n>\n> (./git tag -l 'v[0-9]*'; ./git tag -l 'v[0-9]*' --contains v2.10.1-3-gcf5c7253e0) \\\n> |sort|uniq -c|grep -E '^ *1 '|awk '{print $2}' | tail -n 10\n>\n> But with the --no-contains option you can now get the exact same\n> output with:\n>\n> ./git tag -l 'v[0-9]*' --no-contains v2.10.1-3-gcf5c7253e0|sort|tail -n 10\n\nThis command line, while it may happen to work, logically does not\nmake much sense. Move the pattern to the end, i.e.\n\n\tgit tag -l --no-contains v2.10.1-3-gcf5c7253e0 'v[0-9]*'\n\nAlso if an overlong line in an example disturbs you, do not solve it\nby omitting SP around pipe. If you are trying to make the result\nreadable, pick a readable solution, e.g.\n\n git tag -l --no-contains v2.10.1-3-gcf5c7253e0 'v[0-9]*' |\n sort | tail -n 10\n\nOh, drop ./ from ./git while at it ;-)\n\n> The filtering machinery is generic between the tag, branch &\n> for-each-ref commands, so once I'd implemented it for tag it was\n> trivial to add support for this to the other two.\n\nAlso, we tend not to say \"I did this, I do that\".\n\n\tBecause the filtering machinery is generic ..., support it\n\tfor all three consistently.\n\n> I'm adding a --without option to \"tag\" as an alias for --no-contains\n> for consistency with --with and --contains. Since we don't even\n> document --with anymore (or test it). The --with option is\n> undocumented, and possibly the only user of it is Junio[1]. But it's\n> trivial to support, so let's do that.\n\nThe sentence that begins \"Since we don't\" is unfinished. I think\nit can safely removed without losing any information (the next\nsentence says the same thing).\n\n> Where I'm changing existing documentation lines I'm mainly word\n> wrapping at 75 columns to be consistent with the existing style.\n\nReviewers would appreciate you refrain from doing that in the same\npatch. Do a minimum patch so that the review can concentrate on\nwhat got changed (i.e. contents), followed by a mechanical reflow as\na follow-up, or something like that, would be much nicer to handle.\n\n> Most of the test changes I've made are just doing the inverse of the\n> existing --contains tests, with this change --no-contains for tag,\n> branch & for-each-ref is just as well tested as the existing\n> --contains option.\n\nAgain, we tend to try our commits not about \"I, my, me\".\n\n\tAdd --no-contains tests for tag, branch and for-each-ref\n\tthat mostly do the inverse of the existing tests we have for\n\t--contains.\n\n> This is now based on top of pu, which has Jeff King's \"fix object flag\n> pollution in \"tag --contains\" series.\n\nThanks for this note. I obviously cannot queue on top of 'pu' ;-)\nbut will fork this topic off of the jk/ref-filter-flags-cleanup\ntopic.\n\n> 'git for-each-ref' [--count=] [--shell|--perl|--python|--tcl]\n> \t\t [(--sort=)...] [--format=] [...]\n> \t\t [--points-at ] [(--merged | --no-merged) []]\n> -\t\t [--contains []]\n> +\t\t [(--contains | --no-contains) []]\n\nTHis notation makes sense. We have to have one of these but\n at the end could be omitted (to default to HEAD). I guess\nthe same notation can be used in the log for the other \"filtering\nimplies --list mode for 'git tag'\" topic.\n\n> +--no-contains []::\n> +\tOnly list tags which don't contain the specified commit (HEAD if\n> +\tnot specified).\n\nJust being curious. Can we do\n\n\tfor-each-ref --contains --no-contains \n\nand have both default to HEAD? I know that would not make sense as\na set operation, but I am curious what our command line parser\n(which is oblivious to what the command is doing) does. I am guessing\nthat it would barf saying \"--contains\" needs a commit but \"--no-contains\"\nis not a commit (which is very sensible)?\n\n> +\n> --points-at ::\n> \tOnly list tags of the given object.\n\nThis is not a new issue (and certainly not a problem caused by your\npatch), but unlike \"--contains\", this does not default to HEAD when\n is not explicitly given? It seems a bit inconsistent to me.\n\n> @@ -618,7 +620,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n> \tif (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)\n> \t\tlist = 1;\n> \n> -\tif (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)\n> +\tif (filter.with_commit || filter.no_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)\n> \t\tlist = 1;\n\nOK.\n\n> diff --git a/parse-options.h b/parse-options.h\n> index dcd8a0926c..0eac90b510 100644\n> --- a/parse-options.h\n> +++ b/parse-options.h\n> @@ -258,7 +258,9 @@ extern int parse_opt_passthru_argv(const struct option *, const char *, int);\n> \t PARSE_OPT_LASTARG_DEFAULT | flag, \\\n> \t parse_opt_commits, (intptr_t) \"HEAD\" \\\n> \t}\n> -#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"contains\", v, h, 0)\n> +#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"contains\", v, h, PARSE_OPT_NONEG)\n> +#define OPT_NO_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"no-contains\", v, h, PARSE_OPT_NONEG)\n> #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH(\"with\", v, h, PARSE_OPT_HIDDEN)\n> +#define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH(\"without\", v, h, PARSE_OPT_HIDDEN)\n\nHmph, perhaps WITH/WITHOUT also do not take \"--no-\" form hence need OPT_NONEG?\n\n> @@ -1586,11 +1587,11 @@ static enum contains_result contains_tag_algo(struct commit *candidate,\n> }\n> \n> static int commit_contains(struct ref_filter *filter, struct commit *commit,\n> -\t\t\t struct contains_cache *cache)\n> +\t\t\t struct commit_list *list, struct contains_cache *cache)\n> {\n> \tif (filter->with_commit_tag_algo)\n> -\t\treturn contains_tag_algo(commit, filter->with_commit, cache) == CONTAINS_YES;\n> -\treturn is_descendant_of(commit, filter->with_commit);\n> +\t\treturn contains_tag_algo(commit, list, cache) == CONTAINS_YES;\n> +\treturn is_descendant_of(commit, list);\n> }\n> \n> /*\n> @@ -1780,13 +1781,17 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,\n> \t * obtain the commit using the 'oid' available and discard all\n> \t * non-commits early. The actual filtering is done later.\n> \t */\n> -\tif (filter->merge_commit || filter->with_commit || filter->verbose) {\n> +\tif (filter->merge_commit || filter->with_commit || filter->no_commit || filter->verbose) {\n> \t\tcommit = lookup_commit_reference_gently(oid->hash, 1);\n> \t\tif (!commit)\n> \t\t\treturn 0;\n> -\t\t/* We perform the filtering for the '--contains' option */\n> +\t\t/* We perform the filtering for the '--contains' option... */\n> \t\tif (filter->with_commit &&\n> -\t\t !commit_contains(filter, commit, &ref_cbdata->contains_cache))\n> +\t\t !commit_contains(filter, commit, filter->with_commit, &ref_cbdata->contains_cache))\n> +\t\t\treturn 0;\n> +\t\t/* ...or for the `--no-contains' option */\n> +\t\tif (filter->no_commit &&\n> +\t\t commit_contains(filter, commit, filter->no_commit, &ref_cbdata->no_contains_cache))\n> \t\t\treturn 0;\n> \t}\n\nWhen asking \"--contains A --contains B\", we show refs that contain\n_EITHER_ A or B. Two predicates are ORed together, and I think it\nmakes sense.\n\nWhen asking \"--contains A --no-contains B\", we show refs that\ncontain A but exclude refs that contains B. Two predicates are\nANDed together, and I think this also makes sense.\n\nWhen asking \"--no-contains A --no-contains B\", what should we show?\nThis implementation makes the two predicates ANDed together [*1*].\n\nThe behaviour is sensible, but is it consistent with the way now\nexisting --no-merged works?\n\nI think the rule is something like:\n\n A match with any positive selection criterion (like --contains\n A) makes a ref eligible for output, but then a match with any\n negatigve selection criterion (like --no-merged) filters it out.\n\nIs it easy to explain to the users? Do we need doc updates to\nclarify, or does the description for existing --no-merged already\ncover this?\n\nThanks.\n\n\n[Footnote]\n\n*1* ... because it uses the same commit_contains() machinery that\ncomputes \"contains either A or B\" used for the first one and then\nnegates its result.\n"},{"id":"313847","messageId":"CACBZZX4v49zfyGVpcxGSKsxbMfVaUcGHtitpfaZMUtG82YzW-g@mail.gmail.com","threadId":"45321","inReplyTo":"xmqqwpbvumrk.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4] ref-filter: Add --no-contains option to tag/branch/for-each-ref","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-03-12T09:10:25Z","receivedAt":"2017-03-12T09:11:07Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sun, Mar 12, 2017 at 5:44 AM, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Change the tag, branch & for-each-ref commands to have a --no-contains\n>> option in addition to their longstanding --contains options.\n>>\n>> The use-case I have for this is to find the last-good rollout tag\n>> given a known-bad . Right now, given a hypothetically bad\n>> commit v2.10.1-3-gcf5c7253e0, you can find which git version to revert\n>> to with this hacky two-liner:\n>>\n>> (./git tag -l 'v[0-9]*'; ./git tag -l 'v[0-9]*' --contains v2.10.1-3-gcf5c7253e0) \\\n>> |sort|uniq -c|grep -E '^ *1 '|awk '{print $2}' | tail -n 10\n>>\n>> But with the --no-contains option you can now get the exact same\n>> output with:\n>>\n>> ./git tag -l 'v[0-9]*' --no-contains v2.10.1-3-gcf5c7253e0|sort|tail -n 10\n>\n> This command line, while it may happen to work, logically does not\n> make much sense. Move the pattern to the end, i.e.\n>\n> git tag -l --no-contains v2.10.1-3-gcf5c7253e0 'v[0-9]*'\n\nSure, if you'd like it like that I can change it.\n\nBut as an aside, I don't understand why you think it happens to work\nand doesn't make much sense. To someone reading \"git help tag\" this\nwould be *exactly* how you'd expect to have to write this, since the\nexample given in the synopsis is:\n\n git tag [-n[]] -l [--contains ] [....]\n\nAnd then later in the documentation:\n\n -l , --list \n\nI.e. for git-branch this type of invocation wouldn't make sense and\nwould just happen to work, but for git-tag the --list option is\nexplicitly documented to immediately take a argument.\n\nBut of course the whole branch v.s. tag difference is just more fodder\nfor my \"tag: Implicitly supply --list given another list-like option\"\npatch.\n\n> Also if an overlong line in an example disturbs you, do not solve it\n> by omitting SP around pipe. If you are trying to make the result\n> readable, pick a readable solution, e.g.\n\nFWIW I just write one-liners like this, i.e. I don't add the\nsemantically meaningless spaces around all pipes to save myself some\ntyping, and wasn't trying to squeeze this all on one line, but sure\nI'll change it.\n\n> git tag -l --no-contains v2.10.1-3-gcf5c7253e0 'v[0-9]*' |\n> sort | tail -n 10\n\nAlthough I'll add a \\ to that so you can still paste it to a terminal.\n\n> Oh, drop ./ from ./git while at it ;-)\n\nSure.\n\n>> The filtering machinery is generic between the tag, branch &\n>> for-each-ref commands, so once I'd implemented it for tag it was\n>> trivial to add support for this to the other two.\n>\n> Also, we tend not to say \"I did this, I do that\".\n>\n> Because the filtering machinery is generic ..., support it\n> for all three consistently.\n>\n>> I'm adding a --without option to \"tag\" as an alias for --no-contains\n>> for consistency with --with and --contains. Since we don't even\n>> document --with anymore (or test it). The --with option is\n>> undocumented, and possibly the only user of it is Junio[1]. But it's\n>> trivial to support, so let's do that.\n>\n> The sentence that begins \"Since we don't\" is unfinished. I think\n> it can safely removed without losing any information (the next\n> sentence says the same thing).\n>\n>> Where I'm changing existing documentation lines I'm mainly word\n>> wrapping at 75 columns to be consistent with the existing style.\n\nAck.\n\n> Reviewers would appreciate you refrain from doing that in the same\n> patch. Do a minimum patch so that the review can concentrate on\n> what got changed (i.e. contents), followed by a mechanical reflow as\n> a follow-up, or something like that, would be much nicer to handle.\n\nOkey, so two patches, one where I potentially produce very long lines\n& then re-flow them in a subsequent commit.\n\n>> Most of the test changes I've made are just doing the inverse of the\n>> existing --contains tests, with this change --no-contains for tag,\n>> branch & for-each-ref is just as well tested as the existing\n>> --contains option.\n>\n> Again, we tend to try our commits not about \"I, my, me\".\n>\n> Add --no-contains tests for tag, branch and for-each-ref\n> that mostly do the inverse of the existing tests we have for\n> --contains.\n\n*Nod*\n\n>> This is now based on top of pu, which has Jeff King's \"fix object flag\n>> pollution in \"tag --contains\" series.\n>\n> Thanks for this note. I obviously cannot queue on top of 'pu' ;-)\n> but will fork this topic off of the jk/ref-filter-flags-cleanup\n> topic.\n\nIf I'd like to base on top of that to make things easier for you do\nyou publish jk/ref-filter-flags-cleanup sowhere? I.e. as a git ref\nrather than me also following that topic, applying it on top of\nmaster, and then applying my topic on top of that.\n\n>> 'git for-each-ref' [--count=] [--shell|--perl|--python|--tcl]\n>> [(--sort=)...] [--format=] [...]\n>> [--points-at ] [(--merged | --no-merged) []]\n>> - [--contains []]\n>> + [(--contains | --no-contains) []]\n>\n> THis notation makes sense. We have to have one of these but\n> at the end could be omitted (to default to HEAD). I guess\n> the same notation can be used in the log for the other \"filtering\n> implies --list mode for 'git tag'\" topic.\n\nI don't know what makes to list in the synopsis given the default\narguments to --contains and --no-contains, maybe \"[]?\", but in\nany case, I'm not changing how that part is documented in\nfor-each-ref, just adding an alternative to the existing --contains\noption.\n\n>> +--no-contains []::\n>> + Only list tags which don't contain the specified commit (HEAD if\n>> + not specified).\n>\n> Just being curious. Can we do\n>\n> for-each-ref --contains --no-contains\n>\n> and have both default to HEAD? I know that would not make sense as\n> a set operation, but I am curious what our command line parser\n> (which is oblivious to what the command is doing) does. I am guessing\n> that it would barf saying \"--contains\" needs a commit but \"--no-contains\"\n> is not a commit (which is very sensible)?\n\nIt'll spew out \"error: malformed object name --no-contains\".\n\nYou can do \"--contains HEAD --no-contains HEAD\" to get nothing.\n\nIn an earlier thread I was discussing with Jeff whether it would be\nworthwhile to error out in that case, but his opinion was\n(paraphrasing) \"Nah, GIGO\", which I think makes sense in this case.\n\n>> +\n>> --points-at ::\n>> Only list tags of the given object.\n>\n> This is not a new issue (and certainly not a problem caused by your\n> patch), but unlike \"--contains\", this does not default to HEAD when\n> is not explicitly given? It seems a bit inconsistent to me.\n\nYeah, it doesn't default to HEAD. It's inconsistent, due to a\nhistorical wart in parse-options.[ch]\n\n>> @@ -618,7 +620,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n>> if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)\n>> list = 1;\n>>\n>> - if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)\n>> + if (filter.with_commit || filter.no_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)\n>> list = 1;\n>\n> OK.\n>\n>> diff --git a/parse-options.h b/parse-options.h\n>> index dcd8a0926c..0eac90b510 100644\n>> --- a/parse-options.h\n>> +++ b/parse-options.h\n>> @@ -258,7 +258,9 @@ extern int parse_opt_passthru_argv(const struct option *, const char *, int);\n>> PARSE_OPT_LASTARG_DEFAULT | flag, \\\n>> parse_opt_commits, (intptr_t) \"HEAD\" \\\n>> }\n>> -#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"contains\", v, h, 0)\n>> +#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"contains\", v, h, PARSE_OPT_NONEG)\n>> +#define OPT_NO_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"no-contains\", v, h, PARSE_OPT_NONEG)\n>> #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH(\"with\", v, h, PARSE_OPT_HIDDEN)\n>> +#define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH(\"without\", v, h, PARSE_OPT_HIDDEN)\n>\n> Hmph, perhaps WITH/WITHOUT also do not take \"--no-\" form hence need OPT_NONEG?\n\nI may be missing some subtlety here, but I think you've misread this\n(admittedly dense) chunk. the /WITH/ options don't supply NONEG, just\nHIDDEN.\n\n>> @@ -1586,11 +1587,11 @@ static enum contains_result contains_tag_algo(struct commit *candidate,\n>> }\n>>\n>> static int commit_contains(struct ref_filter *filter, struct commit *commit,\n>> - struct contains_cache *cache)\n>> + struct commit_list *list, struct contains_cache *cache)\n>> {\n>> if (filter->with_commit_tag_algo)\n>> - return contains_tag_algo(commit, filter->with_commit, cache) == CONTAINS_YES;\n>> - return is_descendant_of(commit, filter->with_commit);\n>> + return contains_tag_algo(commit, list, cache) == CONTAINS_YES;\n>> + return is_descendant_of(commit, list);\n>> }\n>>\n>> /*\n>> @@ -1780,13 +1781,17 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,\n>> * obtain the commit using the 'oid' available and discard all\n>> * non-commits early. The actual filtering is done later.\n>> */\n>> - if (filter->merge_commit || filter->with_commit || filter->verbose) {\n>> + if (filter->merge_commit || filter->with_commit || filter->no_commit || filter->verbose) {\n>> commit = lookup_commit_reference_gently(oid->hash, 1);\n>> if (!commit)\n>> return 0;\n>> - /* We perform the filtering for the '--contains' option */\n>> + /* We perform the filtering for the '--contains' option... */\n>> if (filter->with_commit &&\n>> - !commit_contains(filter, commit, &ref_cbdata->contains_cache))\n>> + !commit_contains(filter, commit, filter->with_commit, &ref_cbdata->contains_cache))\n>> + return 0;\n>> + /* ...or for the `--no-contains' option */\n>> + if (filter->no_commit &&\n>> + commit_contains(filter, commit, filter->no_commit, &ref_cbdata->no_contains_cache))\n>> return 0;\n>> }\n>\n> When asking \"--contains A --contains B\", we show refs that contain\n> _EITHER_ A or B. Two predicates are ORed together, and I think it\n> makes sense.\n>\n> When asking \"--contains A --no-contains B\", we show refs that\n> contain A but exclude refs that contains B. Two predicates are\n> ANDed together, and I think this also makes sense.\n>\n> When asking \"--no-contains A --no-contains B\", what should we show?\n> This implementation makes the two predicates ANDed together [*1*].\n>\n> The behaviour is sensible, but is it consistent with the way now\n> existing --no-merged works?\n>\n> I think the rule is something like:\n>\n> A match with any positive selection criterion (like --contains\n> A) makes a ref eligible for output, but then a match with any\n> negatigve selection criterion (like --no-merged) filters it out.\n>\n> Is it easy to explain to the users? Do we need doc updates to\n> clarify, or does the description for existing --no-merged already\n> cover this?\n\nI'm not sure, and wanting to get the rest of this e-mail out the door,\nI didn't have time to fully investigate this. I.e. is it consistent\nwith how --merged works? It uses a slightly different codepath, maybe,\nbut it's definitely consistent with how everything in the ref-filter\nmachinery works in general, and that's used by multiple options.\n\nSo to the extent that this needs docs we should do that in some\ngeneral place that covers existing options that use it.\n"},{"id":"313850","messageId":"20170312111217.swapjaazeg3wbuyt@sigill.intra.peff.net","threadId":"45321","inReplyTo":"CACBZZX5A46-ua9y_HxqrqycQviR0uGnStnaPwH9jukzPVak6ug@mail.gmail.com","subject":"Re: [PATCH 4/4] ref-filter: use separate cache for contains_tag_algo","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-12T11:12:18Z","receivedAt":"2017-03-12T11:12:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 11, 2017 at 09:01:25PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> On Thu, Mar 9, 2017 at 2:29 PM, Jeff King wrote:\n> > [...]\n> > @@ -1874,6 +1886,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int\n> > broken = 1;\n> > filter->kind = type & FILTER_REFS_KIND_MASK;\n> >\n> > + init_contains_cache(&ref_cbdata.contains_cache);\n> > +\n> > /* Simple per-ref filtering */\n> > [...]\n> >\n> > + clear_contains_cache(&ref_cbdata.contains_cache);\n> >\n> > /* Filters that need revision walking */\n> > if (filter->merge_commit)\n> \n> Shouldn't both of those be guarded by a \"if (filter->with_commit)\" test?\n\nI thought about that while writing it, but decided that it was not worth\ncomplicating the initialization and cleanup with conditionals. The\ninitialization is on par with what we would normally do with a static\nstruct initializer, and the cleanup is a noop if the cache wasn't\ntouched.\n\nSo my thinking was that it didn't matter either way, and dropping the\nconditional was one less thing for readers of the code to have to worry\nabout.\n\nThat said, I'm not opposed to doing it the other way.\n\n> That init/clear codepath is rather light, but it seems to me that we\n> can avoid it entirely if filter->with_commit isn't defined. I've\n> tested this locally and it still passes all tests.\n\nYes, it should be correct either way.\n\n-Peff\n"},{"id":"313862","messageId":"xmqqk27uv100.fsf@gitster.mtv.corp.google.com","threadId":"45321","inReplyTo":"CACBZZX4v49zfyGVpcxGSKsxbMfVaUcGHtitpfaZMUtG82YzW-g@mail.gmail.com","subject":"Re: [PATCH v4] ref-filter: Add --no-contains option to tag/branch/for-each-ref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-12T17:49:35Z","receivedAt":"2017-03-12T17:49:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> And then later in the documentation:\n>\n> -l , --list \n>\n> I.e. for git-branch this type of invocation wouldn't make sense and\n> would just happen to work, but for git-tag the --list option is\n> explicitly documented to immediately take a argument.\n\nBut that (i.e. \"to immediately take\") is not how it actually works,\nas you already know after looking at how 'l' is defined as OPT_CMDMODE.\n\nThe command line is more like \"-l\" chooses the \"list mode\", and the\npattern is _NOT_ an argument to the option at all. The command line\nis more like \"-l ...\" and the\n is one of these criteria. The command line convention being\ndashed-options first then other arguments, it makes sense to do\n\n\t-l --contains HEAD v\\*\n\nbecause \"--contains HEAD\" is a dashed-option (which takes an argument)\nand \"v\\*\" is a pattern (which is \"other arguments\").\n\n> I'll change it.\n>\n>> git tag -l --no-contains v2.10.1-3-gcf5c7253e0 'v[0-9]*' |\n>> sort | tail -n 10\n>\n> Although I'll add a \\ to that so you can still paste it to a terminal.\n\nPlease don't. The shell knows, when you end a line with pipe, that\nyou haven't finished your sentence and keeps listening to you.\n\n>> Reviewers would appreciate you refrain from doing that in the same\n>> patch. Do a minimum patch so that the review can concentrate on\n>> what got changed (i.e. contents), followed by a mechanical reflow as\n>> a follow-up, or something like that, would be much nicer to handle.\n>\n> Okey, so two patches, one where I potentially produce very long lines\n> & then re-flow them in a subsequent commit.\n\nOr preferrably, the last \"-\" line in a hunk of your first patch may\nbe longer than the first \"+\" line that replaces it that may be\noverly short (i.e. chopping the end of existing paragraph and\nreplacing the remainder). And then reflow comes, e.g.\n\n -Okey, so two patches, one where I potentially produce very long lines\n +Okey, so two patches, one where I\n +cut an existing line short if it makes the patch churn smaller\n & then re-flow them in a subsequent commit.\n\n> If I'd like to base on top of that to make things easier for you do\n> you publish jk/ref-filter-flags-cleanup sowhere? I.e. as a git ref\n> rather than me also following that topic, applying it on top of\n> master, and then applying my topic on top of that.\n\nDo you mean a repository that holds broken-out topics? If so:\n\n\tgit://github.com/gitster/git/\n\nis what you are looking for, perhaps?\n\n>\n>> and have both default to HEAD? I know that would not make sense as\n>> a set operation, but I am curious what our command line parser\n>> (which is oblivious to what the command is doing) does. I am guessing\n>> that it would barf saying \"--contains\" needs a commit but \"--no-contains\"\n>> is not a commit (which is very sensible)?\n>\n> It'll spew out \"error: malformed object name --no-contains\".\n>\n> You can do \"--contains HEAD --no-contains HEAD\" to get nothing.\n>\n> In an earlier thread I was discussing with Jeff whether it would be\n> worthwhile to error out in that case, but his opinion was\n> (paraphrasing) \"Nah, GIGO\", which I think makes sense in this case.\n\nI agree with Peff (I said \"that would not make sense as a set\noperation\", didn't I? ;-); I was only curious if the notation used\nin the documentation, i.e. \"--opt []\" made sense. It looks\nas if it would accept \"--contains --no-contains\" (omitting arguments\nfrom both options), but it is not so, and I was wondering if we need\nto improve the documentation, or the readers are OK with the notation.\n\n>>> -#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"contains\", v, h, 0)\n>>> +#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"contains\", v, h, PARSE_OPT_NONEG)\n>>> +#define OPT_NO_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"no-contains\", v, h, PARSE_OPT_NONEG)\n>>> #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH(\"with\", v, h, PARSE_OPT_HIDDEN)\n>>> +#define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH(\"without\", v, h, PARSE_OPT_HIDDEN)\n>>\n>> Hmph, perhaps WITH/WITHOUT also do not take \"--no-\" form hence need OPT_NONEG?\n>\n> I may be missing some subtlety here, but I think you've misread this\n> (admittedly dense) chunk. the /WITH/ options don't supply NONEG, just\n> HIDDEN.\n\nMaybe I was unclear. As --contains should not take --no-contains\nand use \"unset\" (because new code wants to see \"no-contains\" and act\non it in the new code, I was wondering if we should forbid --no-with\nand --no-without in a similar way by using OPT_NONEG in addition to\nOPT_HIDDEN.\n"}]}