{"thread":{"id":"9384","subject":"possible bug in git apply?","startedAt":"2007-08-04T19:45:23Z","lastAt":"2007-08-06T09:37:18Z","messageCount":15,"participants":["david@lang.hm","Junio C Hamano","David Kastrup","Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"49770","messageId":"Pine.LNX.4.64.0708041243070.6905@asgard.lang.hm","threadId":"9384","inReplyTo":null,"subject":"possible bug in git apply?","fromName":"","fromEmail":"david@lang.hm","sentAt":"2007-08-04T19:45:23Z","receivedAt":"2007-08-04T19:45:23Z","isPatch":false,"sender":{"key":"david@lang.hm","avatar":null},"body":"since git doesn't track directories, only content (per the big discussion \nrecently) I beleive that doing a checkout would leave Rob without the \ndirectories that he emptied out, so shouldn't git apply also clear the \ndirectories to end up in the same state?\n\nDavid Lang\n\n---------- Forwarded message ----------\nDate: Sat, 4 Aug 2007 15:19:54 -0400\nFrom: Rob Landley \nTo: Geert Uytterhoeven \nCc: Roland Dreier ,\n Linux Kernel , linux-doc@vger.kernel.org,\n Andrew Morton , Paul Mundt \nSubject: Re: [PATCH 1/2] Group architecture Documentation under\n Documentation/arch.\n\nOn Saturday 04 August 2007 2:03:59 pm Rob Landley wrote:\n> Signed-off-by: Rob Landley \n> Amiga part Acked-by: Geert Uytterhoeven \n>\n> Move architecture-specific Documentation into a common subdirectory.\n\nI really, really, really hate git.\n\nOk, on my laptop I just noticed that \"git apply\" of the patch didn't complain\nbut it also left the empty subdirectories it moved stuff out of. (I don't\nbelieve this happened on the version of git I was using on my previous\nlaptop, which ate itself a month and change ago, but obviously I can't\ncheck.)\n\nThere is no \"git rmdir\". \"git rm\" refuses to delete the directory\nwithout -r. \"git rm -r Documentation/x86_64\" listed (as just deleted) all\nthe files that the patch already moved out of the directory.\n\nAm I missing something obvious here?\n\nRob\n-- \n\"One of my most productive days was throwing away 1000 lines of code.\"\n - Ken Thompson.\n"},{"id":"49772","messageId":"7v7iobjcjd.fsf@assigned-by-dhcp.cox.net","threadId":"9384","inReplyTo":"Pine.LNX.4.64.0708041243070.6905@asgard.lang.hm","subject":"Re: possible bug in git apply?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-04T20:00:06Z","receivedAt":"2007-08-04T20:00:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"david@lang.hm writes:\n\n> since git doesn't track directories, only content (per the big\n> discussion recently) I beleive that doing a checkout would leave Rob\n> without the directories that he emptied out, so shouldn't git apply\n> also clear the directories to end up in the same state?\n\nIt should and it does, unless you have untracked files in that\ndirectory that you haven't told git about. Then removing the\ndirectory along with these files will lose the files there,\nwhich is a wrong thing to do.\n\nA simple \"rm -fr this/directory/is/no/longer/interesting/to/me\"\nwould be all that is needed.\n"},{"id":"49773","messageId":"857iobdpw3.fsf@lola.goethe.zz","threadId":"9384","inReplyTo":"Pine.LNX.4.64.0708041243070.6905@asgard.lang.hm","subject":"Re: possible bug in git apply?","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-04T20:08:12Z","receivedAt":"2007-08-04T20:08:12Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"david@lang.hm writes:\n\n> On Saturday 04 August 2007 2:03:59 pm Rob Landley wrote:\n>> Signed-off-by: Rob Landley \n>> Amiga part Acked-by: Geert Uytterhoeven \n>>\n>> Move architecture-specific Documentation into a common subdirectory.\n>\n> I really, really, really hate git.\n>\n> Ok, on my laptop I just noticed that \"git apply\" of the patch didn't\n> complain but it also left the empty subdirectories it moved stuff\n> out of. (I don't believe this happened on the version of git I was\n> using on my previous laptop, which ate itself a month and change\n> ago, but obviously I can't check.)\n>\n> There is no \"git rmdir\". \"git rm\" refuses to delete the directory\n> without -r. \"git rm -r Documentation/x86_64\" listed (as just deleted) all\n> the files that the patch already moved out of the directory.\n>\n> Am I missing something obvious here?\n\nCommitting the change?\n\n> since git doesn't track directories, only content (per the big\n> discussion recently) I beleive that doing a checkout would leave Rob\n> without the directories that he emptied out, so shouldn't git apply\n> also clear the directories to end up in the same state?\n\nYes, once he commits. As long as git keeps files tracked in that\ndirectory, there is no reason for it to delete it.\n\nI agree that it is hard to come up with a good logic for this sort of\nthing. git-add checks the _current_ state of a file into the index.\ngit-rm can actually do the same only by actually _deleting_ the\nworking copy. So when should git try deleting the directory?\nProbably when the directory becomes empty in the index, for\nconsistency. Too bad that the index does not contain any information\nabout directories at all, so there is no good way to figure this\nparticular point in time out efficiently.\n\nI guess that git rather attempts deleting the directory when the tree\nin the _repository_ rather than the index becomes empty. And for that\nyou need to commit.\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"49815","messageId":"alpine.LFD.0.999.0708042141510.5037@woody.linux-foundation.org","threadId":"9384","inReplyTo":"Pine.LNX.4.64.0708041243070.6905@asgard.lang.hm","subject":"Re: possible bug in git apply?","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-08-05T04:48:08Z","receivedAt":"2007-08-05T04:48:08Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 4 Aug 2007, david@lang.hm wrote:\n>\n> since git doesn't track directories, only content (per the big discussion\n> recently) I beleive that doing a checkout would leave Rob without the\n> directories that he emptied out, so shouldn't git apply also clear the\n> directories to end up in the same state?\n\nIt should. I thought we did that, but maybe there's some bug there.\n\nSee \"remove_file()\" in builtin-apply.c.\n\nBut yeah, it seems that the file *rename* ends up not triggering that \nlogic! Very annoying.\n\nDoes this fix it? Totally untested, but it _looks_ obvious enough..\n\n\t\tLinus\n\n---\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 0a0b4a9..da27075 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -2508,7 +2508,7 @@ static void write_out_one_result(struct patch *patch, int phase)\n \t * thing: remove the old, write the new\n \t */\n \tif (phase == 0)\n-\t\tremove_file(patch, 0);\n+\t\tremove_file(patch, patch->is_rename);\n \tif (phase == 1)\n \t\tcreate_file(patch);\n }\n"},{"id":"49817","messageId":"alpine.LFD.0.999.0708042148440.5037@woody.linux-foundation.org","threadId":"9384","inReplyTo":"alpine.LFD.0.999.0708042141510.5037@woody.linux-foundation.org","subject":"Re: possible bug in git apply?","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-08-05T04:53:07Z","receivedAt":"2007-08-05T04:53:07Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 4 Aug 2007, Linus Torvalds wrote:\n>\n> But yeah, it seems that the file *rename* ends up not triggering that \n> logic! Very annoying.\n> \n> Does this fix it? Totally untested, but it _looks_ obvious enough..\n\nSide note: git will never remove a directory even if it becomes empty as \nfar as *git* is concerned, if there are other files that git doesn't know \nabout in there. So if you really really want to remove that directory, you \ndo end up having to often just doing\n\n\trm -rf some/dir/\n\nand the fact that you don't find a \"git rmdir\" is that with git, you \nreally should not even need it.\n\nWe do \"git mv\" and friends, but the fact is, that may have confused people \ninto thinking that git cares.\n\nThe \"default mindset\" should be to just do everything directly in the \nfilesystem, and git will just figure things out on its own. The \"git mv\" \nand \"git rm\" stuff is purely *convenience* features, nothing more.\n\nOther SCM's really want you to do \"scm rm\" or \"scm cp\" or whatever. Git \nreally really really doesn't care, and I think people are a bit too afraid \nof just doing the operation the normal UNIX way.\n\nSo next time you want to remove a directory, just remove it. With \nbog-standard unix tools. Use \"rm\". Or a graphical file manager. Or muck \nwith the disk directly using a binary editor. Git won't care. It will \nnotice that the file is gone, and do the right thing.\n\n\t\tLinus\n"},{"id":"49820","messageId":"7vvebuh8g8.fsf@assigned-by-dhcp.cox.net","threadId":"9384","inReplyTo":"alpine.LFD.0.999.0708042141510.5037@woody.linux-foundation.org","subject":"Re: possible bug in git apply?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-05T05:11:19Z","receivedAt":"2007-08-05T05:11:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> It should. I thought we did that, but maybe there's some bug there.\n>\n> See \"remove_file()\" in builtin-apply.c.\n>\n> But yeah, it seems that the file *rename* ends up not triggering that \n> logic! Very annoying.\n>\n> Does this fix it? Totally untested, but it _looks_ obvious enough..\n\nThat would regress the fix made in aea19457, I am afraid. If\nyou are in a subdirectory and the rename patch moves away the\nlast file in your current directory, the shell session you ran\nthe git-apply from will end up in an unlinked directory.\n\nMaybe that is a pilot error, and we can revert aea19457\naltogether?\n"},{"id":"49836","messageId":"857ioact4u.fsf@lola.goethe.zz","threadId":"9384","inReplyTo":"7vvebuh8g8.fsf@assigned-by-dhcp.cox.net","subject":"Re: possible bug in git apply?","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-05T07:55:45Z","receivedAt":"2007-08-05T07:55:45Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano writes:\n\n> Linus Torvalds writes:\n>\n>> It should. I thought we did that, but maybe there's some bug there.\n>>\n>> See \"remove_file()\" in builtin-apply.c.\n>>\n>> But yeah, it seems that the file *rename* ends up not triggering that \n>> logic! Very annoying.\n>>\n>> Does this fix it? Totally untested, but it _looks_ obvious enough..\n>\n> That would regress the fix made in aea19457, I am afraid. If\n> you are in a subdirectory and the rename patch moves away the\n> last file in your current directory, the shell session you ran\n> the git-apply from will end up in an unlinked directory.\n>\n> Maybe that is a pilot error, and we can revert aea19457\n> altogether?\n\nI'd call it pilot error. We can't really have git's behavior depend\non where somebody might point his cwd. If we do that, we may never\nremove any directory. Once git tracks directories, it will be harder\nto make this particular pilot error (git rm ../mycwd is more obviously\nhaving this effect than git rm *), but a pilot error it is in my book.\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"49913","messageId":"alpine.LFD.0.999.0708050949220.5037@woody.linux-foundation.org","threadId":"9384","inReplyTo":"7vvebuh8g8.fsf@assigned-by-dhcp.cox.net","subject":"Re: possible bug in git apply?","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-08-05T16:59:03Z","receivedAt":"2007-08-05T16:59:03Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 4 Aug 2007, Junio C Hamano wrote:\n> >\n> > Does this fix it? Totally untested, but it _looks_ obvious enough..\n> \n> That would regress the fix made in aea19457, I am afraid.\n\nThe fix in aea19457 is broken anyway.\n\nWhy? \n\nThat whole \"do it in two phases\" thing breaks it.\n\nWhat can happen is that you have a directory with 100 files, and:\n - a patch modifies 99 of them\n - and removes one\n\nWhat happens is that during phase 0, we'll remove all the files (and *not* \nwrite new ones), and then beause the last patch entry is a removal, we'll \nalso remove the directory (which succeeds, because all the files that got \nmodified are all long gone).\n\nThen, in phase 1, we'll re-create the files that we modified, and create a \nwhole new directory.\n\nIOW, as far as I can see we _already_ delete and then recreate the \ndirectory structure under some circumstances.\n\nI just extended it to also do it for \"rename\" and not just delete, since a \nrename may be renaming it to another directory.\n\nSo I'd say that my patch is a clear improvement on the current situation. \n\nThat said, if we really wanted to get it right, we should do this as a \nthree-phase thing: (1) remove old files (2) create new files (3) for all \nremovals and renames, try to remove source directories that might have \nbecome empty.\n\nThat would fix it properly and for all cases.\n\n\t\tLinus\n"},{"id":"49918","messageId":"85hcndj2b5.fsf@lola.goethe.zz","threadId":"9384","inReplyTo":"alpine.LFD.0.999.0708050949220.5037@woody.linux-foundation.org","subject":"Re: possible bug in git apply?","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-05T17:53:18Z","receivedAt":"2007-08-05T17:53:18Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Linus Torvalds writes:\n\n> On Sat, 4 Aug 2007, Junio C Hamano wrote:\n>> >\n>> > Does this fix it? Totally untested, but it _looks_ obvious enough..\n>> \n>> That would regress the fix made in aea19457, I am afraid.\n>\n> The fix in aea19457 is broken anyway.\n>\n> Why? \n>\n> That whole \"do it in two phases\" thing breaks it.\n\nDid not know that.\n\n> What can happen is that you have a directory with 100 files, and:\n> - a patch modifies 99 of them\n> - and removes one\n>\n> What happens is that during phase 0, we'll remove all the files (and *not* \n> write new ones), and then beause the last patch entry is a removal, we'll \n> also remove the directory (which succeeds, because all the files that got \n> modified are all long gone).\n>\n> Then, in phase 1, we'll re-create the files that we modified, and create a \n> whole new directory.\n>\n> IOW, as far as I can see we _already_ delete and then recreate the \n> directory structure under some circumstances.\n\nWhich will let the user sit in an empty directory void of . and ..,\nand a parallel directory with the old name elsewhere. Unpretty...\n\n> I just extended it to also do it for \"rename\" and not just delete,\n> since a rename may be renaming it to another directory.\n>\n> So I'd say that my patch is a clear improvement on the current\n> situation.\n>\n> That said, if we really wanted to get it right, we should do this as\n> a three-phase thing: (1) remove old files (2) create new files (3)\n> for all removals and renames, try to remove source directories that\n> might have become empty.\n>\n> That would fix it properly and for all cases.\n\nStupid question from someone without good background: why do we need\ntwo passes in the first place? Can't we just do phase 1/2/3 for every\nfile in one step? Is there any case where not having done (1) for a\n_different_ file is going to cause trouble for (2)? I presume this is\nintended for something like\n\ncreate a (plain file, coming in sort order before a/)\ndelete a/x\ndelete a/y (last file in a/)\n\nin the index and frankly, this will cease working in your three-phase\nscheme.\n\nThe problem is that we really need a -depth sort order for deletion in\nthe index, meaning that\ndelete xxx/yyy\nsorts before\ncreate xxx*\n\nWhen we don't change the sorting order, one can do so with recursion:\nwhen finding\ncreate a\nwe postpone processing it until a prospective\ndelete a/*\nis over. That means first processing a.txt, a.whatever/ and so on. I\nthink that is not sane.\n\nAs far as I can see, changing the index sort order for deletions is\nprobably the sanest _working_ way to go about this.\n\nOne problem is that corresponding \"delete\" and \"create\" items are then\nno longer necessarily adjacent in the index. The sensible way to go\nabout this is to sort the requests into _two_ linked lists, one in\n\"create\" order, one in \"delete\" order, and when merging a \"create\"\nrequest in the index, one compares with the head of the \"create\"\nordered list, and when merging a \"delete\" request in the index, one\ncompares with the head of the \"delete\" ordered list.\n\nIs this pretty? Not at all. But I don't see that any _fixed_ number\nof phases will buy us out of the principal problem, namely that\ndeletions have to be done depth-first, and deletions of a directory\nhave to be finished before we can attempt creating a new file in their\nplace.\n\nYes, this implies changing the index sort order, unfortunately.\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"49922","messageId":"alpine.LFD.0.999.0708051106020.5037@woody.linux-foundation.org","threadId":"9384","inReplyTo":"85hcndj2b5.fsf@lola.goethe.zz","subject":"Re: possible bug in git apply?","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-08-05T18:18:19Z","receivedAt":"2007-08-05T18:18:19Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 5 Aug 2007, David Kastrup wrote:\n> \n> Which will let the user sit in an empty directory void of . and ..,\n> and a parallel directory with the old name elsewhere. Unpretty...\n\nIt actually depends on the OS. POSIX allows the case of \"rmdir()\" \nreturning EBUSY if the directory is \"in use\", where \"in use\" may mean \nbeing somebodys current working directory.\n\nBut yes, most UNIXes (Linux very much included) will just remove the \ndirectory, and yes, the user ends up sitting in a directory that is empty \nand not reachable from anywhere else.\n\nEasy enough to see:\n\n\t[torvalds@woody ~]$ mkdir test-me ; cd test-me ; rmdir $(pwd)\n\t[torvalds@woody test-me]$ ls -a\n\t[torvalds@woody test-me]$ pwd\n\t/home/torvalds/test-me\n\t[torvalds@woody test-me]$ /bin/pwd\n\t/bin/pwd: couldn't find directory entry in `..' with matching i-node\n\n(the first \"pwd\" is a shell built-in, and the shell caches is).\n\nUnder Linux, you can also see funky things like this:\n\n\t[torvalds@woody test-me]$ ls -l /proc/self/cwd\n\tlrwxrwxrwx 1 torvalds torvalds 0 2007-08-05 11:09 /proc/self/cwd -> /home/torvalds/test-me (deleted)\n\nie khe kernel actually shows you that you're in a deleted directory, that \n_used_ to be called \"test-me\".\n\n\n> > That said, if we really wanted to get it right, we should do this as\n> > a three-phase thing: (1) remove old files (2) create new files (3)\n> > for all removals and renames, try to remove source directories that\n> > might have become empty.\n> >\n> > That would fix it properly and for all cases.\n> \n> Stupid question from someone without good background: why do we need\n> two passes in the first place?\n\nFor example, a patch that removes a directory structure \"x/...\" and then \ncreates a file \"x\" in its place.\n\nIn order for the patch ordering to not matter, you want to do the \"remove \nold state\" in an earlier phase.\n\nAnd patch order shouldn't matter, since you can have interesting things \nlike mixing of renames and creates etc (ie maybe it's not \"removing\" that \ndirectory x, it's just moving all the contents somewhere else, and maybe \nthe new file \"x\" is a move from somewhere else).\n\nCriss-crossing renames make it even more interesting.\n\nSo git-apply actually does things in more than two phases: there's a whole \nanother phase that is the \"read the patch and create the in-memory \nresult\". The \"two phases\" above are actually just the two phases concerned \nwith actually modifying the working tree, there's more phases elsewhere.\n\n\t\t\tLinus\n"},{"id":"49925","messageId":"853ayxiznp.fsf@lola.goethe.zz","threadId":"9384","inReplyTo":"alpine.LFD.0.999.0708051106020.5037@woody.linux-foundation.org","subject":"Re: possible bug in git apply?","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-05T18:50:34Z","receivedAt":"2007-08-05T18:50:34Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Linus Torvalds writes:\n\n> On Sun, 5 Aug 2007, David Kastrup wrote:\n\n[...]\n\n>> > That said, if we really wanted to get it right, we should do this as\n>> > a three-phase thing: (1) remove old files (2) create new files (3)\n>> > for all removals and renames, try to remove source directories that\n>> > might have become empty.\n>> >\n>> > That would fix it properly and for all cases.\n>> \n>> Stupid question from someone without good background: why do we need\n>> two passes in the first place?\n>\n> For example, a patch that removes a directory structure \"x/...\" and then \n> creates a file \"x\" in its place.\n>\n> In order for the patch ordering to not matter, you want to do the\n> \"remove old state\" in an earlier phase.\n\nBut your proposed three passes won't work with a patch removing\n\"x/...\" and creating \"x\" in its place, since \"x/\" gets only removed\nin pass 3, and \"x\" needs to created in pass 2 already.\n\nIf you had bothered reading my mail to the end: I explained exactly\nthat. So your three pass scheme actually breaks the case for which\nthe two-pass scheme has been designed.\n\nI propose you read my previous mail to its end: I explain a scheme\nthat will work in this case, but it would, as far as I understand\nindex processing, necessitate changing the index sort order, basically\nhaving -depth order for deletion entries.\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"49928","messageId":"alpine.LFD.0.999.0708051219440.5037@woody.linux-foundation.org","threadId":"9384","inReplyTo":"853ayxiznp.fsf@lola.goethe.zz","subject":"Re: possible bug in git apply?","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-08-05T19:20:48Z","receivedAt":"2007-08-05T19:20:48Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 5 Aug 2007, David Kastrup wrote:\n> \n> But your proposed three passes won't work with a patch removing\n> \"x/...\" and creating \"x\" in its place, since \"x/\" gets only removed\n> in pass 3, and \"x\" needs to created in pass 2 already.\n\nYes, I was wrong. The current two passes are the right thing to do, and we \nshould just always remove empty directories (and my patch was fine: we can \nleave them alone if it's a pure \"modify file in place\", but that's really \nthe only case).\n\n\t\tLinus\n"},{"id":"49931","messageId":"85odhlhiwq.fsf@lola.goethe.zz","threadId":"9384","inReplyTo":"alpine.LFD.0.999.0708051219440.5037@woody.linux-foundation.org","subject":"Re: possible bug in git apply?","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-05T19:37:41Z","receivedAt":"2007-08-05T19:37:41Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Linus Torvalds writes:\n\n> On Sun, 5 Aug 2007, David Kastrup wrote:\n>> \n>> But your proposed three passes won't work with a patch removing\n>> \"x/...\" and creating \"x\" in its place, since \"x/\" gets only removed\n>> in pass 3, and \"x\" needs to created in pass 2 already.\n>\n> Yes, I was wrong. The current two passes are the right thing to do, and we \n> should just always remove empty directories (and my patch was fine: we can \n> leave them alone if it's a pure \"modify file in place\", but that's really \n> the only case).\n\nThe consequence will be that renaming all files in one directory (and\n\"all\" can even be a single file) will temporarily delete and recreate\nthat directory. My proposed change of index sort and processing order\nwould take care of that without requiring multiple passes, at the cost\nof changing the index format and processing. I think that it would be\na sound long-term solution.\n\nAnyway, once directories can be tracked (again necessitating a change\nof index format), surprising directory deletion and recreation should\nbecome less of an issue, but it won't help with projects that continue\nnot tracking directories (presumably most patch-based workflows).\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"49994","messageId":"7vabt5dq11.fsf@assigned-by-dhcp.cox.net","threadId":"9384","inReplyTo":"alpine.LFD.0.999.0708050949220.5037@woody.linux-foundation.org","subject":"Re: possible bug in git apply?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-06T08:29:46Z","receivedAt":"2007-08-06T08:29:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> That said, if we really wanted to get it right, we should do this as a \n> three-phase thing: (1) remove old files (2) create new files (3) for all \n> removals and renames, try to remove source directories that might have \n> become empty.\n>\n> That would fix it properly and for all cases.\n\nActually that will break the case of removing foo/bar, which is\nthe only file in directory foo, and creating a new file foo. So\nif we really want to do all the corner cases, we would need to\ndo something like:\n\n * scan the list of files that will remain (i.e., renamed-to,\n modifed-in-place and created-anew) to note which directories\n should remain in the result;\n\n * remove old files, and remove its empty parent directories\n that are not in the above list;\n\n * create new files.\n\nBut in the meantime for 1.5.3, I think your patch is better than\nwhat we currently have.\n"},{"id":"50001","messageId":"7vir7tc8c1.fsf@assigned-by-dhcp.cox.net","threadId":"9384","inReplyTo":"7vabt5dq11.fsf@assigned-by-dhcp.cox.net","subject":"Re: possible bug in git apply?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-06T09:37:18Z","receivedAt":"2007-08-06T09:37:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Actually that will break the case of removing foo/bar, which is\n> the only file in directory foo, and creating a new file foo.\n\nSorry. The creation codepath already has enough code to remove\nthe left-over directory foo/ when creating the file foo. I\nthink your three-step process should be Ok.\n"}]}