From: Kristoffer Haugsbakk <code@xxxxxxxxxxxxxxx>
We are about to complicate the command handling by allowing *deprecated*
builtins to be shadowed by aliases. We need to organize the code in
order to facilitate that.[1]
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]
[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@xxxxxxxx>
Signed-off-by: Kristoffer Haugsbakk <code@xxxxxxxxxxxxxxx>
---
Notes (series):
v5 (new):
Do a preliminary code movement in order to avoid loop aliasing in the
specific case of alias-shadowing deprecated builtins.
Link: https://lore.kernel.org/git/20250911204302.GA1907101@xxxxxxxxxxxxxxxxxxxxxxx/
As noted in footnote 3: you get one more line of output before the loop
aliasing is detected (and see the next commit):
'whatchanged' is aliased to 'pack-redundant'
'pack-redundant' is aliased to 'whatchanged'
'whatchanged' is aliased to 'pack-redundant'
fatal: alias loop detected: expansion of 'whatchanged' does not terminate:
whatchanged <==
pack-redundant ==>
This is an informational message specifically for `<git cmd> -h`.
This output can be gotten after the next commit.
The output in this case is very incidental:
1. You are aliasing deprecated commands in a chain
2. You are using `-h`
3. You are going to get caught by the alias loop checker
git.c | 47 ++++++++++++++++++++++++-----------------------
1 file changed, 24 insertions(+), 23 deletions(-)
diff --git a/git.c b/git.c
index 511efdf2056..f9c2d8c8d86 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);
@@ -806,8 +826,7 @@ static void execv_dashed_external(const char **argv)
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) {
/*
@@ -859,35 +878,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;
}
--
2.51.0.16.gcd94ab5bf81
