# [RFC PATCH] Allow aliases that include other aliases 3 messages from 2018-09-04 to 2018-09-04. Participants: Tim Schumacher, Junio C Hamano, Jeff King. Thread: https://gitlist.dev/t/49274 ## Tim Schumacher, 2018-09-04 17:39 Subject: [RFC PATCH] Allow aliases that include other aliases Message-ID: <20180904173915.2393-1-timschumi@gmx.de> URL: https://gitlist.dev/e/20180904173915.2393-1-timschumi%40gmx.de ``` Aliases can only contain non-alias git commands and arguments, but not other user-defined aliases. Resolving nested aliases is prevented by breaking the loop after the first alias was processed, git then fails with a command-not-found error. Allow resolving nested aliases by not breaking the loop in run_argv() after the first alias was processed. Instead, continue incrementing done_alias until handle_alias() fails, which means that there are no further aliases that can be processed. --- I submitted this as RFC because I'm not sure whether disallowing nested aliases was an intentional design choice. The done_alias check implies that disallowing is intended, but the direct recursion check for aliases that call themselves opposes that. Furthermore, including this patch allows creating a looping state, since the recursion check only checks if an alias is directly calling itself. One solution would be to break the loop as soon as done_alias reaches a certain value, but that requires setting an arbitrary point of "too many recursions". A list of already resolved aliases and breaking the loop as soon as an alias is resolved twice would probably do the trick, but implementing that is well beyond the point of what I'm capable of doing. --- git.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/git.c b/git.c index c27c38738..9d3cf5797 100644 --- a/git.c +++ b/git.c @@ -695,11 +695,9 @@ static int run_argv(int *argcp, const char ***argv) * of overriding "git log" with "git show" by having * alias.log = show */ - if (done_alias) - break; if (!handle_alias(argcp, argv)) break; - done_alias = 1; + done_alias++; } return done_alias; -- 2.19.0.rc1.2.g7460ee143 ``` ## Junio C Hamano, 2018-09-04 17:55 Subject: Re: [RFC PATCH] Allow aliases that include other aliases Message-ID: URL: https://gitlist.dev/e/xmqqsh2p9mg8.fsf%40gitster-ct.c.googlers.com In-Reply-To: <20180904173915.2393-1-timschumi@gmx.de> ``` Tim Schumacher writes: > I submitted this as RFC because I'm not sure whether disallowing > nested aliases was an intentional design choice. The done_alias > check implies that disallowing is intended, but the direct > recursion check for aliases that call themselves opposes that. "direct recursion check for aliases"? I am not sure what you mean by that, but anyway. If I recall correctly, it is intended that we disallow run_argv() doing handle_alias() twice (or more). But the ultimate objective is to forbid infinite loops, "git foo" expanding to "git bar" which in turn expanding back to "git foo", and the current "do not expand alias to another" is a simple but too strict implementation. As long as a replacement implementation still forbids infinite loops with reasonable cost and complexity, I do not think we would mind such an improvement to allow alias expanding to another alias. ``` ## Jeff King, 2018-09-04 20:11 Subject: Re: [RFC PATCH] Allow aliases that include other aliases Message-ID: <20180904201115.GD17481@sigill.intra.peff.net> URL: https://gitlist.dev/e/20180904201115.GD17481%40sigill.intra.peff.net In-Reply-To: ``` On Tue, Sep 04, 2018 at 10:55:35AM -0700, Junio C Hamano wrote: > Tim Schumacher writes: > > > I submitted this as RFC because I'm not sure whether disallowing > > nested aliases was an intentional design choice. The done_alias > > check implies that disallowing is intended, but the direct > > recursion check for aliases that call themselves opposes that. > > "direct recursion check for aliases"? I am not sure what you mean > by that, but anyway. > > If I recall correctly, it is intended that we disallow run_argv() > doing handle_alias() twice (or more). But the ultimate objective is > to forbid infinite loops, "git foo" expanding to "git bar" which in > turn expanding back to "git foo", and the current "do not expand > alias to another" is a simple but too strict implementation. As > long as a replacement implementation still forbids infinite loops > with reasonable cost and complexity, I do not think we would mind > such an improvement to allow alias expanding to another alias. I agree that this could be looser, if all we care about is infinite loops. But given the issues we've had with aliases and the startup sequence, I also would not be surprised if there is some weird hidden effect if we call handle_alias() more than once. That's not a reason not to pursue this, but just one more thing to look out for. As an aside, you can still do this: $ git config alias.foo '!git bar' $ git config alias.bar '!git foo' $ git foo [boy, my CPU fan is really spinning] I don't know how much effort we actually need to put into people not shooting themselves in the foot, since we can't cover all of the cases anyway (and in a non-infinite configuration, that's another solution for Tim's original problem). -Peff ```