Hi,

On Mon, Nov 16, 2020 at 9:20 PM Elijah Newren [off-list ref] wrote:

On Mon, Nov 16, 2020 at 12:14 PM Junio C Hamano [off-list ref] wrote:

quoted

Matheus Tavares [off-list ref] writes:

quoted

Make git-rm honor the 'sparse.restrictCmds' setting, by restricting its
operation to the paths that match both the command line pathspecs and
the repository's sparsity patterns.

quoted

This better matches the expectations
of users with sparse-checkout definitions, while still allowing them
to optionally enable the old behavior with 'sparse.restrictCmds=false'
or the global '--no-restrict-to-sparse-paths' option.

Hmph.  Is "rm" the only oddball that ignores the sparse setting?

This might make you much less happy, but in general none of the
commands pay attention to the setting; I think a line or two in

This isn't quite right; as noted at the just submitted [1], there are
three different classes of ways that existing commands at least
partially pay attention to the setting.

[1] https://lore.kernel.org/git/5143cba7047d25137b3d7f8c7811a875c1931aee.1605891222.git.gitgitgadget@gmail.com/ (local)

merge-recursive.c is the only part of the codebase outside of
unpack_trees() that pays any attention to it at all.  This was noted
as a problem in the initial review of the sparse-checkout series at
[1], and was the biggest factor behind me requesting the following
being added to the manpage for sparse-checkout[2]:

THIS COMMAND IS EXPERIMENTAL. ITS BEHAVIOR, AND THE BEHAVIOR OF OTHER
COMMANDS IN THE PRESENCE OF SPARSE-CHECKOUTS, WILL LIKELY CHANGE IN
THE FUTURE.

The fact that commands have only somewhat paid attention to this
setting is still a problem, though.  In fact, it was apparently a
known problem as far back as 2009 just from looking at the short list
of TODOs at the end of that file.

quoted

quoted

 to the paths specified by the sparsity patterns, or to the intersection of
 those paths and any (like `*.c`) that the user might also specify on the
 command line. When false, the affected commands will work on full trees,
-ignoring the sparsity patterns. For now, only git-grep honors this setting.
+ignoring the sparsity patterns. For now, only git-grep and git-rm honor this
+setting.

I am not sure if this is a good direction to go---can we make an
inventory of all commands that affect working tree files and see
which ones need the same treatment before going forward with just
"grep" and "rm"?  Documenting the decision on the ones that will not
get the same treatment may also be a good idea.  What I am aiming
for is to prevent users from having to know in which versions of Git
they can rely on the sparsity patterns with what commands, and doing
things piecemeal like these two topics would be a road to confusion.

It's not just commands which affect the working tree that need to be
inventoried and adjusted.  We've made lists of commands in the past:

[3] https://lore.kernel.org/git/CABPp-BEbNCYk0pCuEDQ_ViB2=varJPBsVODxNvJs0EVRyBqjBg@mail.gmail.com/ (local)
[4] https://lore.kernel.org/git/xmqqy2y3ejwe.fsf@gitster-ct.c.googlers.com/ (local)

So, I think there are a few other commands that need to be modified
the same way rm is here by Matheus, a longer list of commands than
what I previously linked to for other modifications, some warnings and
error messages that need to be cleaned up, and a fair amount of
additional testing needed.  I also think we need to revisit the flag
names for --restrict-to-sparse-paths and
--no-restrict-to-sparse-paths; some feedback I'm getting suggest they
might be more frequently used than I originally suspected and thus we
might want shorter names.  (--sparse and --dense?)  So we probably
want to wait off on both mt/grep-sparse-checkout and
mt/rm-sparse-checkout (sorry Matheus) and maybe my recently submitted
stash changes (though those don't have an exposed
--[no]-restrict-to-sparse-paths flag and are modelled on existing
merge behavior) until we have a bigger plan in place.

But I only dug into it a bit while working on the stash apply bug; I'm
going to dig more (probably just after Thanksgiving) and perhaps make
a Documentation/technical/ file of some sort to propose more plans
here.