Hi, On Mon, Nov 16, 2020 at 9:20 PM Elijah Newren <newren@xxxxxxxxx> wrote: > > On Mon, Nov 16, 2020 at 12:14 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > > > Matheus Tavares <matheus.bernardino@xxxxxx> writes: > > > > > 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. > > > > > 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@xxxxxxxxx/ > 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. > > > 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@xxxxxxxxxxxxxx/ > [4] https://lore.kernel.org/git/xmqqy2y3ejwe.fsf@xxxxxxxxxxxxxxxxxxxxxxxxx/ 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.
