{"thread":{"id":"33569","subject":"[PATCH] t4202 (log): add failing test for log with subtree","startedAt":"2013-04-22T12:08:07Z","lastAt":"2013-04-23T16:29:43Z","messageCount":34,"participants":["Ramkumar Ramachandra","Matthieu Moy","Thomas Rast","Junio C Hamano","Philip Oakley"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"215070","messageId":"1366632487-28153-1-git-send-email-artagnon@gmail.com","threadId":"33569","inReplyTo":null,"subject":"[PATCH] t4202 (log): add failing test for log with subtree","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-22T12:08:07Z","receivedAt":"2013-04-22T12:08:07Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"When using 'git subtree' to add an external project at a given prefix,\nlog of a pathspec within the prefix fails to give the expected output.\nThis failure can be interpreted as log expecting all trees to be read\ninto /. Document this bug by adding a failing test.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n I normally don't use git-subtree, but happened to notice this when I\n was checking it out: a 'git log ', when referring to a file\n inside the subtree, doesn't work as expected: it only displays the\n HEAD commit. I know this is not related to git-subtree at all, and\n has to do with how 'git log' (and probably 'git merge -Xsubtree',\n 'git merge -s subtree') works. I suspect that 'git log' expects all\n trees to be read into /, and I've tried to prove this with a failing\n test.\n\n I think this is a bug, but I might be missing something. Can someone\n tell me what is actually happening?\n\n t/t4202-log.sh | 31 +++++++++++++++++++++++++++++++\n 1 file changed, 31 insertions(+)\n\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 9243a97..523c1be 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -168,6 +168,37 @@ test_expect_success 'git log --follow' '\n \n '\n \n+test_expect_failure 'log pathspec in tree read into prefix' '\n+\tgit checkout --orphan subtree &&\n+\tgit rm -rf . &&\n+\techo foodle >ichi &&\n+\tgit add ichi &&\n+\ttest_tick &&\n+\tgit commit -m foom &&\n+\techo moodle >unrelated &&\n+\tgit add unrelated &&\n+\ttest_tick &&\n+\tgit commit -m quux &&\n+\tsubtree_h=$(git rev-parse HEAD) &&\n+\tgit checkout master &&\n+\torig_h=$(git rev-parse HEAD) &&\n+\tgit read-tree --prefix=bar $subtree_h &&\n+\tnew_t=$(git write-tree) &&\n+\tnew_h=$(echo \"new subtree\" |\n+\tgit commit-tree $new_t -p $orig_h -p $subtree_h) &&\n+\tgit reset --hard $new_h &&\n+\t(\n+\t\tcd bar &&\n+\t\tgit log --oneline ichi >../actual\n+\t) &&\n+\tcat >expect <<-\\EOF &&\n+\t61dcd8e new subtree\n+\t130a8fb foom\n+\tEOF\n+\tgit reset --hard HEAD~1 &&\n+\ttest_cmp expect actual\n+'\n+\n cat > expect << EOF\n 804a787 sixth\n 394ef78 fifth\n-- \n1.8.2.1.546.gea3475a\n"},{"id":"215071","messageId":"vpqa9oqpxx7.fsf@grenoble-inp.fr","threadId":"33569","inReplyTo":"1366632487-28153-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-04-22T12:11:48Z","receivedAt":"2013-04-22T12:11:48Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ramkumar Ramachandra writes:\n\n> was checking it out: a 'git log ', when referring to a file\n> inside the subtree, doesn't work as expected: it only displays the\n> HEAD commit.\n\nThis is somehow expected: the subtree merge changed the filename during\nmerge (it is subtree/file.txt after the merge, and just file.txt\nbefore), so \"git log\" without --follow just considers the file appeared.\n\nOTOH, I think this is a known limitation of \"git log --follow\" that it\ndoes not follow renames done by subtree merges.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"215072","messageId":"87r4i2pxo5.fsf@linux-k42r.v.cablecom.net","threadId":"33569","inReplyTo":"vpqa9oqpxx7.fsf@grenoble-inp.fr","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-04-22T12:17:14Z","receivedAt":"2013-04-22T12:17:14Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Matthieu Moy writes:\n\n> Ramkumar Ramachandra writes:\n>\n>> was checking it out: a 'git log ', when referring to a file\n>> inside the subtree, doesn't work as expected: it only displays the\n>> HEAD commit.\n>\n> This is somehow expected: the subtree merge changed the filename during\n> merge (it is subtree/file.txt after the merge, and just file.txt\n> before), so \"git log\" without --follow just considers the file appeared.\n>\n> OTOH, I think this is a known limitation of \"git log --follow\" that it\n> does not follow renames done by subtree merges.\n\nUmm, it should follow the rename. The big limitation is that it is\nunable to follow more than one name at a time, so if the file exists on\nboth sides of the subtree merge, it will follow the original name. And\nthat's the common case; only the very first merge of the subtree has the\nfiles only on one side.\n\nYou can see this by comparing\n\n git log --oneline --follow gitk-git/gitk\n\nwith\n\n git log --oneline -- gitk gitk-git/gitk\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"215073","messageId":"CALkWK0nMgbZY7E-wPNj2C7W98Y4HtQ7Hss6uMKngajB5iJrjDw@mail.gmail.com","threadId":"33569","inReplyTo":"vpqa9oqpxx7.fsf@grenoble-inp.fr","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-22T12:29:25Z","receivedAt":"2013-04-22T12:29:25Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Matthieu Moy wrote:\n> This is somehow expected: the subtree merge changed the filename during\n> merge (it is subtree/file.txt after the merge, and just file.txt\n> before), so \"git log\" without --follow just considers the file appeared.\n\nNo, a merge does not \"change\" any filenames. The history of the file\nis very much present: run a git log HEAD^2 to see the entire history\nof the subtree. Even a git blame (without -M or -C) works just fine.\n\n> OTOH, I think this is a known limitation of \"git log --follow\" that it\n> does not follow renames done by subtree merges.\n\nUm, no. I think --follow is entirely orthogonal to the issue: unless\nI'm mistaken, it looks for other blobs in history with heuristically\nsimilar content.\n\nThe real issue has nothing to do with log itself: it has to do with\nhow rev-parse handles pathspecs. A 'git rev-parse\nHEAD:subproject/README' works fine, but 'git rev-parse\nHEAD^2:subproject/README' fails. However, 'git rev-parse\nHEAD^2:README' works, but it is assuming that the path README is\npresent in /, when it is actually present in subproject/. Now, I'm\nnot sure rev-parse is doing something unexpected, which is why I filed\nthe bug in log.\n"},{"id":"215074","messageId":"vpqip3eoi72.fsf@grenoble-inp.fr","threadId":"33569","inReplyTo":"CALkWK0nMgbZY7E-wPNj2C7W98Y4HtQ7Hss6uMKngajB5iJrjDw@mail.gmail.com","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-04-22T12:36:49Z","receivedAt":"2013-04-22T12:36:49Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ramkumar Ramachandra writes:\n\n> Matthieu Moy wrote:\n>> This is somehow expected: the subtree merge changed the filename during\n>> merge (it is subtree/file.txt after the merge, and just file.txt\n>> before), so \"git log\" without --follow just considers the file appeared.\n>\n> No, a merge does not \"change\" any filenames.\n\nRead again my message, especially the \"it is subtree/file.txt after the\nmerge, and just file.txt before\" part. Replace \"subtree\" with \"bar\" and\n\"file.txt\" with \"ichi\".\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"215075","messageId":"CALkWK0kaYeOt3pdjae8rXPiPBWnfrAPOoOaQ73m2qktVxvoeTw@mail.gmail.com","threadId":"33569","inReplyTo":"87r4i2pxo5.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-22T12:37:17Z","receivedAt":"2013-04-22T12:37:17Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Thomas Rast wrote:\n> Umm, it should follow the rename.\n\nThere is no rename. Unless there is a commit with the following diff\n(with heuristically similar content), I don't see how --follow can\nwork:\n\n diff --git a/README b/README\n deleted file mode 100644\n\n diff --git a/subproject/README b/subproject/README\n new file mode 100644\n\nHere, we just created one tiny merge commit after reading a tree into\na non-/ prefix. There is no diff associated.\n"},{"id":"215076","messageId":"CALkWK0kS4Y-ERMn54J7O+kiQwNNGxqx1a_31y2JAO9UjeVZ7MQ@mail.gmail.com","threadId":"33569","inReplyTo":"CALkWK0kaYeOt3pdjae8rXPiPBWnfrAPOoOaQ73m2qktVxvoeTw@mail.gmail.com","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-22T12:54:29Z","receivedAt":"2013-04-22T12:54:29Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Ramkumar Ramachandra wrote:\n> Unless there is a commit with the following diff\n> (with heuristically similar content), I don't see how --follow can\n> work:\n>\n> diff --git a/README b/README\n> deleted file mode 100644\n>\n> diff --git a/subproject/README b/subproject/README\n> new file mode 100644\n\nIn other words, git diff-tree HEAD^2 HEAD is absolute nonsense in the\nsubtree case.\n\nLet me outline how I think --follow works:\nA 'git log --follow foo' executes a diff-tree with historical trees\nuntil it finds one that removes (or adds, depending on which way you\nlook at it) the 'foo' entry. It then inspects all the trees in the\ncorresponding commit for a blob that is heuristically similar to the\nHEAD:foo blob, and follows it.\n\nHow can you logically extend this concept to handle my case?\n"},{"id":"215077","messageId":"87ppxmogdv.fsf@linux-k42r.v.cablecom.net","threadId":"33569","inReplyTo":"1366632487-28153-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-04-22T13:15:56Z","receivedAt":"2013-04-22T13:15:56Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Ramkumar Ramachandra writes:\n>\n\nSo after a long IRC discussion trying to hash out what you *want* it to\ndo, here's the summary for everyone else. This test is wrong on several\ncounts. For simplicity I'll denote by M the subtree merge, called\n$new_h in the actual test.\n\n> +test_expect_failure 'log pathspec in tree read into prefix' '\n> +\tgit checkout --orphan subtree &&\n> +\tgit rm -rf . &&\n> +\techo foodle >ichi &&\n\n'ichi' also exists in M^1 because you reused a name from another test.\nSo rename detection will never pair the eventual 'bar/ichi' with this\n'ichi', because the 'ichi' path was *modified*, not deleted, w.r.t. M^1.\n\nJust to clarify, rename detection is based on seeing\n\n A foo\n D bar\n\nand changing that to\n\n R bar -> foo\n\nassuming the blobs were reasonably similar. And indeed, *copy*\ndetection (-C) is able to figure it out, because it considers all paths\nthat were modified as possible candidates for a copy source.\n\nBut --follow only uses rename detection.\n\n> +\tgit add ichi &&\n> +\ttest_tick &&\n> +\tgit commit -m foom &&\n> +\techo moodle >unrelated &&\n> +\tgit add unrelated &&\n> +\ttest_tick &&\n> +\tgit commit -m quux &&\n> +\tsubtree_h=$(git rev-parse HEAD) &&\n> +\tgit checkout master &&\n> +\torig_h=$(git rev-parse HEAD) &&\n> +\tgit read-tree --prefix=bar $subtree_h &&\n\nYou need to supply a trailing / for the prefix, it's not implied.\n\n> +\tnew_t=$(git write-tree) &&\n> +\tnew_h=$(echo \"new subtree\" |\n> +\tgit commit-tree $new_t -p $orig_h -p $subtree_h) &&\n> +\tgit reset --hard $new_h &&\n> +\t(\n> +\t\tcd bar &&\n> +\t\tgit log --oneline ichi >../actual\n\nYou need to use --follow, as otherwise the pathspec filtering is\nconsidered fixed.\n\nNote that as discussed in the rest of the thread, --follow is fairly\nlimited and doesn't *really* solve the problem. But it does work\nassuming the filenames are different and you only have one subtree\nmerge, like in this case.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"215078","messageId":"87li8aog7e.fsf@linux-k42r.v.cablecom.net","threadId":"33569","inReplyTo":"87ppxmogdv.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-04-22T13:19:49Z","receivedAt":"2013-04-22T13:19:49Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Thomas Rast writes:\n\n>> +test_expect_failure 'log pathspec in tree read into prefix' '\n>> +\tgit checkout --orphan subtree &&\n>> +\tgit rm -rf . &&\n>> +\techo foodle >ichi &&\n>\n> 'ichi' also exists in M^1 because you reused a name from another test.\n> So rename detection will never pair the eventual 'bar/ichi' with this\n> 'ichi', because the 'ichi' path was *modified*, not deleted, w.r.t. M^1.\n\nArgh, that should read 'w.r.t. M^2', i.e. the subtree side.\n\nThe subtree side brings its own 'ichi', but it is moved to bar/ichi, so\nthere is a large difference between M:ichi (which came from M^1) and\nM^2:ichi.\n\n\nPS: As mentioned on IRC, even if you fix all that, a one-line file is\nprobably too small to pass the rename detection heuristics.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"215080","messageId":"CALkWK0m6vwR9rNNw_GjF4MOK1GZfwjB8ZA5Y0Lo8LbvfAg0g3g@mail.gmail.com","threadId":"33569","inReplyTo":"87ppxmogdv.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-22T14:30:47Z","receivedAt":"2013-04-22T14:30:47Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Thomas Rast wrote:\n> [...]\n\nI think you've misunderstood the whole thing. The histories of M^1\nand M^2 are completely unrelated: they're from different projects\naltogether. Considering the /ichi in M^2 a \"rename\" of the /ichi in\nM^1 is completely wrong. They have nothing to do with each other. I\nintentionally named it \"ichi\" in my orphan branch just to drive my\npoint. I suspect you've got confused because I used an orphan branch\nto emulate a different project's history. If you want an end-user\nunderstanding of the problem, use git subtree:\n\n $ cd /tmp\n $ git clone gh:artagnon/varlog\n $ cd varlog\n $ git subtree add --prefix=clayoven \\\n gh:artagnon/clayoven master\n $ cd clayoven\n $ git log README.md\n\nWhat do you expect? The same output you would get if you cloned\ngh:artagnon/clayoven separately and executed 'git log README.md' on\nit.\n\nNow, clayoven's README.md (the one in HEAD^2) has nothing to do with\nvarlog's README.md (the one in HEAD^1). It's just incidental that\nboth projects have a README.md. I repeat: clayoven and varlog have\n_nothing_ to do with each other. If I say git log --follow README.md\nin the above example, I don't even get the HEAD commit. And I\nwouldn't expect to either.\n\nI will repeat this: --follow has nothing to do with the problem I've\nspecified. And it is not tied to \"renaming\" (ie. changing the\nname/path of a file) as you've made it look. If you're still not\nconvinced, I've included a testcase for --follow \"following\" over a\nmerge commit (include it after the --follow test in the t4202). Try\nit without the --follow and you'll see what I mean. Neither the\nfilename nor the filepath of ichi has changed in this example.\n\n-- 8< --\ntest_expect_success '--follow over merge' '\n\tgit checkout -b featurebranch\n\techo foodle >>ichi &&\n\tgit add ichi &&\n\ttest_tick &&\n\tgit commit -m \"add a line to the end of ichi\" &&\n\techo moodle >unrelated &&\n\tgit add unrelated &&\n\ttest_tick &&\n\tgit commit -m quux &&\n\tgit checkout master &&\n\tmv ichi ichi.bak &&\n\techo gooble >ichi &&\n\tcat ichi.bak >>ichi &&\n\tgit add ichi &&\n\ttest_tick &&\n\tgit commit -m \"add a line to the beginning of ichi\" &&\n\tgit merge featurebranch &&\n\tgit log --follow --oneline ichi >actual &&\n\tcat >expect <<-\\EOF &&\n\tdf26551 add a line to the beginning of ichi\n\t882d8d9 add a line to the end of ichi\n\t2fbe8c0 third\n\tf7dab8e second\n\t3a2fdcb initial\n\tEOF\n\ttest_cmp expect actual\n'\n"},{"id":"215084","messageId":"CALkWK0kKWebGqQr3aNMCbKzGaWX7AMhyCrs3RiYrbgy0XyEJCw@mail.gmail.com","threadId":"33569","inReplyTo":"CALkWK0m6vwR9rNNw_GjF4MOK1GZfwjB8ZA5Y0Lo8LbvfAg0g3g@mail.gmail.com","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-22T14:57:26Z","receivedAt":"2013-04-22T14:57:26Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Ramkumar Ramachandra wrote:\n> $ git log README.md\n>\n> What do you expect? The same output you would get if you cloned\n> gh:artagnon/clayoven separately and executed 'git log README.md' on\n> it.\n\nAnd the reason I brought up rev-parse in the first place is because\nthis problem is not unique to log. Try a 'git shortlog README.md' if\nyou're not convinced. The entire revision-walking-pathspec-filtering\nmechanism in the log-like-family is broken on trees that are read\ninto a non-/ prefix.\n\nAnd no, it doesn't have anything to do with renames or \"rename\ndetection\". If it did, 'git shortlog README.md' should atleast give\nme the commit that was responsible for the rename. And if you're\nstill wondering about --follow, 'git shortlog --follow' (undocumented)\nis completely broken.\n"},{"id":"215088","messageId":"87wqruk2pj.fsf@linux-k42r.v.cablecom.net","threadId":"33569","inReplyTo":"CALkWK0m6vwR9rNNw_GjF4MOK1GZfwjB8ZA5Y0Lo8LbvfAg0g3g@mail.gmail.com","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-04-22T15:24:56Z","receivedAt":"2013-04-22T15:24:56Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Thomas Rast wrote:\n>> [...]\n>\n> I think you've misunderstood the whole thing. The histories of M^1\n> and M^2 are completely unrelated: they're from different projects\n> altogether. Considering the /ichi in M^2 a \"rename\" of the /ichi in\n> M^1 is completely wrong. They have nothing to do with each other. I\n> intentionally named it \"ichi\" in my orphan branch just to drive my\n> point. I suspect you've got confused because I used an orphan branch\n> to emulate a different project's history. If you want an end-user\n> understanding of the problem, use git subtree:\n>\n> $ cd /tmp\n> $ git clone gh:artagnon/varlog\n> $ cd varlog\n> $ git subtree add --prefix=clayoven \\\n> gh:artagnon/clayoven master\n> $ cd clayoven\n> $ git log README.md\n>\n> What do you expect? The same output you would get if you cloned\n> gh:artagnon/clayoven separately and executed 'git log README.md' on\n> it.\n\nNo, I don't. But that's probably because I know a few things about how\ngit-log works that your hypothetical $USER doesn't.\n\nAt the risk of restating what everyone agrees on: It's a design\nprinciple of git that it only stores tree states, and anything about\ndiffs, files, renames, etc. is purely in the imagination of the user.\n\nWe support that imagination by having analysis tools with which some\nthings can be found out, but others can't.\n\nSo (I think?) in the above you claim that $USER interprets\n\n git log -- README.md\n\nas\n\n Show me the history of README.md.\n\nBut there's no such thing as the history of a file! The command instead\nsays\n\n If I filter all history for only changes affecting a path 'README.md'\n in the root of the repository[1], then what does it look like?\n\n\nSo please don't write tests that go contrary to that definition, because\nthey're *wrong*. The current implementation precisely matches the\ncurrent definition of pathspec filtering.\n\nYou can try arguing for changing the definition, but unless you find one\nthat can be implemented fast enough to be generally usable, I will\noppose that change.\n\n\nThe only thing that's broken in any of this is that I think, as\nexplained on IRC, that a hypothetical fixed --follow -C should be able\nto figure out this case. By spending extra cycles on analysis,\nnaturally.\n\n\n[1] and also skipping lines of history that seem uninteresting at this\npoint already, compare --simplify-merges\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"215093","messageId":"CALkWK0kgQqhG_ojuxZ85mTF8rhLf=wv5dGamU8z+cFxpN7QWYQ@mail.gmail.com","threadId":"33569","inReplyTo":"87wqruk2pj.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-22T15:46:40Z","receivedAt":"2013-04-22T15:46:40Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Thomas Rast wrote:\n> So (I think?) in the above you claim that $USER interprets\n>\n> git log -- README.md\n>\n> as\n>\n> Show me the history of README.md.\n>\n> But there's no such thing as the history of a file!\n\nI made no such claims. I might not know as much as you or the others\non the list about git, but I can certainly grok how git stores\nhistory.\n\nThere needs to be some amount of mutual respect for a healthy\nconversation: if you start assuming midway that I don't understand\nwhat history is, we have a problem.\n\n> So please don't write tests that go contrary to that definition, because\n> they're *wrong*. The current implementation precisely matches the\n> current definition of pathspec filtering.\n\nWho said anything about changing any definitions? Where are you\ngetting all this from?\n\nHow does \"git log HEAD~3 -- README\" work? It sets up a revision walk\nto start from HEAD~3 going all the way down to the root commit. In\neach of these commits, it looks for the entry \"README\" in the\ncorresponding tree. It then runs diff-tree with the previous commit's\ntree to see if the object (blob) corresponding to the \"README\" entry\nis different: if so, it selects the commit and displays it.\n\nNow, what am I saying? I'm saying that this approach assumes that all\ntrees are read into /. A pathspec \"subproject/README\" is _only_\npresent in the subtree-merge commit^{tree} and nowhere else. The\ncurrent log algorithm might try to look for the entry\n\"subproject/README\" (your pathspec) in all the commit^{tree}s of the\ncommits leading up to M^2. That is _not_ the problem, as I have\nalready illustrated that --follow follows over merges. The problem is\nlooking for the pathspec \"subproject/README\" in the first place: those\ncommit^{trees}s have the entry stored as \"README\".\n\nAm I making any sense, or are you going to accuse me of not\nunderstanding trees now?\n"},{"id":"215094","messageId":"CALkWK0mUH2m5zJ4MwPWC85CsZZ=2RODumLvsF9q3rLj-+d7vBw@mail.gmail.com","threadId":"33569","inReplyTo":"87wqruk2pj.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-22T15:50:48Z","receivedAt":"2013-04-22T15:50:48Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Thomas Rast wrote:\n> The only thing that's broken in any of this is that I think, as\n> explained on IRC, that a hypothetical fixed --follow -C should be able\n> to figure out this case. By spending extra cycles on analysis,\n> naturally.\n\nFor the 100th time, nothing has been \"copied\". There is no need to\nspend time on any analysis. It's a very straightforward problem that\nrequires no computation or heuristics: it just requires you to strip\nthe leading \"subproject/\" when looking for pathspecs in the M^2\ncommit^{tree}s. Done.\n"},{"id":"215095","messageId":"CALkWK0n5gaz3A7kHT6+5z3YkYdpgU5p6Pv4heMbLzikTbROwkA@mail.gmail.com","threadId":"33569","inReplyTo":"CALkWK0mUH2m5zJ4MwPWC85CsZZ=2RODumLvsF9q3rLj-+d7vBw@mail.gmail.com","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-22T15:54:35Z","receivedAt":"2013-04-22T15:54:35Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Ramkumar Ramachandra wrote:\n> For the 100th time, nothing has been \"copied\". There is no need to\n> spend time on any analysis. It's a very straightforward problem that\n> requires no computation or heuristics: it just requires you to strip\n> the leading \"subproject/\" when looking for pathspecs in the M^2\n> commit^{tree}s. Done.\n\nAnd if you're still not convinced, run 'git log HEAD^2 -- README.md'\nfrom the toplevel directory. You'll get the log of README.md from the\nsubproject.\n"},{"id":"215100","messageId":"7vk3nupltx.fsf@alter.siamese.dyndns.org","threadId":"33569","inReplyTo":"87wqruk2pj.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-22T16:32:58Z","receivedAt":"2013-04-22T16:32:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> So (I think?) in the above you claim that $USER interprets\n>\n> git log -- README.md\n>\n> as\n>\n> Show me the history of README.md.\n>\n> But there's no such thing as the history of a file! The command instead\n> says\n>\n> If I filter all history for only changes affecting a path 'README.md'\n> in the root of the repository[1], then what does it look like?\n\nCorrect. The \"design principle\" I did not quote from your message\ncomes from one of the most important messages in the list archive\n($gmane/217).\n\nThree issues with \"--follow\" are:\n\n (1) It uses the given pathname as single pathspec and drill down\n the same way without \"--follow\" until it notices the path\n disappears and until then there is no attempt to detect renames\n is made. And it only does -M variant of rename detection\n\n (2) The \"same way\" in (1) includes the merge simplification to cull\n side branches if one parent matches the end result. In a\n history where the first parent of a merge M, i.e. M^1, did not\n have path F, its second parent M^2 had path F, and the merge\n result M deposited the contents of M^2:F at M:D/F, then running\n \"log --follow F\" starting from M would notice that the end\n result M did not have F in it, which is shared with its first\n parent M^1, and culls the side branch M^2. The --full-history\n option may let you keep the history leading to M^2, though.\n\n (3) When (1) notices that the path being followed did not exist in\n any of the parents (be it a merge or a non-merge) and finds a\n different path with a similar looking content, it _switches_\n the pathspec to it, but the single pathspec it uses is a global\n state and affects traversals of other ancestry paths at the\n same time. Because of this, \"--follow\" will not work correctly\n in a history that contains merges. It often _appears_ to work\n only by accident.\n\nThe first two are relatively minor (the second is not even an\nissue).\n\nTo solve the last one, you would need to keep track of which path it\nis following per traversal path.\n"},{"id":"215103","messageId":"CALkWK0k0LbAnkhAAqdeAvBnCig_HO+bT+WiQuQQ3Fgc=FzFtRw@mail.gmail.com","threadId":"33569","inReplyTo":"CALkWK0n5gaz3A7kHT6+5z3YkYdpgU5p6Pv4heMbLzikTbROwkA@mail.gmail.com","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-22T17:29:48Z","receivedAt":"2013-04-22T17:29:48Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Ramkumar Ramachandra wrote:\n> And if you're still not convinced, run 'git log HEAD^2 -- README.md'\n> from the toplevel directory. You'll get the log of README.md from the\n> subproject.\n\nOn IRC, Thomas explained to me that mixing in changes from various\nbranches into the pathspec will break this so-called determinism. To\ntry it out for yourself, do:\n\n $ cd /tmp\n $ git clone gh:trast/subtree-mainline-example\n $ cd subtree-mainline-example\n $ git log HEAD^2 -- sub # only lists the side changes\n $ git log -- dir/sub # only lists the mainline changes\n\nWhat we should really expect is a mix of the two.\n"},{"id":"215105","messageId":"CALkWK0madWxOSraZw6u8U_afVeK8tyMB_+D59n1vSBmobxGqqg@mail.gmail.com","threadId":"33569","inReplyTo":"7vk3nupltx.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-22T18:00:21Z","receivedAt":"2013-04-22T18:00:21Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> [...]\n> (3) When (1) notices that the path being followed did not exist in\n> any of the parents (be it a merge or a non-merge) and finds a\n> different path with a similar looking content, it _switches_\n> the pathspec to it, but the single pathspec it uses is a global\n> state and affects traversals of other ancestry paths at the\n> same time. Because of this, \"--follow\" will not work correctly\n> in a history that contains merges. It often _appears_ to work\n> only by accident.\n\nThis explanation is all very nice, but isn't it completely tangential\nto the issue at hand?\n\nLet's say I have a subtree merge M located at HEAD~4. I ask for the\nlog of 'HEAD~4 -- README' with --follow. It follows until it gets to\nM: at M, M^1:README is missing, but M^2:README is present. Should it\nfollow down and show the history of M^2:README?\n\nYou can reserve the discussion about --follow working in the general\ncase for another thread. Meanwhile, you're evading the issue of\nassuming that all trees are read into /, and are really representing\nthe same project's history, while this is not the case.\n"},{"id":"215107","messageId":"vpqzjwqzawf.fsf@grenoble-inp.fr","threadId":"33569","inReplyTo":"CALkWK0madWxOSraZw6u8U_afVeK8tyMB_+D59n1vSBmobxGqqg@mail.gmail.com","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-04-22T18:18:56Z","receivedAt":"2013-04-22T18:18:56Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ramkumar Ramachandra writes:\n\n> Meanwhile, you're evading the issue of assuming that all trees are\n> read into /, and are really representing the same project's history,\n> while this is not the case.\n\nThis is fundamenally how Git works. Git works with commit objects, each\ncommit object points to a tree object that represent \"/\" at this commit.\n\nWhen you do a subtree merge, you include the tree that was in \"/\" in a\nsubtree of the master project. Files used to exist as /file, and now\nexist as /subtree/file. There is nothing recording that the new\n/subtree/file comes from /file in its second parent. Call this a\nrenaming or not, but \"git log subtree/file\" won't show you changes\ntouching \"/file\" by default, and this is the case for the history of the\nsubproject you are merging.\n\nA subtree merge is really a rename of the subproject's file plus a\nmerge, done in the same commit. Try doing something like\n\nmkdir -p subproject\ngit mv * subproject/\ngit commit\n\nin your subproject before doing a merge, you'll get the same result,\nexcept there will be one more commit.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"215108","messageId":"7v7gjupekk.fsf@alter.siamese.dyndns.org","threadId":"33569","inReplyTo":"CALkWK0madWxOSraZw6u8U_afVeK8tyMB_+D59n1vSBmobxGqqg@mail.gmail.com","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-22T19:09:47Z","receivedAt":"2013-04-22T19:09:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Junio C Hamano wrote:\n>> [...]\n>> (3) When (1) notices that the path being followed did not exist in\n>> any of the parents (be it a merge or a non-merge) and finds a\n>> different path with a similar looking content, it _switches_\n>> the pathspec to it, but the single pathspec it uses is a global\n>> state and affects traversals of other ancestry paths at the\n>> same time. Because of this, \"--follow\" will not work correctly\n>> in a history that contains merges. It often _appears_ to work\n>> only by accident.\n>\n> This explanation is all very nice, but isn't it completely tangential\n> to the issue at hand?\n\nI think you are talking mostly about (2), but the primary purpose of\nmy message was not about your specific issue.\n\nIt was to give a larger picture to people who may be inclined to\ntackle, and may be capable of tackling, the real issues in the\n\"--follow\" codepath. Anybody who wants to update it needs to be\naware of all three.\n"},{"id":"215109","messageId":"87zjwqpebl.fsf@hexa.v.cablecom.net","threadId":"33569","inReplyTo":"CALkWK0k0LbAnkhAAqdeAvBnCig_HO+bT+WiQuQQ3Fgc=FzFtRw@mail.gmail.com","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-04-22T19:15:10Z","receivedAt":"2013-04-22T19:15:10Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Ramkumar Ramachandra wrote:\n>> And if you're still not convinced, run 'git log HEAD^2 -- README.md'\n>> from the toplevel directory. You'll get the log of README.md from the\n>> subproject.\n>\n> On IRC, Thomas explained to me that mixing in changes from various\n> branches into the pathspec will break this so-called determinism. To\n> try it out for yourself, do:\n>\n> $ cd /tmp\n> $ git clone gh:trast/subtree-mainline-example\n> $ cd subtree-mainline-example\n> $ git log HEAD^2 -- sub # only lists the side changes\n> $ git log -- dir/sub # only lists the mainline changes\n>\n> What we should really expect is a mix of the two.\n\nSo after lots of confusing misunderstandings across the entire thread,\nand a long IRC discussion, I do have one take-away that I think is worth\nwriting down:\n\nThere is a market for a rename detection that works at the tree level.\n\nBear with me while I explain.\n\nThe average subtree merge (after the initial one) looks like this,\nfocusing on the subtree:\n\n M new state in sub/\n | \\\n | * new state in /\n |\n * old state in sub/\n\n(The example in the quote above additionally complicates the issue by\nchanging sub/ in the mainline.)\n\nIdeally we'd like our hypothetical fixed --follow to accurately track a\npathspec 'sub/foo' so that in the mainline it remains the same, but in\nthe side it becomes 'foo'. Because of reasons already explained in\nearlier mails, -M does not suffice for all cases (in particular, it\nfails if there is a /foo in the mainline too). -C works, but is slow.\n\nSo how can we fix that? We could try to somehow figure out that M:sub/\nrefers to the same thing as M^2:/, by looking at them at the tree level.\nLet's provisionally call this --follow-tree-rename.\n\nI don't quite know how to heuristically[1] detect such a rename, but\nsince 'merge -ssubtree' does it (if you don't specify a tree prefix\nexplicitly), it can't be That Hard(tm). If we're extra lucky it's fast\nenough to be enabled by default.\n\nIn the special case where the subtree was not modified in the mainline\nsince the last merge, the problem is pretty trivial: simply check if any\nsubtree of M agrees with the root tree of each merge parent; if so, diff\nthose trees instead of the root trees.\n\nThat should then help subtree users to get better logs.\n\n\n[1] the quoted example shows that you can't just go looking for\nidentical trees in the general case, so it is really a heuristic\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"215132","messageId":"CALkWK0kVDbyQBTSTbEDoVFs8E6JKWw3Z6L77NCXSb=FU8GNDkw@mail.gmail.com","threadId":"33569","inReplyTo":"87zjwqpebl.fsf@hexa.v.cablecom.net","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-22T19:54:16Z","receivedAt":"2013-04-22T19:54:16Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Thomas Rast wrote:\n> There is a market for a rename detection that works at the tree level.\n\nExactly. And making it auto-follow with a low threshold would be a\ngreat default. We'll have to deal with D/F conflicts that Junio was\ntalking about in (2), every once in a while. We'll have a lot more\nincentive to build the D/F conflict resolution process a nice UI.\n\ngit-core will actually start working with trees the way it works with blobs.\n"},{"id":"215149","messageId":"CALkWK0n20f+cBpKWo9E3OVu_OAxNqiz8tkfspFBTBEaQTfE8NA@mail.gmail.com","threadId":"33569","inReplyTo":"7vk3nupltx.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-22T20:39:44Z","receivedAt":"2013-04-22T20:39:44Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> (1) It uses the given pathname as single pathspec and drill down\n> the same way without \"--follow\" until it notices the path\n> disappears and until then there is no attempt to detect renames\n> is made. And it only does -M variant of rename detection\n\nOn this. It might be profitable to auto-follow at a low threshold\n(and change --follow to a number argument like -M, -C). I'm not\nasking you to do it for more than one-file at a time, but I often view\none file's history: I was just going through the history of\nbuiltin/merge-trees.c (what I was looking for was mostly in the\nbuiltin-* variant), and it would've been nice if I didn't have to quit\nand come back with a --follow.\n"},{"id":"215153","messageId":"B91DA87BC1F548ECB1FDB9550F4AB017@PhilipOakley","threadId":"33569","inReplyTo":"CALkWK0kVDbyQBTSTbEDoVFs8E6JKWw3Z6L77NCXSb=FU8GNDkw@mail.gmail.com","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2013-04-22T21:00:01Z","receivedAt":"2013-04-22T21:00:01Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Ramkumar Ramachandra\" \nSent: Monday, April 22, 2013 8:54 PM\n> Thomas Rast wrote:\n>> There is a market for a rename detection that works at the tree \n>> level.\n>\n> Exactly. And making it auto-follow with a low threshold would be a\n> great default. We'll have to deal with D/F conflicts that Junio was\n> talking about in (2), every once in a while. We'll have a lot more\n> incentive to build the D/F conflict resolution process a nice UI.\n>\n> git-core will actually start working with trees the way it works with \n> blobs.\n> --\n\nIs this not similar to the previous attempts at bulk rename detection? \nSuch as $gmane/160233.\n\nA practical bulk rename detection would be good. It doesn't have to be \nperfect.\n\nPhilip \n"},{"id":"215155","messageId":"vpqr4i2z35n.fsf@grenoble-inp.fr","threadId":"33569","inReplyTo":"87zjwqpebl.fsf@hexa.v.cablecom.net","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-04-22T21:06:12Z","receivedAt":"2013-04-22T21:06:12Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Thomas Rast writes:\n\n> So how can we fix that? We could try to somehow figure out that M:sub/\n> refers to the same thing as M^2:/, by looking at them at the tree level.\n> Let's provisionally call this --follow-tree-rename.\n\nThere was a patch serie long ago that implemented directory rename\ndetection:\n\n http://thread.gmane.org/gmane.comp.version-control.git/163328\n\nI'm not sure why it hasn't been merged.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"215156","messageId":"CALkWK0=EcXP-tUOhcn8vmqPtzetX19QOgRb14Op1ZM9ZtxEz3Q@mail.gmail.com","threadId":"33569","inReplyTo":"B91DA87BC1F548ECB1FDB9550F4AB017@PhilipOakley","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-22T21:08:28Z","receivedAt":"2013-04-22T21:08:28Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Philip Oakley wrote:\n> Is this not similar to the previous attempts at bulk rename detection? Such\n> as $gmane/160233.\n\nYes. Does anyone know what happened to the series?\n\n... and I wonder how git merge -s subtree works (still reading).\n"},{"id":"215158","messageId":"CALkWK0=n=06sd76Ah_kL-a3P0WQ7GkOH24kaNPzfzg=qpinMCQ@mail.gmail.com","threadId":"33569","inReplyTo":"CALkWK0=EcXP-tUOhcn8vmqPtzetX19QOgRb14Op1ZM9ZtxEz3Q@mail.gmail.com","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-22T21:23:44Z","receivedAt":"2013-04-22T21:23:44Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Ramkumar Ramachandra wrote:\n> Philip Oakley wrote:\n>> Is this not similar to the previous attempts at bulk rename detection? Such\n>> as $gmane/160233.\n>\n> Yes. Does anyone know what happened to the series?\n\nHm, this and the series Matthieu posted ($gmane/163328) seems to be\nsolving the more generalized problem. My initial problem was to do it\nonly when following a single file's history (probably a --tree-follow\ncorresponding to --follow). And to probably restrict looking only in\nmerge commits (?).\n"},{"id":"215172","messageId":"7vmwsqns4p.fsf@alter.siamese.dyndns.org","threadId":"33569","inReplyTo":"vpqr4i2z35n.fsf@grenoble-inp.fr","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-22T21:59:50Z","receivedAt":"2013-04-22T21:59:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Thomas Rast writes:\n>\n>> So how can we fix that? We could try to somehow figure out that M:sub/\n>> refers to the same thing as M^2:/, by looking at them at the tree level.\n>> Let's provisionally call this --follow-tree-rename.\n>\n> There was a patch serie long ago that implemented directory rename\n> detection:\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/163328\n>\n> I'm not sure why it hasn't been merged.\n\nI have forgotten about this topic (and its numerous iterations in\nthe past), but it appears me that people mostly lost interest after\nv7 review cycle where the series looked like a solution that is\nlooking for a problem.\n\nI took another quick look at it now, but it tells me that the series\nwas a good platform to discuss the design and the goals but it was\nfar from ready with comments like this \"// FIXME: leaks like sieve\".\n\nAs to the design (not the implementation), I find what are in the\ndocumentation updates in [v9 2/6] more or less reasonable, even\nthough I still doubt that foo/ => bar/, when the constituent files\nare not moved a la \"mv foo/* bar/.\" is useful. The change in [v9\n3/6] is distasteful and should just be killed.\n"},{"id":"215189","messageId":"CALkWK0mO0L4YrZFGmzWkiO4V18p-ydKyjVOe4vJ2H-d7Kz+qOw@mail.gmail.com","threadId":"33569","inReplyTo":"7vmwsqns4p.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-22T22:52:05Z","receivedAt":"2013-04-22T22:52:05Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> I have forgotten about this topic (and its numerous iterations in\n> the past), but it appears me that people mostly lost interest after\n> v7 review cycle where the series looked like a solution that is\n> looking for a problem.\n\nI agree. Over-generalizing the problem is going nowhere. I have some\nideas in my head, but I think I'm at the brink of insanity again:\n\nWe have a special type of merge commit that has 'bind' lines\ncorresponding to the trees of the commits we just merged in: each line\nreferences a tree and a prefix. Then, a diff between the merge's tree\nand one of these trees can easily tell us what changes were introduced\nby each side. And here's the bomb: if we consider extending it to\ninclude a blob-like buffer, we can use it to store submodule-like\ninformation for each prefix. Everything would just work out of the\nbox with a few minor adjustments:\n\n1. We have to modify our packing algorithm to not reach out beyond ^1\nof these special merge commits. This means object-store isolation\n(not $GITDIR isolation). And it means that submodules can be cloned\nand removed at will.\n\n2. We have to maintain a symbolic ref for each prefix. A commit\ninvoked from a special-commit referenced subdirectory will update this\nsymref. Ofcourse, this means that we need to namespace our refs/ and\nrefs/remotes sensibly.\n\n3. Git commands will take into account $CWD very strongly, and just\nDTRT all the time. Whether you're looking from the submodule\nperspective or the superproject perspective.\n\nAm I making any sense?\n"},{"id":"215190","messageId":"CALkWK0mHsMRYc2MZ92ALzfLC9mxytA5OiL3i4Wh+BE6bZMb2hQ@mail.gmail.com","threadId":"33569","inReplyTo":"CALkWK0mO0L4YrZFGmzWkiO4V18p-ydKyjVOe4vJ2H-d7Kz+qOw@mail.gmail.com","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-22T22:59:46Z","receivedAt":"2013-04-22T22:59:46Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Ramkumar Ramachandra wrote:\n> [...]\n\nOh, and I forgot the best part. True floating submodules when the\nbind line references a zeroed-out hex. Naturally, the diff'ing to\nfigure out changes introduced by the submodule will be blocked. Also\ngood for dropping in huge binary files at a particular path.\n"},{"id":"215194","messageId":"7vehe2m88a.fsf@alter.siamese.dyndns.org","threadId":"33569","inReplyTo":"CALkWK0mO0L4YrZFGmzWkiO4V18p-ydKyjVOe4vJ2H-d7Kz+qOw@mail.gmail.com","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-22T23:55:01Z","receivedAt":"2013-04-22T23:55:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Am I making any sense?\n\nI dunno. Depends on the definition of \"sense\".\n\nIt sounds like you are repeating the same old \"let's record renames\nin the commit\", and in a system (not Git) where recording renames\nmay make sense, you may be making sense.\n\nBut we will not record renames in the commit. Time to re-read\n$gmane/217, perhaps?\n\nA subtree merge that slurps everything from a side branch under a\nsingle directory, say gitk/, is not at all different from a normal\nmerge with many renames.\n\nImagine an alternate world where we had a small \"git.git\" project\nwith 11 files totallying 1244 lines. Then Paul Mackerras forks that\nproject, remove everything from the top-level directory and adds a\nTck/Tk script \"gitk\". Linus merges that history as-is, keeping all\nour files intact (i.e. ignoring his removal) but taking the addition\nof \"gitk\" from him.\n\nThen after I inherit the project, I rename \"gitk\" to \"gitk-git/git\".\nPaul does not rename his. We keep developing in parallel and I\noccassionally merge from his tree, which has \"gitk\" at the top,\nwhile mine has it in a directory \"gitk-git\". The ordinary rename\ndetection kicks in and integrates his updates to \"gitk\" into my\n\"gitk-git/gitk\".\n\nThe only difference the above imaginary history has from the reality\nis Paul's history does not share that root, but everything else is\nthe same.\n"},{"id":"215199","messageId":"CALkWK0noVfRoXAH75r4yBijXSHD5yusLcKgGC=8bjOW6=qCMng@mail.gmail.com","threadId":"33569","inReplyTo":"7vehe2m88a.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-23T07:53:57Z","receivedAt":"2013-04-23T07:53:57Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> It sounds like you are repeating the same old \"let's record renames\n> in the commit\", and in a system (not Git) where recording renames\n> may make sense, you may be making sense.\n>\n> But we will not record renames in the commit. Time to re-read\n> $gmane/217, perhaps?\n\nYeah, you're right. It makes no sense to record renames, although I\nhave a different argument against it: any implementation that records\nrenames will depend on the path that was taken to get to the final\nstate, and this is completely wrong. Subversion made this mistake,\nand users pay a very heavy price: if the user didn't explicitly rename\na node (~= tree) and just did a delete + add, the repository is\nmore-or-less screwed wrt merging.\n\nForget everything that I said, and let's start over.\n\nIn the common usecase of subtrees, it might make sense to record some\nadditional submodule-like parameters in the subtree's tree object.\nThis additional information is entirely optional, and doesn't change\nthe way merges happen: we can have it as merely a heuristic-helper\n(for git merge -s subtree). Initially, let's just think of putting a\nref field in the tree, so that I can have the following setup:\n\n- remote origin referring to my superproject.\n- remote git/origin referring to the fetchdefault of my subproject git.\n- remote git/ram referring to the pushdefault of my subproject.\n- the tree object corresponding to quux/baz/git is additionally filled\nin with the ref refs/remotes/git/origin/master.\n\nThen, I can just say git pull quux/baz/git, and it will automatically\nfetch changes from the ref and merge it into the subtree. It's not to\nsay that I can't merge any other ref; just that I merge this ref most\nof the time, and I want a DWIM for this case.\n\nFurther, this can speed up tree-rename detection greatly (in fact, I'm\nthinking the first implementation will depend on this information\nbeing present). I inspect M^{tree} and I want to know how it's\ncomposed from M^1^{tree} and M^2^{tree}. Simple. In M^{tree}, look\nfor trees that have this additional data filled in: then we can just\nshort-circuit the rename detection to matching the similarity of this\ntree with M^1^{tree} and M^2^{tree}.\n\nWhen this aux data is present in the tree, we can do one more thing:\nhave a symref tracking the commit-line corresponding to M^2. This\nmeans that we can DWIM for things like 'git commit' inside the subtree\nvery easily. When the aux information is not present, 'git commit'\nwill obviously commit to HEAD, essentially making the superproject own\nthose changes in the subtree (as it does now).\n\nThis might be the route to implementing narrow clones sensibly. A\nnarrow clone does not mean that any directory in the entire repository\ncan be cloned separately: it just means that a tree with this aux data\nneed not be fetched in the initial clone. For this to work, instead\nof refs/remotes/git/origin/master, the aux data will need a\ncombination of upstream URL and ref: we can then automatically figure\nout the name from the URL and deposit the fetched data in\nrefs/remotes//origin/.\n\nInitializing nested submodules without the container is also easy: in\nthe superproject, you need to have aux-trees corresponding to\nquux/baz/git and quux/baz/git/moo/clayoven. It might additionally be\na good idea to track these aux-trees: but even if we don't go down\nthat route, we can always deposit a \"template\" file from the\nsuperproject that won't interfere with the subtree merging process.\n\nNow, I'm not sure what the value of splitting the object stores at\nthis point is. The aux-tree can have a statthrough field to block\nstat() calls from going through, so there's really no performance\nissue. If you want to separate one tree out of the superproject and\nwork on it separately, all you have to do is fetch the corresponding\nref.\n\nOn the issue of floating submodules. It might make sense to zero out\nthe hex of the tree, as seen by the superproject. The limitation is\nthat we can't introduce any changes to the submodule from the\nsuperproject: it's basically a dummy entry sitting in the\nsuperproject's tree. It's a bit of a hack, but I think it's workable.\n\nSo, what do you think?\n"},{"id":"215244","messageId":"7vobd5jktd.fsf@alter.siamese.dyndns.org","threadId":"33569","inReplyTo":"CALkWK0noVfRoXAH75r4yBijXSHD5yusLcKgGC=8bjOW6=qCMng@mail.gmail.com","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-23T16:03:42Z","receivedAt":"2013-04-23T16:03:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Further, this can speed up tree-rename detection greatly (in fact, I'm\n> thinking the first implementation will depend on this information\n> being present). I inspect M^{tree} and I want to know how it's\n> composed from M^1^{tree} and M^2^{tree}. Simple. In M^{tree}, look\n> for trees that have this additional data filled in: then we can just\n> short-circuit the rename detection to matching the similarity of this\n> tree with M^1^{tree} and M^2^{tree}.\n\nIf you need the history context (i.e. M, M^1, M^2) around it to\ninterpret that additional information, isn't it essentially the same\nas storing renames in the merge commit M?\n\nNot very impressed, at least not yet.\n"},{"id":"215247","messageId":"CALkWK0mACae7VEe2M-UXhmnU9PUDxu1Ph5HSXb7S4fOGecsFrw@mail.gmail.com","threadId":"33569","inReplyTo":"7vobd5jktd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t4202 (log): add failing test for log with subtree","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-23T16:29:43Z","receivedAt":"2013-04-23T16:29:43Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> If you need the history context (i.e. M, M^1, M^2) around it to\n> interpret that additional information, isn't it essentially the same\n> as storing renames in the merge commit M?\n\nThere's one big difference.\n\nThe point is that for any kind of repository composition, you're going\nto have to store tree renames at some level if you want to be able to\nmove the submodule around in the tree. In the current submodule\nsystem, you could say that you're storing tree renames in a blob\n(submodule..path in .gitmodules). Ofcourse, the difference is\nthat the other side of the tree entry is completely opaque to the\nsuperproject (and I think we can do better than that now). You should\nuse heuristics for all other kinds of renames, but I'm arguing that\nrepository composition is a special case that needs more thought.\n\nYou shouldn't be against storing renames in principle, because we\nalready do that in a way. What you should be against is storing\nrenames in a way that will lock up the repository if a different path\nis taken to the same state.\n\nWhat I'm proposing is an optional field to speed up (and make more\nreliable) rename detection in a very specific case. I'm not putting\nthe information in a commit because that will lock up our repository:\nIf there are two different commits representing the same tree, but one\ncontains bind lines and the other doesn't, we've done something\nseriously wrong: rebase and merge are screwed. In my case, if there\nare two perfectly identical trees except that one contains auxiliary\ninformation, nothing goes wrong: the tree without the auxiliary\ninformation will just be a little slower and can't support DWIM git\noperations.\n\n> Not very impressed, at least not yet.\n\nPlease bear with me. I really think the repository composition\nproblem is worth solving.\n"}]}