Re: [BUG] - git-read-tree segfaults

Junio C Hamano [off-list ref] writes:

I suspect that the reason add_entry() passed SKIP_DFCHECK was to work
around an old bug in add_index_entry() that considered it a D/F conflict
if you have a file D at stage N and a file D/F at stage M when N and M are
different.  I think such a bug has been fixed long time ago, and there is
no reason for such a workaround.  Besides, OK_TO_REPLACE only makes sense
when you do check D/F conflict ("replace" in the name of the flag means
"If you want to add 'xxx/zzz' when the index has 'xxx', it is Ok to drop
the existing 'xxx' in order to add your 'xxx/zzz''); it makes no sense to
give it if you are giving SKIP_DFCHECK at the same time.

The ancient D/F conflict detection bug in add_cache_entry() I had in mind
was fixed by b155725 ([PATCH] Fix oversimplified optimization for
add_cache_entry()., 2005-06-25).  The use of SKIP_DFCHECK turns out to
have nothing to do with working that ancient bug around.

The real culprit seems to be 34110cd (Make 'unpack_trees()' have a
separate source and destination index, 2008-03-06).

Before that commit:

 * merge_entry() that records the final tree-level 3-way merge decision
   used to pass OK_TO_REPLACE without SKIP_DFCHECK.

 * unpack_nondirectories() codepath for non-merge multi-tree read-tree
   (the one under discussion in this thread) used to pass SKIP_DFCHECK but
   did not pass OK_TO_REPLACE.

The fact that even merge_entry() side passes SKIP_DFCHECK these days does
not appear to be a workaround for an old bug in D/F conflict detection
code after all; it simply is a bug in the refactoring done with the said
commit.

The unpack_nondirectories() codepath passed SKIP_DFCHECK from ee6566e
(Rewrite read-tree, 2005-09-05), which is the very original implementation
of the modern "read-tree A B C" code.  The ancient bug in the D/F conflict
detection code was killed way before that commit, and SKIP_DFCHECK in the
commit is not a workaround either; it also simply is a bug.

	Side note: I was somewhat surprised that "make test" of that old
	commit dates from September 2005 runs _much_ faster than the test
	suite we have these days.

The only sane use of SKIP_DFCHECK is when the caller knows the addition is
not introducing D/F conflict in the index to avoid the overhead.