{"thread":{"id":"54679","subject":"Bug report: git apply --cached --reject","startedAt":"2020-11-20T14:21:49Z","lastAt":"2020-11-21T20:18:55Z","messageCount":2,"participants":["Patrick Stevens","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"410421","messageId":"31599b45-cf4e-be77-22bb-8fa03f0a52d6@patrickstevens.co.uk","threadId":"54679","inReplyTo":null,"subject":"Bug report: git apply --cached --reject","fromName":"Patrick Stevens","fromEmail":"patrick@patrickstevens.co.uk","sentAt":"2020-11-20T14:21:38Z","receivedAt":"2020-11-20T14:21:49Z","isPatch":false,"sender":{"key":"patrick@patrickstevens.co.uk","avatar":null},"body":"Hi,\n\nBelow the fold is a `git bugreport`-generated report of a bug with `git\napply --cached --reject`, which I have reproduced in three different\nenvironments. Summary: we do not correctly stage the removal of a file\nif there is also an unrelated change that cannot be applied.\n\nI don't think this behaviour is intended; in the report I give a couple\nof variations which correctly do what I expected, and this one breaks\nthe semantics I expect, given the behaviour of those variations. I have\nnot tried to find the source of the bug.\n\nThanks,\nPatrick Stevens\n\n---\n\nThank you for filling out a Git bug report!\nPlease answer the following questions to help us understand your issue.\n\nWhat did you do before the bug happened? (Steps to reproduce your issue)\n\nComplete reproduction instructions:\n\n```\nmkdir badrepo && cd badrepo && git init\necho \"start\" >> file.txt && git add file.txt && git commit -m \"Initial\ncommit\"\ngit tag initialcommit\necho \"another\" >> another.txt && git add another.txt && git commit -m\n\"Something else\"\ngit tag commit2\ngit rm another.txt && git rm file.txt && git commit -m \"Remove both\"\ngit tag commit3\ngit checkout initialcommit\ngit diff commit2 commit3 | git apply --reject --cached\n```\n\nWhat did you expect to happen? (Expected behavior)\n\nWe should be left detached at tag `initialcommit`, with the working copy\nclean (i.e. containing only `file.txt`), and with the deletion of\n`file.txt` staged.\n\nWhat happened instead? (Actual behavior)\n\nThe final `git apply` correctly leaves us detached at tag\n`initialcommit`, with a clean working copy, but incorrectly the deletion\nof `file.txt` is not staged: the index is also clean.\n\nWhat's different between what you expected and what actually happened?\n\nThe file `file.txt` should have had its deletion staged, because this is\npart of the diff which could apply cleanly.\n\nAnything else you want to add:\n\nIf I delete the `--cached` from the last line, the right thing happens:\nthe working copy has `file.txt` deleted in the working copy but the\ndeletion is not staged.\nIf instead I remove the `git rm another.txt`, then similarly the right\nthing happens: now we end up with the deletion of `file.txt` staged (but\nthe working copy is correctly unchanged).\n\nI have reproduced this using git version 2.24.1.windows.2, as well as\n2.29.2 built on Windows using the WSL and invoked through WSL, as well\nas from the Mac HomeBrew install where I produced this bug report.\n\nPlease review the rest of the bug report below.\nYou can delete any lines you don't wish to share.\n\n[System Info]\ngit version:\ngit version 2.29.2\ncpu: x86_64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nuname: Darwin 18.7.0 Darwin Kernel Version 18.7.0: Mon Aug 31 20:53:32\nPDT 2020; root:xnu-4903.278.44~1/RELEASE_X86_64 x86_64\ncompiler info: clang: 11.0.0 (clang-1100.0.33.17)\nlibc info: no libc information available\n$SHELL (typically, interactive shell): /bin/zsh\n\n\n[Enabled Hooks]\n\n\n"},{"id":"410470","messageId":"xmqqlfeusbtk.fsf@gitster.c.googlers.com","threadId":"54679","inReplyTo":"31599b45-cf4e-be77-22bb-8fa03f0a52d6@patrickstevens.co.uk","subject":"Re: Bug report: git apply --cached --reject","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-21T20:18:47Z","receivedAt":"2020-11-21T20:18:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Stevens <patrick@patrickstevens.co.uk> writes:\n\n> I don't think this behaviour is intended; in the report I give a couple\n> of variations which correctly do what I expected, and this one breaks\n> the semantics I expect, given the behaviour of those variations. I have\n> not tried to find the source of the bug.\n\nI do not think this is intended, either.  It certainly is not\nsomething I would expect to see as an end user.  I somehow suspect\nthat when we added \"--reject\", we didn't even mean to make it work\nwith \"--cached\" in the first place.\n\nA more plain-vanilla case like below, where an existing file sees a\ntwo-hunk patch, but only one of the hunks apply cleanly,\ndemonstrates that the command, with an option to touch the index,\ndoes not want to take any ...\n\n--- >8 ------ >8 ------ >8 ------ >8 ------ >8 ------ >8 ---\n\nseq 1 10 >file && git add file && git commit -m \"1 to 10\"\nseq 1 10 | sed -e '/1/s/$/+/' >file\ngit diff --stat -p >patch\n\nseq 1 10 | sed -e '/9/s/$/+/' >file\ngit commit -m \"9 plus\" file\n\nrm -f file.rej\ngit reset --hard\ngit apply --reject patch\n# inspect the index and working tree to see what happened here\n# this one, that does not touch the index, works as expected.\n\nrm -f file.rej\ngit reset --hard\ngit apply --cached --reject patch\n# inspect the index and working tree to see what happened here\n\nrm -f file.rej\ngit reset --hard\ngit apply --index --reject patch\n# inspect the index and working tree to see what happened here\n\n--- 8< ------ 8< ------ 8< ------ 8< ------ 8< ------ 8< ---\n\n... half-baked changes.  The above prepares a 10-line file and a\npatch that touches its first and the last line.  It then makes a\nmodification that textually conflicts the second last line to the\ntarget file, and tries to apply the patch.\n\nWe do get .rej file in all cases, but in the mode that would touch\nthe index, the command refuses to put the result of an incomplete\npatch application to the index, even though it updates the working\ntree files with partial application.\n\nThe log message of 8938045a (git-apply --reject: finishing touches.,\n2006-08-27) does mention that with \"--index\", it is the designed\nbehaviour to update the index for paths to which the patches were\ncleanly applied, with a reasonable justification.  Even though it\nalso implies at the same time that it is also the designed behaviour\nto leave the index untouched for paths that got .rej files, I do not\noffhand see it explains the reason behind the design very well.  I\nsuspect that it was to avoid committing a half-resolved state by\nmistake, but then it does not prevent the user from committing a\nno-modification by mistake, and that is why I say \"not ... very well\"\nhere.  In any case, it does not even mention what ought to happen in\nthe \"--cached\" case, so...\n\n"}]}