Re: [PATCH v2 1/5] sparse-checkout: refactor skip worktree retry logic

[Elijah Newren <newren@gmail.com>]

On Wed, Jun 26, 2024 at 7:29 AM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Derrick Stolee <stolee@gmail.com>
>
[...]
> If users are having trouble with the performance of this operation and
> don't care about paths outside of the sparse-checkout, they can disable
> them using the sparse.expectFilesOutsideOfPatterns config option
> introduced in ecc7c8841d (repo_read_index: add config to expect files
> outside sparse patterns, 2022-02-25).

So, I had some heartburn with this paragraph when reading v1, but
decided to focus on other stuff.  But it still really bugs me while
reading v2.  So...

The purpose for the sparse.expectFilesOutsideOfPatterns option is very
specifically for the virtual-filesystem usecase (Google, specifically)
where sparse files aren't meant to stay sparse but be brought to life
the instant they are accessed (via a specialized filesystem and kernel
modules and whatnot).  Using it for any other reason, such as a
workaround for performance issues here, seems risky to me and I don't
like seeing it suggested.  The "if users...don't care about paths
outside of the sparse-checkout" really doesn't do it justice.  See the
"User-facing issues" section of
https://lore.kernel.org/git/b263cc75b7d4f426fb051d2cae5ae0fabeebb9c9.1642092230.git.gitgitgadget@gmail.com/;
we're foisting all those bugs back on users if they don't have such a
specialized filesystem and turn this knob on.  And then we'll get many
bug reports that are close to impossible to track down, and virtually
impossible to fix even if we do track it down.  I had for a while
suggested numerous fixes we needed in order to make
SKIP_WORKTREE-but-actually-present files work better, which required
fixes all over the codebase and which was serving as an impediment
e.g. to Victoria's desired sparse-index changes.  We gave up on those
other efforts long ago in favor of this much cleaner and simpler
solution of just clearing the SKIP_WORKTREE bit if the file wasn't
missing.

Further, I've seen people read git.git commit messages on Stack
Overflow and make suggestions based off them, so I'm a bit worried
this paragraph as worded will end up causing active harm, when I think
it's original intent was merely to provide full context around the
history of the clear_skip_worktree_from_present_files_sparse()
function.  While this bit of information is part of the history of
this function, I'm not sure it's really all that relevant to your
series.

Could we omit this paragraph, or if you want to keep it, reword it in
some way that doesn't make it look like some tradeoff that isn't that
big of a deal for non-specialized-vfs users?