git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Derrick Stolee <stolee@gmail.com>
Date
Oct 4, 2021, 22:29 UTC
Message-ID
<a075091c-d0d4-db5d-fa21-c9d6c90c343e@gmail.com>
In-Reply-To
<xmqqzgrov7g2.fsf@gitster.g>
On 10/4/2021 4:52 PM, Junio C Hamano wrote:
Show 43 quoted lines
> 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

Previous: Junio C HamanoNext: René Scharfe
Message 7 of 11 in “Bug/regression report - 'git stash push -u' fatal errors when sub-repo present”
  1. Robert LeftwichSep 30, 2021
  2. René ScharfeSep 30, 2021
  3. Junio C HamanoSep 30, 2021
  4. René ScharfeOct 1, 2021
  5. Derrick StoleeOct 4, 2021
  6. Junio C HamanoOct 4, 2021
  7. Derrick StoleeOct 4, 2021
  8. 1/3 t3905: show failure to ignore sub-repoRené Scharfe, Oct 7, 2021
  9. 2/3 read-cache: add verify_path_internal()René Scharfe, Oct 7, 2021
  10. 3/3 read-cache: let verify_path() reject trailing dir separators againRené Scharfe, Oct 7, 2021
  11. Junio C HamanoOct 8, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.