[PATCH v5 0/8] you-still-use-that??: improve breaking changes troubleshooting
- From
- kristofferhaugsbakk@fastmail.com <kristofferhaugsbakk@fastmail.com>
- Date
- Sep 14, 2025, 19:49 UTC
- Message-ID
- <cover.1757879060.git.code@khaugsbakk.name>
- In-Reply-To
- <cover.1757446619.git.code@khaugsbakk.name>
From: Kristoffer Haugsbakk <code@khaugsbakk.name>
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@vger.kernel.org> 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@vger.kernel.org> to let us
know that you still use this command and were unable
to determine a suitable replacementfatal: 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@coredump.intra.peff.net/
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@khaugsbakk.name/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@coredump.intra.peff.net/ [3]: https://lore.kernel.org/git/BL3P221MB0449288C8B0FA448A227FD48833AA@BL3P221MB0449.NAMP221.PROD.OUTLOOK.COM/ + [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@peff.net> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> ## 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@khaugsbakk.name> ## 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@acm.org/#t • https://lore.kernel.org/git/83241BDE-1E0D-489A-9181-C608E9FCC17B@gmail.com/ [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@coredump.intra.peff.net/ + [5]: https://lore.kernel.org/git/20250825085428.GA367101@coredump.intra.peff.net/ Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> ## 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