Re: Bug/regression report - 'git stash push -u' fatal errors when sub-repo present

[Derrick Stolee <stolee@gmail.com>]

On 10/4/2021 4:52 PM, Junio C Hamano wrote:
> Derrick Stolee <stolee@gmail.com> writes:
> 
>> And yes, I believe that make_cache_entry() and add_index_entry_with_check()
>> are the only places that need this internal version. If we find others,
>> then we can add them as necessary. The tests in t1092 are getting rather
>> robust, although they don't do much for this test case:
>>> +test_expect_success 'stash -u ignores sub-repository' '
>>> +	test_when_finished "rm -rf sub-repo" &&
>>> +	git init sub-repo &&
>>> +	git stash -u
>>> +'
>>
>> Seems like a good test to have, anyway.
>>
>> I look forward to seeing this as a full patch.
> 
> Just one thing I want to pick your brains for ;-)
> 
> I said this earlier ...
> 
>>>> "git update-index" should never allow to create a "tree" kind of
>>>> cache entry (making it sparse again should be the task of systems
>>>> internal, and should not be done by end-user feeding a pre-shrunk
>>>> "tree" kind of entry to record a sparsely populated state, if I
>>>> understand correctly), so its call to verify_path(), if it ends with
>>>> a directory separator, should say "that's not a good path".
> 
> ... without knowing what you had in mind when you did the "tree kind
> of entry in the index".  Are we on the same page, or do we think it
> might be beneficial to give end-users a long-enough rope
> to hang themselves, aka get into the lower details of
> implementation?
> 
> One _could_ imagine that allowing
> 
>  $ git update-index --cacheinfo 40000,609869396314577e5a,t/
> 
> given by the end user to drop all entries under t/* and replace them
> with a single sparse-dir-entry might be a good way to allow
> scripters the same power as the C-code to take advantage of the
> sparse checkout feature.  It needs to be paired with some mechanism
> to allow sparse-dir-entry observed by the end users with a plumbing,
> e.g. even though ls-files unconditionally calls ensure_full_index(),

I think this is an interesting capability, but I'm not sure I see
a use case that is worth the footgun, especially with the accidents
that can happen when working with submodules.

I think we can happily extend to include this functionality in the
future. Having an error condition now that we relax in the future is
the good kind of behavior change.

>  $ git ls-files --show-sparse
> 
> may show the sparse-dir-entry by bypassing the call.

I have something in the works for this, but I'm letting others send
their sparse-index work first. I have not forgotten your request for
such a feature!

Thanks,
-Stolee