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

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

[Ævar Arnfjörð Bjarmason]

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(-)

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

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

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> 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.

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

[Jeff King]

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.

> 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.

> -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.

> @@ -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

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

[Ævar Arnfjörð Bjarmason]

On Thu, Mar 9, 2017 at 11:09 AM, Jeff King <peff@peff.net> wrote:
> 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.

>> 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

>> 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.

>> -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.

> 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.

>> @@ -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.

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

[Jeff King]

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.

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.

-Peff

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

[Ævar Arnfjörð Bjarmason]

On Thu, Mar 9, 2017 at 11:46 AM, Jeff King <peff@peff.net> wrote:
> 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.

> 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.

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

[Jeff King]

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.

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.

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), \

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

[Jeff King]

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

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

[Jeff King]

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(-)

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

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

[Jeff King]

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(-)

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

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

[Jeff King]

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(-)

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

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

[Jeff King]

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(-)

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

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

[Ævar Arnfjörð Bjarmason]

On Thu, Mar 9, 2017 at 1:51 PM, Jeff King <peff@peff.net> wrote:
> 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:

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), \

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

[Jeff King]

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.

> > 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

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

[Ævar Arnfjörð Bjarmason]

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(-)

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

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

[Christian Couder]

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/

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

[Ævar Arnfjörð Bjarmason]

On Thu, Mar 9, 2017 at 3:55 PM, Jeff King <peff@peff.net> wrote:
> 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.

>> > 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.

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

[Ævar Arnfjörð Bjarmason]

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>]"


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?

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

[Ævar Arnfjörð Bjarmason]

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

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?

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

[Ævar Arnfjörð Bjarmason]

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(-)

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

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

[Ævar Arnfjörð Bjarmason]

On Thu, Mar 9, 2017 at 2:27 PM, Jeff King <peff@peff.net> wrote:
> 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

> 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.

>   [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

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

[Ævar Arnfjörð Bjarmason]

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.

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

[Ævar Arnfjörð Bjarmason]

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(-)

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

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

[Ævar Arnfjörð Bjarmason]

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

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

[Junio C Hamano]

Æ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]*'

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.

> 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.

>  '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.

> @@ -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?

> @@ -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.

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

[Ævar Arnfjörð Bjarmason]

On Sun, Mar 12, 2017 at 5:44 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Æ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.

>> 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.

>> 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*

>> 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.

>>  '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.

>> +--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.

>> +
>>  --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]

>> @@ -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.

>> @@ -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.

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

[Jeff King]

On Sat, Mar 11, 2017 at 09:01:25PM +0100, Ævar Arnfjörð Bjarmason wrote:

> 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

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

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> 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").

> 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.

>> 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?

>
>> 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.

>>> -#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.