Re: [PATCH 7/8] checkout: allow ignoring unmatched pathspec

[Thomas Gummerer <t.gummerer@gmail.com>]

On 12/10, Elijah Newren wrote:
> On Sun, Dec 9, 2018 at 12:05 PM Thomas Gummerer <t.gummerer@gmail.com> wrote:
> >
> > Currently when 'git checkout -- <pathspec>...' is invoked with
> > multiple pathspecs, where one or more of the pathspecs don't match
> > anything, checkout errors out.
> >
> > This can be inconvenient in some cases, such as when using git
> > checkout from a script.  Introduce a new --ignore-unmatched option,
> > which which allows us to ignore a non-matching pathspec instead of
> > erroring out.
> >
> > In a subsequent commit we're going to start using 'git checkout' in
> > 'git stash' and are going to make use of this feature.
> 
> This makes sense, but seems incomplete.  But to explain it, I think
> there's another bug I need to demonstrate first because it's related
> on builds on it.  First, the setup:
> 
>   $ echo foo >subdir/newfile
>   $ git add subdir/newfile
>   $ echo bar >>subdir/newfile
>   $ git status
>   On branch A
>   Changes to be committed:
>     (use "git reset HEAD <file>..." to unstage)
> 
>       new file:   subdir/newfile
> 
>   Changes not staged for commit:
>     (use "git add <file>..." to update what will be committed)
>     (use "git checkout -- <file>..." to discard changes in working directory)
> 
>       modified:   subdir/newfile
> 
> Now, does it do what we expect?
> 
>   $ git checkout HEAD -- subdir/newfile
>   error: pathspec 'subdir/newfile' did not match any file(s) known to git
> 
> This is the old overlay behavior; kinda lame, but you made no claims
> about fixing the default behavior.  What about with your new option?
> 
>   $ git checkout --no-overlay HEAD -- subdir
>   $ git status
>   On branch A
>   nothing to commit, working tree clean
> 
> Yes, the feature seems to work as advertised.  However, let's try
> again with a different variant:
> 
>   $ echo foo >subdir/newfile
>   $ git checkout --no-overlay HEAD -- subdir
>   $ git status
>   On branch A
>   Untracked files:
>     (use "git add <file>..." to include in what will be committed)
> 
>       subdir/newfile
> 
> Why is the file ignored and left there?  Also:
> 
>   $ git checkout --no-overlay HEAD -- subdir/newfile
>   error: pathspec 'subdir/newfile' did not match any file(s) known to git
> 
> That seems wrong to me.

Ah interesting, this is a case I didn't consider.  I'm a bit torn on
this one.  My intention for the no overlay mode was that it would work
similar to what I'd expect 'git reset --hard -- <pathspec>' to work if
it existed, which means not removing untracked files if they exist.

While I think in the example you have above removing subdir/newfile
may be the right behaviour I'm not so sure in the case of 'git
checkout --no-overlay HEAD -- .' or ''git checkout --no-overlay HEAD
-- t/*' for example.  I don't think that should remove all untracked
files in the repository or in the t/ directory.  Removing untracked
files in that case would probably surprise users more than your case
above would.

I think it's okay to keep considering untracked files as special with
respect to how they are treated by 'git checkout --no-overlay'.

>                          The point of no-overlay is to make it match
> HEAD, and while subdir/newfile doesn't exist in HEAD or the index it
> does match in the working tree so the intent is clear. But let's say
> that the user did go ahead and specify your new flag:
> 
> 
>   $ git checkout --no-overlay --ignore-unmatch HEAD -- subdir/newfile
>   $ git status
>   On branch A
>   Untracked files:
>     (use "git add <file>..." to include in what will be committed)
> 
>       subdir/newfile
> 
>   nothing added to commit but untracked files present (use "git add" to track)
> 
> So now it avoids erroring out when the user does more work than
> necessary, but it still misses appropriately cleaning up the file.

Yeah this is a good point, this could be more confusing to the user
than the previous case in my opinion.  Maybe I'll just drop this patch
for now (and the next one, as it's better to hold of until stash in C
lands anyway), and then try to do all this in-core for 'git stash'.