Re: [PATCH 4/4] restore: add --intent-to-add (restoring worktree only)

[Duy Nguyen <pclouds@gmail.com>]

On Thu, Jun 20, 2019 at 9:34 PM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 6/20/2019 5:55 AM, Nguyễn Thái Ngọc Duy wrote:
> > "git restore --source" (without --staged) could create new files
> > (i.e. not present in index) on worktree to match the given source. But
> > the new files are not tracked, so both "git diff" and "git diff
> > <source>" ignore new files. "git commit -a" will not recreate a commit
> > exactly as the given source either.
> >
> > Add --intent-to-add to help track new files in this case, which is the
> > default on the least surprise principle.
>
> I was unfamiliar with this behavior, but did check the 'restore' command
> myself and saw that it would register the file as untracked. I agree that
> could be confusing for a user, so adding it to the staging environment
> makes this more in-line with `git checkout <rev> -- <path>`.

It's actually not the same as "git checkout <rev>" which would restore
<path> in both index and worktree, while "git restore" (no --staged)
only touches worktree . Try "git diff --cached" and "git diff" in both
cases, you'll see the differences.

Or in other words, "git commit" (no -a) after "git checkout" records
the version of <path> from <rev>, while "git commit" after "git
restore" will commit whatever you have before git-restore. "git commit
-a" behaves the same way for both (though it drops <path> without this
patch).

> > @@ -392,6 +393,69 @@ static int checkout_worktree(const struct checkout_opts *opts)
> >       return errs;
> >  }
> >
> > +/*
> > + * Input condition: r->index contains the file list matching worktree.
> > + *
> > + * r->index is reloaded with $GIT_DIR/index. Files that exist in the
> > + * current worktree but not in $GIT_DIR/index are added back as
> > + * intent-to-add.
> > + */
>
> Reading this code (and being unfamiliar with the cache array) I thought
> it might accidentally add untracked files from the working directory into
> the index. A local test verified that was not the case. Is that worth
> adding to your test below?

It never occured to me because r->index (before this function) should
be the same as <rev>, more or less. But yeah, adding a garbage file
and checking that it remains garbage is a good idea. I'll rename it
"untracked" though to be clear.
-- 
Duy