{"thread":{"id":"7288","subject":"[RFH] straightening out \"read-tree -m\"","startedAt":"2007-03-18T07:25:40Z","lastAt":"2007-03-19T00:04:06Z","messageCount":6,"participants":["Junio C Hamano","Matthias Lederhofer","Brian Gernhardt","Steven Grimm"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"37357","messageId":"7vtzwjnhcb.fsf@assigned-by-dhcp.cox.net","threadId":"7288","inReplyTo":null,"subject":"[RFH] straightening out \"read-tree -m\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-18T07:25:40Z","receivedAt":"2007-03-18T07:25:40Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"While everybody else was having fun with low-hanging fruit\nperformance tweaking ;-), I was looking at rather depressing\npicture.\n\nI do not have much energy left to look into this further\ntonight, but I noticed this while drawing a case table to deal\nwith the recent \"why can't I switch between these two branches\"\nproblem.\n\nFirst part is sort of \"accomplishment\" for tonight. Two case\ntables that describe desired semantics (meaning, the current\n\"read-tree -m -u\" does not do this, and neither does\nmerge-recursive because it shares the same unpack-trees\nbackend).\n\n* 2-way \"read-tree -m -u A B\".\n\nWhile switching from branch A to branch B, if a path \"foo\" is\ndirectory/file in A while the path is directory/file in B, then\nthese things should happen...\n\nA\tB\tOutcome\n----------------------------------------------------------------\nD\tD\tNo problem.\n\nD\tF\tThis should result in refusal when there are\n local modification to paths in \"foo\" directory,\n\t\tor there are untracked (but unignored) paths in\n \"foo/\".\n\nF\tD\tThis should result in refusal if \"foo\" has local\n modification.\n\nF\tF\tThis should result in refusal if \"foo\" has local\n\t\tmodification.\n\n\nThe patch to \"verify_absent()\" I sent out earlier tries to deal\nwith the second case above, although it still has issues\n(namely, the *indpos needs to be recomputed after replacing\npotentially multiple entries under \"foo/\" with \"foo\" from B,\nwhich the patch does not do).\n\nThen I started to wonder if we have similar problem on the 3-way\nside.\n\n* 3-way \"read-tree -m -u O A B\"\n\nWhile on branch A, if you try to merge branch B using O as the\nmerge base, if a path \"foo\" is D/F in A/B/C (there are 8\ncombinations), these things should happen...\n\nO\tA\tB\tOutcome\n----------------------------------------------------------------\nD\tD\tD\tNo problem. Paths in the \"foo/\" directory \n\t\t\tare merged subject to the usual rule.\n\nD\tD\tF\tFile \"foo\", which did not exist in ancestor\n\t\t\tand we did not touch, is created. Which means\n\t\t\tthat contents of \"foo/\" is about to be\n\t\t\tlost. We need to refuse this unless\n\t\t\teverything in \"foo/\" matches between O\n\t\t\tand A, there is no local modifications\n\t\t\tin \"foo/\", and there is no untracked but\n\t\t\tunignored paths in \"foo/\".\n\nD\tF\tD\tFile \"foo\", which did not exist in\n\t\t\tancestor and they did not touch, is\n\t\t\tcreated, so we would want to keep it.\n\t\t\tWhich means that their changes to paths\n\t\t\tin \"foo/\" will be lost. We should leave\n\t\t\tconflict for paths in \"foo/\" that are\n\t\t\tdifferent between O and B.\n\nD\tF\tF\tNo problem. This is usual \"added on\n\t\t\tboth branches\" conflict.\n\nF\tD\tD\tNo problem. Paths in the \"foo/\"\n \t\tdirectory have usual \"added on both\n \t\tbranches\" conflict.\n\nF\tD\tF\tNo problem. If file \"foo\" was modified\n\t\t\tby them (i.e. O!=B) then usual \"remove-modify\"\n\t\t\tconflict arises.\n\nF\tF\tD\tNo problem. If file \"foo\" was modified\n\t\t\tby us (i.e. O!=A) then usual \"remove-modify\"\n\t\t\tconflict arises. Also \"foo\" must not\n\t\t\thave local modifications.\n\nF\tF\tF\tNo problem, usual 3-way rule applies.\n\t\t\tAlso \"foo\" must not have local modifications.\n\n\nThe sad story is how confused the current unpack-trees is, given\nthe test case for the above 3-way.\n\nThe attached script demonstrates one aspect of the sorry state\nof three-way merge done with unpack-trees. You would notice\nthat there are multiple paths that have both stage '0' entry and\nhigher stage entries at the same time --- I do not think this\nshould not happen.\n\nGit v1.0.0 to v1.2.0 refuse to merge but leaves two stage '0'\nentries for path '4' and '8' (which is already incorrect but in\na different way). v1.3.0 and later leaves stage 0/1/2/3 entry\nfor path '8'.\n\nDepressing...\n\n\n#!/bin/sh\n\n# If your 'test' is precious, do not run this script!\n/bin/rm -fr test\n\ncreate () {\n\trm -f .git/index\n\tmark=\"$1\"\n\tshift\n\tnum=0\n\tfor t\n\tdo\n\t\tnum=$(($num+1))\n\t\trm -fr \"$num\"\n\t\tcase \"$t\" in\n\t\tD)\n\t\t\tmkdir \"$num\"; t=\"$num/$num\" ;;\n\t\tF)\n\t\t\tt=\"$num\" ;;\n\t\tesac\n\t\techo \"$mark\" >\"$t\"\n\t\tgit add \"$t\"\n\tdone\n\tgit commit -m \"$mark: $*\"\n}\n\nmkdir test || exit\ncd test\ngit init-db\n\ncreate O D D D D F F F F\ngit tag O\ngit branch a\ngit branch b\n\nrm -rf ?\ngit checkout a\ncreate A D D F F D D F F\ngit tag A\n\nrm -rf ?\ngit checkout b\ncreate B D F D F D F D F\ngit tag B\n\nrm -rf ?\ngit checkout a\ngit read-tree -m O A B\n\ngit ls-files -s\n"},{"id":"37371","messageId":"20070318121810.GA17807@moooo.ath.cx","threadId":"7288","inReplyTo":"7vtzwjnhcb.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFH] straightening out \"read-tree -m\"","fromName":"Matthias Lederhofer","fromEmail":"matled@gmx.net","sentAt":"2007-03-18T12:18:10Z","receivedAt":"2007-03-18T12:18:10Z","isPatch":false,"sender":{"key":"matled@gmx.net","avatar":null},"body":"Junio C Hamano wrote:\n> A\tB\tOutcome\n> ----------------------------------------------------------------\n> D\tD\tNo problem.\n> \n> D\tF\tThis should result in refusal when there are\n> local modification to paths in \"foo\" directory,\n> \t\tor there are untracked (but unignored) paths in\n> \"foo/\".\n\nWhat about ignored configuration files like config.mak?\nLet's say there is an ignored configuration file in a subdirectory.\nIf this subdirectory is replaced by a file git would delete the\nignored configuration file?\n"},{"id":"37381","messageId":"16CD10D5-3BDA-4DFE-A8C6-96D48F511824@silverinsanity.com","threadId":"7288","inReplyTo":"20070318121810.GA17807@moooo.ath.cx","subject":"Re: [RFH] straightening out \"read-tree -m\"","fromName":"Brian Gernhardt","fromEmail":"benji@silverinsanity.com","sentAt":"2007-03-18T16:06:12Z","receivedAt":"2007-03-18T16:06:12Z","isPatch":false,"sender":{"key":"benji@silverinsanity.com","avatar":"https://gravatar.com/avatar/e06c101dbc25c68114d859b4a9ec7cf8a2c52fd2b0270ef0eac0e2e63ff22311?d=mp&s=160"},"body":"\nOn Mar 18, 2007, at 8:18 AM, Matthias Lederhofer wrote:\n\n> Junio C Hamano wrote:\n>> A\tB\tOutcome\n>> ----------------------------------------------------------------\n>> D\tD\tNo problem.\n>>\n>> D\tF\tThis should result in refusal when there are\n>> local modification to paths in \"foo\" directory,\n>> \t\tor there are untracked (but unignored) paths in\n>> \"foo/\".\n>\n> What about ignored configuration files like config.mak?\n> Let's say there is an ignored configuration file in a subdirectory.\n> If this subdirectory is replaced by a file git would delete the\n> ignored configuration file?\n\nI would sincerely hope that a file not being tracked by git would \ncount as \"local modifications\" in that case. \"Ignored\" does not mean \n\"unimportant\".\n\nMy $0.02,\n~~ Brian\n"},{"id":"37395","messageId":"7v8xdul2rt.fsf@assigned-by-dhcp.cox.net","threadId":"7288","inReplyTo":"20070318121810.GA17807@moooo.ath.cx","subject":"Re: [RFH] straightening out \"read-tree -m\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-18T20:23:18Z","receivedAt":"2007-03-18T20:23:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthias Lederhofer writes:\n\n> Junio C Hamano wrote:\n>> A\tB\tOutcome\n>> ----------------------------------------------------------------\n>> D\tD\tNo problem.\n>> \n>> D\tF\tThis should result in refusal when there are\n>> local modification to paths in \"foo\" directory,\n>> \t\tor there are untracked (but unignored) paths in\n>> \"foo/\".\n>\n> What about ignored configuration files like config.mak?\n> Let's say there is an ignored configuration file in a subdirectory.\n> If this subdirectory is replaced by a file git would delete the\n> ignored configuration file?\n\nYou certainly could argue that way. At that point switching\nfrom branch A to branch B would become impossible after building\nin A (\"foo/hello.o\" is built but that is ignored) without\nrunning \"make clean\" first, but I do not think it is a big deal.\n\nAnyway, my Request For Help was not about such details about\nwhat to do with untracked working tree files. It was about a\nmore fundamental issue, the confused behaviour of unpack-trees\nengine (switching branches, and performing tree-level 3-way\nmerges), which is used by both read-tree and merge-recursive.\nIt's so central in everyday git operation, that leaving it buggy\nin the presense of D/F conflict is an more serious issue, and\ncompared to that what we may end up doing to untracked working\ntree files is secondary. We'll get to it eventually but we need\nto deal with bigger issue first, as its resolution would change\nhow the code to deal with the untracked files will be\nimplemented.\n\nIn any case, the reason why for full two years this switching\nbetween branches that have \"foo/\" and \"foo\" never surfaced as an\nissue is that people are saner than doing that in practice (and\nI think people coming from CVS couldn't do that before so they\nmay not even think about doing so).\n\nFor example, if you are writing a paper, it may start out as a\nsmall single file, paper.txt (or paper.tex), then as it grows\nyou might split it and give it its own subdirectory. You would\nnot create paper.txt directory and have its chapters in separate\nfiles paper.txt/ch01.txt, paper.txt/ch02.txt, etc. when you do\nso in practice. Your directory would most likely be named just\n\"paper\", and the chapters are in paper/ch01.txt, paper/ch02.txt.\n\nSo in that sense, switching branches with D/F conflict is much\nlower priority item in the real world sense than others cooking\nright now. But I hate leaving known bugs in such a core piece\nlike unpack-trees, and that was why I sent out my RFH.\n\nBy the way, setting Mail-Followup-To: to me is rude. Please\ndon't. The person who wants to talk to you about _your_ message\nmay not be interested in bothering me.\n"},{"id":"37420","messageId":"45FDC95B.5020106@midwinter.com","threadId":"7288","inReplyTo":"7v8xdul2rt.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFH] straightening out \"read-tree -m\"","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-03-18T23:20:59Z","receivedAt":"2007-03-18T23:20:59Z","isPatch":false,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> In any case, the reason why for full two years this switching\n> between branches that have \"foo/\" and \"foo\" never surfaced as an\n> issue is that people are saner than doing that in practice (and\n> I think people coming from CVS couldn't do that before so they\n> may not even think about doing so).\n> \n\nI can at least give you some insight into the real-world scenario that \nled to the current discussion. It's only slightly insane. :) We had a \nshell script called \"foo\" in our source directory. It had to turn into a \nmulti-source-file app for various reasons. So the standalone script got \nreplaced with a directory \"foo\" containing the source files.\n\nI suppose one could argue that we should have called the original \n\"foo.sh\" or something and renamed it to \"foo\" at build/deploy time, but \ngiven that extensions aren't required on shell scripts (and that we have \nzillions of other scripts happily living without extensions) I suppose \nthat just felt unnecessary.\n\nBy the way, Subversion required that we do this in two separate commits, \nthe first to delete the shell script and the second to make the \ndirectory with the files in it. We originally tried to do both in one \ncommit and it got confused. Then, some time after we'd done the separate \ncommits in svn, we updated our git-svn repositories and ran into the \nproblem.\n\nIn fact, when we tried to merge in the batch of changes from the git-svn \ntracking branch, we also ran into merge problems. Git got rather \nconfused and the merge started complaining about files being untracked \nwhen in fact they were tracked in both branches! It looked like the \nindex and/or working copy were left in an inconsistent state when the \nmerge engine bailed out on the file-to-directory transition.\n\nOn my to-do list for next week is to try to come up with a minimally \ncomplex test case to demonstrate the problem, as the actual repositories \nin question are pretty convoluted and there were a bunch of other \nunrelated changes. I didn't mention it before now because I knew my \ndescription was certain to be too vague to be useful without a test case \nto look at.\n\nWe ended up successfully doing the merge by doing exactly what we had to \ndo on the Subversion side: first explicitly merge up to the \"delete the \nfile\" revision, then merge the \"create the directory\" revision. After \nthat git was able to successfully fast-forward us through the subsequent \nrevisions.\n\n-Steve\n"},{"id":"37428","messageId":"7v1wjmnlop.fsf@assigned-by-dhcp.cox.net","threadId":"7288","inReplyTo":"45FDC95B.5020106@midwinter.com","subject":"Re: [RFH] straightening out \"read-tree -m\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-19T00:04:06Z","receivedAt":"2007-03-19T00:04:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steven Grimm writes:\n\n> On my to-do list for next week is to try to come up with a minimally\n> complex test case to demonstrate the problem, as the actual\n> repositories in question are pretty convoluted and there were a bunch\n> of other unrelated changes. I didn't mention it before now because I\n> knew my description was certain to be too vague to be useful without a\n> test case to look at.\n\nYou are free to come up with test case, but I have a feeling\nthat the problem is already well understood (and that is why the\nlast part of my RFH matters a lot more than the former \"case\ntable\"). In short, \"read-tree -m\" (and unpack-trees) is screwy\nin the presense of D/F conflicts.\n"}]}