{"thread":{"id":"105","subject":"[PATCH] fix bug in read-cache.c which loses files when merging a tree","startedAt":"2005-04-18T18:17:19Z","lastAt":"2005-04-18T22:09:31Z","messageCount":5,"participants":["James Bottomley","Linus Torvalds","Petr Baudis"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"674","messageId":"1113848239.4998.45.camel@mulgrave","threadId":"105","inReplyTo":null,"subject":"[PATCH] fix bug in read-cache.c which loses files when merging a tree","fromName":"James Bottomley","fromEmail":"james.bottomley@steeleye.com","sentAt":"2005-04-18T18:17:19Z","receivedAt":"2005-04-18T18:17:19Z","isPatch":true,"sender":{"key":"james.bottomley@steeleye.com","avatar":null},"body":"I noticed this when I tried a non-trivial scsi merge and checked the\nresults against BK.  The problem is that remove_entry_at() actually\ndecrements active_nr, so decrementing it in add_cache_entry() before\ncalling remove_entry_at() is a double decrement (hence we lose cache\nentries at the end).\n\nJames\n\nread-cache.c: 4d4d94f75cceb8039eb466c1956f8b54dc0e24b6\n--- read-cache.c\n+++ read-cache.c\t2005-04-18 13:08:09.000000000 -0500\n@@ -402,7 +402,6 @@\n \tif (pos < active_nr && ce_stage(ce) == 0) {\n \t\twhile (same_name(active_cache[pos], ce)) {\n \t\t\tok_to_add = 1;\n-\t\t\tactive_nr--;\n \t\t\tif (!remove_entry_at(pos))\n \t\t\t\tbreak;\n \t\t}\n\n\n"},{"id":"682","messageId":"Pine.LNX.4.58.0504181219480.15725@ppc970.osdl.org","threadId":"105","inReplyTo":"1113848239.4998.45.camel@mulgrave","subject":"Re: [PATCH] fix bug in read-cache.c which loses files when merging a tree","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-04-18T19:25:47Z","receivedAt":"2005-04-18T19:25:47Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 18 Apr 2005, James Bottomley wrote:\n>\n> I noticed this when I tried a non-trivial scsi merge and checked the\n> results against BK.  The problem is that remove_entry_at() actually\n> decrements active_nr, so decrementing it in add_cache_entry() before\n> calling remove_entry_at() is a double decrement (hence we lose cache\n> entries at the end).\n\nThanks. Just before I was going to hit the same issue, too.\n\nI've pushed out my first real content merge: since Daniel Barkalow's\nobject model stuff didn't apply to my tree any more (I had added the\ncommit type tracking to mine after Daniel did his conversion), I\ninstead applied his series to the place they were done against,\nand used git to merge the result with my current top-of-tree.\n\nI based it on the two example scripts I had sent out, but obviously never \ntested until this point (since both of them had some serious syntax \nerrors, and thus clearly wouldn't work).\n\nI also checked in the stupid scripts, in the expectation that somebody\nelse can improve on them and make them useful. For example, firing up an \neditor when the merge fails is probably a damn good idea.\n\nAnyway, it seems to prove the concept of a real three-way merge, and it \nall actually worked exactly the way I envisioned. Whether the end result \nworks or not, that's a different issue ;)\n\n\t\t\tLinus\n"},{"id":"696","messageId":"Pine.LNX.4.58.0504181330450.15725@ppc970.osdl.org","threadId":"105","inReplyTo":"1113854941.4998.61.camel@mulgrave","subject":"Re: [PATCH] fix bug in read-cache.c which loses files when merging a tree","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-04-18T21:19:46Z","receivedAt":"2005-04-18T21:19:46Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 18 Apr 2005, James Bottomley wrote:\n> \n> I had a problem with the SCSI tree in that there's a file removal in one\n> branch.  Your git-merge-one-file-script wouldn't have handled this\n> correctly: It seems to think that the file must be removed in both\n> branches, which is wrong.\n\nYes, I agree. My current \"merge-one-file-script\" doesn't actually look at \nwhat the original file was in this situation, and clearly it should. I \nthink I'll leave it for the user to decide what happens when somebody has \nmodified the deleted file, but clearly we should delete it if the other \nbranch has not touched it.\n\nI suspect that I should just pass in the SHA1 of the files to the\n\"merge-one-file-script\" from \"merge-cache\", rather than unpacking it.  \nAfter all, the merging script can do the unpacking itself with a simple\n\"cat-file blob $sha1\".\n\nAnd the fact is, many of the trivial merges should be handled by just\nlooking at the content, and doing a \"cmp\" on the files seems to be a\nstupid way to do that when we had the sha1 earlier.\n\nDone, and pushed out. Does the new merge infrastructure work for you?\n\n\t\tLinus\n"},{"id":"701","messageId":"20050418215819.GH5554@pasky.ji.cz","threadId":"105","inReplyTo":"Pine.LNX.4.58.0504181330450.15725@ppc970.osdl.org","subject":"Re: [PATCH] fix bug in read-cache.c which loses files when merging a tree","fromName":"Petr Baudis","fromEmail":"pasky@ucw.cz","sentAt":"2005-04-18T21:58:20Z","receivedAt":"2005-04-18T21:58:20Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Mon, Apr 18, 2005 at 11:19:46PM CEST, I got a letter\nwhere Linus Torvalds <torvalds@osdl.org> told me that...\n> I suspect that I should just pass in the SHA1 of the files to the\n> \"merge-one-file-script\" from \"merge-cache\", rather than unpacking it.  \n> After all, the merging script can do the unpacking itself with a simple\n> \"cat-file blob $sha1\".\n\nSo, I'm confused. Why did you introduce unpack-file instead of doing\njust this?\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nC++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor\n"},{"id":"704","messageId":"Pine.LNX.4.58.0504181508040.15725@ppc970.osdl.org","threadId":"105","inReplyTo":"20050418215819.GH5554@pasky.ji.cz","subject":"Re: [PATCH] fix bug in read-cache.c which loses files when merging a tree","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-04-18T22:09:31Z","receivedAt":"2005-04-18T22:09:31Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 18 Apr 2005, Petr Baudis wrote:\n> \n> So, I'm confused. Why did you introduce unpack-file instead of doing\n> just this?\n\nIt was code that I already had (ie the old code from \"merge-cache\" just\nmoved over), and thanks to that, I don't have to worry about broken\n\"mktemp\" crap in user space...\n\n\t\tLinus\n"}]}