Re: [PATCH v2 3/5] repo_read_index: clear SKIP_WORKTREE bit from files present in worktree

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Feb 19, 2022, 18:14 UTC
Message-ID: <YhEzmdhxHC3W8ijE@google.com>
In-Reply-To: <CABPp-BHU4VYXF8kNvZEwBLu2BYP2Q1c9dYMW_8QfNmvGjB1ZOA@mail.gmail.com>

Hi,

Elijah Newren wrote:

Show 7 quoted lines

> 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.

[...]

Show 14 quoted lines

>>  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.

Show 8 quoted lines

> 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"