Re: [PATCH] contrib/subtree: Split history with empty trees correctly
- From
David A. Greene <greened@obbligato.org>
- Date
- Jan 20, 2016, 04:05 UTC
- Message-ID
- <871t9cvqsp.fsf@waller.obbligato.org>
- In-Reply-To
- <569EE046.9040506@semantics.de>
Marcus Brinkmann <m.brinkmann@semantics.de> writes:
> 'git subtree split' will fail if the history of the subtree has empty > tree commits (or trees that are considered empty, such as submodules). > This fix keeps track of this condition and correctly follows the history > over such commits.
Thanks for working on this! Please add a test to t7900-subtree.sh.
Show 22 quoted lines
> @@ -625,12 +629,16 @@ cmd_split() > > # ugly. is there no better way to tell if this is a subtree > # vs. a mainline commit? Does it matter? > - if [ -z $tree ]; then > - set_notree $rev > - if [ -n "$newparents" ]; then > - cache_set $rev $rev > + if [ -z $found_first_commit ]; then > + if [ -z $tree ]; then > + set_notree $rev > + if [ -n "$newparents" ]; then > + cache_set $rev $rev > + fi > + continue > + else > + found_first_commit=yes > fi > - continue > fi > > newrev=$(copy_or_skip "$rev" "$tree" "$newparents") || exit $?
Can you explain the logic here? The old code appears to be using the lack of a tree to filter out "mainline" commits from the subtree history when splitting. If that test is only done before seeing a proper subtree commit and never after, then any commit mainline commit following the first subtree commit in the rev list will miss being marked with set_notree and the cache will not have the identity entry added.
Test #36 in t7900-subtree.sh has a mainline commit listed after the first subtree commit in the rev list, I believe.
I'm not positive your change is wrong, I'd just like to understand it better. I'd also like a comment explaining why it works so future developers don't get confused. Overall, I am trying to better comment the code as I make my own changes.
-David