Re: [PATCH v3 7/7] rm: honor sparse checkout patterns

[Derrick Stolee <stolee@gmail.com>]

On 3/12/2021 5:48 PM, Matheus Tavares wrote:
> `git add` refrains from adding or updating index entries that are
> outside the current sparse checkout, but `git rm` doesn't follow the
> same restriction. This is somewhat counter-intuitive and inconsistent.
> So make `rm` honor the sparsity rules and advise on how to remove
> SKIP_WORKTREE entries just like `add` does. Also add some tests for the
> new behavior.
> 
> Suggested-by: Elijah Newren <newren@gmail.com>
> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
> ---
>  Documentation/config/advice.txt  |  5 +-
>  Documentation/git-rm.txt         |  4 +-
>  builtin/rm.c                     | 35 +++++++++-----
>  t/t3602-rm-sparse-checkout.sh    | 78 ++++++++++++++++++++++++++++++++
>  t/t7011-skip-worktree-reading.sh |  5 --
>  5 files changed, 108 insertions(+), 19 deletions(-)
>  create mode 100755 t/t3602-rm-sparse-checkout.sh
> 
> diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
> index 851b83cf30..8b2849ff7b 100644
> --- a/Documentation/config/advice.txt
> +++ b/Documentation/config/advice.txt
> @@ -120,6 +120,7 @@ advice.*::
>  		Advice shown if a user runs the add command without providing
>  		the pathspec parameter.
>  	updateSparsePath::
> -		Advice shown when linkgit:git-add[1] is asked to update index
> -		entries outside the current sparse checkout.
> +		Advice shown when either linkgit:git-add[1] or linkgit:git-rm[1]
> +		is asked to update index entries outside the current sparse
> +		checkout.

I wonder if this is going to need to be updated forever, or if in
the _next_ iteration we just say "Advice shown when a command is
asked to update..." (using the "three is many" rule).

> diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
> index ab750367fd..26e9b28470 100644
> --- a/Documentation/git-rm.txt
> +++ b/Documentation/git-rm.txt
> @@ -23,7 +23,9 @@ branch, and no updates to their contents can be staged in the index,
>  though that default behavior can be overridden with the `-f` option.
>  When `--cached` is given, the staged content has to
>  match either the tip of the branch or the file on disk,
> -allowing the file to be removed from just the index.
> +allowing the file to be removed from just the index. When
> +sparse-checkouts are in use (see linkgit:git-sparse-checkout[1]),
> +`git rm` will only remove paths within the sparse-checkout patterns.

Good to update in the correct spot.

Implementation and tests look good, too.

I just wanted to chime in and voice my support of this general
direction. Thank you!

-Stolee