Re: [PATCH] contrib/subtree: Split history with empty trees correctly

From: Marcus Brinkmann <m.brinkmann@semantics.de>
Date: Jan 24, 2016, 13:07 UTC
Message-ID: <56A4CC85.90705@semantics.de>
In-Reply-To: <871t9cvqsp.fsf@waller.obbligato.org>

With my patch, "git subtree split -P" produces the same result (for my data set) as "git filter-branch --subdirectory-filter", which is much faster, because it selects the revisions to rewrite before rewriting. As I am not using any of the advanced features of "git subtree", I will just use "git filter-branch" instead.

Thanks! Marcus

On 01/20/2016 05:05 AM, David A. Greene wrote:

Show 50 quoted lines

> 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.
> 
>> @@ -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
>

-- 
s<e>mantics GmbH
Viktoriaallee 45
52066 Aachen
Web: www.semantics.de
Registergericht  : Amtsgericht Aachen, HRB 8189
Geschäftsführer  : Kay Heiligenhaus M.A.
                   Dipl. Ing. José de la Rosa