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

[Jonathan Nieder <jrnieder@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"