you-still-use-that??: improve breaking changes troubleshooting

From: Kristoffer Haugsbakk <[email protected]>

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 <[email protected]> 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 <[email protected]> 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/[email protected]/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:

index e11340c1ae7..9311da5bb15 100644
@@ -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

index 1310363e5c4..8c85da84c30 100644
@@ -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);

index 1ac739a2737..07a53e7366e 100755
@@ -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

## 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/[email protected]/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/[email protected]/
[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

## 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