* [PATCH v3 16/24] completion: complete --no-stat
From: Philippe Blain via GitGitGadget @ 2023-06-26 16:24 UTC (permalink / raw)
To: git; +Cc: Eric Sunshine, Philippe Blain, Philippe Blain
In-Reply-To: <pull.1543.v3.git.1687796688.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index f4e773cb997..ec2e4c9e711 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1758,7 +1758,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
--textconv --no-textconv --break-rewrites
--patch --no-patch --cc --combined-all-paths
--anchored= --compact-summary --ignore-matching-lines=
- --irreversible-delete --line-prefix
+ --irreversible-delete --line-prefix --no-stat
"
# Options for diff/difftool
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 19/24] completion: complete --unified
From: Philippe Blain via GitGitGadget @ 2023-06-26 16:24 UTC (permalink / raw)
To: git; +Cc: Eric Sunshine, Philippe Blain, Philippe Blain
In-Reply-To: <pull.1543.v3.git.1687796688.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a69421cd740..7babd95d844 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1745,7 +1745,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
--find-copies-harder --ignore-cr-at-eol
--text --ignore-space-at-eol --ignore-space-change
--ignore-all-space --ignore-blank-lines --exit-code
- --quiet --ext-diff --no-ext-diff
+ --quiet --ext-diff --no-ext-diff --unified=
--no-prefix --src-prefix= --dst-prefix=
--inter-hunk-context= --function-context
--patience --histogram --minimal
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 02/24] completion: complete --break-rewrites
From: Philippe Blain via GitGitGadget @ 2023-06-26 16:24 UTC (permalink / raw)
To: git; +Cc: Eric Sunshine, Philippe Blain, Philippe Blain
In-Reply-To: <pull.1543.v3.git.1687796688.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 90fe292459b..f07b00b9c68 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1753,7 +1753,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
--diff-algorithm=
--submodule --submodule= --ignore-submodules
--indent-heuristic --no-indent-heuristic
- --textconv --no-textconv
+ --textconv --no-textconv --break-rewrites
--patch --no-patch
--anchored=
"
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 07/24] completion: complete --find-copies
From: Philippe Blain via GitGitGadget @ 2023-06-26 16:24 UTC (permalink / raw)
To: git; +Cc: Eric Sunshine, Philippe Blain, Philippe Blain
In-Reply-To: <pull.1543.v3.git.1687796688.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
contrib/completion/git-completion.bash | 1 +
1 file changed, 1 insertion(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6af04932a0a..dd6e12ad8f6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1740,6 +1740,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
--color-moved --color-moved= --no-color-moved
--color-moved-ws= --no-color-moved-ws
--full-index --binary --abbrev --diff-filter=
+ --find-copies
--find-copies-harder --ignore-cr-at-eol
--text --ignore-space-at-eol --ignore-space-change
--ignore-all-space --ignore-blank-lines --exit-code
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 10/24] completion: complete --function-context
From: Philippe Blain via GitGitGadget @ 2023-06-26 16:24 UTC (permalink / raw)
To: git; +Cc: Eric Sunshine, Philippe Blain, Philippe Blain
In-Reply-To: <pull.1543.v3.git.1687796688.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0fa86dcde6f..2610a55487b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1746,7 +1746,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
--ignore-all-space --ignore-blank-lines --exit-code
--quiet --ext-diff --no-ext-diff
--no-prefix --src-prefix= --dst-prefix=
- --inter-hunk-context=
+ --inter-hunk-context= --function-context
--patience --histogram --minimal
--raw --word-diff --word-diff-regex=
--dirstat --dirstat= --dirstat-by-file
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 00/24] completion: add missing diff options
From: Philippe Blain via GitGitGadget @ 2023-06-26 16:24 UTC (permalink / raw)
To: git; +Cc: Eric Sunshine, Philippe Blain
In-Reply-To: <pull.1543.v2.git.1686574374.gitgitgadget@gmail.com>
Changes since v2:
* removed --patch-with-raw on Junio's suggestion, '-p --raw' does the exact
same thing.
Changes since v1:
* correted authorship in 21/25
* fixed typos pointed out by Eric
v1: This series adds missing diff options to the Bash completion script.
Completion often serves as a discovery mechanism for options, so it is
beneficial to users if all options are offered by the completion script.
The list of missing options was generated by:
1. Extracting all diff options from the documentation:
git grep -h --no-column --only-match -e ^--[a-z][a-z-]*
Documentation/diff-options.txt
| sort -u > diff-options.txt
2. Searching for each option in the completion script and visually checking
which one was missing:
while read p; do echo --- $p ---; echo; git grep --color -p -e $p
upstream/master contrib/completion/git-completion.bash done <
diff-options.txt
The only options I left out are --skip-to and --rotate-to, since I agree
with their documentation: they are probably not very useful outside of their
use in 'git difftool'.
Cheers,
Philippe.
Philippe Blain (24):
completion: add comments describing __git_diff_* globals
completion: complete --break-rewrites
completion: complete --cc
completion: complete --combined-all-paths
completion: complete --compact-summary
completion: complete --default-prefix
completion: complete --find-copies
completion: complete --find-object
completion: complete --find-renames
completion: complete --function-context
completion: complete --ignore-matching-lines
completion: complete --irreversible-delete
completion: complete --ita-invisible-in-index and
--ita-visible-in-index
completion: complete --line-prefix
completion: complete --no-relative
completion: complete --no-stat
completion: complete --output
completion: complete --output-indicator-{context,new,old}
completion: complete --unified
completion: complete --ws-error-highlight
completion: move --pickaxe-{all,regex} to __git_diff_common_options
completion: complete --diff-merges, its options and --no-diff-merges
completion: complete --remerge-diff
diff.c: mention completion above add_diff_options
contrib/completion/git-completion.bash | 57 ++++++++++++++++++++++----
diff.c | 4 ++
2 files changed, 52 insertions(+), 9 deletions(-)
base-commit: fe86abd7511a9a6862d5706c6fa1d9b57a63ba09
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1543%2Fphil-blain%2Fcompletion-common-diff-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1543/phil-blain/completion-common-diff-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1543
Range-diff vs v2:
1: 4edabc7f15c = 1: 4edabc7f15c completion: add comments describing __git_diff_* globals
2: 1f3c9e8d417 = 2: 1f3c9e8d417 completion: complete --break-rewrites
3: d38823dd116 = 3: d38823dd116 completion: complete --cc
4: 51024ee2f2c = 4: 51024ee2f2c completion: complete --combined-all-paths
5: 63d70d645e2 = 5: 63d70d645e2 completion: complete --compact-summary
6: 7296a3a8c9d = 6: 7296a3a8c9d completion: complete --default-prefix
7: 1f9b213cee5 = 7: 1f9b213cee5 completion: complete --find-copies
8: 53b1c348f82 = 8: 53b1c348f82 completion: complete --find-object
9: 053f9e8620a = 9: 053f9e8620a completion: complete --find-renames
10: 2503d990e5c = 10: 2503d990e5c completion: complete --function-context
11: 8bd72945a2f = 11: 8bd72945a2f completion: complete --ignore-matching-lines
12: 5d32e972a0c = 12: 5d32e972a0c completion: complete --irreversible-delete
13: fd94e9ae783 = 13: fd94e9ae783 completion: complete --ita-invisible-in-index and --ita-visible-in-index
14: 560ad1cd017 = 14: 560ad1cd017 completion: complete --line-prefix
15: d3242e1f949 = 15: d3242e1f949 completion: complete --no-relative
16: 0f16a466fd9 = 16: 0f16a466fd9 completion: complete --no-stat
17: 761c75d4aec = 17: 761c75d4aec completion: complete --output
18: f8d430639bc = 18: f8d430639bc completion: complete --output-indicator-{context,new,old}
19: 807b8201d14 < -: ----------- completion: complete --patch-with-raw
20: 19507b1a210 = 19: cbf2e59cbea completion: complete --unified
21: c78650f215e = 20: 4750951f120 completion: complete --ws-error-highlight
22: 040248a3868 = 21: eda4d407ded completion: move --pickaxe-{all,regex} to __git_diff_common_options
23: 808e7db20cf = 22: fb23869dfbb completion: complete --diff-merges, its options and --no-diff-merges
24: d5fc5b04b00 = 23: eb9a6a06914 completion: complete --remerge-diff
25: da2cc42cbd4 = 24: 47e81c2add6 diff.c: mention completion above add_diff_options
--
gitgitgadget
^ permalink raw reply
* [PATCH v3 14/24] completion: complete --line-prefix
From: Philippe Blain via GitGitGadget @ 2023-06-26 16:24 UTC (permalink / raw)
To: git; +Cc: Eric Sunshine, Philippe Blain, Philippe Blain
In-Reply-To: <pull.1543.v3.git.1687796688.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 7246ced14ad..13d6730f33d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1757,7 +1757,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
--textconv --no-textconv --break-rewrites
--patch --no-patch --cc --combined-all-paths
--anchored= --compact-summary --ignore-matching-lines=
- --irreversible-delete
+ --irreversible-delete --line-prefix
"
# Options for diff/difftool
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 01/24] completion: add comments describing __git_diff_* globals
From: Philippe Blain via GitGitGadget @ 2023-06-26 16:24 UTC (permalink / raw)
To: git; +Cc: Eric Sunshine, Philippe Blain, Philippe Blain
In-Reply-To: <pull.1543.v3.git.1687796688.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>
Add descriptive comments for '__git_diff_common_options' and
'__git_diff_difftool_options', so that it is clearer when looking at
these variables to know in which command's completion they are used.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
contrib/completion/git-completion.bash | 2 ++
1 file changed, 2 insertions(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index dc95c34cc85..90fe292459b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1733,6 +1733,7 @@ __git_color_moved_opts="no default plain blocks zebra dimmed-zebra"
__git_color_moved_ws_opts="no ignore-space-at-eol ignore-space-change
ignore-all-space allow-indentation-change"
+# Options for the diff machinery (diff, log, show, stash, range-diff, ...)
__git_diff_common_options="--stat --numstat --shortstat --summary
--patch-with-stat --name-only --name-status --color
--no-color --color-words --no-renames --check
@@ -1757,6 +1758,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
--anchored=
"
+# Options for diff/difftool
__git_diff_difftool_options="--cached --staged --pickaxe-all --pickaxe-regex
--base --ours --theirs --no-index --relative --merge-base
$__git_diff_common_options"
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 08/24] completion: complete --find-object
From: Philippe Blain via GitGitGadget @ 2023-06-26 16:24 UTC (permalink / raw)
To: git; +Cc: Eric Sunshine, Philippe Blain, Philippe Blain
In-Reply-To: <pull.1543.v3.git.1687796688.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index dd6e12ad8f6..392fdbedd9f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1740,7 +1740,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
--color-moved --color-moved= --no-color-moved
--color-moved-ws= --no-color-moved-ws
--full-index --binary --abbrev --diff-filter=
- --find-copies
+ --find-copies --find-object
--find-copies-harder --ignore-cr-at-eol
--text --ignore-space-at-eol --ignore-space-change
--ignore-all-space --ignore-blank-lines --exit-code
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 09/24] completion: complete --find-renames
From: Philippe Blain via GitGitGadget @ 2023-06-26 16:24 UTC (permalink / raw)
To: git; +Cc: Eric Sunshine, Philippe Blain, Philippe Blain
In-Reply-To: <pull.1543.v3.git.1687796688.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 392fdbedd9f..0fa86dcde6f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1740,7 +1740,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
--color-moved --color-moved= --no-color-moved
--color-moved-ws= --no-color-moved-ws
--full-index --binary --abbrev --diff-filter=
- --find-copies --find-object
+ --find-copies --find-object --find-renames
--find-copies-harder --ignore-cr-at-eol
--text --ignore-space-at-eol --ignore-space-change
--ignore-all-space --ignore-blank-lines --exit-code
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 05/24] completion: complete --compact-summary
From: Philippe Blain via GitGitGadget @ 2023-06-26 16:24 UTC (permalink / raw)
To: git; +Cc: Eric Sunshine, Philippe Blain, Philippe Blain
In-Reply-To: <pull.1543.v3.git.1687796688.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 58ce64de9e2..4c43d13eef4 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1755,7 +1755,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
--indent-heuristic --no-indent-heuristic
--textconv --no-textconv --break-rewrites
--patch --no-patch --cc --combined-all-paths
- --anchored=
+ --anchored= --compact-summary
"
# Options for diff/difftool
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 15/24] completion: complete --no-relative
From: Philippe Blain via GitGitGadget @ 2023-06-26 16:24 UTC (permalink / raw)
To: git; +Cc: Eric Sunshine, Philippe Blain, Philippe Blain
In-Reply-To: <pull.1543.v3.git.1687796688.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>
Add --no-relative to __git_diff_common_options in the completion script,
and move --relative from __git_diff_difftool_options to
__git_diff_common_options since it applies to more than just diff and
difftool.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
contrib/completion/git-completion.bash | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 13d6730f33d..f4e773cb997 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1741,6 +1741,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
--color-moved-ws= --no-color-moved-ws
--full-index --binary --abbrev --diff-filter=
--find-copies --find-object --find-renames
+ --no-relative --relative
--find-copies-harder --ignore-cr-at-eol
--text --ignore-space-at-eol --ignore-space-change
--ignore-all-space --ignore-blank-lines --exit-code
@@ -1762,7 +1763,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
# Options for diff/difftool
__git_diff_difftool_options="--cached --staged --pickaxe-all --pickaxe-regex
- --base --ours --theirs --no-index --relative --merge-base
+ --base --ours --theirs --no-index --merge-base
--ita-invisible-in-index --ita-visible-in-index
$__git_diff_common_options"
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 13/24] completion: complete --ita-invisible-in-index and --ita-visible-in-index
From: Philippe Blain via GitGitGadget @ 2023-06-26 16:24 UTC (permalink / raw)
To: git; +Cc: Eric Sunshine, Philippe Blain, Philippe Blain
In-Reply-To: <pull.1543.v3.git.1687796688.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>
The options --ita-invisible-in-index and --ita-visible-in-index are
listed in diff-options.txt and so are included in the documentation of
commands which include this file (diff, diff-*, log, show, format-patch)
but they only make sense for diffs relating to the index. As such, add
them to '__git_diff_difftool_options' instead of
'__git_diff_common_options' since it makes more sense to add them there.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
contrib/completion/git-completion.bash | 1 +
1 file changed, 1 insertion(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index e74636ebe86..7246ced14ad 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1763,6 +1763,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
# Options for diff/difftool
__git_diff_difftool_options="--cached --staged --pickaxe-all --pickaxe-regex
--base --ours --theirs --no-index --relative --merge-base
+ --ita-invisible-in-index --ita-visible-in-index
$__git_diff_common_options"
_git_diff ()
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH 2/2] for-each-ref: add --count-matches option
From: Junio C Hamano @ 2023-06-26 16:14 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget; +Cc: git, vdye, me, mjcheetham, Derrick Stolee
In-Reply-To: <9121e027fb9f157878a9624ce6c834b69cd38472.1687792197.git.gitgitgadget@gmail.com>
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> Each of these approaches is wasteful as it requires sending the list of
> matching reference names across a pipe plus the cost of parsing that
> output.
>
> Instead, it would be helpful to have a Git command that counts the
> number of refs matching a list of patterns.
For for-each-ref (which can be reused by branch and tag for no
cost), I am on the fence but slightly in favor (partly because the
code has already been written). But I have to wonder where changes
that come from the above reasoning need to end, though.
Do we count branches and tags because "git branch --list | wc -l" is
too costly? Should we teach "git remote" the same trick? How about
"git stash list"? "git show-index $pack_index"? "git ls-files"?
How do we decide where to draw the line? When a command invocation
in a large repository can produce records exceeding a million?
^ permalink raw reply
* Re: [PATCH] apply: improve error messages when reading patch
From: Junio C Hamano @ 2023-06-26 15:58 UTC (permalink / raw)
To: Phillip Wood via GitGitGadget; +Cc: git, Taylor Blau, Phillip Wood
In-Reply-To: <pull.1552.git.1687772253869.gitgitgadget@gmail.com>
"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Commit f1c0e3946e (apply: reject patches larger than ~1 GiB, 2022-10-25)
> added a limit on the size of patch that apply will process to avoid
> integer overflows. The implementation re-used the existing error message
> for when we are unable to read the patch. This is unfortunate because (a) it
> does not signal to the user that the patch is being rejected because it
> is too large and (b) it uses error_errno() without setting errno.
Good description of the issue, and ...
> static int read_patch_file(struct strbuf *sb, int fd)
> {
> - if (strbuf_read(sb, fd, 0) < 0 || sb->len >= MAX_APPLY_SIZE)
> - return error_errno("git apply: failed to read");
> -
> + if (strbuf_read(sb, fd, 0) < 0)
> + return error_errno(_("failed to read patch"));
> + else if (sb->len >= MAX_APPLY_SIZE)
> + return error(_("patch too large"));
... quite obvious improvement. Nice.
Will queue. Thanks.
^ permalink raw reply
* Re: Clean up stale .gitignore and .gitattribute patterns
From: Junio C Hamano @ 2023-06-26 15:55 UTC (permalink / raw)
To: Sebastian Schuberth; +Cc: Jeff King, Git Mailing List
In-Reply-To: <CAHGBnuPO63Hi8mfA+MkAGES-gs0eNCDPG2FcPZT=YsnVzKd30A@mail.gmail.com>
Sebastian Schuberth <sschuberth@gmail.com> writes:
> Thanks Peff for the suggestion. I ended up scripting something via
> JGit [1], as we're anyway using it as part of our Gradle build system.
>
> PS: As a future idea, it might be good if "git mv" gives a hint about
> updating .gitattributes if files matching .gitattributes pattern are
> moved.
Interesting. "git mv hello.jpg hello.jpeg" would suggest updating
a "*.jpg <list of attribute definitions>" line in the .gitattributes
to begin with "*.jpeg"?
^ permalink raw reply
* Re: [PATCH v0 0/4] Remove obsolete Cygwin support from git-gui
From: Junio C Hamano @ 2023-06-26 15:52 UTC (permalink / raw)
To: Mark Levedahl; +Cc: git, adam, me, johannes.schindelin
In-Reply-To: <b7181a2d-ba97-eae8-6bf4-4fc4b0db64c2@gmail.com>
Mark Levedahl <mlevedahl@gmail.com> writes:
> I had originally organized as you suggest, no problem doing so
> again. What gave me pause was this paragraph I originally wrote for
> the cover letter:
>
> Patches 1/2 cause git-gui to function as it has for the last decade on
> Cygwin, but with Cygwin being detected. However, the browsing and
> shortcut creation menu items, removed in 2012 then re-added when is_Cygwin
> was fixed, do not work, and shortcut creation will crash git-gui if used.
> These are fixed in patches 3 / 4.
As you are removing (ancient) Cygwin specific code that did not work
with modern Cygwin at all in step [2/4], it is not so unexpected
that some stuff does still not work after that step. I am not sure
what your reservation exactly is, but if you are wondering if code
to disable browsing and shortcut creation on Cygwin temporarily
needs to be there in the same step (instead of crashing or not
working), it may make sense if and only if it is done with minimal
changes.
Thanks.
^ permalink raw reply
* RE: Git question rewarding git merge and its exit codes
From: rsbecker @ 2023-06-26 15:25 UTC (permalink / raw)
To: 'Iván Expósito', git
In-Reply-To: <CAPDGPD5UK8+Z0ksX-tv2Rs8ESZ+LCY-Tic-dZSGV0QNStFN=Pg@mail.gmail.com>
On Monday, June 26, 2023 8:15 AM, Iván Expósito wrote:
>Recently, we have been working on some automations for Git, especially auto-
>merging some branches. Just noticed that when using: git merge --commit --no-ff -m
>"test message", and no changes are needed, git returns "Already up-to-date" with
>exit code 0. Is this correct? Git hasn't done anything, so not sure how correct is to
>return 0 for success when nothing has been done. We have here a problem because
>the only way we can detect if a commit has been made or not is to check the return
>text, which is not the best idea for future-proof, or changes in the return text.
>
>Is this exit code as defined, or maybe is something we would need to look into for
>future improvements? Is there any other alternatives to detect what has happened,
>not comparing the standard output text?
If you want to script this, use --no-commit instead of --commit. After the merge runs, use git status --porcelain to determine whether merge performed any actions involving staging to unstaged (conflicts) files. This removes the need to use the completion code.
--Randall
--
Brief whoami: NonStop&UNIX developer since approximately
UNIX(421664400)
NonStop(211288444200000000)
-- In real life, I talk too much.
^ permalink raw reply
* [PATCH 1/2] for-each-ref: extract ref output loop
From: Derrick Stolee via GitGitGadget @ 2023-06-26 15:09 UTC (permalink / raw)
To: git; +Cc: vdye, gitster, me, mjcheetham, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.1548.git.1687792197.gitgitgadget@gmail.com>
From: Derrick Stolee <derrickstolee@github.com>
In preparation for a new output mode of 'git for-each-ref', extract the
loop that outputs references into a static method. No functional change
here.
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
builtin/for-each-ref.c | 58 ++++++++++++++++++++++++++----------------
1 file changed, 36 insertions(+), 22 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 695fc8f4a5e..ce34940e3e6 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -17,17 +17,47 @@ static char const * const for_each_ref_usage[] = {
NULL
};
-int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
+static void filter_and_output_refs(struct repository *r,
+ struct ref_array *array,
+ struct ref_filter *filter,
+ struct ref_format *format,
+ struct ref_sorting *sorting,
+ int maxcount,
+ int omit_empty)
{
int i;
+ struct strbuf err = STRBUF_INIT;
+ struct strbuf output = STRBUF_INIT;
+
+ filter_refs(array, filter, FILTER_REFS_ALL);
+ filter_ahead_behind(r, format, array);
+
+ ref_array_sort(sorting, array);
+
+ if (!maxcount || array->nr < maxcount)
+ maxcount = array->nr;
+ for (i = 0; i < maxcount; i++) {
+ strbuf_reset(&err);
+ strbuf_reset(&output);
+ if (format_ref_array_item(array->items[i], format, &output, &err))
+ die("%s", err.buf);
+ fwrite(output.buf, 1, output.len, stdout);
+ if (output.len || !omit_empty)
+ putchar('\n');
+ }
+
+ strbuf_release(&err);
+ strbuf_release(&output);
+}
+
+int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
+{
struct ref_sorting *sorting;
struct string_list sorting_options = STRING_LIST_INIT_DUP;
- int maxcount = 0, icase = 0, omit_empty = 0;
+ int maxcount = 0, icase = 0, omit_empty = 0, count_matches = 0;
struct ref_array array;
struct ref_filter filter;
struct ref_format format = REF_FORMAT_INIT;
- struct strbuf output = STRBUF_INIT;
- struct strbuf err = STRBUF_INIT;
int from_stdin = 0;
struct strvec vec = STRVEC_INIT;
@@ -101,25 +131,9 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
}
filter.match_as_path = 1;
- filter_refs(&array, &filter, FILTER_REFS_ALL);
- filter_ahead_behind(the_repository, &format, &array);
+ filter_and_output_refs(the_repository, &array, &filter, &format,
+ sorting, maxcount, omit_empty);
- ref_array_sort(sorting, &array);
-
- if (!maxcount || array.nr < maxcount)
- maxcount = array.nr;
- for (i = 0; i < maxcount; i++) {
- strbuf_reset(&err);
- strbuf_reset(&output);
- if (format_ref_array_item(array.items[i], &format, &output, &err))
- die("%s", err.buf);
- fwrite(output.buf, 1, output.len, stdout);
- if (output.len || !omit_empty)
- putchar('\n');
- }
-
- strbuf_release(&err);
- strbuf_release(&output);
ref_array_clear(&array);
free_commit_list(filter.with_commit);
free_commit_list(filter.no_commit);
--
gitgitgadget
^ permalink raw reply related
* [PATCH 2/2] for-each-ref: add --count-matches option
From: Derrick Stolee via GitGitGadget @ 2023-06-26 15:09 UTC (permalink / raw)
To: git; +Cc: vdye, gitster, me, mjcheetham, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.1548.git.1687792197.gitgitgadget@gmail.com>
From: Derrick Stolee <derrickstolee@github.com>
In order to count references of different types based on their initial
prefixes, there are two current approaches:
1. Run 'git for-each-ref' on all refs and parse the output to count
those that match each prefix.
2. Run 'git for-each-ref "prefix/"' and pipe that output to 'wc -l' to
count the number of output lines.
Each of these approaches is wasteful as it requires sending the list of
matching reference names across a pipe plus the cost of parsing that
output.
Instead, it would be helpful to have a Git command that counts the
number of refs matching a list of patterns.
This change adds a new mode, '--count-matches' to 'git for-each-ref' so
we can make use of the existing infrastructure around parsing refspecs
in the correct places. '--count' is already taken as a "maximum number"
of refs to output.
An alternative approach could be to make a brand-new builtin that is
focused on counting ref matches. This would involve duplicating a bit of
code around parsing refpecs, but would not be terribly difficult. The
actual overlap of implementation here with 'git for-each-ref' is small
enough that we could instead extract this elsewhere. My gut feeling is
that this behavior doesn't merit a new builtin.
The implementation is extremely simple: iterate through all references
and compare each ref to each refspec. On a match, increment a counter
value for that refspec.
In the end, output each refspec followed by the count.
If all given refspecs were prefixes, then it is tempting to instead use
a counting behavior that we can use in things like "how many OIDs start
with this short hex string?" Navigating to the start and end of the
range of refs starting with that prefix is possible in the packed-refs
file. However, the records do not have a constant size so we cannot
infer the number of references in that range using the current format.
(Perhaps, in the future we will have a ref storage system that allows
this kind of counting to be easy to do in O(log N) time.)
A new performance test is included to check the performance of iterating
through these references and counting them appropriately. This presents
a 3x improvement over the trivial piping through to 'wc -l', and that
assumes there is a single pattern to match instead of multiple. We can
see that testing three patterns sequentially adds to the total time, but
doing a single process with --count-match continues to be as fast. (It's
difficult to tell since it _also_ matches the sum of the three for this
example repo.)
Test this tree
----------------------------------------------------------------------------
1501.2: count refs/heads/: git for-each-ref | wc -l 0.01(0.00+0.01)
1501.3: count refs/heads/: git for-each-ref --count-match 0.00(0.00+0.00)
1501.4: count refs/tags/: git for-each-ref | wc -l 0.02(0.00+0.02)
1501.5: count refs/tags/: git for-each-ref --count-match 0.00(0.00+0.00)
1501.6: count refs/remotes: git for-each-ref | wc -l 0.15(0.08+0.07)
1501.7: count refs/remotes: git for-each-ref --count-match 0.04(0.01+0.02)
1501.8: count all patterns: git for-each-ref | wc -l 0.18(0.08+0.10)
1501.9: count all patterns: git for-each-ref --count-match 0.04(0.02+0.02)
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
Documentation/git-for-each-ref.txt | 5 ++++
builtin/for-each-ref.c | 26 +++++++++++++++--
ref-filter.c | 47 ++++++++++++++++++++++++++++++
ref-filter.h | 7 +++++
t/perf/p1501-ref-iteration.sh | 35 ++++++++++++++++++++++
t/t6300-for-each-ref.sh | 28 ++++++++++++++++++
6 files changed, 145 insertions(+), 3 deletions(-)
create mode 100755 t/perf/p1501-ref-iteration.sh
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 1e215d4e734..74018755ed5 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -42,6 +42,11 @@ OPTIONS
`<pattern>`. This option makes it stop after showing
that many refs.
+--count-matches::
+ Instead of listing references according to the given format, output
+ the number of references that match each pattern. Incompatible with
+ `--format`, `--sort`, and `--count`.
+
--sort=<key>::
A field name to sort on. Prefix `-` to sort in
descending order of the value. When unspecified,
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index ce34940e3e6..e3db719bb87 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -50,6 +50,17 @@ static void filter_and_output_refs(struct repository *r,
strbuf_release(&output);
}
+static void count_and_output_patterns(struct ref_filter *filter)
+{
+ uint32_t *counts = count_ref_patterns(filter);
+
+ for (int i = 0; filter->name_patterns && filter->name_patterns[i]; i++)
+ fprintf(stdout, "%s %"PRIu32"\n",
+ filter->name_patterns[i], counts[i]);
+
+ free(counts);
+}
+
int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
{
struct ref_sorting *sorting;
@@ -60,6 +71,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
struct ref_format format = REF_FORMAT_INIT;
int from_stdin = 0;
struct strvec vec = STRVEC_INIT;
+ const char *initial_format = "%(objectname) %(objecttype)\t%(refname)";
struct option opts[] = {
OPT_BIT('s', "shell", &format.quote_style,
@@ -72,6 +84,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
N_("quote placeholders suitably for Tcl"), QUOTE_TCL),
OPT_BOOL(0, "omit-empty", &omit_empty,
N_("do not output a newline after empty formatted refs")),
+ OPT_BOOL(0, "count-matches", &count_matches,
+ N_("output number of references matching each pattern instead of any other output")),
OPT_GROUP(""),
OPT_INTEGER( 0 , "count", &maxcount, N_("show only <n> matched refs")),
@@ -93,7 +107,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
memset(&array, 0, sizeof(array));
memset(&filter, 0, sizeof(filter));
- format.format = "%(objectname) %(objecttype)\t%(refname)";
+ format.format = initial_format;
git_config(git_default_config, NULL);
@@ -102,6 +116,9 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
error("invalid --count argument: `%d'", maxcount);
usage_with_options(for_each_ref_usage, opts);
}
+ if (count_matches &&
+ (maxcount || format.format != initial_format || sorting_options.nr))
+ die("--count-matches incompatible with --count, --format, or --sort");
if (HAS_MULTI_BITS(format.quote_style)) {
error("more than one quoting style?");
usage_with_options(for_each_ref_usage, opts);
@@ -131,8 +148,11 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
}
filter.match_as_path = 1;
- filter_and_output_refs(the_repository, &array, &filter, &format,
- sorting, maxcount, omit_empty);
+ if (count_matches)
+ count_and_output_patterns(&filter);
+ else
+ filter_and_output_refs(the_repository, &array, &filter, &format,
+ sorting, maxcount, omit_empty);
ref_array_clear(&array);
free_commit_list(filter.with_commit);
diff --git a/ref-filter.c b/ref-filter.c
index 4991cd4f7a8..4e02a4ae98d 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2560,6 +2560,53 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
return ret;
}
+struct filter_and_counts {
+ struct ref_filter *filter;
+ uint32_t *counts;
+};
+
+static int ref_filter_counter(const char *refname,
+ const struct object_id *oid,
+ int flag, void *cb_data)
+{
+ struct filter_and_counts *fc = cb_data;
+ const char **pattern = fc->filter->name_patterns;
+ size_t namelen = strlen(refname);
+ unsigned flags = fc->filter->ignore_case ? WM_CASEFOLD : 0;
+
+ for (int i = 0; *pattern; i++, pattern++) {
+ const char *p = *pattern;
+ int plen = strlen(p);
+
+ if ((plen <= namelen) &&
+ !strncmp(refname, p, plen) &&
+ (refname[plen] == '\0' ||
+ refname[plen] == '/' ||
+ p[plen-1] == '/'))
+ fc->counts[i]++;
+ else if (!wildmatch(p, refname, flags))
+ fc->counts[i]++;
+ }
+ return 0;
+}
+
+uint32_t *count_ref_patterns(struct ref_filter *filter)
+{
+ int size = 0;
+ struct filter_and_counts fc = {
+ .filter = filter,
+ };
+
+ while (filter->name_patterns[size])
+ size++;
+
+ CALLOC_ARRAY(fc.counts, size);
+
+ for_each_fullref_in_pattern(filter, ref_filter_counter, &fc);
+
+ return fc.counts;
+}
+
static int compare_detached_head(struct ref_array_item *a, struct ref_array_item *b)
{
if (!(a->kind ^ b->kind))
diff --git a/ref-filter.h b/ref-filter.h
index 430701cfb76..b7e5a4f6a80 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -141,6 +141,13 @@ char *get_head_description(void);
/* Set up translated strings in the output. */
void setup_ref_filter_porcelain_msg(void);
+/*
+ * Iterate over all references, counting how many match each filter
+ * pattern. Returns an array of the counts with the ith count matching the
+ * ith filter->name_pattern entry.
+ */
+uint32_t *count_ref_patterns(struct ref_filter *filter);
+
/*
* Print a single ref, outside of any ref-filter. Note that the
* name must be a fully qualified refname.
diff --git a/t/perf/p1501-ref-iteration.sh b/t/perf/p1501-ref-iteration.sh
new file mode 100755
index 00000000000..609487d20b0
--- /dev/null
+++ b/t/perf/p1501-ref-iteration.sh
@@ -0,0 +1,35 @@
+#!/bin/sh
+
+test_description='Ref iteration performance tests'
+. ./perf-lib.sh
+
+test_perf_large_repo
+
+# Optimize ref backend store
+test_expect_success 'setup' '
+ git pack-refs
+'
+
+for pattern in "refs/heads/" "refs/tags/" "refs/remotes"
+do
+ test_perf "count $pattern: git for-each-ref | wc -l" "
+ git for-each-ref $pattern | wc -l
+ "
+
+ test_perf "count $pattern: git for-each-ref --count-match" "
+ git for-each-ref --count-matches $pattern
+ "
+done
+
+test_perf "count all patterns: git for-each-ref | wc -l" "
+ git for-each-ref refs/heads/ | wc -l &&
+ git for-each-ref refs/tags/ | wc -l &&
+ git for-each-ref refs/remotes/ | wc -l
+"
+
+test_perf "count all patterns: git for-each-ref --count-match" "
+ git for-each-ref --count-matches \
+ refs/heads/ refs/tags/ refs/remotes/
+"
+
+test_done
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 5c00607608a..001382956e4 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -486,6 +486,34 @@ for i in "--perl --shell" "-s --python" "--python --tcl" "--tcl --perl"; do
"
done
+test_expect_success '--count-matches incompatible with some options' '
+ for opt in "--format=x" "--sort=refname" "--count=10"
+ do
+ test_must_fail git for-each-ref --count-matches $opt refs/heads/ 2>err &&
+ grep "count-matches incompatible" err || return 1
+ done
+'
+
+test_expect_success '--count-matches tallies the number matching each refspec' '
+ git init multi-refs &&
+ test_commit -C multi-refs A &&
+ git -C multi-refs branch A &&
+ git -C multi-refs branch pre/A &&
+ test_commit -C multi-refs --no-tag B &&
+ git -C multi-refs branch B &&
+ git -C multi-refs for-each-ref --count-matches \
+ refs/heads/ refs/heads/pre/ refs/tags/ "*A*" >actual &&
+
+ cat >expect <<-EOF &&
+ refs/heads/ 4
+ refs/heads/pre/ 1
+ refs/tags/ 1
+ *A* 3
+ EOF
+
+ test_cmp expect actual
+'
+
test_expect_success 'setup for upstream:track[short]' '
test_commit two
'
--
gitgitgadget
^ permalink raw reply related
* [PATCH 0/2] [RFC] for-each-ref: add --count-matches mode
From: Derrick Stolee via GitGitGadget @ 2023-06-26 15:09 UTC (permalink / raw)
To: git; +Cc: vdye, gitster, me, mjcheetham, Derrick Stolee
I'm leaving this as an RFC for now because I can't decide if this new option
in git for-each-ref is good or if this needs an entirely new builtin. I'm
open to whatever people think is best, I'd just like a way to count matches
based on refspecs.
Thanks, -Stolee
Derrick Stolee (2):
for-each-ref: extract ref output loop
for-each-ref: add --count-matches option
Documentation/git-for-each-ref.txt | 5 ++
builtin/for-each-ref.c | 80 +++++++++++++++++++++---------
ref-filter.c | 47 ++++++++++++++++++
ref-filter.h | 7 +++
t/perf/p1501-ref-iteration.sh | 35 +++++++++++++
t/t6300-for-each-ref.sh | 28 +++++++++++
6 files changed, 179 insertions(+), 23 deletions(-)
create mode 100755 t/perf/p1501-ref-iteration.sh
base-commit: d7d8841f67f29e6ecbad85a11805c907d0f00d5d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1548%2Fderrickstolee%2Ffor-each-ref-count-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1548/derrickstolee/for-each-ref-count-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1548
--
gitgitgadget
^ permalink raw reply
* Re: [RFC PATCH 1/2] Add C TAP harness
From: Phillip Wood @ 2023-06-26 13:15 UTC (permalink / raw)
To: Linus Arver, phillip.wood
Cc: git, Ævar Arnfjörð Bjarmason, Josh Steadmon,
Calvin Wan
In-Reply-To: <owly8rcc3j1u.fsf@fine.c.googlers.com>
Hi Linus
On 21/06/2023 16:57, Linus Arver wrote:
> Hello,
>
> Phillip Wood <phillip.wood123@gmail.com> writes:
>> I'd be interested to hear what you think of the alternative
>> approach in the patch below. The patch can be fetched with
>>
>> git fetch https://github.com/phillipwood/git unit-tests
>
> I tried out your unit testing framework to see how it works in practice.
> It has most of what I'd expect in a test framework, e.g. showing file
> and line number in a test failure. Here are some changes I'd like to
> see:
Thanks for taking the time to try this out.
> - Make the 'TEST' macro accept the test description first. Or, keep the
> 'TEST' macro but also name a new macro 'IT' that accepts the
> description first, to encourage usage that reads in a
> behavior-driven-development (BDD) style, like 'IT("should accept foo",
> t_bar(...))'. I find some test descriptions easier to write this way.
The test description is a printf style format string followed by
arguments. This allows parameterized tests to include the parameter
values in the description to aid debugging but it means the test
function must be the first parameter. We could have IT("should accept
%d", t(), i) but that would be a bit weird.
I hope that we will end up with some guidelines for writing unit tests
that recommend testing the expect behavior and to avoid testing
implementation details. I'm don't think we necessarily need an IT()
macro to do that.
> - The 'check_int(result, ==, expected)' usage would be better without
> the commas, like 'check_int(result == expected)'. Is this possible?
I don't particularly like the comma's either but in order to have decent
diagnostic messages we need to pass 'result' and 'expected' as separate
parameters so we can print them. We could have separate macros for
different comparisons check_int_eq(a, b), check_int_lt(a, b), ... I
don't have a strong preference either way.
> - It would be nice to report the test name for failing tests, so that
> this output can be parsed to check for the failing test name.
I'm not quite sure what you mean here. The test name in printed in the
"not ok" lines for failing tests as shown in the output from your tests
below.
> There are other smaller changes I'd like (such as optional
> colorization), but I think these bits can be sorted out much later.
Sure
> My patch can be found at
>
> git fetch https://github.com/listx/git trailer-unit-tests
>
> and also inline at the bottom of this email message.
>
> Build and test with
>
> make unit-tests && t/unit-tests/t-trailer
>
> Sample output:
>
> $ make -j47 unit-tests && t/unit-tests/t-trailer
> CC t/unit-tests/t-trailer.o
> LINK t/unit-tests/t-trailer
> ok 1 - empty trailer_item and arg_item
> ok 2 - identical
> ok 3 - case should not matter
> ok 4 - arg_item is longer than trailer_item
> ok 5 - trailer_item is longer than arg_item
> ok 6 - no similarity
> ok 7 - accept WHERE_AFTER
> ok 8 - accept WHERE_END
> ok 9 - reject WHERE_END
> ok 10 - token with trailing punctuation (colon)
> ok 11 - token without trailing punctuation
> ok 12 - token with spaces with trailing punctuation (colon)
> ok 13 - token with spaces without trailing punctuation
> ok 14 - token with leading non-separator punctuation
> ok 15 - token with leading non-separator punctuation and space
> ok 16 - one-letter token
> ok 17 - token with punctuation in-between
> ok 18 - token with multiple leading punctuation chars
> # check "result == expected" failed at t/unit-tests/t-trailer.c:25
> # left: 1
> # right: 0
> not ok 19 - empty trailer_item
This test fail because same_token() tests that the longer of its two
arguments starts with the shorter argument, not that the two arguments
are equal. As the shorter argument is empty in this case it returns
true, not false.
> # check "result == expected" failed at t/unit-tests/t-trailer.c:25
> # left: 1
> # right: 0
> not ok 20 - empty arg_item
> 1..20
>
> Note that some of the tests I expect to fail are passing.
Which ones are those?
> These may be
> bugs but I am not sure because I don't fully understand the
> interpret-trailers machinery yet.
Thanks for sharing these tests, it is helpful to see examples of the
test framework being used. In this case the tests are checking static
functions from trailer.c. This mean that you have to export a number of
functions and structs that are really implementation details. Where
possible I think we want to concentrate our testing effort on the public
API as (a) that is what matters to callers and (b) it avoids having to
export internal functions and encourages people to test behavior rather
than implementation details. There may be some cases where there are
internal helpers that would benefit from tests. In those cases we can
either hide the export behind a #define so it is only visible to the
test code or just #include the file being tested in the test file (gross
but works). An alternative to both of those is for the tests to live in
the implementation files and have them conditionally compiled (a bit
like rust).
Thanks again for your comments, Best Wishes
Phillip
> Linus
>
> ---- >8 ----
> From e5907b328d35eb37cfd1feb3a2431cb672beb4c8 Mon Sep 17 00:00:00 2001
> From: Linus Arver <linusa@google.com>
> Date: Thu, 15 Jun 2023 18:25:47 -0700
> Subject: [PATCH] add some unit tests for interpret-trailers
>
> Signed-off-by: Linus Arver <linusa@google.com>
> ---
> Makefile | 5 +++
> t/unit-tests/t-trailer.c | 77 ++++++++++++++++++++++++++++++++++++++++
> trailer.c | 33 ++---------------
> trailer.h | 32 +++++++++++++++++
> 4 files changed, 117 insertions(+), 30 deletions(-)
> create mode 100644 t/unit-tests/t-trailer.c
>
> diff --git a/Makefile b/Makefile
> index f0ca5bae4b..9d68518168 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2668,6 +2668,7 @@ scalar-objs: $(SCALAR_OBJS)
>
> UNIT_TEST_PROGRAMS += t-basic
> UNIT_TEST_PROGRAMS += t-strbuf
> +UNIT_TEST_PROGRAMS += t-trailer
> UNIT_TEST_PROGS = $(patsubst %,t/unit-tests/%$X,$(UNIT_TEST_PROGRAMS))
> UNIT_TEST_OBJS = $(patsubst %,t/unit-tests/%.o,$(UNIT_TEST_PROGRAMS))
> UNIT_TEST_OBJS += t/unit-tests/test-lib.o
> @@ -3848,5 +3849,9 @@ t/unit-tests/t-strbuf$X: t/unit-tests/t-strbuf.o t/unit-tests/test-lib.o $(GITLI
> $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \
> $(filter %.o,$^) $(filter %.a,$^) $(LIBS)
>
> +t/unit-tests/t-trailer$X: t/unit-tests/t-trailer.o t/unit-tests/test-lib.o builtin/interpret-trailers.o $(GITLIBS) GIT-LDFLAGS
> + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \
> + $(filter %.o,$^) $(filter %.a,$^) $(LIBS)
> +
> .PHONY: unit-tests
> unit-tests: $(UNIT_TEST_PROGS)
> diff --git a/t/unit-tests/t-trailer.c b/t/unit-tests/t-trailer.c
> new file mode 100644
> index 0000000000..150b5606fa
> --- /dev/null
> +++ b/t/unit-tests/t-trailer.c
> @@ -0,0 +1,77 @@
> +#include "test-lib.h"
> +#include "trailer.h"
> +
> +static void t_token_len_without_separator(const char *token, size_t expected)
> +{
> + size_t result;
> + result = token_len_without_separator(token, strlen(token));
> + check_uint(result, ==, expected);
> +}
> +
> +static void t_after_or_end(enum trailer_where where, int expected)
> +{
> + size_t result;
> + result = after_or_end(where);
> + check_int(result, ==, expected);
> +}
> +
> +
> +static void t_same_token(char *a, char *b, int expected)
> +{
> + struct trailer_item trailer_item = { .token = a };
> + struct arg_item arg_item = { .token = b };
> + size_t result;
> + result = same_token(&trailer_item, &arg_item);
> + check_int(result, ==, expected);
> +}
> +
> +int cmd_main(int argc, const char **argv)
> +{
> + TEST(t_same_token("", "", 1),
> + "empty trailer_item and arg_item");
> + TEST(t_same_token("foo", "foo", 1),
> + "identical");
> + TEST(t_same_token("foo", "FOO", 1),
> + "case should not matter");
> + TEST(t_same_token("foo", "foobar", 1),
> + "arg_item is longer than trailer_item");
> + TEST(t_same_token("foobar", "foo", 1),
> + "trailer_item is longer than arg_item");
> + TEST(t_same_token("foo", "bar", 0),
> + "no similarity");
> +
> + TEST(t_after_or_end(WHERE_AFTER, 1), "accept WHERE_AFTER");
> + TEST(t_after_or_end(WHERE_END, 1), "accept WHERE_END");
> + TEST(t_after_or_end(WHERE_DEFAULT, 0), "reject WHERE_END");
> +
> + TEST(t_token_len_without_separator("Signed-off-by:", 13),
> + "token with trailing punctuation (colon)");
> + TEST(t_token_len_without_separator("Signed-off-by", 13),
> + "token without trailing punctuation");
> + TEST(t_token_len_without_separator("Foo bar:", 7),
> + "token with spaces with trailing punctuation (colon)");
> + TEST(t_token_len_without_separator("Foo bar", 7),
> + "token with spaces without trailing punctuation");
> + TEST(t_token_len_without_separator("-Foo bar:", 8),
> + "token with leading non-separator punctuation");
> + TEST(t_token_len_without_separator("- Foo bar:", 9),
> + "token with leading non-separator punctuation and space");
> + TEST(t_token_len_without_separator("F:", 1),
> + "one-letter token");
> + TEST(t_token_len_without_separator("abc%de#f@", 8),
> + "token with punctuation in-between");
> + TEST(t_token_len_without_separator(":;*%_.#f@", 8),
> + "token with multiple leading punctuation chars");
> +
> + /*
> + * These tests fail unexpectedly. They are probably bugs, although it may be
> + * the case that these bugs never bubble up to the user because of other
> + * checks we do elsewhere up the stack.
> + */
> + TEST(t_same_token("", "foo", 0),
> + "empty trailer_item");
> + TEST(t_same_token("foo", "", 0),
> + "empty arg_item");
> +
> + return test_done();
> +}
> diff --git a/trailer.c b/trailer.c
> index a2c3ed6f28..9f59d8d7a6 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -13,35 +13,8 @@
> * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
> */
>
> -struct conf_info {
> - char *name;
> - char *key;
> - char *command;
> - char *cmd;
> - enum trailer_where where;
> - enum trailer_if_exists if_exists;
> - enum trailer_if_missing if_missing;
> -};
> -
> static struct conf_info default_conf_info;
>
> -struct trailer_item {
> - struct list_head list;
> - /*
> - * If this is not a trailer line, the line is stored in value
> - * (excluding the terminating newline) and token is NULL.
> - */
> - char *token;
> - char *value;
> -};
> -
> -struct arg_item {
> - struct list_head list;
> - char *token;
> - char *value;
> - struct conf_info conf;
> -};
> -
> static LIST_HEAD(conf_head);
>
> static char *separators = ":";
> @@ -62,7 +35,7 @@ static const char *git_generated_prefixes[] = {
> pos != (head); \
> pos = is_reverse ? pos->prev : pos->next)
>
> -static int after_or_end(enum trailer_where where)
> +int after_or_end(enum trailer_where where)
> {
> return (where == WHERE_AFTER) || (where == WHERE_END);
> }
> @@ -73,14 +46,14 @@ static int after_or_end(enum trailer_where where)
> * 13, stripping the trailing punctuation but retaining
> * internal punctuation.
> */
> -static size_t token_len_without_separator(const char *token, size_t len)
> +size_t token_len_without_separator(const char *token, size_t len)
> {
> while (len > 0 && !isalnum(token[len - 1]))
> len--;
> return len;
> }
>
> -static int same_token(struct trailer_item *a, struct arg_item *b)
> +int same_token(struct trailer_item *a, struct arg_item *b)
> {
> size_t a_len, b_len, min_len;
>
> diff --git a/trailer.h b/trailer.h
> index 795d2fccfd..b2031eb305 100644
> --- a/trailer.h
> +++ b/trailer.h
> @@ -146,4 +146,36 @@ int trailer_iterator_advance(struct trailer_iterator *iter);
> */
> void trailer_iterator_release(struct trailer_iterator *iter);
>
> +int after_or_end(enum trailer_where where);
> +size_t token_len_without_separator(const char *token, size_t len);
> +
> +struct conf_info {
> + char *name;
> + char *key;
> + char *command;
> + char *cmd;
> + enum trailer_where where;
> + enum trailer_if_exists if_exists;
> + enum trailer_if_missing if_missing;
> +};
> +
> +struct trailer_item {
> + struct list_head list;
> + /*
> + * If this is not a trailer line, the line is stored in value
> + * (excluding the terminating newline) and token is NULL.
> + */
> + char *token;
> + char *value;
> +};
> +
> +struct arg_item {
> + struct list_head list;
> + char *token;
> + char *value;
> + struct conf_info conf;
> +};
> +
> +int same_token(struct trailer_item *a, struct arg_item *b);
> +
> #endif /* TRAILER_H */
^ permalink raw reply
* Git question rewarding git merge and its exit codes
From: Iván Expósito @ 2023-06-26 12:14 UTC (permalink / raw)
To: git
Hi, my name is Ivan.
Recently, we have been working on some automations for Git, especially
auto-merging some branches. Just noticed that when using: git merge
--commit --no-ff -m "test message", and no changes are needed, git
returns "Already up-to-date" with exit code 0. Is this correct? Git
hasn't done anything, so not sure how correct is to return 0 for
success when nothing has been done. We have here a problem because the
only way we can detect if a commit has been made or not is to check
the return text, which is not the best idea for future-proof, or
changes in the return text.
Is this exit code as defined, or maybe is something we would need to
look into for future improvements? Is there any other alternatives to
detect what has happened, not comparing the standard output text?
Thanks for your work, If any information is needed, don't hesitate to
contact me.
Thanks,
Ivan
^ permalink raw reply
* [PATCH] apply: improve error messages when reading patch
From: Phillip Wood via GitGitGadget @ 2023-06-26 9:37 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Phillip Wood, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Commit f1c0e3946e (apply: reject patches larger than ~1 GiB, 2022-10-25)
added a limit on the size of patch that apply will process to avoid
integer overflows. The implementation re-used the existing error message
for when we are unable to read the patch. This is unfortunate because (a) it
does not signal to the user that the patch is being rejected because it
is too large and (b) it uses error_errno() without setting errno.
This patch adds a specific error message for the case when a patch is
too large. It also updates the existing message to make it clearer that
it is the patch that cannot be read rather than any other file and marks
both messages for translation. The "git apply" prefix is also dropped to
match most of the rest of the error messages in apply.c (there are still
a few error messages that prefixed with "git apply" and are not marked
for translation after this patch). The test added in f1c0e3946e is
updated accordingly.
Reported-by: Premek Vysoky <Premek.Vysoky@microsoft.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
apply: improve error messages when reading patch
I haven't changed it but I found the test a bit confusing as it
superficially looks like it is generating a valid patch that is too
large, but it isn't actually a valid patch because the changed line is
lacking a leading '+' and a trailing '\n'. As we are checking for the
error message that does not matter but it might be clearer to just do
sz=$((1024 * 1024 * 1023)) &&
test-tool genzeros $sz | test_must_fail git apply &&
grep "patch too large" err
if we don't care about the patch being valid. Alternatively we could
create a valid patch with
sz=$((1024 * 1024 * 1023)) &&
{
cat <<-\EOF &&
diff --git a/file b/file
new file mode 100644
--- /dev/null
+++ b/file
@@ -0,0 +1 @@
EOF
printf "+%${sz}s\n" x
} | test_must_fail git apply 2>err &&
grep "patch too large" err
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1552%2Fphillipwood%2Fapply-error-message-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1552/phillipwood/apply-error-message-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1552
apply.c | 7 ++++---
t/t4141-apply-too-large.sh | 2 +-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/apply.c b/apply.c
index 6212ab3a1b3..21c7f92ada8 100644
--- a/apply.c
+++ b/apply.c
@@ -410,9 +410,10 @@ static void say_patch_name(FILE *output, const char *fmt, struct patch *patch)
static int read_patch_file(struct strbuf *sb, int fd)
{
- if (strbuf_read(sb, fd, 0) < 0 || sb->len >= MAX_APPLY_SIZE)
- return error_errno("git apply: failed to read");
-
+ if (strbuf_read(sb, fd, 0) < 0)
+ return error_errno(_("failed to read patch"));
+ else if (sb->len >= MAX_APPLY_SIZE)
+ return error(_("patch too large"));
/*
* Make sure that we have some slop in the buffer
* so that we can do speculative "memcmp" etc, and
diff --git a/t/t4141-apply-too-large.sh b/t/t4141-apply-too-large.sh
index 58742d4fc5d..20cc1209f62 100755
--- a/t/t4141-apply-too-large.sh
+++ b/t/t4141-apply-too-large.sh
@@ -17,7 +17,7 @@ test_expect_success EXPENSIVE 'git apply rejects patches that are too large' '
EOF
test-tool genzeros
} | test_copy_bytes $sz | test_must_fail git apply 2>err &&
- grep "git apply: failed to read" err
+ grep "patch too large" err
'
test_done
base-commit: fe86abd7511a9a6862d5706c6fa1d9b57a63ba09
--
gitgitgadget
^ permalink raw reply related
* Re: Clean up stale .gitignore and .gitattribute patterns
From: Sebastian Schuberth @ 2023-06-26 7:51 UTC (permalink / raw)
To: Jeff King; +Cc: Git Mailing List
In-Reply-To: <20230624011234.GA95358@coredump.intra.peff.net>
Thanks Peff for the suggestion. I ended up scripting something via
JGit [1], as we're anyway using it as part of our Gradle build system.
PS: As a future idea, it might be good if "git mv" gives a hint about
updating .gitattributes if files matching .gitattributes pattern are
moved.
[1]: https://github.com/oss-review-toolkit/ort/pull/7195/commits/e01945d41012db2d0bc2e53d7be4abd513888ba6
--
Sebastian Schuberth
On Sat, Jun 24, 2023 at 3:12 AM Jeff King <peff@peff.net> wrote:
>
> On Fri, Jun 23, 2023 at 05:29:42PM +0200, Sebastian Schuberth wrote:
>
> > is there a command to easily check patterns in .gitignore and
> > .gitattributes to still match something? I'd like to remove / correct
> > patterns that don't match anything anymore due to (re)moved files.
>
> I don't think there's a solution that matches "easily", but you can do a
> bit with some scripting. See below.
>
> For checking .gitignore, I don't think you can ever say (at the git
> level) that a certain pattern is useless, because it is inherently about
> matching things that not tracked, and hence generated elsewhere. So if
> you have a "*.foo" pattern, you can check if it matches anything
> _currently_ in your working tree, but if it doesn't that may mean that
> you simply did not trigger the build rule that makes the garbage ".foo"
> file.
>
> So with that caveat, we can ask Git which rules _do_ have a match, and
> then eliminate them as "definitely useful", and print the others. The
> logic is sufficiently tricky that I turned to perl:
>
> -- >8 show-unmatched-ignore.pl 8< --
> #!/usr/bin/perl
>
> # The general idea here is to read "filename:linenr ..." output from
> # "check-ignore -v". For each filename we learn about, we'll load the
> # complete set of lines into an array and then "cross them off" as
> # check-ignore tells us they were used.
> #
> # Note that we'd fail to mention an ignore file which matches nothing.
> # Probably the list of filenames could be generated independently. I'll
> # that as an exercise for the reader.
> while (<>) {
> /^(.*?):(\d+):/
> or die "puzzling input: $_";
> if (!defined $files{$1}) {
> $files{$1} = do {
> open(my $fh, '<', $1)
> or die "unable to open $1: $!";
> [<$fh>]
> };
> }
> $files{$1}->[$2] = undef;
> }
>
> # With that done, whatever is left is unmatched. Print them.
> for my $fn (sort keys(%files)) {
> my $lines = $files{$fn};
> for my $nr (1..@$lines) {
> my $line = $lines->[$nr-1];
> print "$fn:$nr $line" if defined $line;
> }
> }
> -- >8 --
>
> And you'd use it something like:
>
> git ls-files -o |
> git check-ignore --stdin -v |
> perl show-unmatched-ignore.pl
>
> Pretty clunky, but it works OK in git.git (and shows that there are many
> "not matched but probably still useful" entries; e.g., "*.dll" will
> never match for me on Linux, but is probably something we still want to
> keep). So I wouldn't use it as an automated tool, but it might give a
> starting point for a human looking to clean things up manually.
>
> For attributes, I think the situation is better; we only need them to
> match tracked files (though technically speaking, you may want to keep
> attributes around for historical files as we use the checked-out
> attributes during "git log", etc). Unfortunately we don't have an
> equivalent of "-v" for check-attr. It might be possible to add that ,but
> in the meantime, the best I could come up with is to munge each pattern
> to add a sentinel attribute, and see if it matches anything.
>
> Something like:
>
> # Maybe also pipe in .git/info/attributes and core.attributesFile
> # if you want to check those.
> git ls-files '.gitattributes' '**/.gitattributes' |
> while read fn; do
> lines=$(wc -l <"$fn")
> mv "$fn" "$fn.orig"
> nr=1
> while test $nr -le $lines; do
> sed "${nr}s/$/ is-matched/" <"$fn.orig" >"$fn"
> git ls-files | git check-attr --stdin is-matched |
> grep -q "is-matched: set" ||
> echo "$fn:$nr $(sed -n ${nr}p "$fn.orig")"
> nr=$((nr+1))
> done
> mv "$fn.orig" "$fn"
> done
>
> It produces no output in git.git (we are using all of our attributes),
> but you can add a useless one like:
>
> echo '*.c -diff' >>Documentation/.gitattributes
>
> and then the loop yields:
>
> Documentation/.gitattributes:2 *.c -diff
>
> So I definitely wouldn't call any of that "easy", but it may help you.
>
> -Peff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox