On Tue Aug 4, 2020 at 7:49 PM EDT, Junio C Hamano wrote:
> How exactly does "git add -p" fail for such a patch? What operation
> does it exactly want to do ("apply --cached"???) and is it "apply"
> that is wrong, or is it "git add -p" that fails to remove the i-t-a
> entry from the index before running "git apply" that is at fault?
Yes, "add -p" uses "apply --cached". I do believe this belongs in apply,
both because all "add -p" really does is assemble things to be fed to
apply and also for the more detailed reasons below.
The index and the filesystem are both able to represent "no file" and "a
file exists" states, but the index has an additional state (i-t-a) with
no direct representation in the worktree. By (correctly) emitting "new
file" patches when comparing a file to an i-t-a index entry, we are
setting down the rule that a "new file" patch is not merely the diff
between "no file" and "a file exists", but also the diff between i-t-a
and "a file exists".
Similarly, "deleted file" patches are the diff between "a file exists"
and "no file exists", but they are also the diff between i-t-a and "no
file exists" - if you add -N a file and then delete it from the
worktree, "deleted file" is what git diff (correctly) shows. As a
consequence of these rules, "new file" and "deleted file" diffs are now
the only diffs that validly apply to an i-t-a entry. So apply needs to
handle them (in "--cached" mode, anyway).
But the worktree lives in the filesystem, where there are no i-t-a
entries. So the question seems to me to be whether "no file" in the
worktree matches an i-t-a entry in the index for the purposes of "add
--index". I count a couple options here:
- Nothing on the filesystem can accurately match an i-t-a entry in the
index, so all attempts at "apply --index" when there is an i-t-a in
the index fail with "file: does not match index". "apply --cached",
which "add -p" uses, applies only to the index and continues to work.
I think I prefer this one; additionally, the comment in read-cache.c
indicate that this is supposed to be the case already, so I just need
to make sure this check is not skipped on "new file" patches.
- The current (as of this patch) behavior: a "new file" patch applies
both to an i-t-a in the index, and to the lack of a file in the
worktree. This may seem strange, but it may also seem strange that an
identical new file patch, which can be applied either to just the
worktree or just the index successfully, fails when applied to both at
the same time with "apply --index". However, this is precisely what is
done anyway by "apply --index" when there are no i-t-a entries
involved, so I lean towards i-t-a entries never matching the worktree.
Patch for the first option in progress.
