git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Derrick Stolee <stolee@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "René Scharfe" <l.s.r@web.de>,
	"Robert Leftwich" <robert@gitpod.io>,
	git@vger.kernel.org, "Derrick Stolee" <dstolee@microsoft.com>
Subject: Re: Bug/regression report - 'git stash push -u' fatal errors when sub-repo present
Date: Mon, 4 Oct 2021 18:29:52 -0400	[thread overview]
Message-ID: <a075091c-d0d4-db5d-fa21-c9d6c90c343e@gmail.com> (raw)
In-Reply-To: <xmqqzgrov7g2.fsf@gitster.g>

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

  reply	other threads:[~2021-10-04 22:29 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-30  0:49 Robert Leftwich
2021-09-30 10:06 ` René Scharfe
2021-09-30 16:35   ` Junio C Hamano
2021-10-01 14:25     ` René Scharfe
2021-10-04 20:06       ` Derrick Stolee
2021-10-04 20:52         ` Junio C Hamano
2021-10-04 22:29           ` Derrick Stolee [this message]
2021-10-07 20:31 ` [PATCH 1/3] t3905: show failure to ignore sub-repo René Scharfe
2021-10-07 20:31 ` [PATCH 2/3] read-cache: add verify_path_internal() René Scharfe
2021-10-07 20:31 ` [PATCH 3/3] read-cache: let verify_path() reject trailing dir separators again René Scharfe
2021-10-08  9:04   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a075091c-d0d4-db5d-fa21-c9d6c90c343e@gmail.com \
    --to=stolee@gmail.com \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=l.s.r@web.de \
    --cc=robert@gitpod.io \
    --subject='Re: Bug/regression report - '\''git stash push -u'\'' fatal errors when sub-repo present' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).