Subject: Re: [PATCH v4 2/7] git: allow alias-shadowing deprecated builtins

From: "Kristoffer Haugsbakk" <[email-masked]>
Date: Thu, 11 Sep 2025 19:31:04 +0200

On Wed, Sep 10, 2025, at 20:34, Jeff King wrote:
> On Wed, Sep 10, 2025 at 07:58:32PM +0200, Kristoffer Haugsbakk wrote:
>
>> > So we just need to loop again when it told us that it found an alias (it
>> > is tempting to just continue in the loop body, but that would miss the
>> > case of a deprecated alias that resolves to another deprecated alias).
>>
>> Yeah, that’s subtle.
>>
>> Do you think this should be a test case?  I guess it doesn’t hurt.
>
> Yeah, probably would be easy to add. We need two deprecated command
> names, but it looks like we do have that at this point. :)

Thanks. That works fine:

     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
     '

But it turns out there is a regression in my code with the recursion
detection.  Compare:

    $ ./git -c alias.one=two -c alias.two=two one -h
    'one' is aliased to 'two'
    'two' is aliased to 'two'
    fatal: recursive alias: two

with:

    $ ./git -c alias.whatchanged=pack-redundant -c alias.pack-redundant=whatchanged whatchanged -h
    'whatchanged' is aliased to 'pack-redundant'
    'pack-redundant' is aliased to 'whatchanged'
    'whatchanged' is aliased to 'pack-redundant'
    'pack-redundant' is aliased to 'whatchanged'
    'whatchanged' is aliased to 'pack-redundant'
    'pack-redundant' is aliased to 'whatchanged'
    'whatchanged' is aliased to 'pack-redundant'
    'pack-redundant' is aliased to 'whatchanged'
    [forever]

Both for your suggested change and my v4.