Re: [PATCH v2] ref-filter: Add --no-contains option to tag/branch/for-each-ref
On Fri, Mar 10, 2017 at 12:46 PM, Ævar Arnfjörð Bjarmason
wrote:
> On Thu, Mar 9, 2017 at 9:31 PM, Christian Couder
> wrote:
>> On Thu, Mar 9, 2017 at 9:02 PM, Ævar Arnfjörð Bjarmason
>> 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 ] [-f] [-m | -F ]
>>> [ | ]
>>> 'git tag' -d ...
>>> -'git tag' [-n[]] -l [--contains ] [--points-at ]
>>> +'git tag' [-n[]] -l [--[no-]contains ] [--points-at ]
>>> [--column[=] | --no-column] [--create-reflog]
>>> [--sort=]
>>> [--format=] [--[no-]merged []] [...]
>>> 'git tag' -v [--format=] ...
>>> @@ -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 []::
>>> + Only list tags which don't contain the specified commit (HEAD if
>>> + not secified).
>>
>> s/secified/specified/
>>
>>> +
>>> --points-at ::
>>> 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 []]"),
> + N_("git for-each-ref [(--contains | --no-contains) []]"),
> 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[]] [--contains ] [--points-at
> ]"
> + N_("git tag -l [-n[]] [--[no-]contains ]
> [--points-at ]"
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?
Re: [PATCH v2] ref-filter: Add --no-contains option to tag/branch/for-each-ref
On Thu, Mar 9, 2017 at 9:31 PM, Christian Couder
wrote:
> On Thu, Mar 9, 2017 at 9:02 PM, Ævar Arnfjörð Bjarmason
> 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 ] [-f] [-m | -F ]
>> [ | ]
>> 'git tag' -d ...
>> -'git tag' [-n[]] -l [--contains ] [--points-at ]
>> +'git tag' [-n[]] -l [--[no-]contains ] [--points-at ]
>> [--column[=] | --no-column] [--create-reflog] [--sort=]
>> [--format=] [--[no-]merged []] [...]
>> 'git tag' -v [--format=] ...
>> @@ -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 []::
>> + Only list tags which don't contain the specified commit (HEAD if
>> + not secified).
>
> s/secified/specified/
>
>> +
>> --points-at ::
>> 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 []]"),
+ N_("git for-each-ref [(--contains | --no-contains) []]"),
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[]] [--contains ] [--points-at ]"
+ N_("git tag -l [-n[]] [--[no-]contains ]
[--points-at ]"
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
On Thu, Mar 9, 2017 at 9:02 PM, Ævar Arnfjörð Bjarmason
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 ] [-f] [-m | -F ]
> [ | ]
> 'git tag' -d ...
> -'git tag' [-n[]] -l [--contains ] [--points-at ]
> +'git tag' [-n[]] -l [--[no-]contains ] [--points-at ]
> [--column[=] | --no-column] [--create-reflog] [--sort=]
> [--format=] [--[no-]merged []] [...]
> 'git tag' -v [--format=] ...
> @@ -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 []::
> + Only list tags which don't contain the specified commit (HEAD if
> + not secified).
s/secified/specified/
> +
> --points-at ::
> 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/
[PATCH v2] ref-filter: Add --no-contains option to tag/branch/for-each-ref
Change the tag, branch & for-each-ref commands to have a --no-contains
option in addition to their longstanding --contains options.
The use-case I have for this is to find the last-good rollout tag
given a known-bad . 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.
2. <20170309125132.tubwxtneffok4...@sigill.intra.peff.net>
3.
Signed-off-by: Ævar Arnfjörð Bjarmason
---
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[=] | --no-color] [-r | -a]
[--list] [-v [--abbrev= | --no-abbrev]]
[--column[=] | --no-column]
- [(--merged | --no-merged | --contains) []] [--sort=]
+ [(--merged | --no-merged | --contains | --no-contains) []]
[--s
