{"thread":{"id":"50230","subject":"[PATCH 0/1] Be careful about left-over files from git add --edit runs","startedAt":"2019-01-15T15:42:55Z","lastAt":"2019-01-16T13:48:11Z","messageCount":5,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"366718","messageId":"pull.107.git.gitgitgadget@gmail.com","threadId":"50230","inReplyTo":null,"subject":"[PATCH 0/1] Be careful about left-over files from git add --edit runs","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-01-15T15:42:51Z","receivedAt":"2019-01-15T15:42:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"J Wyman reported almost a year ago (and I fixed this issue in Git for\nWindows around that time) that the .git/ADD_EDIT.patch file might still lie\naround at the beginning of git add --edit from previous runs, and if the new\npatch is smaller than the old one, the resulting diff is obviously corrupt.\n\nThis is yet another Git for Windows patch finally making it to the Git\nmailing list.\n\nJohannes Schindelin (1):\n add --edit: truncate the patch file\n\n builtin/add.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\n\nbase-commit: ecbdaf0899161c067986e9d9d564586d4b045d62\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-107%2Fdscho%2Fadd-e-truncate-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-107/dscho/add-e-truncate-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/107\n-- \ngitgitgadget\n"},{"id":"366719","messageId":"52f74c5d356325660ca1b494e78fb99a3c6c3861.1547566970.git.gitgitgadget@gmail.com","threadId":"50230","inReplyTo":"pull.107.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] add --edit: truncate the patch file","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-01-15T15:42:52Z","receivedAt":"2019-01-15T15:42:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIf there is already a .git/ADD_EDIT.patch file, we fail to truncate it\nproperly, which could result in very funny errors.\n\nOf course, this file should not be left lying around. But at least in\none case, there was a stale copy, larger than the current diff. So the\nresult was a corrupt diff.\n\nLet's just truncate the file when we write it and not worry about it too\nmuch.\n\nReported by J Wyman.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/add.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex f65c172299..53c18ea429 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -239,7 +239,7 @@ static int edit_patch(int argc, const char **argv, const char *prefix)\n \trev.diffopt.output_format = DIFF_FORMAT_PATCH;\n \trev.diffopt.use_color = 0;\n \trev.diffopt.flags.ignore_dirty_submodules = 1;\n-\tout = open(file, O_CREAT | O_WRONLY, 0666);\n+\tout = open(file, O_CREAT | O_WRONLY | O_TRUNC, 0666);\n \tif (out < 0)\n \t\tdie(_(\"Could not open '%s' for writing.\"), file);\n \trev.diffopt.file = xfdopen(out, \"w\");\n-- \ngitgitgadget\n"},{"id":"366733","messageId":"xmqqef9dsqkh.fsf@gitster-ct.c.googlers.com","threadId":"50230","inReplyTo":"pull.107.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/1] Be careful about left-over files from git add --edit runs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-15T18:50:38Z","receivedAt":"2019-01-15T18:50:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> J Wyman reported almost a year ago (and I fixed this issue in Git for\n> Windows around that time) that the .git/ADD_EDIT.patch file might still lie\n> around at the beginning of git add --edit from previous runs, and if the new\n> patch is smaller than the old one, the resulting diff is obviously corrupt.\n>\n> This is yet another Git for Windows patch finally making it to the Git\n> mailing list.\n\nGood, finally ;-). \n\nI didn't realize that \"add -e\" codepath was so old until today to\ncheck with \"git blame\" to see how old this bug was (it predates the\ntransition from builtin-foo.c to builtin/foo.c); it seems that it\nwas from day one of the feature.\n\n>\n> Johannes Schindelin (1):\n> add --edit: truncate the patch file\n>\n> builtin/add.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n>\n> base-commit: ecbdaf0899161c067986e9d9d564586d4b045d62\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-107%2Fdscho%2Fadd-e-truncate-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-107/dscho/add-e-truncate-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/107\n"},{"id":"366734","messageId":"xmqqa7k1sq24.fsf@gitster-ct.c.googlers.com","threadId":"50230","inReplyTo":"52f74c5d356325660ca1b494e78fb99a3c6c3861.1547566970.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] add --edit: truncate the patch file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-15T19:01:39Z","receivedAt":"2019-01-15T19:01:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> Of course, this file should not be left lying around. But at least in\n> one case, there was a stale copy, larger than the current diff. So the\n> result was a corrupt diff.\n\nAfter creating the fd (now with O_TRUNC), we die in a few places.\n\n - Failure to open the file for writing. This is OK and do not need\n any further clean-up.\n\n - Failure to write a patch into that file. In this case, it might\n be worth unlinking the file.\n\n - Error exit from the editor. In this case, the user may want to\n take a look and use the contents in it manually, so not removing\n the file, which is what we do right now, may be the right thing.\n\n On the other hand, such a patch file is easy to recreate with\n \"git diff\", so it may be worth unlinking the file.\n\n - Failure from stat() after editing the file. In this case, it\n might be worth unlinking the file.\n\nSo I agree that \"this file should not be left lying around\" in most\nof the cases (except a half).\n\nIn any case, when we are about to use the file for a newly created\npatch at the beginning of this function, we know that the old\ncontent is no longer relevant, so the change to truncate before\nusing it is absolutely the right thing to do, and makes it moot to\nask \"should we remove it after an error, or should we leave it to\navoid information loss?\".\n\nGood ;-)\n\n"},{"id":"366850","messageId":"nycvar.QRO.7.76.6.1901161447150.41@tvgsbejvaqbjf.bet","threadId":"50230","inReplyTo":"xmqqef9dsqkh.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/1] Be careful about left-over files from git add --edit runs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-01-16T13:47:49Z","receivedAt":"2019-01-16T13:48:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Tue, 15 Jan 2019, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n> \n> I didn't realize that \"add -e\" codepath was so old until today to\n> check with \"git blame\" to see how old this bug was (it predates the\n> transition from builtin-foo.c to builtin/foo.c); it seems that it\n> was from day one of the feature.\n\nYes, I introduced that bug right from the start. On the good side: this is\n*not* an off-by-one bug!\n\nCiao,\nDscho\n\n> \n> >\n> > Johannes Schindelin (1):\n> > add --edit: truncate the patch file\n> >\n> > builtin/add.c | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> >\n> > base-commit: ecbdaf0899161c067986e9d9d564586d4b045d62\n> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-107%2Fdscho%2Fadd-e-truncate-v1\n> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-107/dscho/add-e-truncate-v1\n> > Pull-Request: https://github.com/gitgitgadget/git/pull/107\n> \n"}]}