Re: [PATCH 4/5] allow deepening of a shallow repository

Johannes Schindelin [off-list ref] writes:

quoted

Although I am reasonably sure we can eventually make it work, it
is very subtle and fragile -- somebody touching this code can
easily break it.

I fully agree. Even the OA did not understand the code fully ;-)

How about adding "int force_reparse" to the signature of 
unregister_shallow()? (Just like we added "int cleanup" to 
get_merge_bases() to avoid pilot errors.)

I do not think that's where its fragility lies.  It is that any
random place can later call parse_object() on the commit, after
you elaborately pre-parsed it with shallow so that it appears to
have fewer parents, with the expectation that nobody ever would
clear the parsed bit and cause it to be reparsed again.  With
that assumption, find_common() manipulated the shallow entry
after setting that scheme up.  A mechanism to prevent the commit
from getting re-parsed would have made it more robust.

Hi,

On Fri, 17 Nov 2006, Junio C Hamano wrote:

Johannes Schindelin [off-list ref] writes:

quoted

quoted

Although I am reasonably sure we can eventually make it work, it
is very subtle and fragile -- somebody touching this code can
easily break it.

I fully agree. Even the OA did not understand the code fully ;-)

How about adding "int force_reparse" to the signature of 
unregister_shallow()? (Just like we added "int cleanup" to 
get_merge_bases() to avoid pilot errors.)

I do not think that's where its fragility lies.  It is that any
random place can later call parse_object() on the commit, after
you elaborately pre-parsed it with shallow so that it appears to
have fewer parents, with the expectation that nobody ever would
clear the parsed bit and cause it to be reparsed again.  With
that assumption, find_common() manipulated the shallow entry
after setting that scheme up.  A mechanism to prevent the commit

quoted

from getting re-parsed would have made it more robust.

How about putting yet more meaning into nr_parents: if it is negative, it 
is a shallow commit, but write_shallow_commits() only writes the SHA1 if 
nr_parents is -1. Hmm?

Ciao,
Dscho