Re: [PATCH] sparse-checkout.txt: new document with sparse-checkout directions

From: Elijah Newren <newren@gmail.com>
Date: Sep 27, 2022, 03:05 UTC
Message-ID: <CABPp-BFo_0MaBfyhDBfRby4CSHy8brjMY_8nif2S_Aogn-NqUg@mail.gmail.com>
In-Reply-To: <xmqqwn9qox1z.fsf@gitster.g>

On Mon, Sep 26, 2022 at 10:38 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 9 quoted lines

>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > +    In the case of am and apply, those commands only operate on the
> > +    working tree, so they are kind of in the same boat as stash.
>
> "apply" does not touch the HEAD but it can touch the index; when it
> operates with the "--cached" or the "--index" option, it should not
> be considered as a working-tree-only command.

Ah, right, good flag. This helps resolve part of my question, but gives me a new question as well.

Without --cached or --index, I think we'd need to make `apply` behave like `stash` and just auto-vivify any files being tweaked. If we don't, we'll lose changes from the patch.

"apply --cached" could possibly just update the index. However, it appears to have another bug I need to add to the known bugs section. `apply --cached` updates the index, but the new index entry fails to carry over the "SKIP_WORKTREE" bit, making it appear there is an unstaged deletion of the file. (Users can run `git sparse-checkout reapply` afterwards as a workaround.). This is slightly weird for files with conflicts (created when running `git apply -3 --cached`) since those files with content conflicts will not be present in the working tree, but that's in line with the fact that `git apply -3 --cached` refuses to touch the working tree in general.

In line with `--cached`, we could have "apply --index" do updates to both the index and the working copy, while ensuring any "SKIP_WORKTREE" bits are preserved for non-conflicted files. However, would preserving "SKIP_WORKTREE" bits be weird for users? On one hand, `git apply` without `--index` auto-vivifies files and `--index` says to "also apply changes to the index" -- but preserving SKIP_WORKTREE bits would make the `--index` flag also affect how the working tree is treated, which might seem odd. On the other hand, merge/cherry-pick/rebase will update files in the index while leaving the file missing from the working tree when not conflicted, so there is some precedent for such behavior. The question might just be whether `git apply --index` should be more like mergy behavior, or more like `git apply`/`git stash` behavior.

> "am" is about recording what is in the patch as a commit.

Does that mean it should behave like "apply --index"? Or more like cherry-pick? (This question might be moot depending on what we choose for "apply --index", in particular, it won't matter if we preserve SKIP_WORKTREE bits on non-conflicted files.)

Show 6 quoted lines

> > +    Perhaps `git am` could run `git sparse-checkout reapply`
> > +    automatically afterward and move into a category more similar to
> > +    merge/rebase/cherry-pick, but it'd still be weird because it'd
> > +    vivify files besides just conflicted ones when there are conflicts.
>
> I do not particularly think it is so bad.

For some reason I was thinking of running `git sparse-checkout reapply` only if the `am` operation succeeded, which would give us a special one-off command treatment. If we instead view it as always running `git sparse-checkout reapply` whether or not we hit conflicts, or equivalently, if we view `git am` preserving SKIP_WORKTREE bits on non-conflicted files, then I agree it's not weird anymore and can be classified in the same group as merge/rebase/cherry-pick.

But something else you said confuses me...

Show 15 quoted lines

> How would we handle the case where the user modifies paths outside
> the sparse specification and makes a commit out of the result,
> without using "am"?  We should be consistent with that use case, i.e.
>
>     $ edit path/outside/sparse/specification
>     $ git add path/outside/sparse/specification
>     $ git commit
>
> Do we require some "Yes, I am aware that I need to widen my sparse
> specification to do this, because I am now stepping out of it, and I
> understand that my sparse specification becomes wider after doing
> this operation" confirmation with "add" or "commit"?  If not, then I
> think "am" should silently widen just like these commands.  If they
> do, then "am" should also require such an option.  Perhaps call it
> "--widen-sparse" or whatever.

The command
    $ edit path/outside/sparse/specification
doesn't make sense to me; the file (and perhaps also its leading
directories) are missing.  Most editors will probably tell you that
you are editing a new file, but then it's more of a "rewrite from
scratch" than an "edit".

Typically, we'd expect users who want to edit such files to do so by first running the `add` or `set` subcommands of sparse-checkout to change their sparse specification so that the file becomes present. But then it's no longer outside the sparse specification. So, I'm not sure how this angle could help guide our direction.

Show 5 quoted lines

> By the way, I like the term "sparse specification" very much, as
> we should worry about non-cone mode as well.  Please use it
> consistently in this document after getting a concensus that it
> is a good phrase to use from others---I saw some other words
> used after "sparse" elsewhere in this patch.

:-)

Show 6 quoted lines

> > +    In the case of ls-files, `git ls-files -t` is often used to see what
> > +    is sparse and not, in which case restricting would not make sense.
>
> I suspect that leaving it tree-wide would allow scripters come up
> with Porcelains that restricts to the sparse specification more
> easily.