git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Elijah Newren <newren@gmail.com>
Cc: Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Victoria Dye <vdye@github.com>, Derrick Stolee <stolee@gmail.com>,
	Lessley Dennington <lessleydennington@gmail.com>,
	Jonathan Tan <jonathantanmy@google.com>,
	Jose Lopes <jabolopes@google.com>,
	Jeff Hostetler <jeffhostetler@github.com>
Subject: Re: [PATCH v2 3/5] repo_read_index: clear SKIP_WORKTREE bit from files present in worktree
Date: Sat, 19 Feb 2022 10:14:49 -0800	[thread overview]
Message-ID: <YhEzmdhxHC3W8ijE@google.com> (raw)
In-Reply-To: <CABPp-BHU4VYXF8kNvZEwBLu2BYP2Q1c9dYMW_8QfNmvGjB1ZOA@mail.gmail.com>

Hi,

Elijah Newren wrote:

> And, of course, you're trying to do more than just detect
> inconsistencies -- you want the vfs to fully control the sparsity
> patterns and expand them based on dynamic file accesses by the user.
> That dynamic bit doesn't play well with the non-vfs workaround.
>
>
> Does that sound right?

You captured some of it.  There's a bit more: typically when using
sparse checkout the traditional way, you will not have files in your
working directory that do not match the sparse checkout pattern.  That
way, the disk usage in the working copy is nice and small.  But with a
vfsd like described in [2], having other files in the working
directory does not cost disk usage because the corresponding data even
in compressed (git object) form does not have to be present locally
until the files are accessed.  So a vfsd gives an end user the
illusion that all files are present, whereas git only operates on a
small subset (the working set).

With this change, git would start operating on all the files.

[...]
> Side note: I thought Microsoft's vfs was first named GVFS and then
> based on naming collisions renamed to VFS for Git.  Sounds like you
> have something that is probably a bit different, but which you are
> also calling VFS for Git?

No, sorry for the lack of clarity.  When I say "VFS for Git", I
genuinely mean https://vfsforgit.org/, which was authored by Microsoft
and to my understanding is still used by Microsoft's Windows team and
is available for anyone to use.  (That page currently returns a cert
error because their SSL cert expired 10 days ago.  But hopefully it
conveys the idea, and the content is still there if you go through the
interstitial.)

I agree that it can be kind of confusing to talk about that alongside
VFSes in general, but I didn't choose the name. :)

[...]
> On Fri, Feb 18, 2022 at 5:07 PM Jonathan Nieder <jrnieder@gmail.com> wrote:

>>  a. We could guard it with a config option.  It would still be a
>>     regression for VFS for Git users, but they'd be able to use the
>>     config option to restore the expected behavior.
[...]
>>  b. We could distinguish between the vfsd and the "interrupted and
>>     forgot to update SKIP_WORKTREE bits in the index" cases somehow.
>>     This sounds complex.
>>
>>  c. Something else?
>>
>> (b) and (c) aren't sounding obviously good, so (a) seems tempting.
>> What do you think?
>
> Yeah, I'm having a hard time coming up with a way that either the VFS
> could recognize these special Git present-despite-skip-worktree checks
> and treat them differently, or having Git recognize that it is running
> under a special VFS that likes to aggressively and automagically
> expand the sparsity patterns.  So (a) seems tempting to me too.

Thanks.  In a way it feels like giving up (isn't it better when things
automagically Just Work?), but I think it's the right thing to do.

> Got any name suggestions?  core.avoidPresentDespiteSkipWorktreeCheck
> (defaulting to false)?  core.sparsityConsistencyCheck (defaulting to
> true)?
>
> Did your team want to implement that on top of
> en/present-despite-skipped since you can verify if it works for you,
> or did you want me to take a stab at it?  Should be a pretty simple
> change.

Monday is a holiday here; it shouldn't be hard to get a patch out
later this week.  Happy to write one but I also won't be at all offended
if someone else writes it first.

Ideally the config name should describe the intent from the user's
perspective instead of the implementation.  So something like
"sparsecheckout.expectFilesOutsideSparsePattern".

Thanks,
Jonathan

>> [2] see
>> https://lore.kernel.org/git/20220207190320.2960362-1-jonathantanmy@google.com/
>> for what I mean by "vfsd"

  reply	other threads:[~2022-02-19 18:15 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-13 16:43 [PATCH 0/5] Remove the present-despite-SKIP_WORKTREE class of bugs (for sparse-checkouts) Elijah Newren via GitGitGadget
2022-01-13 16:43 ` [PATCH 1/5] t1011: add testcase demonstrating accidental loss of user modifications Elijah Newren via GitGitGadget
2022-01-13 16:43 ` [PATCH 2/5] unpack-trees: fix accidental loss of user changes Elijah Newren via GitGitGadget
2022-01-13 16:43 ` [PATCH 3/5] repo_read_index: clear SKIP_WORKTREE bit from files present in worktree Elijah Newren via GitGitGadget
2022-01-13 16:43 ` [PATCH 4/5] Update documentation related to sparsity and the skip-worktree bit Elijah Newren via GitGitGadget
2022-01-13 16:43 ` [PATCH 5/5] Accelerate clear_skip_worktree_from_present_files() by caching Elijah Newren via GitGitGadget
2022-01-13 23:35   ` Elijah Newren
2022-01-14 15:59 ` [PATCH v2 0/5] Remove the present-despite-SKIP_WORKTREE class of bugs (for sparse-checkouts) Elijah Newren via GitGitGadget
2022-01-14 15:59   ` [PATCH v2 1/5] t1011: add testcase demonstrating accidental loss of user modifications Elijah Newren via GitGitGadget
2022-02-16  8:51     ` Ævar Arnfjörð Bjarmason
2022-02-16 16:02       ` Elijah Newren
2022-01-14 15:59   ` [PATCH v2 2/5] unpack-trees: fix accidental loss of user changes Elijah Newren via GitGitGadget
2022-01-14 15:59   ` [PATCH v2 3/5] repo_read_index: clear SKIP_WORKTREE bit from files present in worktree Elijah Newren via GitGitGadget
2022-02-16  8:57     ` Ævar Arnfjörð Bjarmason
2022-02-16 16:08       ` Elijah Newren
2022-02-19  1:06     ` Jonathan Nieder
2022-02-19 16:42       ` Elijah Newren
2022-02-19 18:14         ` Jonathan Nieder [this message]
2022-02-20  5:28           ` Elijah Newren
2022-02-20 16:56       ` Derrick Stolee
2022-02-22 23:17         ` Jonathan Nieder
2022-01-14 15:59   ` [PATCH v2 4/5] Update documentation related to sparsity and the skip-worktree bit Elijah Newren via GitGitGadget
2022-02-16  9:15     ` Ævar Arnfjörð Bjarmason
2022-02-16 16:21       ` Elijah Newren
2022-01-14 15:59   ` [PATCH v2 5/5] Accelerate clear_skip_worktree_from_present_files() by caching Elijah Newren via GitGitGadget
2022-01-15  1:39     ` Victoria Dye
2022-02-16  9:32     ` Ævar Arnfjörð Bjarmason
2022-02-16 16:30       ` Elijah Newren
2022-02-17  4:40         ` Elijah Newren
2022-01-15  1:51   ` [PATCH v2 0/5] Remove the present-despite-SKIP_WORKTREE class of bugs (for sparse-checkouts) Victoria Dye

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=YhEzmdhxHC3W8ijE@google.com \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=jabolopes@google.com \
    --cc=jeffhostetler@github.com \
    --cc=jonathantanmy@google.com \
    --cc=lessleydennington@gmail.com \
    --cc=newren@gmail.com \
    --cc=stolee@gmail.com \
    --cc=vdye@github.com \
    --subject='Re: [PATCH v2 3/5] repo_read_index: clear SKIP_WORKTREE bit from files present in worktree' \
    /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

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).