Re: [PATCH v6] subtree: fix split processing with multiple subtrees present

From: Zach FettersMoore <zach.fetters@apollographql.com>
Date: Dec 11, 2023, 15:39 UTC
Message-ID: <CAEWN6q3RTbVuMb0VyCYz196ZL+OGAAHbJLZ2-MnW1RVVabg7Mw@mail.gmail.com>
In-Reply-To: <CAP8UFD3FzP6QW4dJ9yiG1BAytLcsk+zGE+CBeArRJBJ8gsaDMQ@mail.gmail.com>

Show 26 quoted lines

>>
>> From: Zach FettersMoore <zach.fetters@apollographql.com>
>>
>> When there are multiple subtrees present in a repository and they are
>> all using 'git subtree split', the 'split' command can take a
>> significant (and constantly growing) amount of time to run even when
>> using the '--rejoin' flag. This is due to the fact that when processing
>> commits to determine the last known split to start from when looking
>> for changes, if there has been a split/merge done from another subtree
>> there will be 2 split commits, one mainline and one subtree, for the
>> second subtree that are part of the processing. The non-mainline
>> subtree split commit will cause the processing to always need to search
>> the entire history of the given subtree as part of its processing even
>> though those commits are totally irrelevant to the current subtree
>> split being run.
>>
>> To see this in practice you can use the open source GitHub repo
>> 'apollo-ios-dev' and do the following in order:
>>
>> -Make a changes to a file in 'apollo-ios' and 'apollo-ios-codegen'
>> directories
>> -Create a commit containing these changes
>> -Do a split on apollo-ios-codegen
>> - Do a fetch on the subtree repo
>> - git fetch git@github.com:apollographql/apollo-ios-codegen.git
>> - git subtree split --prefix=apollo-ios-codegen --squash --rejoin

Show 15 quoted lines

> Now I get the following without your patch at this step:
>
> $ git subtree split --prefix=apollo-ios-codegen --squash --rejoin
> [...]/libexec/git-core/git-subtree: 318: Maximum function recursion
> depth (1000) reached
>
> Line 318 in git-subtree.sh contains the following:
>
> missed=$(cache_miss "$@") || exit $?
>
> With your patch it seems to work:
>
> $ git subtree split --prefix=apollo-ios-codegen --squash --rejoin
> Merge made by the 'ort' strategy.
> e274aed3ba6d0659fb4cc014587cf31c1e8df7f4

Looking into this some it looks like it could be a bash config difference? My machine always runs it all the way through vs failing for recursion depth. Although that would also be an issue which is solved by this fix.

>> - Depending on the current state of the 'apollo-ios-dev' repo
>> you may see the issue at this point if the last split was on
>> apollo-ios

> I guess I see it, but it seems a bit different for me than what you describe.
>
> Otherwise your patch looks good to me now.

Yea I hadn't accounted for/realized that some folks may see a recursion depth error vs it just taking a long time like it does for me. Also what I was saying with the apollo-ios-dev repo is you may not need all the steps to see the issue, because its possible the state of the repo is already in a position to display the issue just by doing a split on apollo-ios-codegen.

Great! Thanks again for all the feedback and guidance! Is there anything else I need to do to get this across the finish line and merged in?