From: Kristoffer Haugsbakk <code@xxxxxxxxxxxxxxx>
Based on the recent i-still-use-that reports about whatchanged, improve
the error reporting with this command in mind:
1. Give more possible actions instead of just (only) asking them to send
an email
2. Hint how to replace their git-whatchanged(1) use with git-log(1) or
an alias `whatchanged` (you can alias deprecated commands now)
3. Minor documentation changes
4. Add `deprecated` to `git --list-cmds`
§ What the errors now look like
$ ./git whatchanged
'git whatchanged' is nominated for removal.
hint: You can replace 'git whatchanged <opts>' with:
hint: git log <opts> --raw --no-merges
hint: Or make an alias:
hint: git config set --global alias.whatchanged 'log --raw --no-merges'
If you still use this command, here's what you can do:
- read https://git-scm.com/docs/BreakingChanges.html
- check if anyone has discussed this on the mailing
list and if they came up with something that can
help you: https://lore.kernel.org/git/?q=git%20whatchanged
- send an email to <git@xxxxxxxxxxxxxxx> to let us
know that you still use this command and were unable
to determine a suitable replacement
fatal: refusing to run without --i-still-use-this
$ ./git pack-redundant
'git pack-redundant' is nominated for removal.
If you still use this command, here's what you can do:
- read https://git-scm.com/docs/BreakingChanges.html
- check if anyone has discussed this on the mailing
list and if they came up with something that can
help you: https://lore.kernel.org/git/?q=git%20pack-redundant
- send an email to <git@xxxxxxxxxxxxxxx> to let us
know that you still use this command and were unable
to determine a suitable replacement
fatal: refusing to run without --i-still-use-this
§ Changes in v5
Fix the leak that Peff found (with Peff’s suggestion). Also fix (with
Peff’s help) a regression where you were able to get into an infinite
alias loop specifically when using deprecated builtin names.
Link: https://lore.kernel.org/git/20250910051347.GA556174@xxxxxxxxxxxxxxxxxxxxxxx/
Also bring back the `BUG` check.
Cheers
Kristoffer Haugsbakk (8):
git: add `deprecated` category to --list-cmds
git: move seen-alias bookkeeping into handle_alias(...)
git: allow alias-shadowing deprecated builtins
t0014: test shadowing of aliases for a sample of builtins
you-still-use-that??: help the user help themselves
whatchanged: hint about git-log(1) and aliasing
whatchanged: remove not-even-shorter clause
BreakingChanges: remove claim about whatchanged reports
Documentation/BreakingChanges.adoc | 2 +-
Documentation/config/alias.adoc | 3 +-
Documentation/git-whatchanged.adoc | 8 ++-
Documentation/git.adoc | 3 +-
builtin/log.c | 8 ++-
builtin/pack-redundant.c | 2 +-
git-compat-util.h | 2 +-
git.c | 90 ++++++++++++++++++++----------
t/t0014-alias.sh | 58 +++++++++++++++++++
usage.c | 33 ++++++++---
10 files changed, 163 insertions(+), 46 deletions(-)
Interdiff against v4:
diff --git a/git.c b/git.c
index b3aafebfe4c..1310363e5c4 100644
--- a/git.c
+++ b/git.c
@@ -365,7 +365,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
return (*argv) - orig_argv;
}
-static int handle_alias(struct strvec *args)
+static int handle_alias(struct strvec *args, struct string_list *expanded_aliases)
{
int envchanged = 0, ret = 0, saved_errno = errno;
int count, option_count;
@@ -376,6 +376,8 @@ static int handle_alias(struct strvec *args)
alias_command = args->v[0];
alias_string = alias_lookup(alias_command);
if (alias_string) {
+ struct string_list_item *seen;
+
if (args->nr == 2 && !strcmp(args->v[1], "-h"))
fprintf_ln(stderr, _("'%s' is aliased to '%s'"),
alias_command, alias_string);
@@ -423,6 +425,24 @@ static int handle_alias(struct strvec *args)
if (!strcmp(alias_command, new_argv[0]))
die(_("recursive alias: %s"), alias_command);
+ seen = unsorted_string_list_lookup(expanded_aliases, args->v[0]);
+ if (seen) {
+ struct strbuf sb = STRBUF_INIT;
+ for (size_t i = 0; i < expanded_aliases->nr; i++) {
+ struct string_list_item *item = &expanded_aliases->items[i];
+
+ strbuf_addf(&sb, "\n %s", item->string);
+ if (item == seen)
+ strbuf_addstr(&sb, " <==");
+ else if (i == expanded_aliases->nr - 1)
+ strbuf_addstr(&sb, " ==>");
+ }
+ die(_("alias loop detected: expansion of '%s' does"
+ " not terminate:%s"), expanded_aliases->items[0].string, sb.buf);
+ }
+
+ string_list_append(expanded_aliases, args->v[0]);
+
trace_argv_printf(new_argv,
"trace: alias expansion: %s =>",
alias_command);
@@ -677,6 +697,8 @@ static void list_builtins(struct string_list *out,
unsigned int include_option,
unsigned int exclude_option)
{
+ if (include_option && exclude_option)
+ BUG("'include_option' and 'exclude_option' are mutually exclusive");
for (size_t i = 0; i < ARRAY_SIZE(commands); i++) {
if (include_option && !(commands[i].option & include_option))
continue;
@@ -810,8 +832,7 @@ static int is_deprecated_command(const char *cmd)
static int run_argv(struct strvec *args)
{
int done_alias = 0;
- struct string_list cmd_list = STRING_LIST_INIT_DUP;
- struct string_list_item *seen;
+ struct string_list expanded_aliases = STRING_LIST_INIT_DUP;
while (1) {
/*
@@ -821,9 +842,7 @@ static int run_argv(struct strvec *args)
* deprecation complaint in the meantime.
*/
if (is_deprecated_command(args->v[0]) &&
- alias_lookup(args->v[0])) {
- if (!handle_alias(args))
- break;
+ handle_alias(args, &expanded_aliases)) {
done_alias = 1;
continue;
}
@@ -876,35 +895,17 @@ static int run_argv(struct strvec *args)
/* .. then try the external ones */
execv_dashed_external(args->v);
- seen = unsorted_string_list_lookup(&cmd_list, args->v[0]);
- if (seen) {
- struct strbuf sb = STRBUF_INIT;
- for (size_t i = 0; i < cmd_list.nr; i++) {
- struct string_list_item *item = &cmd_list.items[i];
-
- strbuf_addf(&sb, "\n %s", item->string);
- if (item == seen)
- strbuf_addstr(&sb, " <==");
- else if (i == cmd_list.nr - 1)
- strbuf_addstr(&sb, " ==>");
- }
- die(_("alias loop detected: expansion of '%s' does"
- " not terminate:%s"), cmd_list.items[0].string, sb.buf);
- }
-
- string_list_append(&cmd_list, args->v[0]);
-
/*
* It could be an alias -- this works around the insanity
* of overriding "git log" with "git show" by having
* alias.log = show
*/
- if (!handle_alias(args))
+ if (!handle_alias(args, &expanded_aliases))
break;
done_alias = 1;
}
- string_list_clear(&cmd_list, 0);
+ string_list_clear(&expanded_aliases, 0);
return done_alias;
}
diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh
index bf7e6512bb1..1ac739a2737 100755
--- a/t/t0014-alias.sh
+++ b/t/t0014-alias.sh
@@ -27,6 +27,21 @@ test_expect_success 'looping aliases - internal execution' '
test_grep "^fatal: alias loop detected: expansion of" output
'
+test_expect_success 'looping aliases - deprecated builtins' '
+ test_config alias.whatchanged pack-redundant &&
+ test_config alias.pack-redundant whatchanged &&
+ cat >expect <<-EOF &&
+ ${SQ}whatchanged${SQ} is aliased to ${SQ}pack-redundant${SQ}
+ ${SQ}pack-redundant${SQ} is aliased to ${SQ}whatchanged${SQ}
+ ${SQ}whatchanged${SQ} is aliased to ${SQ}pack-redundant${SQ}
+ fatal: alias loop detected: expansion of ${SQ}whatchanged${SQ} does not terminate:
+ whatchanged <==
+ pack-redundant ==>
+ EOF
+ test_must_fail git whatchanged -h 2>actual &&
+ test_cmp expect actual
+'
+
# This test is disabled until external loops are fixed, because would block
# the test suite for a full minute.
#
@@ -72,6 +87,15 @@ test_expect_success 'can alias-shadow deprecated builtins' '
done
'
+test_expect_success 'can alias-shadow via two deprecated builtins' '
+ # some git(1) commands will fail... (see above)
+ test_might_fail git status -h >expect &&
+ test_file_not_empty expect &&
+ test_might_fail git -c alias.whatchanged=pack-redundant \
+ -c alias.pack-redundant=status whatchanged -h >actual &&
+ test_cmp expect actual
+'
+
cannot_alias_regular_builtin () {
cmd="$1" &&
# some git(1) commands will fail... (see above)
Range-diff against v4:
1: 66e6a9554b1 ! 1: 13682553018 git: add `deprecated` category to --list-cmds
@@ Commit message
## Notes (series) ##
+ v5:
+
+ Add back the `BUG` check from v3 because I think it makes sense to only
+ populate one of the two options.
+
+ Link: https://lore.kernel.org/git/cover.1757345711.git.code@xxxxxxxxxxxxxxx/T/#m922b852384911511c45afd458051f52b50dce62f
+
v4:
Incorporate Patrick’s suggestions about the for-loop refactor and
@@ git.c: int is_builtin(const char *s)
+ unsigned int include_option,
+ unsigned int exclude_option)
{
++ if (include_option && exclude_option)
++ BUG("'include_option' and 'exclude_option' are mutually exclusive");
for (size_t i = 0; i < ARRAY_SIZE(commands); i++) {
- if (exclude_option &&
- (commands[i].option & exclude_option))
-: ----------- > 2: 7f82ef6e96f git: move seen-alias bookkeeping into handle_alias(...)
2: 672253e0e71 ! 3: 4bfe36cccd0 git: allow alias-shadowing deprecated builtins
@@ Commit message
the command is removed for good.
Let’s lift this limitation by allowing *deprecated* builtins to be
- shadowed by aliases.
+ shadowed by aliases.[4]
The only observed demand for aliasing has been for git-whatchanged(1),
not for git-pack-redundant(1). But let’s be consistent and treat all
@@ Commit message
have empty commits (no changes)[2]
[2]: https://lore.kernel.org/git/20250825085428.GA367101@xxxxxxxxxxxxxxxxxxxxxxx/
[3]: https://lore.kernel.org/git/BL3P221MB0449288C8B0FA448A227FD48833AA@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
+ [4]: The test “looping aliases - deprecated builtins” covers the
+ case of the extra output mentioned in footnote number 3 of the
+ previous commit
Based-on-patch-by: Jeff King <peff@xxxxxxxx>
Signed-off-by: Kristoffer Haugsbakk <code@xxxxxxxxxxxxxxx>
## Notes (series) ##
+ v5:
+
+ Fix a leak caused by calling `alias_lookup(...)` without freeing the
+ return value by not calling that function.
+
+ Add tests for deprecated builtins for infinite alias looping as well as
+ simple chaining. This is to exercise all code paths that we could think
+ of.
+
+ Expand on the infinite alias looping test on the whole error
+ output (compared to the one above). Link this change to the
+ previous patch/commit by mentioning it in a footnote.
+
+ See the last footnote and the note on the previous commit/patch. I have
+ opted not to get rid of this redundant line. But this documents the
+ change and future changes that affect it will trigger this
+ assertion (say if they remove the redundant line).
+
v4:
Better `is_deprecated_command` implementation.
@@ git.c: static void execv_dashed_external(const char **argv)
static int run_argv(struct strvec *args)
{
int done_alias = 0;
-@@ git.c: static int run_argv(struct strvec *args)
- struct string_list_item *seen;
+ struct string_list expanded_aliases = STRING_LIST_INIT_DUP;
while (1) {
+ /*
@@ git.c: static int run_argv(struct strvec *args)
+ * deprecation complaint in the meantime.
+ */
+ if (is_deprecated_command(args->v[0]) &&
-+ alias_lookup(args->v[0])) {
-+ if (!handle_alias(args))
-+ break;
++ handle_alias(args, &expanded_aliases)) {
+ done_alias = 1;
+ continue;
+ }
@@ git.c: static int run_argv(struct strvec *args)
* it no longer is safe to invoke builtins directly in
## t/t0014-alias.sh ##
+@@ t/t0014-alias.sh: test_expect_success 'looping aliases - internal execution' '
+ test_grep "^fatal: alias loop detected: expansion of" output
+ '
+
++test_expect_success 'looping aliases - deprecated builtins' '
++ test_config alias.whatchanged pack-redundant &&
++ test_config alias.pack-redundant whatchanged &&
++ cat >expect <<-EOF &&
++ ${SQ}whatchanged${SQ} is aliased to ${SQ}pack-redundant${SQ}
++ ${SQ}pack-redundant${SQ} is aliased to ${SQ}whatchanged${SQ}
++ ${SQ}whatchanged${SQ} is aliased to ${SQ}pack-redundant${SQ}
++ fatal: alias loop detected: expansion of ${SQ}whatchanged${SQ} does not terminate:
++ whatchanged <==
++ pack-redundant ==>
++ EOF
++ test_must_fail git whatchanged -h 2>actual &&
++ test_cmp expect actual
++'
++
+ # This test is disabled until external loops are fixed, because would block
+ # the test suite for a full minute.
+ #
@@ t/t0014-alias.sh: test_expect_success 'tracing a shell alias with arguments shows trace of prepare
test_cmp expect actual
'
@@ t/t0014-alias.sh: test_expect_success 'tracing a shell alias with arguments show
+ can_alias_deprecated_builtin "$cmd" || return 1
+ done
+'
++
++test_expect_success 'can alias-shadow via two deprecated builtins' '
++ # some git(1) commands will fail... (see above)
++ test_might_fail git status -h >expect &&
++ test_file_not_empty expect &&
++ test_might_fail git -c alias.whatchanged=pack-redundant \
++ -c alias.pack-redundant=status whatchanged -h >actual &&
++ test_cmp expect actual
++'
+
test_done
3: 00108f28f82 ! 4: a04c6ae968e t0014: test shadowing of aliases for a sample of builtins
@@ Metadata
## Commit message ##
t0014: test shadowing of aliases for a sample of builtins
- The previous commit added a test for shadowing deprecated builtins.
+ The previous commit added tests for shadowing deprecated builtins.
Let’s make the test suite more complete by exercising a sample of
the builtins and in turn test the documentation for git-config(1):
@@ Notes (series)
if I test all of them.
## t/t0014-alias.sh ##
-@@ t/t0014-alias.sh: test_expect_success 'can alias-shadow deprecated builtins' '
- done
+@@ t/t0014-alias.sh: test_expect_success 'can alias-shadow via two deprecated builtins' '
+ test_cmp expect actual
'
+cannot_alias_regular_builtin () {
4: 6bdcaf7f80f = 5: 2f78ab2e28c you-still-use-that??: help the user help themselves
5: 58de9767b22 ! 6: 34443066645 whatchanged: tell users the git-log(1) equivalent
@@ Metadata
Author: Kristoffer Haugsbakk <code@xxxxxxxxxxxxxxx>
## Commit message ##
- whatchanged: tell users the git-log(1) equivalent
+ whatchanged: hint about git-log(1) and aliasing
There have been quite a few `--i-still-use-this` user reports since Git
2.51.0 was released.[1][2] And it doesn’t seem like they are reading
the man page about the git-log(1) equivalent.
- Tell them what options to plug into git-log(1). That template produces
- almost the same output[3] and is arguably a plug-in replacement.
- Concretely, add an optional `hint` argument so that we can use it right
- after the initial error line.
+ Tell them what options to plug into git-log(1), either as a replacement
+ command or as an alias.[3] That template produces almost the same
+ output[4] and is arguably a plug-in replacement. Concretely, add
+ an optional `hint` argument so that we can use it right after the
+ initial error line.
Also mention the same concrete options in the documentation while we’re
at it.
@@ Commit message
• https://lore.kernel.org/git/9fcbfcc4-79f9-421f-b9a4-dc455f7db485@xxxxxxx/#t
• https://lore.kernel.org/git/83241BDE-1E0D-489A-9181-C608E9FCC17B@xxxxxxxxx/
[2]: The error message on 2.51.0 does tell them to report it, unconditionally
- [3]: You only get different outputs if you happen to have empty
+ [3]: We allow aliasing deprecated builtins now for people who are very
+ used to the command name or just like it a lot
+ [4]: You only get different outputs if you happen to have empty
commits (no changes)[4]
- [4]: https://lore.kernel.org/git/20250825085428.GA367101@xxxxxxxxxxxxxxxxxxxxxxx/
+ [5]: https://lore.kernel.org/git/20250825085428.GA367101@xxxxxxxxxxxxxxxxxxxxxxx/
Signed-off-by: Kristoffer Haugsbakk <code@xxxxxxxxxxxxxxx>
## Notes (series) ##
+ v5:
+
+ Mention the aliasing hint in the commit message.
+
v3:
Add an alias hint now that that is possible. Also prefix each hint-line
6: 407b430d02c = 7: b0aabd793f0 whatchanged: remove not-even-shorter clause
7: fee752d2fb0 = 8: 5021647c245 BreakingChanges: remove claim about whatchanged reports
base-commit: c44beea485f0f2feaf460e2ac87fdd5608d63cf0
--
2.51.0.16.gcd94ab5bf81
