From: Elijah Newren <newren@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Matheus Tavares <matheus.bernardino@usp.br>,
Git Mailing List <git@vger.kernel.org>,
Derrick Stolee <stolee@gmail.com>
Subject: Re: [PATCH] rm: honor sparse checkout patterns
Date: Fri, 20 Nov 2020 09:06:58 -0800 [thread overview]
Message-ID: <CABPp-BGMX3wb7LiS1HkJpGveoW3J1oR0vVHbKTF5+qYLRF+59g@mail.gmail.com> (raw)
In-Reply-To: <CABPp-BHvZyb4cF29HqDYgMHTMEr2LdvKYATWqADRyhqJzB=Liw@mail.gmail.com>
Hi,
On Mon, Nov 16, 2020 at 9:20 PM Elijah Newren <newren@gmail.com> wrote:
>
> On Mon, Nov 16, 2020 at 12:14 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Matheus Tavares <matheus.bernardino@usp.br> 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@gmail.com/
> 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@mail.gmail.com/
> [4] https://lore.kernel.org/git/xmqqy2y3ejwe.fsf@gitster-ct.c.googlers.com/
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.
next prev parent reply other threads:[~2020-11-20 17:07 UTC|newest]
Thread overview: 93+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-12 21:01 [PATCH] rm: honor sparse checkout patterns Matheus Tavares
2020-11-12 23:54 ` Elijah Newren
2020-11-13 13:47 ` Derrick Stolee
2020-11-15 20:12 ` Matheus Tavares Bernardino
2020-11-15 21:42 ` Johannes Sixt
2020-11-16 12:37 ` Matheus Tavares Bernardino
2020-11-23 13:23 ` Johannes Schindelin
2020-11-24 2:48 ` Matheus Tavares Bernardino
2020-11-16 14:30 ` Jeff Hostetler
2020-11-17 4:53 ` Elijah Newren
2020-11-16 13:58 ` [PATCH v2] " Matheus Tavares
2021-02-17 21:02 ` [RFC PATCH 0/7] add/rm: honor sparse checkout and warn on sparse paths Matheus Tavares
2021-02-17 21:02 ` [RFC PATCH 1/7] add --chmod: don't update index when --dry-run is used Matheus Tavares
2021-02-17 21:45 ` Junio C Hamano
2021-02-18 1:33 ` Matheus Tavares
2021-02-17 21:02 ` [RFC PATCH 2/7] add: include magic part of pathspec on --refresh error Matheus Tavares
2021-02-17 22:20 ` Junio C Hamano
2021-02-17 21:02 ` [RFC PATCH 3/7] t3705: add tests for `git add` in sparse checkouts Matheus Tavares
2021-02-17 23:01 ` Junio C Hamano
2021-02-17 23:22 ` Eric Sunshine
2021-02-17 23:34 ` Junio C Hamano
2021-02-18 3:11 ` Matheus Tavares Bernardino
2021-02-18 3:07 ` Matheus Tavares Bernardino
2021-02-18 14:38 ` Matheus Tavares
2021-02-18 19:05 ` Junio C Hamano
2021-02-18 19:02 ` Junio C Hamano
2021-02-22 18:53 ` Elijah Newren
2021-02-17 21:02 ` [RFC PATCH 4/7] add: make --chmod and --renormalize honor " Matheus Tavares
2021-02-17 21:02 ` [RFC PATCH 5/7] pathspec: allow to ignore SKIP_WORKTREE entries on index matching Matheus Tavares
2021-02-17 21:02 ` [RFC PATCH 6/7] add: warn when pathspec only matches SKIP_WORKTREE entries Matheus Tavares
2021-02-19 0:34 ` Junio C Hamano
2021-02-19 17:11 ` Matheus Tavares Bernardino
2021-02-17 21:02 ` [RFC PATCH 7/7] rm: honor sparse checkout patterns Matheus Tavares
2021-02-22 18:57 ` [RFC PATCH 0/7] add/rm: honor sparse checkout and warn on sparse paths Elijah Newren
2021-02-24 4:05 ` [PATCH v2 " Matheus Tavares
2021-02-24 4:05 ` [PATCH v2 1/7] add: include magic part of pathspec on --refresh error Matheus Tavares
2021-02-24 4:05 ` [PATCH v2 2/7] t3705: add tests for `git add` in sparse checkouts Matheus Tavares
2021-02-24 5:15 ` Elijah Newren
2021-02-24 4:05 ` [PATCH v2 3/7] add: make --chmod and --renormalize honor " Matheus Tavares
2021-02-24 4:05 ` [PATCH v2 4/7] pathspec: allow to ignore SKIP_WORKTREE entries on index matching Matheus Tavares
2021-02-24 5:23 ` Elijah Newren
2021-02-24 4:05 ` [PATCH v2 5/7] refresh_index(): add REFRESH_DONT_MARK_SPARSE_MATCHES flag Matheus Tavares
2021-02-24 4:05 ` [PATCH v2 6/7] add: warn when pathspec only matches SKIP_WORKTREE entries Matheus Tavares
2021-02-24 6:50 ` Elijah Newren
2021-02-24 15:33 ` Matheus Tavares
2021-03-04 15:23 ` Matheus Tavares
2021-03-04 17:21 ` Elijah Newren
2021-03-04 21:03 ` Junio C Hamano
2021-03-04 22:48 ` Elijah Newren
2021-03-04 21:26 ` Matheus Tavares Bernardino
2021-02-24 4:05 ` [PATCH v2 7/7] rm: honor sparse checkout patterns Matheus Tavares
2021-02-24 6:59 ` Elijah Newren
2021-02-24 7:05 ` [PATCH v2 0/7] add/rm: honor sparse checkout and warn on sparse paths Elijah Newren
2021-03-12 22:47 ` [PATCH v3 " Matheus Tavares
2021-03-12 22:47 ` [PATCH v3 1/7] add: include magic part of pathspec on --refresh error Matheus Tavares
2021-03-12 22:47 ` [PATCH v3 2/7] t3705: add tests for `git add` in sparse checkouts Matheus Tavares
2021-03-23 20:00 ` Derrick Stolee
2021-03-12 22:47 ` [PATCH v3 3/7] add: make --chmod and --renormalize honor " Matheus Tavares
2021-03-12 22:47 ` [PATCH v3 4/7] pathspec: allow to ignore SKIP_WORKTREE entries on index matching Matheus Tavares
2021-03-12 22:48 ` [PATCH v3 5/7] refresh_index(): add REFRESH_DONT_MARK_SPARSE_MATCHES flag Matheus Tavares
2021-03-18 23:45 ` Junio C Hamano
2021-03-19 0:00 ` Junio C Hamano
2021-03-19 12:23 ` Matheus Tavares Bernardino
2021-03-19 16:05 ` Junio C Hamano
2021-03-30 18:51 ` Matheus Tavares Bernardino
2021-03-31 9:14 ` Elijah Newren
2021-03-12 22:48 ` [PATCH v3 6/7] add: warn when asked to update SKIP_WORKTREE entries Matheus Tavares
2021-03-12 22:48 ` [PATCH v3 7/7] rm: honor sparse checkout patterns Matheus Tavares
2021-03-21 23:03 ` Ævar Arnfjörð Bjarmason
2021-03-22 1:08 ` Matheus Tavares Bernardino
2021-03-23 20:47 ` Derrick Stolee
2021-03-13 7:07 ` [PATCH v3 0/7] add/rm: honor sparse checkout and warn on sparse paths Elijah Newren
2021-04-08 20:41 ` [PATCH v4 " Matheus Tavares
2021-04-08 20:41 ` [PATCH v4 1/7] add: include magic part of pathspec on --refresh error Matheus Tavares
2021-04-08 20:41 ` [PATCH v4 2/7] t3705: add tests for `git add` in sparse checkouts Matheus Tavares
2021-04-14 16:39 ` Derrick Stolee
2021-04-08 20:41 ` [PATCH v4 3/7] add: make --chmod and --renormalize honor " Matheus Tavares
2021-04-08 20:41 ` [PATCH v4 4/7] pathspec: allow to ignore SKIP_WORKTREE entries on index matching Matheus Tavares
2021-04-08 20:41 ` [PATCH v4 5/7] refresh_index(): add flag to ignore SKIP_WORKTREE entries Matheus Tavares
2021-04-08 20:41 ` [PATCH v4 6/7] add: warn when asked to update " Matheus Tavares
2021-04-08 20:41 ` [PATCH v4 7/7] rm: honor sparse checkout patterns Matheus Tavares
2021-04-14 16:36 ` [PATCH v4 0/7] add/rm: honor sparse checkout and warn on sparse paths Elijah Newren
2021-04-14 18:04 ` Matheus Tavares Bernardino
2021-04-16 21:33 ` Junio C Hamano
2021-04-16 23:17 ` Elijah Newren
2020-11-16 20:14 ` [PATCH] rm: honor sparse checkout patterns Junio C Hamano
2020-11-17 5:20 ` Elijah Newren
2020-11-20 17:06 ` Elijah Newren [this message]
2020-12-31 20:03 ` sparse-checkout questions and proposals [Was: Re: [PATCH] rm: honor sparse checkout patterns] Elijah Newren
2021-01-04 3:02 ` Derrick Stolee
2021-01-06 19:15 ` Elijah Newren
2021-01-07 12:53 ` Derrick Stolee
2021-01-07 17:36 ` Elijah Newren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CABPp-BGMX3wb7LiS1HkJpGveoW3J1oR0vVHbKTF5+qYLRF+59g@mail.gmail.com \
--to=newren@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=matheus.bernardino@usp.br \
--cc=stolee@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).