Re: [PATCH] unpack-trees: fix sparse directory recursion check

[Derrick Stolee <derrickstolee@github.com>]

On 9/1/2022 5:02 PM, Victoria Dye via GitGitGadget wrote:
> From: Victoria Dye <vdye@github.com>
> 
> Ensure 'is_sparse_directory_entry()' receives a valid 'name_entry *' if one
> exists in the list of tree(s) being unpacked in 'unpack_callback()'.
> 
> Currently, 'is_sparse_directory_entry()' is called with the first
> 'name_entry' in the 'names' list of entries on 'unpack_callback()'. However,
> this entry may be empty even when other elements of 'names' are not (such as
> when switching from an orphan branch back to a "normal" branch). As a
> result, 'is_sparse_directory_entry()' could incorrectly indicate that a
> sparse directory is *not* actually sparse because the name of the index
> entry does not match the (empty) 'name_entry' path.

Thank you for finding the root cause here. It was very non-obvious!

> +test_expect_success 'checkout orphan then non-orphan' '
> +	init_repos &&
> +
> +	test_all_match git checkout --orphan test-orphan &&
> +	test_all_match git status --porcelain=v2 &&
> +	test_all_match git checkout base &&
> +	test_all_match git status --porcelain=v2
> +'
> +

This test demonstrates how specific the case needs to be for this to
actually happen.

Could it also happen if we are going from a commit without that
sparse directory and then to a commit with that sparse directory? I
think that would be a more common case, but I was unable to
manipulate the test repo in t1092 to trigger this bug in the existing
test cases.

This makes me think that this bug _is_ extremely rare, so we don't
need to rush this into a 2.37.4 or anything. It would be good to
bring it into 2.38.0-rc0, though.

Thanks!
-Stolee