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

On 12/10, Elijah Newren wrote:

On Sun, Dec 9, 2018 at 12:05 PM Thomas Gummerer [off-list ref] wrote:

quoted

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'.