[PATCH v6 0/9] you-still-use-that??: improve breaking changes troubleshooting

[kristofferhaugsbakk@fastmail.com]

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`
5. (new in v6) Don’t list git-whatchanged(1) as a builtin when
   `WITH_BREAKING_CHANGES`

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

    fatal: refusing to run without --i-still-use-this

§ Changes in v6

Use Peff’s patch to get rid of “the extra output”.[1]  Update commit
messages to not mention that.

Also use another fix from Peff (patch 1); don’t list git-whatchanged(1)
as a builtin when `WITH_BREAKING_CHANGES`.

🔗 1: https://lore.kernel.org/git/cover.1757446619.git.code@khaugsbakk.name/T/#mb3ac9c32c8527675a1f9cdf7709f2c74b3dd2a6c

Thanks

Kristoffer Haugsbakk (9):
  Makefile: don’t add whatchanged after it has been removed
  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 +-
 Makefile                           |  2 +
 builtin/log.c                      |  8 ++-
 builtin/pack-redundant.c           |  2 +-
 git-compat-util.h                  |  2 +-
 git.c                              | 91 ++++++++++++++++++++----------
 t/t0014-alias.sh                   | 57 +++++++++++++++++++
 usage.c                            | 33 ++++++++---
 11 files changed, 165 insertions(+), 46 deletions(-)

Interdiff against v5:
diff --git a/Makefile b/Makefile
index e11340c1ae7..9311da5bb15 100644
--- a/Makefile
+++ b/Makefile
@@ -883,7 +883,9 @@ BUILT_INS += git-stage$X
 BUILT_INS += git-status$X
 BUILT_INS += git-switch$X
 BUILT_INS += git-version$X
+ifndef WITH_BREAKING_CHANGES
 BUILT_INS += git-whatchanged$X
+endif
 
 # what 'all' will build but not install in gitexecdir
 OTHER_PROGRAMS += git$X
diff --git a/git.c b/git.c
index 1310363e5c4..8c85da84c30 100644
--- a/git.c
+++ b/git.c
@@ -425,7 +425,10 @@ static int handle_alias(struct strvec *args, struct string_list *expanded_aliase
 		if (!strcmp(alias_command, new_argv[0]))
 			die(_("recursive alias: %s"), alias_command);
 
-		seen = unsorted_string_list_lookup(expanded_aliases, args->v[0]);
+		string_list_append(expanded_aliases, alias_command);
+		seen = unsorted_string_list_lookup(expanded_aliases,
+						   new_argv[0]);
+
 		if (seen) {
 			struct strbuf sb = STRBUF_INIT;
 			for (size_t i = 0; i < expanded_aliases->nr; i++) {
@@ -441,8 +444,6 @@ static int handle_alias(struct strvec *args, struct string_list *expanded_aliase
 			      " 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);
diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh
index 1ac739a2737..07a53e7366e 100755
--- a/t/t0014-alias.sh
+++ b/t/t0014-alias.sh
@@ -33,7 +33,6 @@ test_expect_success 'looping aliases - deprecated builtins' '
 	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 ==>
Range-diff against v5:
 -:  ----------- >  1:  8b34edf876e Makefile: don’t add whatchanged after it has been removed
 1:  13682553018 =  2:  c10a24018ce git: add `deprecated` category to --list-cmds
 2:  7f82ef6e96f !  3:  90e35bfc063 git: move seen-alias bookkeeping into handle_alias(...)
    @@ Commit message
         The code in the `while(1)` speculatively adds commands to the list
         before finding out if it’s an alias.  Let’s instead move it inside
         `handle_alias(...)`—where it conceptually belongs anyway—and in turn
    -    only run this logic when we have found an alias.[2][3]
    +    only run this logic when we have found an alias.[2]
     
         [1]: We will do that with an additional call to `handle_alias(1)` inside
             the loop.  *Not* moving this code leaves a blind spot; we will miss
             alias looping crafted via deprecated builtin names
         [2]: Also rename the list to a more descriptive name
    -    [3]: The error output `'<cmd1>' is aliased to '<cmd2>'` is used for the
    -        `<cmd2> -h` case.  We will get one more (redundant) line of output
    -        in the case of alias looping; that is covered in the test “looping
    -        aliases - deprecated builtins” in the next commit
     
         Based-on-patch-by: Jeff King <peff@peff.net>
         Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
     
     
      ## Notes (series) ##
    +    v6:
    +
    +    Apply Peff’s patch[1] in order to get rid of the extra output line.
    +
    +    Drop the footnote from the commit message now that we don’t have that
    +    problem any more.
    +
    +    🔗 1: https://lore.kernel.org/git/cover.1757446619.git.code@khaugsbakk.name/T/#mb3ac9c32c8527675a1f9cdf7709f2c74b3dd2a6c
    +
         v5 (new):
     
         Do a preliminary code movement in order to avoid loop aliasing in the
    @@ git.c: 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]);
    ++		string_list_append(expanded_aliases, alias_command);
    ++		seen = unsorted_string_list_lookup(expanded_aliases,
    ++						   new_argv[0]);
    ++
     +		if (seen) {
     +			struct strbuf sb = STRBUF_INIT;
     +			for (size_t i = 0; i < expanded_aliases->nr; i++) {
    @@ git.c: static int handle_alias(struct strvec *args)
     +			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 =>",
 3:  4bfe36cccd0 !  4:  2cac74d731c 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.[4]
    +    shadowed by aliases.
     
         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) ##
    +    v6:
    +
    +    Drop footnote now that we don’t have the “extra output line” problem.
    +
         v5:
     
         Fix a leak caused by calling `alias_lookup(...)` without freeing the
    @@ t/t0014-alias.sh: test_expect_success 'looping aliases - internal execution' '
     +	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 ==>
 4:  a04c6ae968e =  5:  3576439add5 t0014: test shadowing of aliases for a sample of builtins
 5:  2f78ab2e28c =  6:  7499890ed49 you-still-use-that??: help the user help themselves
 6:  34443066645 =  7:  b0831e8967a whatchanged: hint about git-log(1) and aliasing
 7:  b0aabd793f0 =  8:  2f281944fbf whatchanged: remove not-even-shorter clause
 8:  5021647c245 =  9:  7dc3d03d626 BreakingChanges: remove claim about whatchanged reports

base-commit: c44beea485f0f2feaf460e2ac87fdd5608d63cf0
-- 
2.51.0.274.gdcb64e51a0f