Bug report: git apply --cached --reject

Hi,

Below the fold is a `git bugreport`-generated report of a bug with `git apply --cached --reject`, which I have reproduced in three different environments. Summary: we do not correctly stage the removal of a file if there is also an unrelated change that cannot be applied.

I don't think this behaviour is intended; in the report I give a couple of variations which correctly do what I expected, and this one breaks the semantics I expect, given the behaviour of those variations. I have not tried to find the source of the bug.

Thanks, Patrick Stevens

---

Thank you for filling out a Git bug report! Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)

Complete reproduction instructions:

``` mkdir badrepo && cd badrepo && git init echo "start" >> file.txt && git add file.txt && git commit -m "Initial commit" git tag initialcommit echo "another" >> another.txt && git add another.txt && git commit -m "Something else" git tag commit2 git rm another.txt && git rm file.txt && git commit -m "Remove both" git tag commit3 git checkout initialcommit git diff commit2 commit3 | git apply --reject --cached ```

What did you expect to happen? (Expected behavior)

We should be left detached at tag `initialcommit`, with the working copy clean (i.e. containing only `file.txt`), and with the deletion of `file.txt` staged.

What happened instead? (Actual behavior)

The final `git apply` correctly leaves us detached at tag `initialcommit`, with a clean working copy, but incorrectly the deletion of `file.txt` is not staged: the index is also clean.

What's different between what you expected and what actually happened?

The file `file.txt` should have had its deletion staged, because this is part of the diff which could apply cleanly.

Anything else you want to add:

If I delete the `--cached` from the last line, the right thing happens: the working copy has `file.txt` deleted in the working copy but the deletion is not staged. If instead I remove the `git rm another.txt`, then similarly the right thing happens: now we end up with the deletion of `file.txt` staged (but the working copy is correctly unchanged).

I have reproduced this using git version 2.24.1.windows.2, as well as 2.29.2 built on Windows using the WSL and invoked through WSL, as well as from the Mac HomeBrew install where I produced this bug report.

Please review the rest of the bug report below. You can delete any lines you don't wish to share.

[System Info] git version: git version 2.29.2 cpu: x86_64 no commit associated with this build sizeof-long: 8 sizeof-size_t: 8 shell-path: /bin/sh uname: Darwin 18.7.0 Darwin Kernel Version 18.7.0: Mon Aug 31 20:53:32 PDT 2020; root:xnu-4903.278.44~1/RELEASE_X86_64 x86_64 compiler info: clang: 11.0.0 (clang-1100.0.33.17) libc info: no libc information available $SHELL (typically, interactive shell): /bin/zsh

[Enabled Hooks]

Patrick Stevens <patrick@patrickstevens.co.uk> writes:

I do not think this is intended, either. It certainly is not something I would expect to see as an end user. I somehow suspect that when we added "--reject", we didn't even mean to make it work with "--cached" in the first place.

A more plain-vanilla case like below, where an existing file sees a two-hunk patch, but only one of the hunks apply cleanly, demonstrates that the command, with an option to touch the index, does not want to take any ...

--- >8 ------ >8 ------ >8 ------ >8 ------ >8 ------ >8 ---

seq 1 10 >file && git add file && git commit -m "1 to 10" seq 1 10 | sed -e '/1/s/$/+/' >file git diff --stat -p >patch

seq 1 10 | sed -e '/9/s/$/+/' >file git commit -m "9 plus" file

rm -f file.rej git reset --hard git apply --reject patch # inspect the index and working tree to see what happened here # this one, that does not touch the index, works as expected.

rm -f file.rej git reset --hard git apply --cached --reject patch # inspect the index and working tree to see what happened here

rm -f file.rej git reset --hard git apply --index --reject patch # inspect the index and working tree to see what happened here

--- 8< ------ 8< ------ 8< ------ 8< ------ 8< ------ 8< ---

... half-baked changes. The above prepares a 10-line file and a patch that touches its first and the last line. It then makes a modification that textually conflicts the second last line to the target file, and tries to apply the patch.

We do get .rej file in all cases, but in the mode that would touch the index, the command refuses to put the result of an incomplete patch application to the index, even though it updates the working tree files with partial application.

The log message of 8938045a (git-apply --reject: finishing touches., 2006-08-27) does mention that with "--index", it is the designed behaviour to update the index for paths to which the patches were cleanly applied, with a reasonable justification. Even though it also implies at the same time that it is also the designed behaviour to leave the index untouched for paths that got .rej files, I do not offhand see it explains the reason behind the design very well. I suspect that it was to avoid committing a half-resolved state by mistake, but then it does not prevent the user from committing a no-modification by mistake, and that is why I say "not ... very well" here. In any case, it does not even mention what ought to happen in the "--cached" case, so...