threads / patch / 45321

patchbranch & tag: Add a --no-contains option

Subject: [PATCH] branch & tag: Add a --no-contains option

## tl;dr

28 messages between Mar 8, 2017 and Mar 12, 2017. Diffs are folded; open one to read it.

replies: 27people: 4as markdown or json

Ævar Arnfjörð Bjarmason· Mar 8, 2017, 20:20 UTC · lore

Change the branch & tag commands to have a --no-contains option in addition to their longstanding --contains options.

The use-case I have for this is mainly to find the last-good rollout tag given a known-bad <commit>. Right given a hypothetically bad commit v2.10.1-3-gcf5c7253e0 now you can find that with this hacky one-liner:

    (./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}'

But with the --no-contains option you can now get the exact same output with:

    ./git tag -l 'v[0-9]*' --no-contains v2.10.1-3-gcf5c7253e0 | sort

Once I'd implemented this for "tag" it was easy enough to add it for "branch". I haven't added it to "for-each-ref" but that would be trivial if anyone cares, but that use-case would be even more obscure than adding it to "branch", so I haven't bothered. The "describe" command also has a --contains option, but its semantics are unrelated to what tag/branch/for-each-ref use --contains for, and I don't see how a --no-contains option for it would make any sense.

More notes about this patch:
 * I'm not really happy with the "special attention" documentation
   example in git-branch.txt, but it follows logically from the
   description for --contains just above it which I think is overly
   specific as well. IMO that entire NOTES section in git-branch.txt
   could just be removed.
 * I'm adding a --without option as an alias for --no-contains for
   consistency with --with and --contains. Since we don't even
   document --with anymore (or test it) perhaps we shouldn't be adding
   --without.
 * Where I'm changing existing documentation lines I'm mainly word
   wrapping at 75 columns to be consistent with the existing style.
 * Ditto the minor change to git-completion.bash.
 * Perhaps we should just copy/paste commit_contains() into
   commit_no_contains() and skip the ternary struct assignment. It's a
   hot loop, but I have faith in modern compilers.
 * All of the test changes I've made are just doing the inverse of the
   existing --contains tests, with this --no-contains for both tag &
   branch should be just as tested as the existing --contains option.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-branch.txt           |  24 +++--
 Documentation/git-tag.txt              |   6 +-
 builtin/branch.c                       |   4 +-
 builtin/tag.c                          |   2 +
 contrib/completion/git-completion.bash |   9 +-
 parse-options.h                        |   4 +-
 ref-filter.c                           |  17 ++--
 ref-filter.h                           |   1 +
 t/t3201-branch-contains.sh             |  40 +++++++-
 t/t7004-tag.sh                         | 163 ++++++++++++++++++++++++++++++++-
 10 files changed, 245 insertions(+), 25 deletions(-)
Show changes to 10 files +245 −25

Documentation/git-branch.txt, Documentation/git-tag.txt, builtin/branch.c, builtin/tag.c, contrib/completion/git-completion.bash, parse-options.h, ref-filter.c, ref-filter.h, t/t3201-branch-contains.sh, t/t7004-tag.sh

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 092f1bcf9f..316ec5b2d4 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git branch' [--color[=<when>] | --no-color] [-r | -a]
 	[--list] [-v [--abbrev=<length> | --no-abbrev]]
 	[--column[=<options>] | --no-column]
-	[(--merged | --no-merged | --contains) [<commit>]] [--sort=<key>]
+	[(--merged | --no-merged | --contains | --no-contains) [<commit>]] [--sort=<key>]
 	[--points-at <object>] [--format=<format>] [<pattern>...]
 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
 'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
@@ -35,11 +35,12 @@ as branch creation.
 
 With `--contains`, shows only the branches that contain the named commit
 (in other words, the branches whose tip commits are descendants of the
-named commit).  With `--merged`, only branches merged into the named
-commit (i.e. the branches whose tip commits are reachable from the named
-commit) will be listed.  With `--no-merged` only branches not merged into
-the named commit will be listed.  If the <commit> argument is missing it
-defaults to `HEAD` (i.e. the tip of the current branch).
+named commit), `--no-contains` inverts it. With `--merged`, only branches
+merged into the named commit (i.e. the branches whose tip commits are
+reachable from the named commit) will be listed.  With `--no-merged` only
+branches not merged into the named commit will be listed.  If the <commit>
+argument is missing it defaults to `HEAD` (i.e. the tip of the current
+branch).
 
 The command's second form creates a new branch head named <branchname>
 which points to the current `HEAD`, or <start-point> if given.
@@ -213,6 +214,10 @@ start-point is either a local or remote-tracking branch.
 	Only list branches which contain the specified commit (HEAD
 	if not specified). Implies `--list`.
 
+--no-contains [<commit>]::
+	Only list branches which don't contain the specified commit
+	(HEAD if not specified). Implies `--list`.
+
 --merged [<commit>]::
 	Only list branches whose tips are reachable from the
 	specified commit (HEAD if not specified). Implies `--list`.
@@ -296,13 +301,16 @@ If you are creating a branch that you want to checkout immediately, it is
 easier to use the git checkout command with its `-b` option to create
 a branch and check it out with a single command.
 
-The options `--contains`, `--merged` and `--no-merged` serve three related
-but different purposes:
+The options `--contains`, `--no-contains`, `--merged` and `--no-merged`
+serve three related but different purposes:
 
 - `--contains <commit>` is used to find all branches which will need
   special attention if <commit> were to be rebased or amended, since those
   branches contain the specified <commit>.
 
+- `--no-contains <commit>` is used to find those branches which won't need
+  that special attention.
+
 - `--merged` is used to find all branches which can be safely deleted,
   since those branches are fully contained by HEAD.
 
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 525737a5d8..4938496194 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git tag' [-a | -s | -u <keyid>] [-f] [-m <msg> | -F <file>]
 	<tagname> [<commit> | <object>]
 'git tag' -d <tagname>...
-'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
+'git tag' [-n[<num>]] -l [--[no-]contains <commit>] [--points-at <object>]
 	[--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]
 	[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]
 'git tag' -v [--format=<format>] <tagname>...
@@ -124,6 +124,10 @@ This option is only applicable when listing tags without annotation lines.
 	Only list tags which contain the specified commit (HEAD if not
 	specified).
 
+--no-contains [<commit>]::
+	Only list tags which don't contain the specified commit (HEAD if
+	not secified).
+
 --points-at <object>::
 	Only list tags of the given object.
 
diff --git a/builtin/branch.c b/builtin/branch.c
index 94f7de7fa5..e8d534604c 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -548,7 +548,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT('r', "remotes",     &filter.kind, N_("act on remote-tracking branches"),
 			FILTER_REFS_REMOTES),
 		OPT_CONTAINS(&filter.with_commit, N_("print only branches that contain the commit")),
+		OPT_NO_CONTAINS(&filter.no_commit, N_("print only branches that don't contain the commit")),
 		OPT_WITH(&filter.with_commit, N_("print only branches that contain the commit")),
+		OPT_WITHOUT(&filter.with_commit, N_("print only branches that don't contain the commit")),
 		OPT__ABBREV(&filter.abbrev),
 
 		OPT_GROUP(N_("Specific git-branch actions:")),
@@ -604,7 +606,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)
 		list = 1;
 
-	if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)
+	if (filter.with_commit || filter.no_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)
 		list = 1;
 
 	if (!!delete + !!rename + !!new_upstream +
diff --git a/builtin/tag.c b/builtin/tag.c
index ad29be6923..737a83028a 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -424,7 +424,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		OPT_GROUP(N_("Tag listing options")),
 		OPT_COLUMN(0, "column", &colopts, N_("show tag list in columns")),
 		OPT_CONTAINS(&filter.with_commit, N_("print only tags that contain the commit")),
+		OPT_NO_CONTAINS(&filter.no_commit, N_("print only tags that don't contain the commit")),
 		OPT_WITH(&filter.with_commit, N_("print only tags that contain the commit")),
+		OPT_WITHOUT(&filter.no_commit, N_("print only tags that don't contain the commit")),
 		OPT_MERGED(&filter, N_("print only tags that are merged")),
 		OPT_NO_MERGED(&filter, N_("print only tags that are not merged")),
 		OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index fc32286a43..fa3da49478 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1093,9 +1093,9 @@ _git_branch ()
 	--*)
 		__gitcomp "
 			--color --no-color --verbose --abbrev= --no-abbrev
-			--track --no-track --contains --merged --no-merged
-			--set-upstream-to= --edit-description --list
-			--unset-upstream --delete --move --remotes
+			--track --no-track --contains --no-contains --merged
+			--no-merged --set-upstream-to= --edit-description
+			--list --unset-upstream --delete --move --remotes
 			--column --no-column --sort= --points-at
 			"
 		;;
@@ -2862,7 +2862,8 @@ _git_tag ()
 		__gitcomp "
 			--list --delete --verify --annotate --message --file
 			--sign --cleanup --local-user --force --column --sort=
-			--contains --points-at --merged --no-merged --create-reflog
+			--contains --no-contains --points-at --merged
+			--no-merged --create-reflog
 			"
 		;;
 	esac
diff --git a/parse-options.h b/parse-options.h
index dcd8a0926c..0eac90b510 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -258,7 +258,9 @@ extern int parse_opt_passthru_argv(const struct option *, const char *, int);
 	  PARSE_OPT_LASTARG_DEFAULT | flag, \
 	  parse_opt_commits, (intptr_t) "HEAD" \
 	}
-#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, 0)
+#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, PARSE_OPT_NONEG)
+#define OPT_NO_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("no-contains", v, h, PARSE_OPT_NONEG)
 #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH("with", v, h, PARSE_OPT_HIDDEN)
+#define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH("without", v, h, PARSE_OPT_HIDDEN)
 
 #endif
diff --git a/ref-filter.c b/ref-filter.c
index 1ec0fb8391..6a7ca1cdac 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1571,11 +1571,12 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
 	return contains_test(candidate, want);
 }
 
-static int commit_contains(struct ref_filter *filter, struct commit *commit)
+static int commit_contains(struct ref_filter *filter, struct commit *commit, const int with_commit)
 {
+	struct commit_list *tmp = with_commit ? filter->with_commit : filter->no_commit;
 	if (filter->with_commit_tag_algo)
-		return contains_tag_algo(commit, filter->with_commit);
-	return is_descendant_of(commit, filter->with_commit);
+		return contains_tag_algo(commit, tmp);
+	return is_descendant_of(commit, tmp);
 }
 
 /*
@@ -1765,13 +1766,17 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
 	 * obtain the commit using the 'oid' available and discard all
 	 * non-commits early. The actual filtering is done later.
 	 */
-	if (filter->merge_commit || filter->with_commit || filter->verbose) {
+	if (filter->merge_commit || filter->with_commit || filter->no_commit || filter->verbose) {
 		commit = lookup_commit_reference_gently(oid->hash, 1);
 		if (!commit)
 			return 0;
-		/* We perform the filtering for the '--contains' option */
+		/* We perform the filtering for the '--contains' option... */
 		if (filter->with_commit &&
-		    !commit_contains(filter, commit))
+		    !commit_contains(filter, commit, 1))
+			return 0;
+		/* ...or for the `--no-contains' option */
+		if (filter->no_commit &&
+		    commit_contains(filter, commit, 0))
 			return 0;
 	}
 
diff --git a/ref-filter.h b/ref-filter.h
index 154e24c405..af85eb4592 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -53,6 +53,7 @@ struct ref_filter {
 	const char **name_patterns;
 	struct sha1_array points_at;
 	struct commit_list *with_commit;
+	struct commit_list *no_commit;
 
 	enum {
 		REF_FILTER_MERGED_NONE = 0,
diff --git a/t/t3201-branch-contains.sh b/t/t3201-branch-contains.sh
index 7f3ec47241..9fb79e66f0 100755
--- a/t/t3201-branch-contains.sh
+++ b/t/t3201-branch-contains.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='branch --contains <commit>, --merged, and --no-merged'
+test_description='branch --contains <commit>, --no-contains <commit> --merged, and --no-merged'
 
 . ./test-lib.sh
 
@@ -45,6 +45,22 @@ test_expect_success 'branch --contains master' '
 
 '
 
+test_expect_success 'branch --no-contains=master' '
+
+	git branch --no-contains=master >actual &&
+	>expect &&
+	test_cmp expect actual
+
+'
+
+test_expect_success 'branch --no-contains master' '
+
+	git branch --no-contains master >actual &&
+	>expect &&
+	test_cmp expect actual
+
+'
+
 test_expect_success 'branch --contains=side' '
 
 	git branch --contains=side >actual &&
@@ -55,6 +71,16 @@ test_expect_success 'branch --contains=side' '
 
 '
 
+test_expect_success 'branch --no-contains=side' '
+
+	git branch --no-contains=side >actual &&
+	{
+		echo "  master"
+	} >expect &&
+	test_cmp expect actual
+
+'
+
 test_expect_success 'branch --contains with pattern implies --list' '
 
 	git branch --contains=master master >actual &&
@@ -65,6 +91,14 @@ test_expect_success 'branch --contains with pattern implies --list' '
 
 '
 
+test_expect_success 'branch --no-contains with pattern implies --list' '
+
+	git branch --no-contains=master master >actual &&
+	>expect &&
+	test_cmp expect actual
+
+'
+
 test_expect_success 'side: branch --merged' '
 
 	git branch --merged >actual &&
@@ -126,7 +160,9 @@ test_expect_success 'branch --no-merged with pattern implies --list' '
 test_expect_success 'implicit --list conflicts with modification options' '
 
 	test_must_fail git branch --contains=master -d &&
-	test_must_fail git branch --contains=master -m foo
+	test_must_fail git branch --contains=master -m foo &&
+	test_must_fail git branch --no-contains=master -d &&
+	test_must_fail git branch --no-contains=master -m foo
 
 '
 
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index b4698ab5f5..f01bcafea4 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1385,6 +1385,23 @@ test_expect_success 'checking that first commit is in all tags (relative)' "
 	test_cmp expected actual
 "
 
+# All the --contains tests above, but with --no-contains
+test_expect_success 'checking that first commit is not listed in any tag with --no-contains  (hash)' "
+	>expected &&
+	git tag -l --no-contains $hash1 v* >actual &&
+	test_cmp expected actual
+"
+
+test_expect_success 'checking that first commit is in all tags (tag)' "
+	git tag -l --no-contains v1.0 v* >actual &&
+	test_cmp expected actual
+"
+
+test_expect_success 'checking that first commit is in all tags (relative)' "
+	git tag -l --no-contains HEAD~2 v* >actual &&
+	test_cmp expected actual
+"
+
 cat > expected <<EOF
 v2.0
 EOF
@@ -1394,6 +1411,17 @@ test_expect_success 'checking that second commit only has one tag' "
 	test_cmp expected actual
 "
 
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+EOF
+
+test_expect_success 'inverse of the last test, with --no-contains' "
+	git tag -l --no-contains $hash2 v* >actual &&
+	test_cmp expected actual
+"
 
 cat > expected <<EOF
 EOF
@@ -1403,6 +1431,19 @@ test_expect_success 'checking that third commit has no tags' "
 	test_cmp expected actual
 "
 
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+v2.0
+EOF
+
+test_expect_success 'conversely --no-contains on the third commit lists all tags' "
+	git tag -l --no-contains $hash3 v* >actual &&
+	test_cmp expected actual
+"
+
 # how about a simple merge?
 
 test_expect_success 'creating simple branch' '
@@ -1424,6 +1465,19 @@ test_expect_success 'checking that branch head only has one tag' "
 	test_cmp expected actual
 "
 
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+v2.0
+EOF
+
+test_expect_success 'checking that branch head with --no-contains lists all but one tag' "
+	git tag -l --no-contains $hash4 v* >actual &&
+	test_cmp expected actual
+"
+
 test_expect_success 'merging original branch into this branch' '
 	git merge --strategy=ours master &&
         git tag v4.0
@@ -1445,6 +1499,20 @@ v1.0.1
 v1.1.3
 v2.0
 v3.0
+EOF
+
+test_expect_success 'checking that original branch head with --no-contains lists all but one tag now' "
+	git tag -l --no-contains $hash3 v* >actual &&
+	test_cmp expected actual
+"
+
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+v2.0
+v3.0
 v4.0
 EOF
 
@@ -1453,6 +1521,12 @@ test_expect_success 'checking that initial commit is in all tags' "
 	test_cmp expected actual
 "
 
+test_expect_success 'checking that initial commit is in all tags with --no-contains' "
+	>expected &&
+	git tag -l --no-contains $hash1 v* >actual &&
+	test_cmp expected actual
+"
+
 # mixing modes and options:
 
 test_expect_success 'mixing incompatibles modes and options is forbidden' '
@@ -1708,8 +1782,91 @@ run_with_limited_stack () {
 
 test_lazy_prereq ULIMIT_STACK_SIZE 'run_with_limited_stack true'
 
+# These are all the tags we've created above
+cat >expect.no-contains <<EOF
+a1
+aa1
+annotated-again-v4.0
+annotated-tag
+annotated-v4.0
+blank-annotated-tag
+blank-signed-tag
+blankfile-annotated-tag
+blankfile-signed-tag
+blanknonlfile-annotated-tag
+blanknonlfile-signed-tag
+blanks-annotated-tag
+blanks-signed-tag
+cba
+comment-annotated-tag
+comment-signed-tag
+commentfile-annotated-tag
+commentfile-signed-tag
+commentnonlfile-annotated-tag
+commentnonlfile-signed-tag
+comments-annotated-tag
+comments-signed-tag
+empty-annotated-tag
+empty-signed-tag
+emptyfile-annotated-tag
+emptyfile-signed-tag
+far-far-away
+file-annotated-tag
+file-signed-tag
+foo1.10
+foo1.10-alpha
+foo1.10-beta
+foo1.10-delta
+foo1.10-gamma
+foo1.10-unlisted-suffix
+foo1.3
+foo1.6
+foo1.6-rc1
+foo1.6-rc2
+foo1.7
+foo1.7-after1
+foo1.7-before1
+foo1.8
+foo1.8-foo-bar
+foo1.8-foo-baz
+foo1.9-pre1
+foo1.9-pre2
+foo1.9-prerelease1
+forcesignannotated-annotate
+forcesignannotated-disabled
+forcesignannotated-implied-sign
+forcesignannotated-lightweight
+forged-tag
+implied-annotate
+implied-sign
+non-annotated-tag
+reuse
+signed-tag
+stag-lines
+stag-one-line
+stag-zero-lines
+stdin-annotated-tag
+stdin-signed-tag
+t210
+t211
+tag-from-subdir
+tag-from-subdir-2
+tag-lines
+tag-one-line
+tag-signed-tag
+tag-zero-lines
+u-signed-tag
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+v2.0
+v3.0
+v4.0
+EOF
+
 # we require ulimit, this excludes Windows
-test_expect_success ULIMIT_STACK_SIZE '--contains works in a deep repo' '
+test_expect_success ULIMIT_STACK_SIZE '--contains and --no-contains work in a deep repo' '
 	>expect &&
 	i=1 &&
 	while test $i -lt 8000
@@ -1725,7 +1882,9 @@ EOF"
 	git checkout master &&
 	git tag far-far-away HEAD^ &&
 	run_with_limited_stack git tag --contains HEAD >actual &&
-	test_cmp expect actual
+	test_cmp expect actual &&
+	run_with_limited_stack git tag --no-contains HEAD >actual &&
+	test_cmp expect.no-contains actual
 '
 
 test_expect_success '--format should list tags as per format given' '
-- 
2.11.0
Junio C Hamano· Mar 8, 2017, 23:02 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] branch & tag: Add a --no-contains option

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
Show 7 quoted lines
> More notes about this patch:
>
>  * I'm not really happy with the "special attention" documentation
>    example in git-branch.txt, but it follows logically from the
>    description for --contains just above it which I think is overly
>    specific as well. IMO that entire NOTES section in git-branch.txt
>    could just be removed.

The first paragraph of the section is unrelated to the topic and I do not think anybody would miss it if it goes, but I always feel uncomfortable between --contains and --merged. I do not expect anybody needs lengthy explanation to tell --merged and --no-merged (similarly --contains and --no-contains) apart, but perhaps because I often use --with (which is a hidden synonym to --contains) and almost never --merged, and as we are adding the fourth, I find it a very good idea to extend the description to tell users what they want to use "contains" for (i.e. find the set of containers given a commit) and what they want to use "merged" for (i.e. find the set of containees given a commit).

>  * I'm adding a --without option as an alias for --no-contains for
>    consistency with --with and --contains. Since we don't even
>    document --with anymore (or test it) perhaps we shouldn't be adding
>    --without.

I do not think anybody other than me uses "--with" to begin with, so I do not care too deeply about it. If it makes the patch simpler not to support "--without", I'd be supportive if you want to drop it.

I'll review the body of the patch later.
Thanks.
Jeff King· Mar 9, 2017, 10:09 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] branch & tag: Add a --no-contains option

On Wed, Mar 08, 2017 at 08:20:25PM +0000, Ævar Arnfjörð Bjarmason wrote:
Show 14 quoted lines
> Change the branch & tag commands to have a --no-contains option in
> addition to their longstanding --contains options.
> 
> The use-case I have for this is mainly to find the last-good rollout
> tag given a known-bad <commit>. Right given a hypothetically bad
> commit v2.10.1-3-gcf5c7253e0 now you can find that with this hacky
> one-liner:
> 
>     (./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}'
> 
> But with the --no-contains option you can now get the exact same
> output with:
> 
>     ./git tag -l 'v[0-9]*' --no-contains v2.10.1-3-gcf5c7253e0 | sort
I think that's a good goal.

I'm not sure about the name. I would have expected "--no-contains" to reset the list of "--contains" commits to the empty set. That's an option convention we've been slowly moving towards (e.g., with OPT_STRING_LIST).

What you've added here _does_ match "--no-merged", though. I'm not sure of the best way forward. At the very least, "--no-contains" is currently an error, so you would not be changing existing behavior.

> Once I'd implemented this for "tag" it was easy enough to add it for
> "branch". I haven't added it to "for-each-ref" but that would be
> trivial if anyone cares, but that use-case would be even more obscure
> than adding it to "branch", so I haven't bothered.

I'd prefer to have it consistently in all three. We should be able to tell people to use for-each-ref in their scripts, and that's harder if it is missing features.

> The "describe" command also has a --contains option, but its semantics
> are unrelated to what tag/branch/for-each-ref use --contains for, and
> I don't see how a --no-contains option for it would make any sense.
Yeah, I think that feature is orthogonal.
Show 10 quoted lines
> -static int commit_contains(struct ref_filter *filter, struct commit *commit)
> +static int commit_contains(struct ref_filter *filter, struct commit *commit, const int with_commit)
>  {
> +	struct commit_list *tmp = with_commit ? filter->with_commit : filter->no_commit;
>  	if (filter->with_commit_tag_algo)
> -		return contains_tag_algo(commit, filter->with_commit);
> -	return is_descendant_of(commit, filter->with_commit);
> +		return contains_tag_algo(commit, tmp);
> +	return is_descendant_of(commit, tmp);
>  }

Perhaps it would be simpler if the caller just passed the right commit_list rather than a flag. We unfortunately do still need to pass the "filter" (for the algorithm field), but the caller is then:

  if (filter->with_commit &&
      !commit_contains(filter, filter->with_commit, commit))
          return 0;
  if (filter->no_commit &&
      commit_contains(filter, filter->no_commit, commit))
          return 0;

which avoids the 0/1 flag whose meaning is not immediately apparent at the callsite. One day we can hopefully unify the two algorithms and ditch the extra filter parameter.

I almost suggested that there simply be an option to invert the match (like --invert-contains or something). But what you have here is more flexible, if somebody ever wanted to do:

  git tag --contains X --no-contains Y

That could be useful if a feature was introduced in X and then got buggy in Y.

Show 8 quoted lines
> @@ -1708,8 +1782,91 @@ run_with_limited_stack () {
>  
>  test_lazy_prereq ULIMIT_STACK_SIZE 'run_with_limited_stack true'
>  
> +# These are all the tags we've created above
> +cat >expect.no-contains <<EOF
> [...80 tags...]
> +EOF

That's a lot of tags, and I'd worry it makes the test a little brittle. Can we limit the set of tags with a name-match? It shouldn't affect the purpose of the test (the deep stack comes from traversing the commits, not the number of tags).

-Peff
Ævar Arnfjörð Bjarmason· Mar 9, 2017, 10:41 UTC · re: Jeff King · lore

Re: [PATCH] branch & tag: Add a --no-contains option

On Thu, Mar 9, 2017 at 11:09 AM, Jeff King <peff@peff.net> wrote:
Show 27 quoted lines
> On Wed, Mar 08, 2017 at 08:20:25PM +0000, Ævar Arnfjörð Bjarmason wrote:
>
>> Change the branch & tag commands to have a --no-contains option in
>> addition to their longstanding --contains options.
>>
>> The use-case I have for this is mainly to find the last-good rollout
>> tag given a known-bad <commit>. Right given a hypothetically bad
>> commit v2.10.1-3-gcf5c7253e0 now you can find that with this hacky
>> one-liner:
>>
>>     (./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}'
>>
>> But with the --no-contains option you can now get the exact same
>> output with:
>>
>>     ./git tag -l 'v[0-9]*' --no-contains v2.10.1-3-gcf5c7253e0 | sort
>
> I think that's a good goal.
>
> I'm not sure about the name. I would have expected "--no-contains" to
> reset the list of "--contains" commits to the empty set. That's an
> option convention we've been slowly moving towards (e.g., with
> OPT_STRING_LIST).
>
> What you've added here _does_ match "--no-merged", though. I'm not sure
> of the best way forward. At the very least, "--no-contains" is currently
> an error, so you would not be changing existing behavior.

I initially started hacking this up as --not-contains, but after briefly chatting with Christian about it off-list he suggested --no-*. Since as you point out it's consistent with --no-merge. I have no strong view on it, I just want the feature whatever the flag is called.

Show 8 quoted lines
>> Once I'd implemented this for "tag" it was easy enough to add it for
>> "branch". I haven't added it to "for-each-ref" but that would be
>> trivial if anyone cares, but that use-case would be even more obscure
>> than adding it to "branch", so I haven't bothered.
>
> I'd prefer to have it consistently in all three. We should be able to
> tell people to use for-each-ref in their scripts, and that's harder if
> it is missing features.

Agreed. I'd already hacked that up this morning for a v2. It works & has tests at https://github.com/avar/git/tree/avar/no-contains-2

Show 5 quoted lines
>> The "describe" command also has a --contains option, but its semantics
>> are unrelated to what tag/branch/for-each-ref use --contains for, and
>> I don't see how a --no-contains option for it would make any sense.
>
> Yeah, I think that feature is orthogonal.

*Nod* just adding a note about it in case anyone's puzzled about why describe doesn't have --no-contains, elaborated & clarified this a bit in my WIP v2.

Show 25 quoted lines
>> -static int commit_contains(struct ref_filter *filter, struct commit *commit)
>> +static int commit_contains(struct ref_filter *filter, struct commit *commit, const int with_commit)
>>  {
>> +     struct commit_list *tmp = with_commit ? filter->with_commit : filter->no_commit;
>>       if (filter->with_commit_tag_algo)
>> -             return contains_tag_algo(commit, filter->with_commit);
>> -     return is_descendant_of(commit, filter->with_commit);
>> +             return contains_tag_algo(commit, tmp);
>> +     return is_descendant_of(commit, tmp);
>>  }
>
> Perhaps it would be simpler if the caller just passed the right
> commit_list rather than a flag. We unfortunately do still need to pass
> the "filter" (for the algorithm field), but the caller is then:
>
>   if (filter->with_commit &&
>       !commit_contains(filter, filter->with_commit, commit))
>           return 0;
>   if (filter->no_commit &&
>       commit_contains(filter, filter->no_commit, commit))
>           return 0;
>
> which avoids the 0/1 flag whose meaning is not immediately apparent at
> the callsite. One day we can hopefully unify the two algorithms and
> ditch the extra filter parameter.

My C rustyness is showing. Yeah that's much better, thanks, changed it to that in my WIP v2.

Show 5 quoted lines
> I almost suggested that there simply be an option to invert the match
> (like --invert-contains or something).  But what you have here is more
> flexible, if somebody ever wanted to do:
>
>   git tag --contains X --no-contains Y

Yeah that's really useful. E.g. this shows the branches I branched off (or have locally) from 2.6..2.8:

    $ ./git branch --contains v2.6.0 --no-contains v2.8.0
      avar/monkeypatch-untracked-cache-disabled
      avar/uc-notifs21
      dturner/pclouds-watchman-noshm
But I'd expect this to show all the tags between the two:
    $ ./git tag --contains v2.6.0 --no-contains v2.8.0
    $

But it just returns an empty list. Manually disabling the contains_tag_algo() path (i.e. effectively locally reverting your ffc4b8012d) makes it "work", but of course it's much slower now. I haven't dug into why it's not working yet.

Also I wonder if this should be an error:
    $ ./git [tag|branch|for-each-ref] --contains A --no-contains A

I.e. when you give the same argument to both, this can never return anything for obvious reasons.

Show 13 quoted lines
>> @@ -1708,8 +1782,91 @@ run_with_limited_stack () {
>>
>>  test_lazy_prereq ULIMIT_STACK_SIZE 'run_with_limited_stack true'
>>
>> +# These are all the tags we've created above
>> +cat >expect.no-contains <<EOF
>> [...80 tags...]
>> +EOF
>
> That's a lot of tags, and I'd worry it makes the test a little brittle.
> Can we limit the set of tags with a name-match? It shouldn't affect the
> purpose of the test (the deep stack comes from traversing the commits,
> not the number of tags).

I'll make this less sucky in v2 somehow. I did it this way because no existing test was checking all the tags we'd created at the end, so this does that by proxy now, but I agree it's too verbose. Will fix it.

Jeff King· Mar 9, 2017, 10:46 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] branch & tag: Add a --no-contains option

On Thu, Mar 09, 2017 at 11:41:59AM +0100, Ævar Arnfjörð Bjarmason wrote:
Show 13 quoted lines
> > I almost suggested that there simply be an option to invert the match
> > (like --invert-contains or something).  But what you have here is more
> > flexible, if somebody ever wanted to do:
> >
> >   git tag --contains X --no-contains Y
> 
> Yeah that's really useful. E.g. this shows the branches I branched off
> (or have locally) from 2.6..2.8:
> 
>     $ ./git branch --contains v2.6.0 --no-contains v2.8.0
>       avar/monkeypatch-untracked-cache-disabled
>       avar/uc-notifs21
>       dturner/pclouds-watchman-noshm
Oh, that's a clever application.
Show 9 quoted lines
> But I'd expect this to show all the tags between the two:
> 
>     $ ./git tag --contains v2.6.0 --no-contains v2.8.0
>     $
> 
> But it just returns an empty list. Manually disabling the
> contains_tag_algo() path (i.e. effectively locally reverting your
> ffc4b8012d) makes it "work", but of course it's much slower now. I
> haven't dug into why it's not working yet.

I'm almost certain this is because the contains_tag_algo one doesn't clean up the flag bits it sets on the commit objects. So running it twice in the same process is going to give you nonsense results.

Coincidentally, I've been looking into resurrecting the cleaner approach that I sent long ago:

  http://public-inbox.org/git/20140625233429.GA20457@sigill.intra.peff.net/

But it's sufficiently complex that it's probably worth fixing the existing algorithm to clean up its bits in the meantime.

Show 6 quoted lines
> Also I wonder if this should be an error:
> 
>     $ ./git [tag|branch|for-each-ref] --contains A --no-contains A
> 
> I.e. when you give the same argument to both, this can never return
> anything for obvious reasons.

It's clearly nonsense, but I don't think there's any need for it to be an error. GIGO.

-Peff
Ævar Arnfjörð Bjarmason· Mar 9, 2017, 12:12 UTC · re: Jeff King · lore

Re: [PATCH] branch & tag: Add a --no-contains option

On Thu, Mar 9, 2017 at 11:46 AM, Jeff King <peff@peff.net> wrote:
Show 31 quoted lines
> On Thu, Mar 09, 2017 at 11:41:59AM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> > I almost suggested that there simply be an option to invert the match
>> > (like --invert-contains or something).  But what you have here is more
>> > flexible, if somebody ever wanted to do:
>> >
>> >   git tag --contains X --no-contains Y
>>
>> Yeah that's really useful. E.g. this shows the branches I branched off
>> (or have locally) from 2.6..2.8:
>>
>>     $ ./git branch --contains v2.6.0 --no-contains v2.8.0
>>       avar/monkeypatch-untracked-cache-disabled
>>       avar/uc-notifs21
>>       dturner/pclouds-watchman-noshm
>
> Oh, that's a clever application.
>
>> But I'd expect this to show all the tags between the two:
>>
>>     $ ./git tag --contains v2.6.0 --no-contains v2.8.0
>>     $
>>
>> But it just returns an empty list. Manually disabling the
>> contains_tag_algo() path (i.e. effectively locally reverting your
>> ffc4b8012d) makes it "work", but of course it's much slower now. I
>> haven't dug into why it's not working yet.
>
> I'm almost certain this is because the contains_tag_algo one doesn't
> clean up the flag bits it sets on the commit objects. So running it
> twice in the same process is going to give you nonsense results.
Yeah indeed.

I tried to hack something up to avoid this, but the lookup_commit_reference_gently() we call will return the same object.parent pointer for two invocations, i.e. the underlying {commit,object}.c API has a cache of objects it returns, couldn't find some way to quickly make it burst that cache.

The other approach of making contains_tag_algo() itself detect that it's been called before (or us passing a flag) and going around setting commit.object.flags on everything to 0 seemed even more brittle, particularly since I think between filter->with_commit & filter->no_commit we might end up visiting different commits, so it's not easy to just clear it.

I'm happy to hack on it given some pointers, will visit it again, but for now unless I'm missing something obvious / you can point out some way to hack this up I'll just submit v2 with the combination of --contains & --no-contains dying with a TODO message.

The patch without that functionality is still really useful, and we can implement that later.

Show 17 quoted lines
> Coincidentally, I've been looking into resurrecting the cleaner approach
> that I sent long ago:
>
>   http://public-inbox.org/git/20140625233429.GA20457@sigill.intra.peff.net/
>
> But it's sufficiently complex that it's probably worth fixing the
> existing algorithm to clean up its bits in the meantime.
>
>> Also I wonder if this should be an error:
>>
>>     $ ./git [tag|branch|for-each-ref] --contains A --no-contains A
>>
>> I.e. when you give the same argument to both, this can never return
>> anything for obvious reasons.
>
> It's clearly nonsense, but I don't think there's any need for it to be
> an error. GIGO.
Yeah, make sense.
Jeff King· Mar 9, 2017, 12:51 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] branch & tag: Add a --no-contains option

On Thu, Mar 09, 2017 at 01:12:08PM +0100, Ævar Arnfjörð Bjarmason wrote:
Show 11 quoted lines
> > I'm almost certain this is because the contains_tag_algo one doesn't
> > clean up the flag bits it sets on the commit objects. So running it
> > twice in the same process is going to give you nonsense results.
> 
> Yeah indeed.
> 
> I tried to hack something up to avoid this, but the
> lookup_commit_reference_gently() we call will return the same
> object.parent pointer for two invocations, i.e. the underlying
> {commit,object}.c API has a cache of objects it returns, couldn't find
> some way to quickly make it burst that cache.
Yeah, you'll always get the same struct for a given sha1.
Show 6 quoted lines
> The other approach of making contains_tag_algo() itself detect that
> it's been called before (or us passing a flag) and going around
> setting commit.object.flags on everything to 0 seemed even more
> brittle, particularly since I think between filter->with_commit &
> filter->no_commit we might end up visiting different commits, so it's
> not easy to just clear it.

You can clear the marks with clear_object_flags(). But I don't think that type of solution will quite help us here. We consider each ref independently and ask "does it match --contains" and "does it match --no-contains?". So there is no moment where we are done with all of the --contains marks, and can move on to the --no-contains ones. The lookups are interleaved.

We could move to doing them in chunks (the way filter->merge_commit works), and then clearing in between. Or we could use a separate bitset. The patch below does that.

Show 7 quoted lines
> I'm happy to hack on it given some pointers, will visit it again, but
> for now unless I'm missing something obvious / you can point out some
> way to hack this up I'll just submit v2 with the combination of
> --contains & --no-contains dying with a TODO message.
> 
> The patch without that functionality is still really useful, and we
> can implement that later.

I'm not opposed to that, though see what you think of the patch below. It's a bit noisy but it's conceptually pretty straightforward. It should hopefully be obvious how you'd add in a separate contains_cache for the "without" case.

Looking at this, I'm pretty sure that using "--contains" with "--merged" has similar problems, as they both use the UNINTERESTING bit. So even without your patch, there is a lurking bug.

Show changes to 2 files +43 −29

ref-filter.c, ref-filter.h

diff --git a/ref-filter.c b/ref-filter.c
index 3820b21cc..42b1bc463 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -14,6 +14,7 @@
 #include "git-compat-util.h"
 #include "version.h"
 #include "trailer.h"
+#include "commit-slab.h"
 
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 
@@ -1137,10 +1138,22 @@ static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom
 	*v = &ref->value[atom];
 }
 
+/*
+ * Unknown has to be "0" here, because that's what unfilled entries in our slab
+ * will return.
+ */
 enum contains_result {
-	CONTAINS_UNKNOWN = -1,
-	CONTAINS_NO = 0,
-	CONTAINS_YES = 1
+	CONTAINS_UNKNOWN = 0,
+	CONTAINS_NO = 1,
+	CONTAINS_YES = 2,
+};
+
+define_commit_slab(contains_cache, enum contains_result);
+
+struct ref_filter_cbdata {
+	struct ref_array *array;
+	struct ref_filter *filter;
+	struct contains_cache contains_cache;
 };
 
 /*
@@ -1171,24 +1184,25 @@ static int in_commit_list(const struct commit_list *want, struct commit *c)
  * Do not recurse to find out, though, but return -1 if inconclusive.
  */
 static enum contains_result contains_test(struct commit *candidate,
-			    const struct commit_list *want)
+			    const struct commit_list *want,
+			    struct contains_cache *cache)
 {
-	/* was it previously marked as containing a want commit? */
-	if (candidate->object.flags & TMP_MARK)
-		return 1;
-	/* or marked as not possibly containing a want commit? */
-	if (candidate->object.flags & UNINTERESTING)
-		return 0;
+	enum contains_result *cached = contains_cache_at(cache, candidate);
+
+	/* if we already found the answer, return it without traversing */
+	if (*cached)
+		return *cached;
+
 	/* or are we it? */
 	if (in_commit_list(want, candidate)) {
-		candidate->object.flags |= TMP_MARK;
-		return 1;
+		*cached = CONTAINS_YES;
+		return *cached;
 	}
 
 	if (parse_commit(candidate) < 0)
-		return 0;
+		return CONTAINS_NO;
 
-	return -1;
+	return CONTAINS_UNKNOWN;
 }
 
 static void push_to_contains_stack(struct commit *candidate, struct contains_stack *contains_stack)
@@ -1199,10 +1213,11 @@ static void push_to_contains_stack(struct commit *candidate, struct contains_sta
 }
 
 static enum contains_result contains_tag_algo(struct commit *candidate,
-		const struct commit_list *want)
+					      const struct commit_list *want,
+					      struct contains_cache *cache)
 {
 	struct contains_stack contains_stack = { 0, 0, NULL };
-	int result = contains_test(candidate, want);
+	enum contains_result result = contains_test(candidate, want, cache);
 
 	if (result != CONTAINS_UNKNOWN)
 		return result;
@@ -1214,16 +1229,16 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
 		struct commit_list *parents = entry->parents;
 
 		if (!parents) {
-			commit->object.flags |= UNINTERESTING;
+			*contains_cache_at(cache, commit) = CONTAINS_NO;
 			contains_stack.nr--;
 		}
 		/*
 		 * If we just popped the stack, parents->item has been marked,
-		 * therefore contains_test will return a meaningful 0 or 1.
+		 * therefore contains_test will return a meaningful yes/no.
 		 */
-		else switch (contains_test(parents->item, want)) {
+		else switch (contains_test(parents->item, want, cache)) {
 		case CONTAINS_YES:
-			commit->object.flags |= TMP_MARK;
+			*contains_cache_at(cache, commit) = CONTAINS_YES;
 			contains_stack.nr--;
 			break;
 		case CONTAINS_NO:
@@ -1235,13 +1250,14 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
 		}
 	}
 	free(contains_stack.contains_stack);
-	return contains_test(candidate, want);
+	return contains_test(candidate, want, cache);
 }
 
-static int commit_contains(struct ref_filter *filter, struct commit *commit)
+static int commit_contains(struct ref_filter *filter, struct commit *commit,
+			   struct contains_cache *cache)
 {
 	if (filter->with_commit_tag_algo)
-		return contains_tag_algo(commit, filter->with_commit);
+		return contains_tag_algo(commit, filter->with_commit, cache) == CONTAINS_YES;
 	return is_descendant_of(commit, filter->with_commit);
 }
 
@@ -1438,7 +1454,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
 			return 0;
 		/* We perform the filtering for the '--contains' option */
 		if (filter->with_commit &&
-		    !commit_contains(filter, commit))
+		    !commit_contains(filter, commit, &ref_cbdata->contains_cache))
 			return 0;
 	}
 
@@ -1538,6 +1554,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
 		broken = 1;
 	filter->kind = type & FILTER_REFS_KIND_MASK;
 
+	init_contains_cache(&ref_cbdata.contains_cache);
+
 	/*  Simple per-ref filtering */
 	if (!filter->kind)
 		die("filter_refs: invalid type");
@@ -1560,6 +1578,7 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
 			head_ref(ref_filter_handler, &ref_cbdata);
 	}
 
+	clear_contains_cache(&ref_cbdata.contains_cache);
 
 	/*  Filters that need revision walking */
 	if (filter->merge_commit)
diff --git a/ref-filter.h b/ref-filter.h
index 7b05592ba..89af9f451 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -71,11 +71,6 @@ struct ref_filter {
 		verbose;
 };
 
-struct ref_filter_cbdata {
-	struct ref_array *array;
-	struct ref_filter *filter;
-};
-
 /*  Macros for checking --merged and --no-merged options */
 #define _OPT_MERGED_NO_MERGED(option, filter, h) \
 	{ OPTION_CALLBACK, 0, option, (filter), N_("commit"), (h), \
Jeff King· Mar 9, 2017, 13:27 UTC · re: Jeff King · lore

[PATCH 0/4] fix object flag pollution in "tag --contains"

On Thu, Mar 09, 2017 at 07:51:32AM -0500, Jeff King wrote:
> Looking at this, I'm pretty sure that using "--contains" with "--merged"
> has similar problems, as they both use the UNINTERESTING bit. So even
> without your patch, there is a lurking bug.

I wasn't able to come up with a simple case that actually demonstrates the bug. But I feel like it has to be triggerable with the right sequence of history.

Even without that, though, I feel like moving away from this flag usage is a good cleanup. Here's a cleaned-up series. What do you think of building your patch on top?

We can do it the other way around if you prefer.
  [1/4]: ref-filter: move ref_cbdata definition into ref-filter.c
  [2/4]: ref-filter: use contains_result enum consistently
  [3/4]: ref-filter: die on parse_commit errors
  [4/4]: ref-filter: use separate cache for contains_tag_algo
 ref-filter.c | 70 ++++++++++++++++++++++++++++++++++++++----------------------
 ref-filter.h |  5 -----
 2 files changed, 44 insertions(+), 31 deletions(-)
-Peff
Jeff King· Mar 9, 2017, 13:27 UTC · re: Jeff King · lore

[PATCH 1/4] ref-filter: move ref_cbdata definition into ref-filter.c

This is an implementation detail of how filter_refs() works, and does not need to be exposed to the outside world. This will become more important in future patches as we add new private data types to it.

Signed-off-by: Jeff King <peff@peff.net>
---
 ref-filter.c | 5 +++++
 ref-filter.h | 5 -----
 2 files changed, 5 insertions(+), 5 deletions(-)
Show changes to 2 files +5 −5

ref-filter.c, ref-filter.h

diff --git a/ref-filter.c b/ref-filter.c
index 1ec0fb839..6546dba73 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1476,6 +1476,11 @@ enum contains_result {
 	CONTAINS_YES = 1
 };
 
+struct ref_filter_cbdata {
+	struct ref_array *array;
+	struct ref_filter *filter;
+};
+
 /*
  * Mimicking the real stack, this stack lives on the heap, avoiding stack
  * overflows.
diff --git a/ref-filter.h b/ref-filter.h
index 154e24c40..e738c5dfd 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -71,11 +71,6 @@ struct ref_filter {
 		verbose;
 };
 
-struct ref_filter_cbdata {
-	struct ref_array *array;
-	struct ref_filter *filter;
-};
-
 /*  Macros for checking --merged and --no-merged options */
 #define _OPT_MERGED_NO_MERGED(option, filter, h) \
 	{ OPTION_CALLBACK, 0, option, (filter), N_("commit"), (h), \
-- 
2.12.0.445.g818af77e0
Jeff King· Mar 9, 2017, 13:28 UTC · re: Jeff King · lore

[PATCH 2/4] ref-filter: use contains_result enum consistently

Commit cbc60b672 (git tag --contains: avoid stack overflow, 2014-04-24) adapted the -1/0/1 contains status into a tri-state enum. However, some of the code still used the numeric values, or assumed that no/yes correspond to C's boolean true/false.

Let's switch to using the symbolic values everywhere, which will make it easier to change them.

Signed-off-by: Jeff King <peff@peff.net>
---
 ref-filter.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
Show changes to ref-filter.c +8 −8
diff --git a/ref-filter.c b/ref-filter.c
index 6546dba73..631978a4f 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1513,20 +1513,20 @@ static enum contains_result contains_test(struct commit *candidate,
 {
 	/* was it previously marked as containing a want commit? */
 	if (candidate->object.flags & TMP_MARK)
-		return 1;
+		return CONTAINS_YES;
 	/* or marked as not possibly containing a want commit? */
 	if (candidate->object.flags & UNINTERESTING)
-		return 0;
+		return CONTAINS_NO;
 	/* or are we it? */
 	if (in_commit_list(want, candidate)) {
 		candidate->object.flags |= TMP_MARK;
-		return 1;
+		return CONTAINS_YES;
 	}
 
 	if (parse_commit(candidate) < 0)
-		return 0;
+		return CONTAINS_NO;
 
-	return -1;
+	return CONTAINS_UNKNOWN;
 }
 
 static void push_to_contains_stack(struct commit *candidate, struct contains_stack *contains_stack)
@@ -1540,7 +1540,7 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
 		const struct commit_list *want)
 {
 	struct contains_stack contains_stack = { 0, 0, NULL };
-	int result = contains_test(candidate, want);
+	enum contains_result result = contains_test(candidate, want);
 
 	if (result != CONTAINS_UNKNOWN)
 		return result;
@@ -1557,7 +1557,7 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
 		}
 		/*
 		 * If we just popped the stack, parents->item has been marked,
-		 * therefore contains_test will return a meaningful 0 or 1.
+		 * therefore contains_test will return a meaningful yes/no.
 		 */
 		else switch (contains_test(parents->item, want)) {
 		case CONTAINS_YES:
@@ -1579,7 +1579,7 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
 static int commit_contains(struct ref_filter *filter, struct commit *commit)
 {
 	if (filter->with_commit_tag_algo)
-		return contains_tag_algo(commit, filter->with_commit);
+		return contains_tag_algo(commit, filter->with_commit) == CONTAINS_YES;
 	return is_descendant_of(commit, filter->with_commit);
 }
 
-- 
2.12.0.445.g818af77e0
Jeff King· Mar 9, 2017, 13:29 UTC · re: Jeff King · lore

[PATCH 3/4] ref-filter: die on parse_commit errors

The tag-contains algorithm quietly returns "does not contain" when parse_commit() fails. But a parse failure is an indication that the repository is corrupt. We should die loudly rather than producing a bogus result.

Signed-off-by: Jeff King <peff@peff.net>
---
 ref-filter.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
Show changes to ref-filter.c +1 −3
diff --git a/ref-filter.c b/ref-filter.c
index 631978a4f..5cb49b7c2 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1523,9 +1523,7 @@ static enum contains_result contains_test(struct commit *candidate,
 		return CONTAINS_YES;
 	}
 
-	if (parse_commit(candidate) < 0)
-		return CONTAINS_NO;
-
+	parse_commit_or_die(candidate);
 	return CONTAINS_UNKNOWN;
 }
 
-- 
2.12.0.445.g818af77e0
Jeff King· Mar 9, 2017, 13:29 UTC · re: Jeff King · lore

[PATCH 4/4] ref-filter: use separate cache for contains_tag_algo

The algorithm which powers "tag --contains" uses the TMP_MARK and UNINTERESTING bits, but never cleans up after itself. As a result, stale UNINTERESTING bits may impact later traversals (like "--merged").

We could fix this by clearing the bits after we're done with the --contains traversal. That would be enough to fix the existing problem, but it leaves future developers in a bad spot: they cannot add other traversals that operate simultaneously with --contains (e.g., if you wanted to add "--no-contains" and use both filters at the same time).

Instead, we can use a commit slab to store our cached results, which will store the bits outside of the commit structs entirely. This adds an extra level of indirection, but in my tests (running "git tag --contains HEAD" on linux.git), there was no measurable slowdown.

Signed-off-by: Jeff King <peff@peff.net>
---
 ref-filter.c | 55 +++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 20 deletions(-)
Show changes to ref-filter.c +35 −20
diff --git a/ref-filter.c b/ref-filter.c
index 5cb49b7c2..7eeecc608 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -15,6 +15,7 @@
 #include "version.h"
 #include "trailer.h"
 #include "wt-status.h"
+#include "commit-slab.h"
 
 static struct ref_msg {
 	const char *gone;
@@ -1470,15 +1471,22 @@ static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom
 	*v = &ref->value[atom];
 }
 
+/*
+ * Unknown has to be "0" here, because that's the default value for
+ * contains_cache slab entries that have not yet been assigned.
+ */
 enum contains_result {
-	CONTAINS_UNKNOWN = -1,
-	CONTAINS_NO = 0,
-	CONTAINS_YES = 1
+	CONTAINS_UNKNOWN = 0,
+	CONTAINS_NO,
+	CONTAINS_YES
 };
 
+define_commit_slab(contains_cache, enum contains_result);
+
 struct ref_filter_cbdata {
 	struct ref_array *array;
 	struct ref_filter *filter;
+	struct contains_cache contains_cache;
 };
 
 /*
@@ -1509,20 +1517,22 @@ static int in_commit_list(const struct commit_list *want, struct commit *c)
  * Do not recurse to find out, though, but return -1 if inconclusive.
  */
 static enum contains_result contains_test(struct commit *candidate,
-			    const struct commit_list *want)
+					  const struct commit_list *want,
+					  struct contains_cache *cache)
 {
-	/* was it previously marked as containing a want commit? */
-	if (candidate->object.flags & TMP_MARK)
-		return CONTAINS_YES;
-	/* or marked as not possibly containing a want commit? */
-	if (candidate->object.flags & UNINTERESTING)
-		return CONTAINS_NO;
+	enum contains_result *cached = contains_cache_at(cache, candidate);
+
+	/* If we already have the answer cached, return that. */
+	if (*cached)
+		return *cached;
+
 	/* or are we it? */
 	if (in_commit_list(want, candidate)) {
-		candidate->object.flags |= TMP_MARK;
+		*cached = CONTAINS_YES;
 		return CONTAINS_YES;
 	}
 
+	/* Otherwise, we don't know; prepare to recurse */
 	parse_commit_or_die(candidate);
 	return CONTAINS_UNKNOWN;
 }
@@ -1535,10 +1545,11 @@ static void push_to_contains_stack(struct commit *candidate, struct contains_sta
 }
 
 static enum contains_result contains_tag_algo(struct commit *candidate,
-		const struct commit_list *want)
+					      const struct commit_list *want,
+					      struct contains_cache *cache)
 {
 	struct contains_stack contains_stack = { 0, 0, NULL };
-	enum contains_result result = contains_test(candidate, want);
+	enum contains_result result = contains_test(candidate, want, cache);
 
 	if (result != CONTAINS_UNKNOWN)
 		return result;
@@ -1550,16 +1561,16 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
 		struct commit_list *parents = entry->parents;
 
 		if (!parents) {
-			commit->object.flags |= UNINTERESTING;
+			*contains_cache_at(cache, commit) = CONTAINS_NO;
 			contains_stack.nr--;
 		}
 		/*
 		 * If we just popped the stack, parents->item has been marked,
 		 * therefore contains_test will return a meaningful yes/no.
 		 */
-		else switch (contains_test(parents->item, want)) {
+		else switch (contains_test(parents->item, want, cache)) {
 		case CONTAINS_YES:
-			commit->object.flags |= TMP_MARK;
+			*contains_cache_at(cache, commit) = CONTAINS_YES;
 			contains_stack.nr--;
 			break;
 		case CONTAINS_NO:
@@ -1571,13 +1582,14 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
 		}
 	}
 	free(contains_stack.contains_stack);
-	return contains_test(candidate, want);
+	return contains_test(candidate, want, cache);
 }
 
-static int commit_contains(struct ref_filter *filter, struct commit *commit)
+static int commit_contains(struct ref_filter *filter, struct commit *commit,
+			   struct contains_cache *cache)
 {
 	if (filter->with_commit_tag_algo)
-		return contains_tag_algo(commit, filter->with_commit) == CONTAINS_YES;
+		return contains_tag_algo(commit, filter->with_commit, cache) == CONTAINS_YES;
 	return is_descendant_of(commit, filter->with_commit);
 }
 
@@ -1774,7 +1786,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
 			return 0;
 		/* We perform the filtering for the '--contains' option */
 		if (filter->with_commit &&
-		    !commit_contains(filter, commit))
+		    !commit_contains(filter, commit, &ref_cbdata->contains_cache))
 			return 0;
 	}
 
@@ -1874,6 +1886,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
 		broken = 1;
 	filter->kind = type & FILTER_REFS_KIND_MASK;
 
+	init_contains_cache(&ref_cbdata.contains_cache);
+
 	/*  Simple per-ref filtering */
 	if (!filter->kind)
 		die("filter_refs: invalid type");
@@ -1896,6 +1910,7 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
 			head_ref(ref_filter_handler, &ref_cbdata);
 	}
 
+	clear_contains_cache(&ref_cbdata.contains_cache);
 
 	/*  Filters that need revision walking */
 	if (filter->merge_commit)
-- 
2.12.0.445.g818af77e0
Ævar Arnfjörð Bjarmason· Mar 11, 2017, 20:01 UTC · re: Jeff King · lore

Re: [PATCH 4/4] ref-filter: use separate cache for contains_tag_algo

On Thu, Mar 9, 2017 at 2:29 PM, Jeff King <peff@peff.net> wrote:
Show 14 quoted lines
> [...]
> @@ -1874,6 +1886,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
>                 broken = 1;
>         filter->kind = type & FILTER_REFS_KIND_MASK;
>
> +       init_contains_cache(&ref_cbdata.contains_cache);
> +
>         /*  Simple per-ref filtering */
> [...]
>
> +       clear_contains_cache(&ref_cbdata.contains_cache);
>
>         /*  Filters that need revision walking */
>         if (filter->merge_commit)
Shouldn't both of those be guarded by a "if (filter->with_commit)" test?

That init/clear codepath is rather light, but it seems to me that we can avoid it entirely if filter->with_commit isn't defined. I've tested this locally and it still passes all tests.

Ævar Arnfjörð Bjarmason· Mar 11, 2017, 20:21 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 4/4] ref-filter: use separate cache for contains_tag_algo

On Sat, Mar 11, 2017 at 9:01 PM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 21 quoted lines
> On Thu, Mar 9, 2017 at 2:29 PM, Jeff King <peff@peff.net> wrote:
>> [...]
>> @@ -1874,6 +1886,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
>>                 broken = 1;
>>         filter->kind = type & FILTER_REFS_KIND_MASK;
>>
>> +       init_contains_cache(&ref_cbdata.contains_cache);
>> +
>>         /*  Simple per-ref filtering */
>> [...]
>>
>> +       clear_contains_cache(&ref_cbdata.contains_cache);
>>
>>         /*  Filters that need revision walking */
>>         if (filter->merge_commit)
>
> Shouldn't both of those be guarded by a "if (filter->with_commit)" test?
>
> That init/clear codepath is rather light, but it seems to me that we
> can avoid it entirely if filter->with_commit isn't defined. I've
> tested this locally and it still passes all tests.

Since that seems to work perfectly and we're doing less work I've incorporated that into the v4 of my series at <20170311201858.27555-1-avarab@gmail.com>.

Jeff King· Mar 12, 2017, 11:12 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 4/4] ref-filter: use separate cache for contains_tag_algo

On Sat, Mar 11, 2017 at 09:01:25PM +0100, Ævar Arnfjörð Bjarmason wrote:
Show 17 quoted lines
> On Thu, Mar 9, 2017 at 2:29 PM, Jeff King <peff@peff.net> wrote:
> > [...]
> > @@ -1874,6 +1886,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
> >                 broken = 1;
> >         filter->kind = type & FILTER_REFS_KIND_MASK;
> >
> > +       init_contains_cache(&ref_cbdata.contains_cache);
> > +
> >         /*  Simple per-ref filtering */
> > [...]
> >
> > +       clear_contains_cache(&ref_cbdata.contains_cache);
> >
> >         /*  Filters that need revision walking */
> >         if (filter->merge_commit)
> 
> Shouldn't both of those be guarded by a "if (filter->with_commit)" test?

I thought about that while writing it, but decided that it was not worth complicating the initialization and cleanup with conditionals. The initialization is on par with what we would normally do with a static struct initializer, and the cleanup is a noop if the cache wasn't touched.

So my thinking was that it didn't matter either way, and dropping the conditional was one less thing for readers of the code to have to worry about.

That said, I'm not opposed to doing it the other way.
> That init/clear codepath is rather light, but it seems to me that we
> can avoid it entirely if filter->with_commit isn't defined. I've
> tested this locally and it still passes all tests.
Yes, it should be correct either way.
-Peff
Ævar Arnfjörð Bjarmason· Mar 11, 2017, 13:06 UTC · re: Jeff King · lore

Re: [PATCH 0/4] fix object flag pollution in "tag --contains"

On Thu, Mar 9, 2017 at 2:27 PM, Jeff King <peff@peff.net> wrote:
Show 5 quoted lines
> On Thu, Mar 09, 2017 at 07:51:32AM -0500, Jeff King wrote:
>
>> Looking at this, I'm pretty sure that using "--contains" with "--merged"
>> has similar problems, as they both use the UNINTERESTING bit. So even
>> without your patch, there is a lurking bug.

Sorry about the late reply. Been a while since I was active on the git ML, and my broken list search was searching for cc:me, not to:me. So I sent my v3 in <20170310203348.675-1-avarab@gmail.com> without reading this.

> I wasn't able to come up with a simple case that actually demonstrates
> the bug. But I feel like it has to be triggerable with the right
> sequence of history.

The tag brute force script I hacked up (https://gist.github.com/avar/45cf288ce7cdc43e7395c6cbf9a98d68) is now at >1k iterations without finding anything. But of course it may be broken / this may not be producible on git.git

Show 5 quoted lines
> Even without that, though, I feel like moving away from this flag usage
> is a good cleanup. Here's a cleaned-up series. What do you think of
> building your patch on top?
>
> We can do it the other way around if you prefer.

Getting this in master first sounds good. I already have a working v4 on top of this, which is of course much faster for the --contains combined with --no-contains case. Gotta run now, but will clean up that patch & submit it to the list soon.

Show 10 quoted lines
>   [1/4]: ref-filter: move ref_cbdata definition into ref-filter.c
>   [2/4]: ref-filter: use contains_result enum consistently
>   [3/4]: ref-filter: die on parse_commit errors
>   [4/4]: ref-filter: use separate cache for contains_tag_algo
>
>  ref-filter.c | 70 ++++++++++++++++++++++++++++++++++++++----------------------
>  ref-filter.h |  5 -----
>  2 files changed, 44 insertions(+), 31 deletions(-)
>
> -Peff
Ævar Arnfjörð Bjarmason· Mar 11, 2017, 20:18 UTC · re: Jeff King · lore

[PATCH v4] ref-filter: Add --no-contains option to tag/branch/for-each-ref

Change the tag, branch & for-each-ref commands to have a --no-contains option in addition to their longstanding --contains options.

The use-case I have for this is to find the last-good rollout tag given a known-bad <commit>. Right now, given a hypothetically bad commit v2.10.1-3-gcf5c7253e0, you can find which git version to revert to with this hacky two-liner:

    (./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}' | tail -n 10

But with the --no-contains option you can now get the exact same output with:

    ./git tag -l 'v[0-9]*' --no-contains v2.10.1-3-gcf5c7253e0|sort|tail -n 10

The filtering machinery is generic between the tag, branch & for-each-ref commands, so once I'd implemented it for tag it was trivial to add support for this to the other two.

A practical use for this with "branch" is e.g. finding branches which diverged between 2.8 & 2.10:

    ./git branch --contains v2.8.0 --no-contains v2.10.0

The "describe" command also has a --contains option, but its semantics are unrelated to what tag/branch/for-each-ref use --contains for. I don't see how a --no-contains option for "describe" would make any sense, other than being exactly equivalent to not supplying --contains at all, which would be confusing at best.

I'm adding a --without option to "tag" as an alias for --no-contains for consistency with --with and --contains. Since we don't even document --with anymore (or test it). The --with option is undocumented, and possibly the only user of it is Junio[1]. But it's trivial to support, so let's do that.

Where I'm changing existing documentation lines I'm mainly word wrapping at 75 columns to be consistent with the existing style. The changes to Documentation/ are much smaller with: git show --word-diff, same for the minor change to git-completion.bash.

Most of the test changes I've made are just doing the inverse of the existing --contains tests, with this change --no-contains for tag, branch & for-each-ref is just as well tested as the existing --contains option.

In addition to those tests I've added tests for --contains in combination with --no-contains for all three commands, and a test for "tag" which asserts that --no-contains won't find tree/blob tags, which is slightly unintuitive, but consistent with how --contains works.

Since we don't need to init/clear the contains_cache unless we're actually using this filter, I skip that unless filter->no_commit is true, and while I'm at it amend the code added in Jeff King's "ref-filter: use separate cache for contains_tag_algo" patch to do that for filter->with_commit as well.

1. <xmqqefy71iej.fsf@gitster.mtv.corp.google.com>
2. <20170309125132.tubwxtneffok4nrc@sigill.intra.peff.net>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

This is now based on top of pu, which has Jeff King's "fix object flag pollution in "tag --contains" series. Thus the whole hack to disable the optimized codepath when both --contains and --no-contains are provided is gone, since we can use the optimized codepath now.

 Documentation/git-branch.txt           |  24 ++++---
 Documentation/git-for-each-ref.txt     |   6 +-
 Documentation/git-tag.txt              |   6 +-
 builtin/branch.c                       |   4 +-
 builtin/for-each-ref.c                 |   3 +-
 builtin/tag.c                          |   6 +-
 contrib/completion/git-completion.bash |   9 +--
 parse-options.h                        |   4 +-
 ref-filter.c                           |  27 +++++---
 ref-filter.h                           |   1 +
 t/t3201-branch-contains.sh             |  51 +++++++++++++-
 t/t6302-for-each-ref-filter.sh         |  16 +++++
 t/t7004-tag.sh                         | 123 ++++++++++++++++++++++++++++++++-
 13 files changed, 250 insertions(+), 30 deletions(-)
Show changes to 13 files +250 −30

Documentation/git-branch.txt, Documentation/git-for-each-ref.txt, Documentation/git-tag.txt, builtin/branch.c, builtin/for-each-ref.c, builtin/tag.c, contrib/completion/git-completion.bash, parse-options.h, ref-filter.c, ref-filter.h, t/t3201-branch-contains.sh, t/t6302-for-each-ref-filter.sh, t/t7004-tag.sh

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 092f1bcf9f..28a36a8a0a 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git branch' [--color[=<when>] | --no-color] [-r | -a]
 	[--list] [-v [--abbrev=<length> | --no-abbrev]]
 	[--column[=<options>] | --no-column]
-	[(--merged | --no-merged | --contains) [<commit>]] [--sort=<key>]
+	[(--merged | --no-merged | --contains | --no-contains) [<commit>]] [--sort=<key>]
 	[--points-at <object>] [--format=<format>] [<pattern>...]
 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
 'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
@@ -35,11 +35,12 @@ as branch creation.
 
 With `--contains`, shows only the branches that contain the named commit
 (in other words, the branches whose tip commits are descendants of the
-named commit).  With `--merged`, only branches merged into the named
-commit (i.e. the branches whose tip commits are reachable from the named
-commit) will be listed.  With `--no-merged` only branches not merged into
-the named commit will be listed.  If the <commit> argument is missing it
-defaults to `HEAD` (i.e. the tip of the current branch).
+named commit), `--no-contains` inverts it. With `--merged`, only branches
+merged into the named commit (i.e. the branches whose tip commits are
+reachable from the named commit) will be listed.  With `--no-merged` only
+branches not merged into the named commit will be listed.  If the <commit>
+argument is missing it defaults to `HEAD` (i.e. the tip of the current
+branch).
 
 The command's second form creates a new branch head named <branchname>
 which points to the current `HEAD`, or <start-point> if given.
@@ -213,6 +214,10 @@ start-point is either a local or remote-tracking branch.
 	Only list branches which contain the specified commit (HEAD
 	if not specified). Implies `--list`.
 
+--no-contains [<commit>]::
+	Only list branches which don't contain the specified commit
+	(HEAD if not specified). Implies `--list`.
+
 --merged [<commit>]::
 	Only list branches whose tips are reachable from the
 	specified commit (HEAD if not specified). Implies `--list`.
@@ -296,13 +301,16 @@ If you are creating a branch that you want to checkout immediately, it is
 easier to use the git checkout command with its `-b` option to create
 a branch and check it out with a single command.
 
-The options `--contains`, `--merged` and `--no-merged` serve three related
-but different purposes:
+The options `--contains`, `--no-contains`, `--merged` and `--no-merged`
+serve four related but different purposes:
 
 - `--contains <commit>` is used to find all branches which will need
   special attention if <commit> were to be rebased or amended, since those
   branches contain the specified <commit>.
 
+- `--no-contains <commit>` is the inverse of that, i.e. branches that don't
+  contain the specified <commit>.
+
 - `--merged` is used to find all branches which can be safely deleted,
   since those branches are fully contained by HEAD.
 
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 111e1be6f5..83b93c75a8 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
 		   [(--sort=<key>)...] [--format=<format>] [<pattern>...]
 		   [--points-at <object>] [(--merged | --no-merged) [<object>]]
-		   [--contains [<object>]]
+		   [(--contains | --no-contains) [<object>]]
 
 DESCRIPTION
 -----------
@@ -79,6 +79,10 @@ OPTIONS
 	Only list refs which contain the specified commit (HEAD if not
 	specified).
 
+--no-contains [<object>]::
+	Only list refs which don't contain the specified commit (HEAD
+	if not specified).
+
 --ignore-case::
 	Sorting and filtering refs are case insensitive.
 
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 525737a5d8..d9243bf5e4 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git tag' [-a | -s | -u <keyid>] [-f] [-m <msg> | -F <file>]
 	<tagname> [<commit> | <object>]
 'git tag' -d <tagname>...
-'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
+'git tag' [-n[<num>]] -l [--[no-]contains <commit>] [--points-at <object>]
 	[--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]
 	[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]
 'git tag' -v [--format=<format>] <tagname>...
@@ -124,6 +124,10 @@ This option is only applicable when listing tags without annotation lines.
 	Only list tags which contain the specified commit (HEAD if not
 	specified).
 
+--no-contains [<commit>]::
+	Only list tags which don't contain the specified commit (HEAD if
+	not specified).
+
 --points-at <object>::
 	Only list tags of the given object.
 
diff --git a/builtin/branch.c b/builtin/branch.c
index 52688f2e1b..b4871db9cb 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -562,7 +562,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT('r', "remotes",     &filter.kind, N_("act on remote-tracking branches"),
 			FILTER_REFS_REMOTES),
 		OPT_CONTAINS(&filter.with_commit, N_("print only branches that contain the commit")),
+		OPT_NO_CONTAINS(&filter.no_commit, N_("print only branches that don't contain the commit")),
 		OPT_WITH(&filter.with_commit, N_("print only branches that contain the commit")),
+		OPT_WITHOUT(&filter.no_commit, N_("print only branches that don't contain the commit")),
 		OPT__ABBREV(&filter.abbrev),
 
 		OPT_GROUP(N_("Specific git-branch actions:")),
@@ -618,7 +620,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)
 		list = 1;
 
-	if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)
+	if (filter.with_commit || filter.no_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)
 		list = 1;
 
 	if (!!delete + !!rename + !!new_upstream +
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index df41fa0350..a11542c4fd 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -9,7 +9,7 @@ static char const * const for_each_ref_usage[] = {
 	N_("git for-each-ref [<options>] [<pattern>]"),
 	N_("git for-each-ref [--points-at <object>]"),
 	N_("git for-each-ref [(--merged | --no-merged) [<object>]]"),
-	N_("git for-each-ref [--contains [<object>]]"),
+	N_("git for-each-ref [(--contains | --no-contains) [<object>]]"),
 	NULL
 };
 
@@ -43,6 +43,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_MERGED(&filter, N_("print only refs that are merged")),
 		OPT_NO_MERGED(&filter, N_("print only refs that are not merged")),
 		OPT_CONTAINS(&filter.with_commit, N_("print only refs which contain the commit")),
+		OPT_NO_CONTAINS(&filter.no_commit, N_("print only refs which don't contain the commit")),
 		OPT_BOOL(0, "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
 		OPT_END(),
 	};
diff --git a/builtin/tag.c b/builtin/tag.c
index ad29be6923..da1fca3677 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -22,7 +22,7 @@
 static const char * const git_tag_usage[] = {
 	N_("git tag [-a | -s | -u <key-id>] [-f] [-m <msg> | -F <file>] <tagname> [<head>]"),
 	N_("git tag -d <tagname>..."),
-	N_("git tag -l [-n[<num>]] [--contains <commit>] [--points-at <object>]"
+	N_("git tag -l [-n[<num>]] [--[no-]contains <commit>] [--points-at <object>]"
 		"\n\t\t[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]"),
 	N_("git tag -v [--format=<format>] <tagname>..."),
 	NULL
@@ -424,7 +424,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		OPT_GROUP(N_("Tag listing options")),
 		OPT_COLUMN(0, "column", &colopts, N_("show tag list in columns")),
 		OPT_CONTAINS(&filter.with_commit, N_("print only tags that contain the commit")),
+		OPT_NO_CONTAINS(&filter.no_commit, N_("print only tags that don't contain the commit")),
 		OPT_WITH(&filter.with_commit, N_("print only tags that contain the commit")),
+		OPT_WITHOUT(&filter.no_commit, N_("print only tags that don't contain the commit")),
 		OPT_MERGED(&filter, N_("print only tags that are merged")),
 		OPT_NO_MERGED(&filter, N_("print only tags that are not merged")),
 		OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
@@ -488,6 +490,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		die(_("-n option is only allowed with -l."));
 	if (filter.with_commit)
 		die(_("--contains option is only allowed with -l."));
+	if (filter.no_commit)
+		die(_("--no-contains option is only allowed with -l."));
 	if (filter.points_at.nr)
 		die(_("--points-at option is only allowed with -l."));
 	if (filter.merge_commit)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2121211cb4..d121c45709 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1159,9 +1159,9 @@ _git_branch ()
 	--*)
 		__gitcomp "
 			--color --no-color --verbose --abbrev= --no-abbrev
-			--track --no-track --contains --merged --no-merged
-			--set-upstream-to= --edit-description --list
-			--unset-upstream --delete --move --remotes
+			--track --no-track --contains --no-contains --merged
+			--no-merged --set-upstream-to= --edit-description
+			--list --unset-upstream --delete --move --remotes
 			--column --no-column --sort= --points-at
 			"
 		;;
@@ -2928,7 +2928,8 @@ _git_tag ()
 		__gitcomp "
 			--list --delete --verify --annotate --message --file
 			--sign --cleanup --local-user --force --column --sort=
-			--contains --points-at --merged --no-merged --create-reflog
+			--contains --no-contains --points-at --merged
+			--no-merged --create-reflog
 			"
 		;;
 	esac
diff --git a/parse-options.h b/parse-options.h
index dcd8a0926c..0eac90b510 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -258,7 +258,9 @@ extern int parse_opt_passthru_argv(const struct option *, const char *, int);
 	  PARSE_OPT_LASTARG_DEFAULT | flag, \
 	  parse_opt_commits, (intptr_t) "HEAD" \
 	}
-#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, 0)
+#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, PARSE_OPT_NONEG)
+#define OPT_NO_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("no-contains", v, h, PARSE_OPT_NONEG)
 #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH("with", v, h, PARSE_OPT_HIDDEN)
+#define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH("without", v, h, PARSE_OPT_HIDDEN)
 
 #endif
diff --git a/ref-filter.c b/ref-filter.c
index 9c82b5b9d6..6f68ae84d4 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1487,6 +1487,7 @@ struct ref_filter_cbdata {
 	struct ref_array *array;
 	struct ref_filter *filter;
 	struct contains_cache contains_cache;
+	struct contains_cache no_contains_cache;
 };
 
 /*
@@ -1586,11 +1587,11 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
 }
 
 static int commit_contains(struct ref_filter *filter, struct commit *commit,
-			   struct contains_cache *cache)
+			   struct commit_list *list, struct contains_cache *cache)
 {
 	if (filter->with_commit_tag_algo)
-		return contains_tag_algo(commit, filter->with_commit, cache) == CONTAINS_YES;
-	return is_descendant_of(commit, filter->with_commit);
+		return contains_tag_algo(commit, list, cache) == CONTAINS_YES;
+	return is_descendant_of(commit, list);
 }
 
 /*
@@ -1780,13 +1781,17 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
 	 * obtain the commit using the 'oid' available and discard all
 	 * non-commits early. The actual filtering is done later.
 	 */
-	if (filter->merge_commit || filter->with_commit || filter->verbose) {
+	if (filter->merge_commit || filter->with_commit || filter->no_commit || filter->verbose) {
 		commit = lookup_commit_reference_gently(oid->hash, 1);
 		if (!commit)
 			return 0;
-		/* We perform the filtering for the '--contains' option */
+		/* We perform the filtering for the '--contains' option... */
 		if (filter->with_commit &&
-		    !commit_contains(filter, commit, &ref_cbdata->contains_cache))
+		    !commit_contains(filter, commit, filter->with_commit, &ref_cbdata->contains_cache))
+			return 0;
+		/* ...or for the `--no-contains' option */
+		if (filter->no_commit &&
+		    commit_contains(filter, commit, filter->no_commit, &ref_cbdata->no_contains_cache))
 			return 0;
 	}
 
@@ -1886,7 +1891,10 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
 		broken = 1;
 	filter->kind = type & FILTER_REFS_KIND_MASK;
 
-	init_contains_cache(&ref_cbdata.contains_cache);
+	if (filter->with_commit)
+		init_contains_cache(&ref_cbdata.contains_cache);
+	if (filter->no_commit)
+		init_contains_cache(&ref_cbdata.no_contains_cache);
 
 	/*  Simple per-ref filtering */
 	if (!filter->kind)
@@ -1910,7 +1918,10 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
 			head_ref(ref_filter_handler, &ref_cbdata);
 	}
 
-	clear_contains_cache(&ref_cbdata.contains_cache);
+	if (filter->with_commit)
+		clear_contains_cache(&ref_cbdata.contains_cache);
+	if (filter->no_commit)
+		clear_contains_cache(&ref_cbdata.no_contains_cache);
 
 	/*  Filters that need revision walking */
 	if (filter->merge_commit)
diff --git a/ref-filter.h b/ref-filter.h
index e738c5dfd3..dde40f6849 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -53,6 +53,7 @@ struct ref_filter {
 	const char **name_patterns;
 	struct sha1_array points_at;
 	struct commit_list *with_commit;
+	struct commit_list *no_commit;
 
 	enum {
 		REF_FILTER_MERGED_NONE = 0,
diff --git a/t/t3201-branch-contains.sh b/t/t3201-branch-contains.sh
index 7f3ec47241..506415dbd3 100755
--- a/t/t3201-branch-contains.sh
+++ b/t/t3201-branch-contains.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='branch --contains <commit>, --merged, and --no-merged'
+test_description='branch --contains <commit>, --no-contains <commit> --merged, and --no-merged'
 
 . ./test-lib.sh
 
@@ -45,6 +45,22 @@ test_expect_success 'branch --contains master' '
 
 '
 
+test_expect_success 'branch --no-contains=master' '
+
+	git branch --no-contains=master >actual &&
+	>expect &&
+	test_cmp expect actual
+
+'
+
+test_expect_success 'branch --no-contains master' '
+
+	git branch --no-contains master >actual &&
+	>expect &&
+	test_cmp expect actual
+
+'
+
 test_expect_success 'branch --contains=side' '
 
 	git branch --contains=side >actual &&
@@ -55,6 +71,16 @@ test_expect_success 'branch --contains=side' '
 
 '
 
+test_expect_success 'branch --no-contains=side' '
+
+	git branch --no-contains=side >actual &&
+	{
+		echo "  master"
+	} >expect &&
+	test_cmp expect actual
+
+'
+
 test_expect_success 'branch --contains with pattern implies --list' '
 
 	git branch --contains=master master >actual &&
@@ -65,6 +91,14 @@ test_expect_success 'branch --contains with pattern implies --list' '
 
 '
 
+test_expect_success 'branch --no-contains with pattern implies --list' '
+
+	git branch --no-contains=master master >actual &&
+	>expect &&
+	test_cmp expect actual
+
+'
+
 test_expect_success 'side: branch --merged' '
 
 	git branch --merged >actual &&
@@ -126,7 +160,9 @@ test_expect_success 'branch --no-merged with pattern implies --list' '
 test_expect_success 'implicit --list conflicts with modification options' '
 
 	test_must_fail git branch --contains=master -d &&
-	test_must_fail git branch --contains=master -m foo
+	test_must_fail git branch --contains=master -m foo &&
+	test_must_fail git branch --no-contains=master -d &&
+	test_must_fail git branch --no-contains=master -m foo
 
 '
 
@@ -159,4 +195,15 @@ test_expect_success 'branch --merged with --verbose' '
 	test_i18ncmp expect actual
 '
 
+test_expect_success 'branch --contains combined with --no-contains' '
+	git branch --contains zzz --no-contains topic >actual &&
+	cat >expect <<-\EOF &&
+	  master
+	  side
+	  zzz
+	EOF
+	test_cmp expect actual
+
+'
+
 test_done
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index a09a1a46ef..4902ba5f16 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -93,6 +93,22 @@ test_expect_success 'filtering with --contains' '
 	test_cmp expect actual
 '
 
+test_expect_success 'filtering with --no-contains' '
+	cat >expect <<-\EOF &&
+	refs/tags/one
+	EOF
+	git for-each-ref --format="%(refname)" --no-contains=two >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'filtering with --contains and --no-contains' '
+	cat >expect <<-\EOF &&
+	refs/tags/two
+	EOF
+	git for-each-ref --format="%(refname)" --contains=two --no-contains=three >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success '%(color) must fail' '
 	test_must_fail git for-each-ref --format="%(color)%(refname)"
 '
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index b4698ab5f5..bec3d0fb42 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1385,6 +1385,23 @@ test_expect_success 'checking that first commit is in all tags (relative)' "
 	test_cmp expected actual
 "
 
+# All the --contains tests above, but with --no-contains
+test_expect_success 'checking that first commit is not listed in any tag with --no-contains  (hash)' "
+	>expected &&
+	git tag -l --no-contains $hash1 v* >actual &&
+	test_cmp expected actual
+"
+
+test_expect_success 'checking that first commit is in all tags (tag)' "
+	git tag -l --no-contains v1.0 v* >actual &&
+	test_cmp expected actual
+"
+
+test_expect_success 'checking that first commit is in all tags (relative)' "
+	git tag -l --no-contains HEAD~2 v* >actual &&
+	test_cmp expected actual
+"
+
 cat > expected <<EOF
 v2.0
 EOF
@@ -1394,6 +1411,17 @@ test_expect_success 'checking that second commit only has one tag' "
 	test_cmp expected actual
 "
 
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+EOF
+
+test_expect_success 'inverse of the last test, with --no-contains' "
+	git tag -l --no-contains $hash2 v* >actual &&
+	test_cmp expected actual
+"
 
 cat > expected <<EOF
 EOF
@@ -1403,6 +1431,19 @@ test_expect_success 'checking that third commit has no tags' "
 	test_cmp expected actual
 "
 
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+v2.0
+EOF
+
+test_expect_success 'conversely --no-contains on the third commit lists all tags' "
+	git tag -l --no-contains $hash3 v* >actual &&
+	test_cmp expected actual
+"
+
 # how about a simple merge?
 
 test_expect_success 'creating simple branch' '
@@ -1424,6 +1465,19 @@ test_expect_success 'checking that branch head only has one tag' "
 	test_cmp expected actual
 "
 
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+v2.0
+EOF
+
+test_expect_success 'checking that branch head with --no-contains lists all but one tag' "
+	git tag -l --no-contains $hash4 v* >actual &&
+	test_cmp expected actual
+"
+
 test_expect_success 'merging original branch into this branch' '
 	git merge --strategy=ours master &&
         git tag v4.0
@@ -1445,6 +1499,20 @@ v1.0.1
 v1.1.3
 v2.0
 v3.0
+EOF
+
+test_expect_success 'checking that original branch head with --no-contains lists all but one tag now' "
+	git tag -l --no-contains $hash3 v* >actual &&
+	test_cmp expected actual
+"
+
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+v2.0
+v3.0
 v4.0
 EOF
 
@@ -1453,6 +1521,12 @@ test_expect_success 'checking that initial commit is in all tags' "
 	test_cmp expected actual
 "
 
+test_expect_success 'checking that initial commit is in all tags with --no-contains' "
+	>expected &&
+	git tag -l --no-contains $hash1 v* >actual &&
+	test_cmp expected actual
+"
+
 # mixing modes and options:
 
 test_expect_success 'mixing incompatibles modes and options is forbidden' '
@@ -1709,7 +1783,7 @@ run_with_limited_stack () {
 test_lazy_prereq ULIMIT_STACK_SIZE 'run_with_limited_stack true'
 
 # we require ulimit, this excludes Windows
-test_expect_success ULIMIT_STACK_SIZE '--contains works in a deep repo' '
+test_expect_success ULIMIT_STACK_SIZE '--contains and --no-contains work in a deep repo' '
 	>expect &&
 	i=1 &&
 	while test $i -lt 8000
@@ -1725,7 +1799,9 @@ EOF"
 	git checkout master &&
 	git tag far-far-away HEAD^ &&
 	run_with_limited_stack git tag --contains HEAD >actual &&
-	test_cmp expect actual
+	test_cmp expect actual &&
+	run_with_limited_stack git tag --no-contains HEAD >actual &&
+	test_line_count ">" 10 actual
 '
 
 test_expect_success '--format should list tags as per format given' '
@@ -1773,4 +1849,47 @@ test_expect_success 'ambiguous branch/tags not marked' '
 	test_cmp expect actual
 '
 
+test_expect_success '--contains combined with --no-contains' '
+	(
+		git init no-contains &&
+		cd no-contains &&
+		test_commit v0.1 &&
+		test_commit v0.2 &&
+		test_commit v0.3 &&
+		test_commit v0.4 &&
+		test_commit v0.5 &&
+		cat >expected <<-\EOF &&
+		v0.2
+		v0.3
+		v0.4
+		EOF
+		git tag --contains v0.2 --no-contains v0.5 >actual &&
+		test_cmp expected actual
+	)
+'
+
+# As the docs say, list tags which contain a specified *commit*. We
+# don't recurse down to tags for trees or blobs pointed to by *those*
+# commits.
+test_expect_success 'Does --[no-]contains stop at commits? Yes!' '
+	cd no-contains &&
+	blob=$(git rev-parse v0.3:v0.3.t) &&
+	tree=$(git rev-parse v0.3^{tree}) &&
+	git tag tag-blob $blob &&
+	git tag tag-tree $tree &&
+	git tag --contains v0.3 >actual &&
+	cat >expected <<-\EOF &&
+	v0.3
+	v0.4
+	v0.5
+	EOF
+	test_cmp expected actual &&
+	git tag --no-contains v0.3 >actual &&
+	cat >expected <<-\EOF &&
+	v0.1
+	v0.2
+	EOF
+	test_cmp expected actual
+'
+
 test_done
-- 
2.11.0
Junio C Hamano· Mar 12, 2017, 04:44 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v4] ref-filter: Add --no-contains option to tag/branch/for-each-ref

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
Show 15 quoted lines
> Change the tag, branch & for-each-ref commands to have a --no-contains
> option in addition to their longstanding --contains options.
>
> The use-case I have for this is to find the last-good rollout tag
> given a known-bad <commit>. Right now, given a hypothetically bad
> commit v2.10.1-3-gcf5c7253e0, you can find which git version to revert
> to with this hacky two-liner:
>
>     (./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}' | tail -n 10
>
> But with the --no-contains option you can now get the exact same
> output with:
>
>     ./git tag -l 'v[0-9]*' --no-contains v2.10.1-3-gcf5c7253e0|sort|tail -n 10

This command line, while it may happen to work, logically does not make much sense. Move the pattern to the end, i.e.

	git tag -l --no-contains v2.10.1-3-gcf5c7253e0 'v[0-9]*'

Also if an overlong line in an example disturbs you, do not solve it by omitting SP around pipe. If you are trying to make the result readable, pick a readable solution, e.g.

    git tag -l --no-contains v2.10.1-3-gcf5c7253e0 'v[0-9]*' |
    sort | tail -n 10
Oh, drop ./ from ./git while at it ;-)
> The filtering machinery is generic between the tag, branch &
> for-each-ref commands, so once I'd implemented it for tag it was
> trivial to add support for this to the other two.
Also, we tend not to say "I did this, I do that".
	Because the filtering machinery is generic ..., support it
	for all three consistently.
Show 5 quoted lines
> I'm adding a --without option to "tag" as an alias for --no-contains
> for consistency with --with and --contains. Since we don't even
> document --with anymore (or test it). The --with option is
> undocumented, and possibly the only user of it is Junio[1]. But it's
> trivial to support, so let's do that.

The sentence that begins "Since we don't" is unfinished. I think it can safely removed without losing any information (the next sentence says the same thing).

> Where I'm changing existing documentation lines I'm mainly word
> wrapping at 75 columns to be consistent with the existing style.

Reviewers would appreciate you refrain from doing that in the same patch. Do a minimum patch so that the review can concentrate on what got changed (i.e. contents), followed by a mechanical reflow as a follow-up, or something like that, would be much nicer to handle.

> Most of the test changes I've made are just doing the inverse of the
> existing --contains tests, with this change --no-contains for tag,
> branch & for-each-ref is just as well tested as the existing
> --contains option.
Again, we tend to try our commits not about "I, my, me".
	Add --no-contains tests for tag, branch and for-each-ref
	that mostly do the inverse of the existing tests we have for
	--contains.
> This is now based on top of pu, which has Jeff King's "fix object flag
> pollution in "tag --contains" series.

Thanks for this note. I obviously cannot queue on top of 'pu' ;-) but will fork this topic off of the jk/ref-filter-flags-cleanup topic.

Show 5 quoted lines
>  'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
>  		   [(--sort=<key>)...] [--format=<format>] [<pattern>...]
>  		   [--points-at <object>] [(--merged | --no-merged) [<object>]]
> -		   [--contains [<object>]]
> +		   [(--contains | --no-contains) [<object>]]

THis notation makes sense. We have to have one of these but <object> at the end could be omitted (to default to HEAD). I guess the same notation can be used in the log for the other "filtering implies --list mode for 'git tag'" topic.

> +--no-contains [<commit>]::
> +	Only list tags which don't contain the specified commit (HEAD if
> +	not specified).
Just being curious.  Can we do
	for-each-ref --contains --no-contains 

and have both default to HEAD? I know that would not make sense as a set operation, but I am curious what our command line parser (which is oblivious to what the command is doing) does. I am guessing that it would barf saying "--contains" needs a commit but "--no-contains" is not a commit (which is very sensible)?

> +
>  --points-at <object>::
>  	Only list tags of the given object.

This is not a new issue (and certainly not a problem caused by your patch), but unlike "--contains", this does not default to HEAD when <object> is not explicitly given? It seems a bit inconsistent to me.

Show 7 quoted lines
> @@ -618,7 +620,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>  	if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)
>  		list = 1;
>  
> -	if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)
> +	if (filter.with_commit || filter.no_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)
>  		list = 1;
OK.
Show 13 quoted lines
> diff --git a/parse-options.h b/parse-options.h
> index dcd8a0926c..0eac90b510 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -258,7 +258,9 @@ extern int parse_opt_passthru_argv(const struct option *, const char *, int);
>  	  PARSE_OPT_LASTARG_DEFAULT | flag, \
>  	  parse_opt_commits, (intptr_t) "HEAD" \
>  	}
> -#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, 0)
> +#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, PARSE_OPT_NONEG)
> +#define OPT_NO_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("no-contains", v, h, PARSE_OPT_NONEG)
>  #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH("with", v, h, PARSE_OPT_HIDDEN)
> +#define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH("without", v, h, PARSE_OPT_HIDDEN)
Hmph, perhaps WITH/WITHOUT also do not take "--no-" form hence need OPT_NONEG?
Show 35 quoted lines
> @@ -1586,11 +1587,11 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
>  }
>  
>  static int commit_contains(struct ref_filter *filter, struct commit *commit,
> -			   struct contains_cache *cache)
> +			   struct commit_list *list, struct contains_cache *cache)
>  {
>  	if (filter->with_commit_tag_algo)
> -		return contains_tag_algo(commit, filter->with_commit, cache) == CONTAINS_YES;
> -	return is_descendant_of(commit, filter->with_commit);
> +		return contains_tag_algo(commit, list, cache) == CONTAINS_YES;
> +	return is_descendant_of(commit, list);
>  }
>  
>  /*
> @@ -1780,13 +1781,17 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
>  	 * obtain the commit using the 'oid' available and discard all
>  	 * non-commits early. The actual filtering is done later.
>  	 */
> -	if (filter->merge_commit || filter->with_commit || filter->verbose) {
> +	if (filter->merge_commit || filter->with_commit || filter->no_commit || filter->verbose) {
>  		commit = lookup_commit_reference_gently(oid->hash, 1);
>  		if (!commit)
>  			return 0;
> -		/* We perform the filtering for the '--contains' option */
> +		/* We perform the filtering for the '--contains' option... */
>  		if (filter->with_commit &&
> -		    !commit_contains(filter, commit, &ref_cbdata->contains_cache))
> +		    !commit_contains(filter, commit, filter->with_commit, &ref_cbdata->contains_cache))
> +			return 0;
> +		/* ...or for the `--no-contains' option */
> +		if (filter->no_commit &&
> +		    commit_contains(filter, commit, filter->no_commit, &ref_cbdata->no_contains_cache))
>  			return 0;
>  	}

When asking "--contains A --contains B", we show refs that contain _EITHER_ A or B. Two predicates are ORed together, and I think it makes sense.

When asking "--contains A --no-contains B", we show refs that contain A but exclude refs that contains B. Two predicates are ANDed together, and I think this also makes sense.

When asking "--no-contains A --no-contains B", what should we show? This implementation makes the two predicates ANDed together [*1*].

The behaviour is sensible, but is it consistent with the way now existing --no-merged works?

I think the rule is something like:
    A match with any positive selection criterion (like --contains
    A) makes a ref eligible for output, but then a match with any
    negatigve selection criterion (like --no-merged) filters it out.

Is it easy to explain to the users? Do we need doc updates to clarify, or does the description for existing --no-merged already cover this?

Thanks.
[Footnote]

*1* ... because it uses the same commit_contains() machinery that computes "contains either A or B" used for the first one and then negates its result.

Ævar Arnfjörð Bjarmason· Mar 12, 2017, 09:10 UTC · re: Junio C Hamano · lore

Re: [PATCH v4] ref-filter: Add --no-contains option to tag/branch/for-each-ref

On Sun, Mar 12, 2017 at 5:44 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 22 quoted lines
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> Change the tag, branch & for-each-ref commands to have a --no-contains
>> option in addition to their longstanding --contains options.
>>
>> The use-case I have for this is to find the last-good rollout tag
>> given a known-bad <commit>. Right now, given a hypothetically bad
>> commit v2.10.1-3-gcf5c7253e0, you can find which git version to revert
>> to with this hacky two-liner:
>>
>>     (./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}' | tail -n 10
>>
>> But with the --no-contains option you can now get the exact same
>> output with:
>>
>>     ./git tag -l 'v[0-9]*' --no-contains v2.10.1-3-gcf5c7253e0|sort|tail -n 10
>
> This command line, while it may happen to work, logically does not
> make much sense.  Move the pattern to the end, i.e.
>
>         git tag -l --no-contains v2.10.1-3-gcf5c7253e0 'v[0-9]*'
Sure, if you'd like it like that I can change it.

But as an aside, I don't understand why you think it happens to work and doesn't make much sense. To someone reading "git help tag" this would be *exactly* how you'd expect to have to write this, since the example given in the synopsis is:

    git tag [-n[<num>]] -l [--contains <commit>] [....]
And then later in the documentation:
    -l <pattern>, --list <pattern>

I.e. for git-branch this type of invocation wouldn't make sense and would just happen to work, but for git-tag the --list option is explicitly documented to immediately take a <pattern> argument.

But of course the whole branch v.s. tag difference is just more fodder for my "tag: Implicitly supply --list given another list-like option" patch.

> Also if an overlong line in an example disturbs you, do not solve it
> by omitting SP around pipe.  If you are trying to make the result
> readable, pick a readable solution, e.g.

FWIW I just write one-liners like this, i.e. I don't add the semantically meaningless spaces around all pipes to save myself some typing, and wasn't trying to squeeze this all on one line, but sure I'll change it.

>     git tag -l --no-contains v2.10.1-3-gcf5c7253e0 'v[0-9]*' |
>     sort | tail -n 10
Although I'll add a \ to that so you can still paste it to a terminal.
> Oh, drop ./ from ./git while at it ;-)
Sure.
Show 21 quoted lines
>> The filtering machinery is generic between the tag, branch &
>> for-each-ref commands, so once I'd implemented it for tag it was
>> trivial to add support for this to the other two.
>
> Also, we tend not to say "I did this, I do that".
>
>         Because the filtering machinery is generic ..., support it
>         for all three consistently.
>
>> I'm adding a --without option to "tag" as an alias for --no-contains
>> for consistency with --with and --contains. Since we don't even
>> document --with anymore (or test it). The --with option is
>> undocumented, and possibly the only user of it is Junio[1]. But it's
>> trivial to support, so let's do that.
>
> The sentence that begins "Since we don't" is unfinished.  I think
> it can safely removed without losing any information (the next
> sentence says the same thing).
>
>> Where I'm changing existing documentation lines I'm mainly word
>> wrapping at 75 columns to be consistent with the existing style.
Ack.
> Reviewers would appreciate you refrain from doing that in the same
> patch.  Do a minimum patch so that the review can concentrate on
> what got changed (i.e. contents), followed by a mechanical reflow as
> a follow-up, or something like that, would be much nicer to handle.

Okey, so two patches, one where I potentially produce very long lines & then re-flow them in a subsequent commit.

Show 10 quoted lines
>> Most of the test changes I've made are just doing the inverse of the
>> existing --contains tests, with this change --no-contains for tag,
>> branch & for-each-ref is just as well tested as the existing
>> --contains option.
>
> Again, we tend to try our commits not about "I, my, me".
>
>         Add --no-contains tests for tag, branch and for-each-ref
>         that mostly do the inverse of the existing tests we have for
>         --contains.
*Nod*
Show 6 quoted lines
>> This is now based on top of pu, which has Jeff King's "fix object flag
>> pollution in "tag --contains" series.
>
> Thanks for this note.  I obviously cannot queue on top of 'pu' ;-)
> but will fork this topic off of the jk/ref-filter-flags-cleanup
> topic.

If I'd like to base on top of that to make things easier for you do you publish jk/ref-filter-flags-cleanup sowhere? I.e. as a git ref rather than me also following that topic, applying it on top of master, and then applying my topic on top of that.

Show 10 quoted lines
>>  'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
>>                  [(--sort=<key>)...] [--format=<format>] [<pattern>...]
>>                  [--points-at <object>] [(--merged | --no-merged) [<object>]]
>> -                [--contains [<object>]]
>> +                [(--contains | --no-contains) [<object>]]
>
> THis notation makes sense.  We have to have one of these but
> <object> at the end could be omitted (to default to HEAD).  I guess
> the same notation can be used in the log for the other "filtering
> implies --list mode for 'git tag'" topic.

I don't know what makes to list in the synopsis given the default arguments to --contains and --no-contains, maybe "[<object>]?", but in any case, I'm not changing how that part is documented in for-each-ref, just adding an alternative to the existing --contains option.

Show 13 quoted lines
>> +--no-contains [<commit>]::
>> +     Only list tags which don't contain the specified commit (HEAD if
>> +     not specified).
>
> Just being curious.  Can we do
>
>         for-each-ref --contains --no-contains
>
> and have both default to HEAD?  I know that would not make sense as
> a set operation, but I am curious what our command line parser
> (which is oblivious to what the command is doing) does.  I am guessing
> that it would barf saying "--contains" needs a commit but "--no-contains"
> is not a commit (which is very sensible)?
It'll spew out "error: malformed object name --no-contains".
You can do "--contains HEAD --no-contains HEAD" to get nothing.

In an earlier thread I was discussing with Jeff whether it would be worthwhile to error out in that case, but his opinion was (paraphrasing) "Nah, GIGO", which I think makes sense in this case.

Show 7 quoted lines
>> +
>>  --points-at <object>::
>>       Only list tags of the given object.
>
> This is not a new issue (and certainly not a problem caused by your
> patch), but unlike "--contains", this does not default to HEAD when
> <object> is not explicitly given?  It seems a bit inconsistent to me.

Yeah, it doesn't default to HEAD. It's inconsistent, due to a historical wart in parse-options.[ch]

Show 25 quoted lines
>> @@ -618,7 +620,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>>       if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)
>>               list = 1;
>>
>> -     if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)
>> +     if (filter.with_commit || filter.no_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)
>>               list = 1;
>
> OK.
>
>> diff --git a/parse-options.h b/parse-options.h
>> index dcd8a0926c..0eac90b510 100644
>> --- a/parse-options.h
>> +++ b/parse-options.h
>> @@ -258,7 +258,9 @@ extern int parse_opt_passthru_argv(const struct option *, const char *, int);
>>         PARSE_OPT_LASTARG_DEFAULT | flag, \
>>         parse_opt_commits, (intptr_t) "HEAD" \
>>       }
>> -#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, 0)
>> +#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, PARSE_OPT_NONEG)
>> +#define OPT_NO_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("no-contains", v, h, PARSE_OPT_NONEG)
>>  #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH("with", v, h, PARSE_OPT_HIDDEN)
>> +#define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH("without", v, h, PARSE_OPT_HIDDEN)
>
> Hmph, perhaps WITH/WITHOUT also do not take "--no-" form hence need OPT_NONEG?

I may be missing some subtlety here, but I think you've misread this (admittedly dense) chunk. the /WITH/ options don't supply NONEG, just HIDDEN.

Show 59 quoted lines
>> @@ -1586,11 +1587,11 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
>>  }
>>
>>  static int commit_contains(struct ref_filter *filter, struct commit *commit,
>> -                        struct contains_cache *cache)
>> +                        struct commit_list *list, struct contains_cache *cache)
>>  {
>>       if (filter->with_commit_tag_algo)
>> -             return contains_tag_algo(commit, filter->with_commit, cache) == CONTAINS_YES;
>> -     return is_descendant_of(commit, filter->with_commit);
>> +             return contains_tag_algo(commit, list, cache) == CONTAINS_YES;
>> +     return is_descendant_of(commit, list);
>>  }
>>
>>  /*
>> @@ -1780,13 +1781,17 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
>>        * obtain the commit using the 'oid' available and discard all
>>        * non-commits early. The actual filtering is done later.
>>        */
>> -     if (filter->merge_commit || filter->with_commit || filter->verbose) {
>> +     if (filter->merge_commit || filter->with_commit || filter->no_commit || filter->verbose) {
>>               commit = lookup_commit_reference_gently(oid->hash, 1);
>>               if (!commit)
>>                       return 0;
>> -             /* We perform the filtering for the '--contains' option */
>> +             /* We perform the filtering for the '--contains' option... */
>>               if (filter->with_commit &&
>> -                 !commit_contains(filter, commit, &ref_cbdata->contains_cache))
>> +                 !commit_contains(filter, commit, filter->with_commit, &ref_cbdata->contains_cache))
>> +                     return 0;
>> +             /* ...or for the `--no-contains' option */
>> +             if (filter->no_commit &&
>> +                 commit_contains(filter, commit, filter->no_commit, &ref_cbdata->no_contains_cache))
>>                       return 0;
>>       }
>
> When asking "--contains A --contains B", we show refs that contain
> _EITHER_ A or B.  Two predicates are ORed together, and I think it
> makes sense.
>
> When asking "--contains A --no-contains B", we show refs that
> contain A but exclude refs that contains B.  Two predicates are
> ANDed together, and I think this also makes sense.
>
> When asking "--no-contains A --no-contains B", what should we show?
> This implementation makes the two predicates ANDed together [*1*].
>
> The behaviour is sensible, but is it consistent with the way now
> existing --no-merged works?
>
> I think the rule is something like:
>
>     A match with any positive selection criterion (like --contains
>     A) makes a ref eligible for output, but then a match with any
>     negatigve selection criterion (like --no-merged) filters it out.
>
> Is it easy to explain to the users?  Do we need doc updates to
> clarify, or does the description for existing --no-merged already
> cover this?

I'm not sure, and wanting to get the rest of this e-mail out the door, I didn't have time to fully investigate this. I.e. is it consistent with how --merged works? It uses a slightly different codepath, maybe, but it's definitely consistent with how everything in the ref-filter machinery works in general, and that's used by multiple options.

So to the extent that this needs docs we should do that in some general place that covers existing options that use it.

Junio C Hamano· Mar 12, 2017, 17:49 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v4] ref-filter: Add --no-contains option to tag/branch/for-each-ref

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 7 quoted lines
> And then later in the documentation:
>
>     -l <pattern>, --list <pattern>
>
> I.e. for git-branch this type of invocation wouldn't make sense and
> would just happen to work, but for git-tag the --list option is
> explicitly documented to immediately take a <pattern> argument.

But that (i.e. "to immediately take") is not how it actually works, as you already know after looking at how 'l' is defined as OPT_CMDMODE.

The command line is more like "-l" chooses the "list mode", and the pattern is _NOT_ an argument to the option at all. The command line is more like "-l <options to affect selection criteria>..." and the <pattern> is one of these criteria. The command line convention being dashed-options first then other arguments, it makes sense to do

	-l --contains HEAD v\*

because "--contains HEAD" is a dashed-option (which takes an argument) and "v\*" is a pattern (which is "other arguments").

Show 6 quoted lines
> I'll change it.
>
>>     git tag -l --no-contains v2.10.1-3-gcf5c7253e0 'v[0-9]*' |
>>     sort | tail -n 10
>
> Although I'll add a \ to that so you can still paste it to a terminal.

Please don't. The shell knows, when you end a line with pipe, that you haven't finished your sentence and keeps listening to you.

Show 7 quoted lines
>> Reviewers would appreciate you refrain from doing that in the same
>> patch.  Do a minimum patch so that the review can concentrate on
>> what got changed (i.e. contents), followed by a mechanical reflow as
>> a follow-up, or something like that, would be much nicer to handle.
>
> Okey, so two patches, one where I potentially produce very long lines
> & then re-flow them in a subsequent commit.

Or preferrably, the last "-" line in a hunk of your first patch may be longer than the first "+" line that replaces it that may be overly short (i.e. chopping the end of existing paragraph and replacing the remainder). And then reflow comes, e.g.

    -Okey, so two patches, one where I potentially produce very long lines
    +Okey, so two patches, one where I
    +cut an existing line short if it makes the patch churn smaller
     & then re-flow them in a subsequent commit.
> If I'd like to base on top of that to make things easier for you do
> you publish  jk/ref-filter-flags-cleanup sowhere? I.e. as a git ref
> rather than me also following that topic, applying it on top of
> master, and then applying my topic on top of that.
Do you mean a repository that holds broken-out topics?  If so:
	git://github.com/gitster/git/
is what you are looking for, perhaps?
Show 14 quoted lines
>
>> and have both default to HEAD?  I know that would not make sense as
>> a set operation, but I am curious what our command line parser
>> (which is oblivious to what the command is doing) does.  I am guessing
>> that it would barf saying "--contains" needs a commit but "--no-contains"
>> is not a commit (which is very sensible)?
>
> It'll spew out "error: malformed object name --no-contains".
>
> You can do "--contains HEAD --no-contains HEAD" to get nothing.
>
> In an earlier thread I was discussing with Jeff whether it would be
> worthwhile to error out in that case, but his opinion was
> (paraphrasing) "Nah, GIGO", which I think makes sense in this case.

I agree with Peff (I said "that would not make sense as a set operation", didn't I? ;-); I was only curious if the notation used in the documentation, i.e. "--opt [<object>]" made sense. It looks as if it would accept "--contains --no-contains" (omitting arguments from both options), but it is not so, and I was wondering if we need to improve the documentation, or the readers are OK with the notation.

Show 11 quoted lines
>>> -#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, 0)
>>> +#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, PARSE_OPT_NONEG)
>>> +#define OPT_NO_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("no-contains", v, h, PARSE_OPT_NONEG)
>>>  #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH("with", v, h, PARSE_OPT_HIDDEN)
>>> +#define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH("without", v, h, PARSE_OPT_HIDDEN)
>>
>> Hmph, perhaps WITH/WITHOUT also do not take "--no-" form hence need OPT_NONEG?
>
> I may be missing some subtlety here, but I think you've misread this
> (admittedly dense) chunk. the /WITH/ options don't supply NONEG, just
> HIDDEN.

Maybe I was unclear. As --contains should not take --no-contains and use "unset" (because new code wants to see "no-contains" and act on it in the new code, I was wondering if we should forbid --no-with and --no-without in a similar way by using OPT_NONEG in addition to OPT_HIDDEN.

Ævar Arnfjörð Bjarmason· Mar 9, 2017, 14:52 UTC · re: Jeff King · lore

Re: [PATCH] branch & tag: Add a --no-contains option

On Thu, Mar 9, 2017 at 1:51 PM, Jeff King <peff@peff.net> wrote:
Show 46 quoted lines
> On Thu, Mar 09, 2017 at 01:12:08PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> > I'm almost certain this is because the contains_tag_algo one doesn't
>> > clean up the flag bits it sets on the commit objects. So running it
>> > twice in the same process is going to give you nonsense results.
>>
>> Yeah indeed.
>>
>> I tried to hack something up to avoid this, but the
>> lookup_commit_reference_gently() we call will return the same
>> object.parent pointer for two invocations, i.e. the underlying
>> {commit,object}.c API has a cache of objects it returns, couldn't find
>> some way to quickly make it burst that cache.
>
> Yeah, you'll always get the same struct for a given sha1.
>
>> The other approach of making contains_tag_algo() itself detect that
>> it's been called before (or us passing a flag) and going around
>> setting commit.object.flags on everything to 0 seemed even more
>> brittle, particularly since I think between filter->with_commit &
>> filter->no_commit we might end up visiting different commits, so it's
>> not easy to just clear it.
>
> You can clear the marks with clear_object_flags(). But I don't think
> that type of solution will quite help us here. We consider each ref
> independently and ask "does it match --contains" and "does it match
> --no-contains?". So there is no moment where we are done with all of the
> --contains marks, and can move on to the --no-contains ones. The lookups
> are interleaved.
>
> We could move to doing them in chunks (the way filter->merge_commit
> works), and then clearing in between. Or we could use a separate bitset.
> The patch below does that.
>
>> I'm happy to hack on it given some pointers, will visit it again, but
>> for now unless I'm missing something obvious / you can point out some
>> way to hack this up I'll just submit v2 with the combination of
>> --contains & --no-contains dying with a TODO message.
>>
>> The patch without that functionality is still really useful, and we
>> can implement that later.
>
> I'm not opposed to that, though see what you think of the patch below.
> It's a bit noisy but it's conceptually pretty straightforward. It should
> hopefully be obvious how you'd add in a separate contains_cache for the
> "without" case.

I wonder how worthwhile making the combination of options case fast & adding more complexity is, as opposed to just using the slower path for those cases via:

Show changes to builtin/tag.c +6 −1
diff --git a/builtin/tag.c b/builtin/tag.c
index 737a83028a..d90e8675a8 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -53,7 +53,10 @@ static int list_tags(struct ref_filter *filter,
struct ref_sorting *sorting, con
        }

        verify_ref_format(format);
-       filter->with_commit_tag_algo = 1;
+       if ((filter->merge_commit + filter->with_commit +
filter->no_commit) > 1)
+               filter->with_commit_tag_algo = 0;
+       else
+               filter->with_commit_tag_algo = 1;
        filter_refs(&array, filter, FILTER_REFS_TAGS);
        ref_array_sort(sorting, &array);

I think I'll amend the tip of my WIP v2 to have something like that,
and then we can follow-up with making these cases where you supply
multiple options faster.

> Looking at this, I'm pretty sure that using "--contains" with "--merged"
> has similar problems, as they both use the UNINTERESTING bit. So even
> without your patch, there is a lurking bug.

I'm currently running this:
https://gist.github.com/avar/45cf288ce7cdc43e7395c6cbf9a98d68

with this patch to builtin/tag.c:

diff --git a/builtin/tag.c b/builtin/tag.c
index 737a83028a..b93c5e1754 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -56,2 +56,4 @@ static int list_tags(struct ref_filter *filter,
struct ref_sorting *sorting, con
        filter->with_commit_tag_algo = 1;
+       if (getenv("GIT_NO_TAG_ALGO"))
+               filter->with_commit_tag_algo = 0;

To smoke out any combinations of commits in git.git where with &
without the tag algo we already return different results, nothing so
far, but it's slow going.

> diff --git a/ref-filter.c b/ref-filter.c
> index 3820b21cc..42b1bc463 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -14,6 +14,7 @@
>  #include "git-compat-util.h"
>  #include "version.h"
>  #include "trailer.h"
> +#include "commit-slab.h"
>
>  typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
>
> @@ -1137,10 +1138,22 @@ static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom
>         *v = &ref->value[atom];
>  }
>
> +/*
> + * Unknown has to be "0" here, because that's what unfilled entries in our slab
> + * will return.
> + */
>  enum contains_result {
> -       CONTAINS_UNKNOWN = -1,
> -       CONTAINS_NO = 0,
> -       CONTAINS_YES = 1
> +       CONTAINS_UNKNOWN = 0,
> +       CONTAINS_NO = 1,
> +       CONTAINS_YES = 2,
> +};
> +
> +define_commit_slab(contains_cache, enum contains_result);
> +
> +struct ref_filter_cbdata {
> +       struct ref_array *array;
> +       struct ref_filter *filter;
> +       struct contains_cache contains_cache;
>  };
>
>  /*
> @@ -1171,24 +1184,25 @@ static int in_commit_list(const struct commit_list *want, struct commit *c)
>   * Do not recurse to find out, though, but return -1 if inconclusive.
>   */
>  static enum contains_result contains_test(struct commit *candidate,
> -                           const struct commit_list *want)
> +                           const struct commit_list *want,
> +                           struct contains_cache *cache)
>  {
> -       /* was it previously marked as containing a want commit? */
> -       if (candidate->object.flags & TMP_MARK)
> -               return 1;
> -       /* or marked as not possibly containing a want commit? */
> -       if (candidate->object.flags & UNINTERESTING)
> -               return 0;
> +       enum contains_result *cached = contains_cache_at(cache, candidate);
> +
> +       /* if we already found the answer, return it without traversing */
> +       if (*cached)
> +               return *cached;
> +
>         /* or are we it? */
>         if (in_commit_list(want, candidate)) {
> -               candidate->object.flags |= TMP_MARK;
> -               return 1;
> +               *cached = CONTAINS_YES;
> +               return *cached;
>         }
>
>         if (parse_commit(candidate) < 0)
> -               return 0;
> +               return CONTAINS_NO;
>
> -       return -1;
> +       return CONTAINS_UNKNOWN;
>  }
>
>  static void push_to_contains_stack(struct commit *candidate, struct contains_stack *contains_stack)
> @@ -1199,10 +1213,11 @@ static void push_to_contains_stack(struct commit *candidate, struct contains_sta
>  }
>
>  static enum contains_result contains_tag_algo(struct commit *candidate,
> -               const struct commit_list *want)
> +                                             const struct commit_list *want,
> +                                             struct contains_cache *cache)
>  {
>         struct contains_stack contains_stack = { 0, 0, NULL };
> -       int result = contains_test(candidate, want);
> +       enum contains_result result = contains_test(candidate, want, cache);
>
>         if (result != CONTAINS_UNKNOWN)
>                 return result;
> @@ -1214,16 +1229,16 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
>                 struct commit_list *parents = entry->parents;
>
>                 if (!parents) {
> -                       commit->object.flags |= UNINTERESTING;
> +                       *contains_cache_at(cache, commit) = CONTAINS_NO;
>                         contains_stack.nr--;
>                 }
>                 /*
>                  * If we just popped the stack, parents->item has been marked,
> -                * therefore contains_test will return a meaningful 0 or 1.
> +                * therefore contains_test will return a meaningful yes/no.
>                  */
> -               else switch (contains_test(parents->item, want)) {
> +               else switch (contains_test(parents->item, want, cache)) {
>                 case CONTAINS_YES:
> -                       commit->object.flags |= TMP_MARK;
> +                       *contains_cache_at(cache, commit) = CONTAINS_YES;
>                         contains_stack.nr--;
>                         break;
>                 case CONTAINS_NO:
> @@ -1235,13 +1250,14 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
>                 }
>         }
>         free(contains_stack.contains_stack);
> -       return contains_test(candidate, want);
> +       return contains_test(candidate, want, cache);
>  }
>
> -static int commit_contains(struct ref_filter *filter, struct commit *commit)
> +static int commit_contains(struct ref_filter *filter, struct commit *commit,
> +                          struct contains_cache *cache)
>  {
>         if (filter->with_commit_tag_algo)
> -               return contains_tag_algo(commit, filter->with_commit);
> +               return contains_tag_algo(commit, filter->with_commit, cache) == CONTAINS_YES;
>         return is_descendant_of(commit, filter->with_commit);
>  }
>
> @@ -1438,7 +1454,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
>                         return 0;
>                 /* We perform the filtering for the '--contains' option */
>                 if (filter->with_commit &&
> -                   !commit_contains(filter, commit))
> +                   !commit_contains(filter, commit, &ref_cbdata->contains_cache))
>                         return 0;
>         }
>
> @@ -1538,6 +1554,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
>                 broken = 1;
>         filter->kind = type & FILTER_REFS_KIND_MASK;
>
> +       init_contains_cache(&ref_cbdata.contains_cache);
> +
>         /*  Simple per-ref filtering */
>         if (!filter->kind)
>                 die("filter_refs: invalid type");
> @@ -1560,6 +1578,7 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
>                         head_ref(ref_filter_handler, &ref_cbdata);
>         }
>
> +       clear_contains_cache(&ref_cbdata.contains_cache);
>
>         /*  Filters that need revision walking */
>         if (filter->merge_commit)
> diff --git a/ref-filter.h b/ref-filter.h
> index 7b05592ba..89af9f451 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -71,11 +71,6 @@ struct ref_filter {
>                 verbose;
>  };
>
> -struct ref_filter_cbdata {
> -       struct ref_array *array;
> -       struct ref_filter *filter;
> -};
> -
>  /*  Macros for checking --merged and --no-merged options */
>  #define _OPT_MERGED_NO_MERGED(option, filter, h) \
>         { OPTION_CALLBACK, 0, option, (filter), N_("commit"), (h), \
Jeff King· Mar 9, 2017, 14:55 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] branch & tag: Add a --no-contains option

On Thu, Mar 09, 2017 at 03:52:09PM +0100, Ævar Arnfjörð Bjarmason wrote:
Show 12 quoted lines
> -       filter->with_commit_tag_algo = 1;
> +       if ((filter->merge_commit + filter->with_commit +
> filter->no_commit) > 1)
> +               filter->with_commit_tag_algo = 0;
> +       else
> +               filter->with_commit_tag_algo = 1;
>         filter_refs(&array, filter, FILTER_REFS_TAGS);
>         ref_array_sort(sorting, &array);
> 
> I think I'll amend the tip of my WIP v2 to have something like that,
> and then we can follow-up with making these cases where you supply
> multiple options faster.

Yeah, that's another option. I think you may find that it's unbearably slow if you have a lot of tags.

Show 6 quoted lines
> > Looking at this, I'm pretty sure that using "--contains" with "--merged"
> > has similar problems, as they both use the UNINTERESTING bit. So even
> > without your patch, there is a lurking bug.
> 
> I'm currently running this:
> https://gist.github.com/avar/45cf288ce7cdc43e7395c6cbf9a98d68
Cute. I'll be curious if it turns up anything.
-Peff
Ævar Arnfjörð Bjarmason· Mar 10, 2017, 11:31 UTC · re: Jeff King · lore

Re: [PATCH] branch & tag: Add a --no-contains option

On Thu, Mar 9, 2017 at 3:55 PM, Jeff King <peff@peff.net> wrote:
Show 17 quoted lines
> On Thu, Mar 09, 2017 at 03:52:09PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> -       filter->with_commit_tag_algo = 1;
>> +       if ((filter->merge_commit + filter->with_commit +
>> filter->no_commit) > 1)
>> +               filter->with_commit_tag_algo = 0;
>> +       else
>> +               filter->with_commit_tag_algo = 1;
>>         filter_refs(&array, filter, FILTER_REFS_TAGS);
>>         ref_array_sort(sorting, &array);
>>
>> I think I'll amend the tip of my WIP v2 to have something like that,
>> and then we can follow-up with making these cases where you supply
>> multiple options faster.
>
> Yeah, that's another option.  I think you may find that it's unbearably
> slow if you have a lot of tags.

It's what I'm going for in my v2 currently. I think it's a rare enough (and new) use-case that it's OK to slow it down for now & investigate your line of patching separately.

Show 8 quoted lines
>> > Looking at this, I'm pretty sure that using "--contains" with "--merged"
>> > has similar problems, as they both use the UNINTERESTING bit. So even
>> > without your patch, there is a lurking bug.
>>
>> I'm currently running this:
>> https://gist.github.com/avar/45cf288ce7cdc43e7395c6cbf9a98d68
>
> Cute. I'll be curious if it turns up anything.
Currently at ~500 attempts out of 5K with no bad results.
Ævar Arnfjörð Bjarmason· Mar 9, 2017, 20:02 UTC · re: Jeff King · lore

[PATCH v2] ref-filter: Add --no-contains option to tag/branch/for-each-ref

Change the tag, branch & for-each-ref commands to have a --no-contains option in addition to their longstanding --contains options.

The use-case I have for this is to find the last-good rollout tag given a known-bad <commit>. Right now, given a hypothetically bad commit v2.10.1-3-gcf5c7253e0, you can find which git version to revert to with this hacky two-liner:

    (./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}' | tail -n 10

But with the --no-contains option you can now get the exact same output with:

    ./git tag -l 'v[0-9]*' --no-contains v2.10.1-3-gcf5c7253e0|sort|tail -n 10

The filtering machinery is generic between the tag, branch & for-each-ref commands, so once I'd implemented it for tag it was trivial to add support for this to the other two.

A practical use for this with "branch" is e.g. finding branches which diverged between 2.8 & 2.10:

    ./git branch --contains v2.8.0 --no-contains v2.10.0

The "describe" command also has a --contains option, but its semantics are unrelated to what tag/branch/for-each-ref use --contains for. I don't see how a --no-contains option for "describe" would make any sense, other than being exactly equivalent to not supplying --contains at all, which would be confusing at best.

I'm adding a --without option to "tag" as an alias for --no-contains for consistency with --with and --contains. Since we don't even document --with anymore (or test it). The --with option is undocumented, and possibly the only user of it is Junio[1]. But it's trivial to support, so let's do that.

Where I'm changing existing documentation lines I'm mainly word wrapping at 75 columns to be consistent with the existing style. The changes to Documentation/ are much smaller with: git show --word-diff, same for the minor change to git-completion.bash.

Most of the test changes I've made are just doing the inverse of the existing --contains tests, with this change --no-contains for tag, branch & for-each-ref is just as well tested as the existing --contains option.

In addition to those tests I've added tests for --contains in combination with --no-contains for all three commands, and a test for "tag" which asserts that --no-contains won't find tree/blob tags, which is slightly unintuitive, but consistent with how --contains works.

When --contains and --no-contains are provided to "tag" we disable the optimized code to find tags added in v1.7.2-rc1-1-gffc4b8012d. That code changes flags on commit objects as an optimization, and thus can't be called twice.

Jeff King has a WIP patch to fix that[2], but rather than try to incorporate that into this already big patch I'm just disabling the optimized codepath. Using both --contains and --no-contains will most likely be rare, and we can get an initial correct version working & optimize it later.

It's possible that the --merge & --no-merge codepaths for "tag" have a similar bug, but as of writing I can't produce any evidence of that via a brute-force test script[3].

1. <xmqqefy71iej.fsf@gitster.mtv.corp.google.com>
2. <20170309125132.tubwxtneffok4nrc@sigill.intra.peff.net>
3. <CACBZZX4W7brFe5ecTvQPMmf4X5_zH+dw3cB5xeVg+2hWYps0Ug@mail.gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Changes since v1:
 * Now git-for-each-ref has --no-contains
 * Doc fixes, happy with the NOTES section in git-branch.txt now.
 * Now tag --contains works with --no-contains, but falls back on
   the much slower pre-1.8 era code pending efforts to make
   commit_contains() re-callable.
 * Now `git tag --no-contains HEAD hi` dies, just like `git tag
   --contains HEAD hi` does.
 * Fixed up my shitty C in ref-filter.c as Peff suggested.
 * Tests for --contains combined with --no-contains for all the
   commands.
 * Rewrote the very verbose t7004-tag.sh tag test Peff complied
   about.
 Documentation/git-branch.txt           |  24 ++++---
 Documentation/git-for-each-ref.txt     |   6 +-
 Documentation/git-tag.txt              |   6 +-
 builtin/branch.c                       |   4 +-
 builtin/for-each-ref.c                 |   1 +
 builtin/tag.c                          |  15 +++-
 contrib/completion/git-completion.bash |   9 +--
 parse-options.h                        |   4 +-
 ref-filter.c                           |  16 +++--
 ref-filter.h                           |   1 +
 t/t3201-branch-contains.sh             |  51 +++++++++++++-
 t/t6302-for-each-ref-filter.sh         |  16 +++++
 t/t7004-tag.sh                         | 123 ++++++++++++++++++++++++++++++++-
 13 files changed, 249 insertions(+), 27 deletions(-)
Show changes to 13 files +249 −27

Documentation/git-branch.txt, Documentation/git-for-each-ref.txt, Documentation/git-tag.txt, builtin/branch.c, builtin/for-each-ref.c, builtin/tag.c, contrib/completion/git-completion.bash, parse-options.h, ref-filter.c, ref-filter.h, t/t3201-branch-contains.sh, t/t6302-for-each-ref-filter.sh, t/t7004-tag.sh

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 092f1bcf9f..28a36a8a0a 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git branch' [--color[=<when>] | --no-color] [-r | -a]
 	[--list] [-v [--abbrev=<length> | --no-abbrev]]
 	[--column[=<options>] | --no-column]
-	[(--merged | --no-merged | --contains) [<commit>]] [--sort=<key>]
+	[(--merged | --no-merged | --contains | --no-contains) [<commit>]] [--sort=<key>]
 	[--points-at <object>] [--format=<format>] [<pattern>...]
 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
 'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
@@ -35,11 +35,12 @@ as branch creation.
 
 With `--contains`, shows only the branches that contain the named commit
 (in other words, the branches whose tip commits are descendants of the
-named commit).  With `--merged`, only branches merged into the named
-commit (i.e. the branches whose tip commits are reachable from the named
-commit) will be listed.  With `--no-merged` only branches not merged into
-the named commit will be listed.  If the <commit> argument is missing it
-defaults to `HEAD` (i.e. the tip of the current branch).
+named commit), `--no-contains` inverts it. With `--merged`, only branches
+merged into the named commit (i.e. the branches whose tip commits are
+reachable from the named commit) will be listed.  With `--no-merged` only
+branches not merged into the named commit will be listed.  If the <commit>
+argument is missing it defaults to `HEAD` (i.e. the tip of the current
+branch).
 
 The command's second form creates a new branch head named <branchname>
 which points to the current `HEAD`, or <start-point> if given.
@@ -213,6 +214,10 @@ start-point is either a local or remote-tracking branch.
 	Only list branches which contain the specified commit (HEAD
 	if not specified). Implies `--list`.
 
+--no-contains [<commit>]::
+	Only list branches which don't contain the specified commit
+	(HEAD if not specified). Implies `--list`.
+
 --merged [<commit>]::
 	Only list branches whose tips are reachable from the
 	specified commit (HEAD if not specified). Implies `--list`.
@@ -296,13 +301,16 @@ If you are creating a branch that you want to checkout immediately, it is
 easier to use the git checkout command with its `-b` option to create
 a branch and check it out with a single command.
 
-The options `--contains`, `--merged` and `--no-merged` serve three related
-but different purposes:
+The options `--contains`, `--no-contains`, `--merged` and `--no-merged`
+serve four related but different purposes:
 
 - `--contains <commit>` is used to find all branches which will need
   special attention if <commit> were to be rebased or amended, since those
   branches contain the specified <commit>.
 
+- `--no-contains <commit>` is the inverse of that, i.e. branches that don't
+  contain the specified <commit>.
+
 - `--merged` is used to find all branches which can be safely deleted,
   since those branches are fully contained by HEAD.
 
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 111e1be6f5..83b93c75a8 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
 		   [(--sort=<key>)...] [--format=<format>] [<pattern>...]
 		   [--points-at <object>] [(--merged | --no-merged) [<object>]]
-		   [--contains [<object>]]
+		   [(--contains | --no-contains) [<object>]]
 
 DESCRIPTION
 -----------
@@ -79,6 +79,10 @@ OPTIONS
 	Only list refs which contain the specified commit (HEAD if not
 	specified).
 
+--no-contains [<object>]::
+	Only list refs which don't contain the specified commit (HEAD
+	if not specified).
+
 --ignore-case::
 	Sorting and filtering refs are case insensitive.
 
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 525737a5d8..4938496194 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git tag' [-a | -s | -u <keyid>] [-f] [-m <msg> | -F <file>]
 	<tagname> [<commit> | <object>]
 'git tag' -d <tagname>...
-'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
+'git tag' [-n[<num>]] -l [--[no-]contains <commit>] [--points-at <object>]
 	[--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]
 	[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]
 'git tag' -v [--format=<format>] <tagname>...
@@ -124,6 +124,10 @@ This option is only applicable when listing tags without annotation lines.
 	Only list tags which contain the specified commit (HEAD if not
 	specified).
 
+--no-contains [<commit>]::
+	Only list tags which don't contain the specified commit (HEAD if
+	not secified).
+
 --points-at <object>::
 	Only list tags of the given object.
 
diff --git a/builtin/branch.c b/builtin/branch.c
index 94f7de7fa5..e8d534604c 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -548,7 +548,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT('r', "remotes",     &filter.kind, N_("act on remote-tracking branches"),
 			FILTER_REFS_REMOTES),
 		OPT_CONTAINS(&filter.with_commit, N_("print only branches that contain the commit")),
+		OPT_NO_CONTAINS(&filter.no_commit, N_("print only branches that don't contain the commit")),
 		OPT_WITH(&filter.with_commit, N_("print only branches that contain the commit")),
+		OPT_WITHOUT(&filter.with_commit, N_("print only branches that don't contain the commit")),
 		OPT__ABBREV(&filter.abbrev),
 
 		OPT_GROUP(N_("Specific git-branch actions:")),
@@ -604,7 +606,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)
 		list = 1;
 
-	if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)
+	if (filter.with_commit || filter.no_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)
 		list = 1;
 
 	if (!!delete + !!rename + !!new_upstream +
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index df41fa0350..b1ae2388e6 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -43,6 +43,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_MERGED(&filter, N_("print only refs that are merged")),
 		OPT_NO_MERGED(&filter, N_("print only refs that are not merged")),
 		OPT_CONTAINS(&filter.with_commit, N_("print only refs which contain the commit")),
+		OPT_NO_CONTAINS(&filter.no_commit, N_("print only refs which don't contain the commit")),
 		OPT_BOOL(0, "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
 		OPT_END(),
 	};
diff --git a/builtin/tag.c b/builtin/tag.c
index ad29be6923..d83674e3e6 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -53,7 +53,16 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
 	}
 
 	verify_ref_format(format);
-	filter->with_commit_tag_algo = 1;
+	if (filter->with_commit && filter->no_commit)
+		/* Due to the way the contains_tag_algo() function
+		   touches object.flags we can only call it once
+		   per-process.
+
+		   For now sacrifice performance for correctness when
+		   both --contains and --no-contains are provided */
+		filter->with_commit_tag_algo = 0;
+	else
+		filter->with_commit_tag_algo = 1;
 	filter_refs(&array, filter, FILTER_REFS_TAGS);
 	ref_array_sort(sorting, &array);
 
@@ -424,7 +433,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		OPT_GROUP(N_("Tag listing options")),
 		OPT_COLUMN(0, "column", &colopts, N_("show tag list in columns")),
 		OPT_CONTAINS(&filter.with_commit, N_("print only tags that contain the commit")),
+		OPT_NO_CONTAINS(&filter.no_commit, N_("print only tags that don't contain the commit")),
 		OPT_WITH(&filter.with_commit, N_("print only tags that contain the commit")),
+		OPT_WITHOUT(&filter.no_commit, N_("print only tags that don't contain the commit")),
 		OPT_MERGED(&filter, N_("print only tags that are merged")),
 		OPT_NO_MERGED(&filter, N_("print only tags that are not merged")),
 		OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
@@ -488,6 +499,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		die(_("-n option is only allowed with -l."));
 	if (filter.with_commit)
 		die(_("--contains option is only allowed with -l."));
+	if (filter.no_commit)
+		die(_("--no-contains option is only allowed with -l."));
 	if (filter.points_at.nr)
 		die(_("--points-at option is only allowed with -l."));
 	if (filter.merge_commit)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index fc32286a43..fa3da49478 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1093,9 +1093,9 @@ _git_branch ()
 	--*)
 		__gitcomp "
 			--color --no-color --verbose --abbrev= --no-abbrev
-			--track --no-track --contains --merged --no-merged
-			--set-upstream-to= --edit-description --list
-			--unset-upstream --delete --move --remotes
+			--track --no-track --contains --no-contains --merged
+			--no-merged --set-upstream-to= --edit-description
+			--list --unset-upstream --delete --move --remotes
 			--column --no-column --sort= --points-at
 			"
 		;;
@@ -2862,7 +2862,8 @@ _git_tag ()
 		__gitcomp "
 			--list --delete --verify --annotate --message --file
 			--sign --cleanup --local-user --force --column --sort=
-			--contains --points-at --merged --no-merged --create-reflog
+			--contains --no-contains --points-at --merged
+			--no-merged --create-reflog
 			"
 		;;
 	esac
diff --git a/parse-options.h b/parse-options.h
index dcd8a0926c..0eac90b510 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -258,7 +258,9 @@ extern int parse_opt_passthru_argv(const struct option *, const char *, int);
 	  PARSE_OPT_LASTARG_DEFAULT | flag, \
 	  parse_opt_commits, (intptr_t) "HEAD" \
 	}
-#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, 0)
+#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, PARSE_OPT_NONEG)
+#define OPT_NO_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("no-contains", v, h, PARSE_OPT_NONEG)
 #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH("with", v, h, PARSE_OPT_HIDDEN)
+#define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH("without", v, h, PARSE_OPT_HIDDEN)
 
 #endif
diff --git a/ref-filter.c b/ref-filter.c
index 1ec0fb8391..b6dab75729 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1571,11 +1571,11 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
 	return contains_test(candidate, want);
 }
 
-static int commit_contains(struct ref_filter *filter, struct commit *commit)
+static int commit_contains(struct ref_filter *filter, struct commit_list *list, struct commit *commit)
 {
 	if (filter->with_commit_tag_algo)
-		return contains_tag_algo(commit, filter->with_commit);
-	return is_descendant_of(commit, filter->with_commit);
+		return contains_tag_algo(commit, list);
+	return is_descendant_of(commit, list);
 }
 
 /*
@@ -1765,13 +1765,17 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
 	 * obtain the commit using the 'oid' available and discard all
 	 * non-commits early. The actual filtering is done later.
 	 */
-	if (filter->merge_commit || filter->with_commit || filter->verbose) {
+	if (filter->merge_commit || filter->with_commit || filter->no_commit || filter->verbose) {
 		commit = lookup_commit_reference_gently(oid->hash, 1);
 		if (!commit)
 			return 0;
-		/* We perform the filtering for the '--contains' option */
+		/* We perform the filtering for the '--contains' option... */
 		if (filter->with_commit &&
-		    !commit_contains(filter, commit))
+		    !commit_contains(filter, filter->with_commit, commit))
+			return 0;
+		/* ...or for the `--no-contains' option */
+		if (filter->no_commit &&
+		    commit_contains(filter, filter->no_commit, commit))
 			return 0;
 	}
 
diff --git a/ref-filter.h b/ref-filter.h
index 154e24c405..af85eb4592 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -53,6 +53,7 @@ struct ref_filter {
 	const char **name_patterns;
 	struct sha1_array points_at;
 	struct commit_list *with_commit;
+	struct commit_list *no_commit;
 
 	enum {
 		REF_FILTER_MERGED_NONE = 0,
diff --git a/t/t3201-branch-contains.sh b/t/t3201-branch-contains.sh
index 7f3ec47241..506415dbd3 100755
--- a/t/t3201-branch-contains.sh
+++ b/t/t3201-branch-contains.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='branch --contains <commit>, --merged, and --no-merged'
+test_description='branch --contains <commit>, --no-contains <commit> --merged, and --no-merged'
 
 . ./test-lib.sh
 
@@ -45,6 +45,22 @@ test_expect_success 'branch --contains master' '
 
 '
 
+test_expect_success 'branch --no-contains=master' '
+
+	git branch --no-contains=master >actual &&
+	>expect &&
+	test_cmp expect actual
+
+'
+
+test_expect_success 'branch --no-contains master' '
+
+	git branch --no-contains master >actual &&
+	>expect &&
+	test_cmp expect actual
+
+'
+
 test_expect_success 'branch --contains=side' '
 
 	git branch --contains=side >actual &&
@@ -55,6 +71,16 @@ test_expect_success 'branch --contains=side' '
 
 '
 
+test_expect_success 'branch --no-contains=side' '
+
+	git branch --no-contains=side >actual &&
+	{
+		echo "  master"
+	} >expect &&
+	test_cmp expect actual
+
+'
+
 test_expect_success 'branch --contains with pattern implies --list' '
 
 	git branch --contains=master master >actual &&
@@ -65,6 +91,14 @@ test_expect_success 'branch --contains with pattern implies --list' '
 
 '
 
+test_expect_success 'branch --no-contains with pattern implies --list' '
+
+	git branch --no-contains=master master >actual &&
+	>expect &&
+	test_cmp expect actual
+
+'
+
 test_expect_success 'side: branch --merged' '
 
 	git branch --merged >actual &&
@@ -126,7 +160,9 @@ test_expect_success 'branch --no-merged with pattern implies --list' '
 test_expect_success 'implicit --list conflicts with modification options' '
 
 	test_must_fail git branch --contains=master -d &&
-	test_must_fail git branch --contains=master -m foo
+	test_must_fail git branch --contains=master -m foo &&
+	test_must_fail git branch --no-contains=master -d &&
+	test_must_fail git branch --no-contains=master -m foo
 
 '
 
@@ -159,4 +195,15 @@ test_expect_success 'branch --merged with --verbose' '
 	test_i18ncmp expect actual
 '
 
+test_expect_success 'branch --contains combined with --no-contains' '
+	git branch --contains zzz --no-contains topic >actual &&
+	cat >expect <<-\EOF &&
+	  master
+	  side
+	  zzz
+	EOF
+	test_cmp expect actual
+
+'
+
 test_done
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index a09a1a46ef..4902ba5f16 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -93,6 +93,22 @@ test_expect_success 'filtering with --contains' '
 	test_cmp expect actual
 '
 
+test_expect_success 'filtering with --no-contains' '
+	cat >expect <<-\EOF &&
+	refs/tags/one
+	EOF
+	git for-each-ref --format="%(refname)" --no-contains=two >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'filtering with --contains and --no-contains' '
+	cat >expect <<-\EOF &&
+	refs/tags/two
+	EOF
+	git for-each-ref --format="%(refname)" --contains=two --no-contains=three >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success '%(color) must fail' '
 	test_must_fail git for-each-ref --format="%(color)%(refname)"
 '
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index b4698ab5f5..8ad5719962 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1385,6 +1385,23 @@ test_expect_success 'checking that first commit is in all tags (relative)' "
 	test_cmp expected actual
 "
 
+# All the --contains tests above, but with --no-contains
+test_expect_success 'checking that first commit is not listed in any tag with --no-contains  (hash)' "
+	>expected &&
+	git tag -l --no-contains $hash1 v* >actual &&
+	test_cmp expected actual
+"
+
+test_expect_success 'checking that first commit is in all tags (tag)' "
+	git tag -l --no-contains v1.0 v* >actual &&
+	test_cmp expected actual
+"
+
+test_expect_success 'checking that first commit is in all tags (relative)' "
+	git tag -l --no-contains HEAD~2 v* >actual &&
+	test_cmp expected actual
+"
+
 cat > expected <<EOF
 v2.0
 EOF
@@ -1394,6 +1411,17 @@ test_expect_success 'checking that second commit only has one tag' "
 	test_cmp expected actual
 "
 
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+EOF
+
+test_expect_success 'inverse of the last test, with --no-contains' "
+	git tag -l --no-contains $hash2 v* >actual &&
+	test_cmp expected actual
+"
 
 cat > expected <<EOF
 EOF
@@ -1403,6 +1431,19 @@ test_expect_success 'checking that third commit has no tags' "
 	test_cmp expected actual
 "
 
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+v2.0
+EOF
+
+test_expect_success 'conversely --no-contains on the third commit lists all tags' "
+	git tag -l --no-contains $hash3 v* >actual &&
+	test_cmp expected actual
+"
+
 # how about a simple merge?
 
 test_expect_success 'creating simple branch' '
@@ -1424,6 +1465,19 @@ test_expect_success 'checking that branch head only has one tag' "
 	test_cmp expected actual
 "
 
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+v2.0
+EOF
+
+test_expect_success 'checking that branch head with --no-contains lists all but one tag' "
+	git tag -l --no-contains $hash4 v* >actual &&
+	test_cmp expected actual
+"
+
 test_expect_success 'merging original branch into this branch' '
 	git merge --strategy=ours master &&
         git tag v4.0
@@ -1445,6 +1499,20 @@ v1.0.1
 v1.1.3
 v2.0
 v3.0
+EOF
+
+test_expect_success 'checking that original branch head with --no-contains lists all but one tag now' "
+	git tag -l --no-contains $hash3 v* >actual &&
+	test_cmp expected actual
+"
+
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+v2.0
+v3.0
 v4.0
 EOF
 
@@ -1453,6 +1521,12 @@ test_expect_success 'checking that initial commit is in all tags' "
 	test_cmp expected actual
 "
 
+test_expect_success 'checking that initial commit is in all tags with --no-contains' "
+	>expected &&
+	git tag -l --no-contains $hash1 v* >actual &&
+	test_cmp expected actual
+"
+
 # mixing modes and options:
 
 test_expect_success 'mixing incompatibles modes and options is forbidden' '
@@ -1709,7 +1783,7 @@ run_with_limited_stack () {
 test_lazy_prereq ULIMIT_STACK_SIZE 'run_with_limited_stack true'
 
 # we require ulimit, this excludes Windows
-test_expect_success ULIMIT_STACK_SIZE '--contains works in a deep repo' '
+test_expect_success ULIMIT_STACK_SIZE '--contains and --no-contains work in a deep repo' '
 	>expect &&
 	i=1 &&
 	while test $i -lt 8000
@@ -1725,7 +1799,9 @@ EOF"
 	git checkout master &&
 	git tag far-far-away HEAD^ &&
 	run_with_limited_stack git tag --contains HEAD >actual &&
-	test_cmp expect actual
+	test_cmp expect actual &&
+	run_with_limited_stack git tag --no-contains HEAD >actual &&
+	test_line_count ">" 70 actual
 '
 
 test_expect_success '--format should list tags as per format given' '
@@ -1773,4 +1849,47 @@ test_expect_success 'ambiguous branch/tags not marked' '
 	test_cmp expect actual
 '
 
+test_expect_success '--contains combined with --no-contains' '
+	(
+		git init no-contains &&
+		cd no-contains &&
+		test_commit v0.1 &&
+		test_commit v0.2 &&
+		test_commit v0.3 &&
+		test_commit v0.4 &&
+		test_commit v0.5 &&
+		cat >expected <<-\EOF &&
+		v0.2
+		v0.3
+		v0.4
+		EOF
+		git tag --contains v0.2 --no-contains v0.5 >actual &&
+		test_cmp expected actual
+	)
+'
+
+# As the docs say, list tags which contain a specified *commit*. We
+# don't recurse down to tags for trees or blobs pointed to by *those*
+# commits.
+test_expect_success 'Does --[no-]contains stop at commits? Yes!' '
+	cd no-contains &&
+	blob=$(git rev-parse v0.3:v0.3.t) &&
+	tree=$(git rev-parse v0.3^{tree}) &&
+	git tag tag-blob $blob &&
+	git tag tag-tree $tree &&
+	git tag --contains v0.3 >actual &&
+	cat >expected <<-\EOF &&
+	v0.3
+	v0.4
+	v0.5
+	EOF
+	test_cmp expected actual &&
+	git tag --no-contains v0.3 >actual &&
+	cat >expected <<-\EOF &&
+	v0.1
+	v0.2
+	EOF
+	test_cmp expected actual
+'
+
 test_done
-- 
2.11.0
Christian Couder· Mar 9, 2017, 20:31 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v2] ref-filter: Add --no-contains option to tag/branch/for-each-ref

On Thu, Mar 9, 2017 at 9:02 PM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 21 quoted lines
>
> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
> index 525737a5d8..4938496194 100644
> --- a/Documentation/git-tag.txt
> +++ b/Documentation/git-tag.txt
> @@ -12,7 +12,7 @@ SYNOPSIS
>  'git tag' [-a | -s | -u <keyid>] [-f] [-m <msg> | -F <file>]
>         <tagname> [<commit> | <object>]
>  'git tag' -d <tagname>...
> -'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
> +'git tag' [-n[<num>]] -l [--[no-]contains <commit>] [--points-at <object>]
>         [--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]
>         [--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]
>  'git tag' -v [--format=<format>] <tagname>...
> @@ -124,6 +124,10 @@ This option is only applicable when listing tags without annotation lines.
>         Only list tags which contain the specified commit (HEAD if not
>         specified).
>
> +--no-contains [<commit>]::
> +       Only list tags which don't contain the specified commit (HEAD if
> +       not secified).
s/secified/specified/
Show 15 quoted lines
> +
>  --points-at <object>::
>         Only list tags of the given object.
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 94f7de7fa5..e8d534604c 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -548,7 +548,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>                 OPT_SET_INT('r', "remotes",     &filter.kind, N_("act on remote-tracking branches"),
>                         FILTER_REFS_REMOTES),
>                 OPT_CONTAINS(&filter.with_commit, N_("print only branches that contain the commit")),
> +               OPT_NO_CONTAINS(&filter.no_commit, N_("print only branches that don't contain the commit")),
>                 OPT_WITH(&filter.with_commit, N_("print only branches that contain the commit")),
> +               OPT_WITHOUT(&filter.with_commit, N_("print only branches that don't contain the commit")),
s/with_commit/no_commit/
Ævar Arnfjörð Bjarmason· Mar 10, 2017, 11:46 UTC · re: Christian Couder · lore

Re: [PATCH v2] ref-filter: Add --no-contains option to tag/branch/for-each-ref

On Thu, Mar 9, 2017 at 9:31 PM, Christian Couder <christian.couder@gmail.com> wrote:

Show 43 quoted lines
> On Thu, Mar 9, 2017 at 9:02 PM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>>
>> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
>> index 525737a5d8..4938496194 100644
>> --- a/Documentation/git-tag.txt
>> +++ b/Documentation/git-tag.txt
>> @@ -12,7 +12,7 @@ SYNOPSIS
>>  'git tag' [-a | -s | -u <keyid>] [-f] [-m <msg> | -F <file>]
>>         <tagname> [<commit> | <object>]
>>  'git tag' -d <tagname>...
>> -'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
>> +'git tag' [-n[<num>]] -l [--[no-]contains <commit>] [--points-at <object>]
>>         [--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]
>>         [--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]
>>  'git tag' -v [--format=<format>] <tagname>...
>> @@ -124,6 +124,10 @@ This option is only applicable when listing tags without annotation lines.
>>         Only list tags which contain the specified commit (HEAD if not
>>         specified).
>>
>> +--no-contains [<commit>]::
>> +       Only list tags which don't contain the specified commit (HEAD if
>> +       not secified).
>
> s/secified/specified/
>
>> +
>>  --points-at <object>::
>>         Only list tags of the given object.
>>
>> diff --git a/builtin/branch.c b/builtin/branch.c
>> index 94f7de7fa5..e8d534604c 100644
>> --- a/builtin/branch.c
>> +++ b/builtin/branch.c
>> @@ -548,7 +548,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>>                 OPT_SET_INT('r', "remotes",     &filter.kind, N_("act on remote-tracking branches"),
>>                         FILTER_REFS_REMOTES),
>>                 OPT_CONTAINS(&filter.with_commit, N_("print only branches that contain the commit")),
>> +               OPT_NO_CONTAINS(&filter.no_commit, N_("print only branches that don't contain the commit")),
>>                 OPT_WITH(&filter.with_commit, N_("print only branches that contain the commit")),
>> +               OPT_WITHOUT(&filter.with_commit, N_("print only branches that don't contain the commit")),
>
> s/with_commit/no_commit/
Thanks!

FWIW this is the current status of my WIP v3. I noticed a couple of other issues where --contains was mentioned without --no-contains.

Show changes to 4 files +4 −5

Documentation/git-tag.txt, builtin/branch.c, builtin/for-each-ref.c, builtin/tag.c

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 4938496194..d9243bf5e4 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -129 +129 @@ This option is only applicable when listing tags
without annotation lines.
-       not secified).
+       not specified).
diff --git a/builtin/branch.c b/builtin/branch.c
index e8d534604c..dd96319726 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -553 +553 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
-               OPT_WITHOUT(&filter.with_commit, N_("print only
branches that don't contain the commit")),
+               OPT_WITHOUT(&filter.no_commit, N_("print only branches
that don't contain the commit")),
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index b1ae2388e6..a11542c4fd 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -12 +12 @@ static char const * const for_each_ref_usage[] = {
-       N_("git for-each-ref [--contains [<object>]]"),
+       N_("git for-each-ref [(--contains | --no-contains) [<object>]]"),
diff --git a/builtin/tag.c b/builtin/tag.c
index d83674e3e6..57289132a9 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -25 +25 @@ static const char * const git_tag_usage[] = {
-       N_("git tag -l [-n[<num>]] [--contains <commit>] [--points-at <object>]"
+       N_("git tag -l [-n[<num>]] [--[no-]contains <commit>]
[--points-at <object>]"


These last two hunks are going to bust the i18n files for most
languages. Junio/Jiang, in cases like these where I could fix those up
with a search/replace on po/* without knowing the languages in
question (since it's purely changing e.g. --contains to
--[no-]contains), what do you prefer to do, have that as part of this
patch, or do it after the fact through the normal i18n maintenance
process?
Ævar Arnfjörð Bjarmason· Mar 10, 2017, 12:09 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v2] ref-filter: Add --no-contains option to tag/branch/for-each-ref

On Fri, Mar 10, 2017 at 12:46 PM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 83 quoted lines
> On Thu, Mar 9, 2017 at 9:31 PM, Christian Couder
> <christian.couder@gmail.com> wrote:
>> On Thu, Mar 9, 2017 at 9:02 PM, Ævar Arnfjörð Bjarmason
>> <avarab@gmail.com> wrote:
>>>
>>> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
>>> index 525737a5d8..4938496194 100644
>>> --- a/Documentation/git-tag.txt
>>> +++ b/Documentation/git-tag.txt
>>> @@ -12,7 +12,7 @@ SYNOPSIS
>>>  'git tag' [-a | -s | -u <keyid>] [-f] [-m <msg> | -F <file>]
>>>         <tagname> [<commit> | <object>]
>>>  'git tag' -d <tagname>...
>>> -'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
>>> +'git tag' [-n[<num>]] -l [--[no-]contains <commit>] [--points-at <object>]
>>>         [--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]
>>>         [--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]
>>>  'git tag' -v [--format=<format>] <tagname>...
>>> @@ -124,6 +124,10 @@ This option is only applicable when listing tags without annotation lines.
>>>         Only list tags which contain the specified commit (HEAD if not
>>>         specified).
>>>
>>> +--no-contains [<commit>]::
>>> +       Only list tags which don't contain the specified commit (HEAD if
>>> +       not secified).
>>
>> s/secified/specified/
>>
>>> +
>>>  --points-at <object>::
>>>         Only list tags of the given object.
>>>
>>> diff --git a/builtin/branch.c b/builtin/branch.c
>>> index 94f7de7fa5..e8d534604c 100644
>>> --- a/builtin/branch.c
>>> +++ b/builtin/branch.c
>>> @@ -548,7 +548,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>>>                 OPT_SET_INT('r', "remotes",     &filter.kind, N_("act on remote-tracking branches"),
>>>                         FILTER_REFS_REMOTES),
>>>                 OPT_CONTAINS(&filter.with_commit, N_("print only branches that contain the commit")),
>>> +               OPT_NO_CONTAINS(&filter.no_commit, N_("print only branches that don't contain the commit")),
>>>                 OPT_WITH(&filter.with_commit, N_("print only branches that contain the commit")),
>>> +               OPT_WITHOUT(&filter.with_commit, N_("print only branches that don't contain the commit")),
>>
>> s/with_commit/no_commit/
>
> Thanks!
>
> FWIW this is the current status of my WIP v3. I noticed a couple of
> other issues where --contains was mentioned without --no-contains.
>
> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
> index 4938496194..d9243bf5e4 100644
> --- a/Documentation/git-tag.txt
> +++ b/Documentation/git-tag.txt
> @@ -129 +129 @@ This option is only applicable when listing tags
> without annotation lines.
> -       not secified).
> +       not specified).
> diff --git a/builtin/branch.c b/builtin/branch.c
> index e8d534604c..dd96319726 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -553 +553 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> -               OPT_WITHOUT(&filter.with_commit, N_("print only
> branches that don't contain the commit")),
> +               OPT_WITHOUT(&filter.no_commit, N_("print only branches
> that don't contain the commit")),
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index b1ae2388e6..a11542c4fd 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -12 +12 @@ static char const * const for_each_ref_usage[] = {
> -       N_("git for-each-ref [--contains [<object>]]"),
> +       N_("git for-each-ref [(--contains | --no-contains) [<object>]]"),
> diff --git a/builtin/tag.c b/builtin/tag.c
> index d83674e3e6..57289132a9 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -25 +25 @@ static const char * const git_tag_usage[] = {
> -       N_("git tag -l [-n[<num>]] [--contains <commit>] [--points-at <object>]"
> +       N_("git tag -l [-n[<num>]] [--[no-]contains <commit>]
> [--points-at <object>]"
Add to that:
Show changes to t/t7004-tag.sh +1 −1
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 8ad5719962..bec3d0fb42 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1804 +1804 @@ EOF"
-       test_line_count ">" 70 actual
+       test_line_count ">" 10 actual

The tests currently fail with v2 if gpg isn't installed, which'll
cause the number to dip below 70, just setting it to a much more
conservative 10, but maybe this should just be "test -s actual" ...

>
> These last two hunks are going to bust the i18n files for most
> languages. Junio/Jiang, in cases like these where I could fix those up
> with a search/replace on po/* without knowing the languages in
> question (since it's purely changing e.g. --contains to
> --[no-]contains), what do you prefer to do, have that as part of this
> patch, or do it after the fact through the normal i18n maintenance
> process?
Ævar Arnfjörð Bjarmason· Mar 10, 2017, 20:33 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v3] ref-filter: Add --no-contains option to tag/branch/for-each-ref

Change the tag, branch & for-each-ref commands to have a --no-contains option in addition to their longstanding --contains options.

The use-case I have for this is to find the last-good rollout tag given a known-bad <commit>. Right now, given a hypothetically bad commit v2.10.1-3-gcf5c7253e0, you can find which git version to revert to with this hacky two-liner:

    (./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}' | tail -n 10

But with the --no-contains option you can now get the exact same output with:

    ./git tag -l 'v[0-9]*' --no-contains v2.10.1-3-gcf5c7253e0|sort|tail -n 10

The filtering machinery is generic between the tag, branch & for-each-ref commands, so once I'd implemented it for tag it was trivial to add support for this to the other two.

A practical use for this with "branch" is e.g. finding branches which diverged between 2.8 & 2.10:

    ./git branch --contains v2.8.0 --no-contains v2.10.0

The "describe" command also has a --contains option, but its semantics are unrelated to what tag/branch/for-each-ref use --contains for. I don't see how a --no-contains option for "describe" would make any sense, other than being exactly equivalent to not supplying --contains at all, which would be confusing at best.

I'm adding a --without option to "tag" as an alias for --no-contains for consistency with --with and --contains. Since we don't even document --with anymore (or test it). The --with option is undocumented, and possibly the only user of it is Junio[1]. But it's trivial to support, so let's do that.

Where I'm changing existing documentation lines I'm mainly word wrapping at 75 columns to be consistent with the existing style. The changes to Documentation/ are much smaller with: git show --word-diff, same for the minor change to git-completion.bash.

Most of the test changes I've made are just doing the inverse of the existing --contains tests, with this change --no-contains for tag, branch & for-each-ref is just as well tested as the existing --contains option.

In addition to those tests I've added tests for --contains in combination with --no-contains for all three commands, and a test for "tag" which asserts that --no-contains won't find tree/blob tags, which is slightly unintuitive, but consistent with how --contains works.

When --contains and --no-contains are provided to "tag" we disable the optimized code to find tags added in v1.7.2-rc1-1-gffc4b8012d. That code changes flags on commit objects as an optimization, and thus can't be called twice.

Jeff King has a WIP patch to fix that[2], but rather than try to incorporate that into this already big patch I'm just disabling the optimized codepath. Using both --contains and --no-contains will most likely be rare, and we can get an initial correct version working & optimize it later.

It's possible that the --merge & --no-merge codepaths for "tag" have a similar bug, but as of writing I can't produce any evidence of that via a brute-force test script[3].

1. <xmqqefy71iej.fsf@gitster.mtv.corp.google.com>
2. <20170309125132.tubwxtneffok4nrc@sigill.intra.peff.net>
3. <CACBZZX4W7brFe5ecTvQPMmf4X5_zH+dw3cB5xeVg+2hWYps0Ug@mail.gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

With the changes noted in my <CACBZZX74fpEzic3Qs0kG5i5pb-up+Ct0cp71beDXaj+BoDcKRQ@mail.gmail.com> && <CACBZZX7EW013ZhDtUWome5nOqLjha34B9gJCRQgc0J0qkvWBuA@mail.gmail.com>.

 Documentation/git-branch.txt           |  24 ++++---
 Documentation/git-for-each-ref.txt     |   6 +-
 Documentation/git-tag.txt              |   6 +-
 builtin/branch.c                       |   4 +-
 builtin/for-each-ref.c                 |   3 +-
 builtin/tag.c                          |  17 ++++-
 contrib/completion/git-completion.bash |   9 +--
 parse-options.h                        |   4 +-
 ref-filter.c                           |  16 +++--
 ref-filter.h                           |   1 +
 t/t3201-branch-contains.sh             |  51 +++++++++++++-
 t/t6302-for-each-ref-filter.sh         |  16 +++++
 t/t7004-tag.sh                         | 123 ++++++++++++++++++++++++++++++++-
 13 files changed, 251 insertions(+), 29 deletions(-)
Show changes to 13 files +251 −29

Documentation/git-branch.txt, Documentation/git-for-each-ref.txt, Documentation/git-tag.txt, builtin/branch.c, builtin/for-each-ref.c, builtin/tag.c, contrib/completion/git-completion.bash, parse-options.h, ref-filter.c, ref-filter.h, t/t3201-branch-contains.sh, t/t6302-for-each-ref-filter.sh, t/t7004-tag.sh

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 092f1bcf9f..28a36a8a0a 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git branch' [--color[=<when>] | --no-color] [-r | -a]
 	[--list] [-v [--abbrev=<length> | --no-abbrev]]
 	[--column[=<options>] | --no-column]
-	[(--merged | --no-merged | --contains) [<commit>]] [--sort=<key>]
+	[(--merged | --no-merged | --contains | --no-contains) [<commit>]] [--sort=<key>]
 	[--points-at <object>] [--format=<format>] [<pattern>...]
 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
 'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
@@ -35,11 +35,12 @@ as branch creation.
 
 With `--contains`, shows only the branches that contain the named commit
 (in other words, the branches whose tip commits are descendants of the
-named commit).  With `--merged`, only branches merged into the named
-commit (i.e. the branches whose tip commits are reachable from the named
-commit) will be listed.  With `--no-merged` only branches not merged into
-the named commit will be listed.  If the <commit> argument is missing it
-defaults to `HEAD` (i.e. the tip of the current branch).
+named commit), `--no-contains` inverts it. With `--merged`, only branches
+merged into the named commit (i.e. the branches whose tip commits are
+reachable from the named commit) will be listed.  With `--no-merged` only
+branches not merged into the named commit will be listed.  If the <commit>
+argument is missing it defaults to `HEAD` (i.e. the tip of the current
+branch).
 
 The command's second form creates a new branch head named <branchname>
 which points to the current `HEAD`, or <start-point> if given.
@@ -213,6 +214,10 @@ start-point is either a local or remote-tracking branch.
 	Only list branches which contain the specified commit (HEAD
 	if not specified). Implies `--list`.
 
+--no-contains [<commit>]::
+	Only list branches which don't contain the specified commit
+	(HEAD if not specified). Implies `--list`.
+
 --merged [<commit>]::
 	Only list branches whose tips are reachable from the
 	specified commit (HEAD if not specified). Implies `--list`.
@@ -296,13 +301,16 @@ If you are creating a branch that you want to checkout immediately, it is
 easier to use the git checkout command with its `-b` option to create
 a branch and check it out with a single command.
 
-The options `--contains`, `--merged` and `--no-merged` serve three related
-but different purposes:
+The options `--contains`, `--no-contains`, `--merged` and `--no-merged`
+serve four related but different purposes:
 
 - `--contains <commit>` is used to find all branches which will need
   special attention if <commit> were to be rebased or amended, since those
   branches contain the specified <commit>.
 
+- `--no-contains <commit>` is the inverse of that, i.e. branches that don't
+  contain the specified <commit>.
+
 - `--merged` is used to find all branches which can be safely deleted,
   since those branches are fully contained by HEAD.
 
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 111e1be6f5..83b93c75a8 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
 		   [(--sort=<key>)...] [--format=<format>] [<pattern>...]
 		   [--points-at <object>] [(--merged | --no-merged) [<object>]]
-		   [--contains [<object>]]
+		   [(--contains | --no-contains) [<object>]]
 
 DESCRIPTION
 -----------
@@ -79,6 +79,10 @@ OPTIONS
 	Only list refs which contain the specified commit (HEAD if not
 	specified).
 
+--no-contains [<object>]::
+	Only list refs which don't contain the specified commit (HEAD
+	if not specified).
+
 --ignore-case::
 	Sorting and filtering refs are case insensitive.
 
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 525737a5d8..d9243bf5e4 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git tag' [-a | -s | -u <keyid>] [-f] [-m <msg> | -F <file>]
 	<tagname> [<commit> | <object>]
 'git tag' -d <tagname>...
-'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
+'git tag' [-n[<num>]] -l [--[no-]contains <commit>] [--points-at <object>]
 	[--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]
 	[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]
 'git tag' -v [--format=<format>] <tagname>...
@@ -124,6 +124,10 @@ This option is only applicable when listing tags without annotation lines.
 	Only list tags which contain the specified commit (HEAD if not
 	specified).
 
+--no-contains [<commit>]::
+	Only list tags which don't contain the specified commit (HEAD if
+	not specified).
+
 --points-at <object>::
 	Only list tags of the given object.
 
diff --git a/builtin/branch.c b/builtin/branch.c
index 94f7de7fa5..dd96319726 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -548,7 +548,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT('r', "remotes",     &filter.kind, N_("act on remote-tracking branches"),
 			FILTER_REFS_REMOTES),
 		OPT_CONTAINS(&filter.with_commit, N_("print only branches that contain the commit")),
+		OPT_NO_CONTAINS(&filter.no_commit, N_("print only branches that don't contain the commit")),
 		OPT_WITH(&filter.with_commit, N_("print only branches that contain the commit")),
+		OPT_WITHOUT(&filter.no_commit, N_("print only branches that don't contain the commit")),
 		OPT__ABBREV(&filter.abbrev),
 
 		OPT_GROUP(N_("Specific git-branch actions:")),
@@ -604,7 +606,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)
 		list = 1;
 
-	if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)
+	if (filter.with_commit || filter.no_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)
 		list = 1;
 
 	if (!!delete + !!rename + !!new_upstream +
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index df41fa0350..a11542c4fd 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -9,7 +9,7 @@ static char const * const for_each_ref_usage[] = {
 	N_("git for-each-ref [<options>] [<pattern>]"),
 	N_("git for-each-ref [--points-at <object>]"),
 	N_("git for-each-ref [(--merged | --no-merged) [<object>]]"),
-	N_("git for-each-ref [--contains [<object>]]"),
+	N_("git for-each-ref [(--contains | --no-contains) [<object>]]"),
 	NULL
 };
 
@@ -43,6 +43,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_MERGED(&filter, N_("print only refs that are merged")),
 		OPT_NO_MERGED(&filter, N_("print only refs that are not merged")),
 		OPT_CONTAINS(&filter.with_commit, N_("print only refs which contain the commit")),
+		OPT_NO_CONTAINS(&filter.no_commit, N_("print only refs which don't contain the commit")),
 		OPT_BOOL(0, "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
 		OPT_END(),
 	};
diff --git a/builtin/tag.c b/builtin/tag.c
index ad29be6923..57289132a9 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -22,7 +22,7 @@
 static const char * const git_tag_usage[] = {
 	N_("git tag [-a | -s | -u <key-id>] [-f] [-m <msg> | -F <file>] <tagname> [<head>]"),
 	N_("git tag -d <tagname>..."),
-	N_("git tag -l [-n[<num>]] [--contains <commit>] [--points-at <object>]"
+	N_("git tag -l [-n[<num>]] [--[no-]contains <commit>] [--points-at <object>]"
 		"\n\t\t[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]"),
 	N_("git tag -v [--format=<format>] <tagname>..."),
 	NULL
@@ -53,7 +53,16 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
 	}
 
 	verify_ref_format(format);
-	filter->with_commit_tag_algo = 1;
+	if (filter->with_commit && filter->no_commit)
+		/* Due to the way the contains_tag_algo() function
+		   touches object.flags we can only call it once
+		   per-process.
+
+		   For now sacrifice performance for correctness when
+		   both --contains and --no-contains are provided */
+		filter->with_commit_tag_algo = 0;
+	else
+		filter->with_commit_tag_algo = 1;
 	filter_refs(&array, filter, FILTER_REFS_TAGS);
 	ref_array_sort(sorting, &array);
 
@@ -424,7 +433,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		OPT_GROUP(N_("Tag listing options")),
 		OPT_COLUMN(0, "column", &colopts, N_("show tag list in columns")),
 		OPT_CONTAINS(&filter.with_commit, N_("print only tags that contain the commit")),
+		OPT_NO_CONTAINS(&filter.no_commit, N_("print only tags that don't contain the commit")),
 		OPT_WITH(&filter.with_commit, N_("print only tags that contain the commit")),
+		OPT_WITHOUT(&filter.no_commit, N_("print only tags that don't contain the commit")),
 		OPT_MERGED(&filter, N_("print only tags that are merged")),
 		OPT_NO_MERGED(&filter, N_("print only tags that are not merged")),
 		OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
@@ -488,6 +499,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		die(_("-n option is only allowed with -l."));
 	if (filter.with_commit)
 		die(_("--contains option is only allowed with -l."));
+	if (filter.no_commit)
+		die(_("--no-contains option is only allowed with -l."));
 	if (filter.points_at.nr)
 		die(_("--points-at option is only allowed with -l."));
 	if (filter.merge_commit)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index fc32286a43..fa3da49478 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1093,9 +1093,9 @@ _git_branch ()
 	--*)
 		__gitcomp "
 			--color --no-color --verbose --abbrev= --no-abbrev
-			--track --no-track --contains --merged --no-merged
-			--set-upstream-to= --edit-description --list
-			--unset-upstream --delete --move --remotes
+			--track --no-track --contains --no-contains --merged
+			--no-merged --set-upstream-to= --edit-description
+			--list --unset-upstream --delete --move --remotes
 			--column --no-column --sort= --points-at
 			"
 		;;
@@ -2862,7 +2862,8 @@ _git_tag ()
 		__gitcomp "
 			--list --delete --verify --annotate --message --file
 			--sign --cleanup --local-user --force --column --sort=
-			--contains --points-at --merged --no-merged --create-reflog
+			--contains --no-contains --points-at --merged
+			--no-merged --create-reflog
 			"
 		;;
 	esac
diff --git a/parse-options.h b/parse-options.h
index dcd8a0926c..0eac90b510 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -258,7 +258,9 @@ extern int parse_opt_passthru_argv(const struct option *, const char *, int);
 	  PARSE_OPT_LASTARG_DEFAULT | flag, \
 	  parse_opt_commits, (intptr_t) "HEAD" \
 	}
-#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, 0)
+#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, PARSE_OPT_NONEG)
+#define OPT_NO_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("no-contains", v, h, PARSE_OPT_NONEG)
 #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH("with", v, h, PARSE_OPT_HIDDEN)
+#define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH("without", v, h, PARSE_OPT_HIDDEN)
 
 #endif
diff --git a/ref-filter.c b/ref-filter.c
index 1ec0fb8391..b6dab75729 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1571,11 +1571,11 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
 	return contains_test(candidate, want);
 }
 
-static int commit_contains(struct ref_filter *filter, struct commit *commit)
+static int commit_contains(struct ref_filter *filter, struct commit_list *list, struct commit *commit)
 {
 	if (filter->with_commit_tag_algo)
-		return contains_tag_algo(commit, filter->with_commit);
-	return is_descendant_of(commit, filter->with_commit);
+		return contains_tag_algo(commit, list);
+	return is_descendant_of(commit, list);
 }
 
 /*
@@ -1765,13 +1765,17 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
 	 * obtain the commit using the 'oid' available and discard all
 	 * non-commits early. The actual filtering is done later.
 	 */
-	if (filter->merge_commit || filter->with_commit || filter->verbose) {
+	if (filter->merge_commit || filter->with_commit || filter->no_commit || filter->verbose) {
 		commit = lookup_commit_reference_gently(oid->hash, 1);
 		if (!commit)
 			return 0;
-		/* We perform the filtering for the '--contains' option */
+		/* We perform the filtering for the '--contains' option... */
 		if (filter->with_commit &&
-		    !commit_contains(filter, commit))
+		    !commit_contains(filter, filter->with_commit, commit))
+			return 0;
+		/* ...or for the `--no-contains' option */
+		if (filter->no_commit &&
+		    commit_contains(filter, filter->no_commit, commit))
 			return 0;
 	}
 
diff --git a/ref-filter.h b/ref-filter.h
index 154e24c405..af85eb4592 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -53,6 +53,7 @@ struct ref_filter {
 	const char **name_patterns;
 	struct sha1_array points_at;
 	struct commit_list *with_commit;
+	struct commit_list *no_commit;
 
 	enum {
 		REF_FILTER_MERGED_NONE = 0,
diff --git a/t/t3201-branch-contains.sh b/t/t3201-branch-contains.sh
index 7f3ec47241..506415dbd3 100755
--- a/t/t3201-branch-contains.sh
+++ b/t/t3201-branch-contains.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='branch --contains <commit>, --merged, and --no-merged'
+test_description='branch --contains <commit>, --no-contains <commit> --merged, and --no-merged'
 
 . ./test-lib.sh
 
@@ -45,6 +45,22 @@ test_expect_success 'branch --contains master' '
 
 '
 
+test_expect_success 'branch --no-contains=master' '
+
+	git branch --no-contains=master >actual &&
+	>expect &&
+	test_cmp expect actual
+
+'
+
+test_expect_success 'branch --no-contains master' '
+
+	git branch --no-contains master >actual &&
+	>expect &&
+	test_cmp expect actual
+
+'
+
 test_expect_success 'branch --contains=side' '
 
 	git branch --contains=side >actual &&
@@ -55,6 +71,16 @@ test_expect_success 'branch --contains=side' '
 
 '
 
+test_expect_success 'branch --no-contains=side' '
+
+	git branch --no-contains=side >actual &&
+	{
+		echo "  master"
+	} >expect &&
+	test_cmp expect actual
+
+'
+
 test_expect_success 'branch --contains with pattern implies --list' '
 
 	git branch --contains=master master >actual &&
@@ -65,6 +91,14 @@ test_expect_success 'branch --contains with pattern implies --list' '
 
 '
 
+test_expect_success 'branch --no-contains with pattern implies --list' '
+
+	git branch --no-contains=master master >actual &&
+	>expect &&
+	test_cmp expect actual
+
+'
+
 test_expect_success 'side: branch --merged' '
 
 	git branch --merged >actual &&
@@ -126,7 +160,9 @@ test_expect_success 'branch --no-merged with pattern implies --list' '
 test_expect_success 'implicit --list conflicts with modification options' '
 
 	test_must_fail git branch --contains=master -d &&
-	test_must_fail git branch --contains=master -m foo
+	test_must_fail git branch --contains=master -m foo &&
+	test_must_fail git branch --no-contains=master -d &&
+	test_must_fail git branch --no-contains=master -m foo
 
 '
 
@@ -159,4 +195,15 @@ test_expect_success 'branch --merged with --verbose' '
 	test_i18ncmp expect actual
 '
 
+test_expect_success 'branch --contains combined with --no-contains' '
+	git branch --contains zzz --no-contains topic >actual &&
+	cat >expect <<-\EOF &&
+	  master
+	  side
+	  zzz
+	EOF
+	test_cmp expect actual
+
+'
+
 test_done
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index a09a1a46ef..4902ba5f16 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -93,6 +93,22 @@ test_expect_success 'filtering with --contains' '
 	test_cmp expect actual
 '
 
+test_expect_success 'filtering with --no-contains' '
+	cat >expect <<-\EOF &&
+	refs/tags/one
+	EOF
+	git for-each-ref --format="%(refname)" --no-contains=two >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'filtering with --contains and --no-contains' '
+	cat >expect <<-\EOF &&
+	refs/tags/two
+	EOF
+	git for-each-ref --format="%(refname)" --contains=two --no-contains=three >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success '%(color) must fail' '
 	test_must_fail git for-each-ref --format="%(color)%(refname)"
 '
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index b4698ab5f5..bec3d0fb42 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1385,6 +1385,23 @@ test_expect_success 'checking that first commit is in all tags (relative)' "
 	test_cmp expected actual
 "
 
+# All the --contains tests above, but with --no-contains
+test_expect_success 'checking that first commit is not listed in any tag with --no-contains  (hash)' "
+	>expected &&
+	git tag -l --no-contains $hash1 v* >actual &&
+	test_cmp expected actual
+"
+
+test_expect_success 'checking that first commit is in all tags (tag)' "
+	git tag -l --no-contains v1.0 v* >actual &&
+	test_cmp expected actual
+"
+
+test_expect_success 'checking that first commit is in all tags (relative)' "
+	git tag -l --no-contains HEAD~2 v* >actual &&
+	test_cmp expected actual
+"
+
 cat > expected <<EOF
 v2.0
 EOF
@@ -1394,6 +1411,17 @@ test_expect_success 'checking that second commit only has one tag' "
 	test_cmp expected actual
 "
 
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+EOF
+
+test_expect_success 'inverse of the last test, with --no-contains' "
+	git tag -l --no-contains $hash2 v* >actual &&
+	test_cmp expected actual
+"
 
 cat > expected <<EOF
 EOF
@@ -1403,6 +1431,19 @@ test_expect_success 'checking that third commit has no tags' "
 	test_cmp expected actual
 "
 
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+v2.0
+EOF
+
+test_expect_success 'conversely --no-contains on the third commit lists all tags' "
+	git tag -l --no-contains $hash3 v* >actual &&
+	test_cmp expected actual
+"
+
 # how about a simple merge?
 
 test_expect_success 'creating simple branch' '
@@ -1424,6 +1465,19 @@ test_expect_success 'checking that branch head only has one tag' "
 	test_cmp expected actual
 "
 
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+v2.0
+EOF
+
+test_expect_success 'checking that branch head with --no-contains lists all but one tag' "
+	git tag -l --no-contains $hash4 v* >actual &&
+	test_cmp expected actual
+"
+
 test_expect_success 'merging original branch into this branch' '
 	git merge --strategy=ours master &&
         git tag v4.0
@@ -1445,6 +1499,20 @@ v1.0.1
 v1.1.3
 v2.0
 v3.0
+EOF
+
+test_expect_success 'checking that original branch head with --no-contains lists all but one tag now' "
+	git tag -l --no-contains $hash3 v* >actual &&
+	test_cmp expected actual
+"
+
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+v2.0
+v3.0
 v4.0
 EOF
 
@@ -1453,6 +1521,12 @@ test_expect_success 'checking that initial commit is in all tags' "
 	test_cmp expected actual
 "
 
+test_expect_success 'checking that initial commit is in all tags with --no-contains' "
+	>expected &&
+	git tag -l --no-contains $hash1 v* >actual &&
+	test_cmp expected actual
+"
+
 # mixing modes and options:
 
 test_expect_success 'mixing incompatibles modes and options is forbidden' '
@@ -1709,7 +1783,7 @@ run_with_limited_stack () {
 test_lazy_prereq ULIMIT_STACK_SIZE 'run_with_limited_stack true'
 
 # we require ulimit, this excludes Windows
-test_expect_success ULIMIT_STACK_SIZE '--contains works in a deep repo' '
+test_expect_success ULIMIT_STACK_SIZE '--contains and --no-contains work in a deep repo' '
 	>expect &&
 	i=1 &&
 	while test $i -lt 8000
@@ -1725,7 +1799,9 @@ EOF"
 	git checkout master &&
 	git tag far-far-away HEAD^ &&
 	run_with_limited_stack git tag --contains HEAD >actual &&
-	test_cmp expect actual
+	test_cmp expect actual &&
+	run_with_limited_stack git tag --no-contains HEAD >actual &&
+	test_line_count ">" 10 actual
 '
 
 test_expect_success '--format should list tags as per format given' '
@@ -1773,4 +1849,47 @@ test_expect_success 'ambiguous branch/tags not marked' '
 	test_cmp expect actual
 '
 
+test_expect_success '--contains combined with --no-contains' '
+	(
+		git init no-contains &&
+		cd no-contains &&
+		test_commit v0.1 &&
+		test_commit v0.2 &&
+		test_commit v0.3 &&
+		test_commit v0.4 &&
+		test_commit v0.5 &&
+		cat >expected <<-\EOF &&
+		v0.2
+		v0.3
+		v0.4
+		EOF
+		git tag --contains v0.2 --no-contains v0.5 >actual &&
+		test_cmp expected actual
+	)
+'
+
+# As the docs say, list tags which contain a specified *commit*. We
+# don't recurse down to tags for trees or blobs pointed to by *those*
+# commits.
+test_expect_success 'Does --[no-]contains stop at commits? Yes!' '
+	cd no-contains &&
+	blob=$(git rev-parse v0.3:v0.3.t) &&
+	tree=$(git rev-parse v0.3^{tree}) &&
+	git tag tag-blob $blob &&
+	git tag tag-tree $tree &&
+	git tag --contains v0.3 >actual &&
+	cat >expected <<-\EOF &&
+	v0.3
+	v0.4
+	v0.5
+	EOF
+	test_cmp expected actual &&
+	git tag --no-contains v0.3 >actual &&
+	cat >expected <<-\EOF &&
+	v0.1
+	v0.2
+	EOF
+	test_cmp expected actual
+'
+
 test_done
-- 
2.11.0

← back to recent threads