{"thread":{"id":"11840","subject":"[BUG?] git log picks up bad commit","startedAt":"2008-02-02T12:21:36Z","lastAt":"2008-02-06T19:26:50Z","messageCount":34,"participants":["Tilman Sauerbeck","Jeff King","Junio C Hamano","Linus Torvalds","Johannes Schindelin","Nicolas Pitre","Karl Hasselström"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"67131","messageId":"20080202122135.GA5783@code-monkey.de","threadId":"11840","inReplyTo":null,"subject":"[BUG?] git log picks up bad commit","fromName":"Tilman Sauerbeck","fromEmail":"tilman@code-monkey.de","sentAt":"2008-02-02T12:21:36Z","receivedAt":"2008-02-02T12:21:36Z","isPatch":false,"sender":{"key":"tilman@code-monkey.de","avatar":null},"body":"Hi,\nI think I either found a bug in git log, or I'm working with a broken\nrepository. I can reproduce this with current git master.\n\nI'm trying to list the last n commits since a given commit on a given\nbranch like this:\n  git log -n N commit.. branch\nThe problem is, if there are less than N commits that match the\ncriteria, git log also prints the very first commit of the repository.\n\nI'm operating on a bare repository here. When I actually check out the\nbranch I'm interested in, git log behaves as expected.\n\nI've uploaded the .git directory to\nhttp://crux.nu/~tilman/broken_repo.tar.bz2 (use -C to extract!)\n\nReproduce like this:\nmkdir /tmp/blah\ncd /tmp/blah\ntar xjf broken_repo.tar.bz2\n\ngit log -n 3 --abbrev-commit --pretty=oneline \\\n1dd567d596b072e3ce44ea5ad8c373871686b078.. 2.4\n\nThe output I'm getting is:\n\n47f585a... syslinux: Updated 3.54 -> 3.60\nb3444e1... lzma: 4.32.4 -> 4.32.5\nd5d6fa1... Created repository\n\nWhen I check out the \"2.4\" branch and run the git log command again, I\nget the expected output:\n\n47f585a... syslinux: Updated 3.54 -> 3.60\nb3444e1... lzma: 4.32.4 -> 4.32.5\n\nAny idea on what's going on there?\n\nThanks,\nTilman\n\n-- \nA: Because it messes up the order in which people normally read text.\nQ: Why is top-posting such a bad thing?\nA: Top-posting.\nQ: What is the most annoying thing on usenet and in e-mail?\n"},{"id":"67177","messageId":"20080203030054.GA18654@coredump.intra.peff.net","threadId":"11840","inReplyTo":"20080202122135.GA5783@code-monkey.de","subject":"Re: [BUG?] git log picks up bad commit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-02-03T03:00:54Z","receivedAt":"2008-02-03T03:00:54Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 02, 2008 at 01:21:36PM +0100, Tilman Sauerbeck wrote:\n\n> I think I either found a bug in git log, or I'm working with a broken\n> repository. I can reproduce this with current git master.\n\nI think it is a bug in your command line.\n\n> git log -n 3 --abbrev-commit --pretty=oneline \\\n> 1dd567d596b072e3ce44ea5ad8c373871686b078.. 2.4\n\nThe space betwen \"..\" and \"2.4\" means that they are two separate\narguments. Thus the second part of your \"..\" operator is blank, which is\ntreated as HEAD. Thus it is equivalent to:\n\n  1dd567d596b072e3ce44ea5ad8c373871686b078..HEAD 2.4\n\nWhen you switch to branch 2.4, then 2.4 becomes your HEAD.\n\nThat being said, the commit in your 'master' branch _is_ part of\n1dd567d5, and should be culled. So I'm not clear on why it shows up only\nwhen you ask to see both branches, and that may be a bug.\n\n-Peff\n"},{"id":"67181","messageId":"20080203043310.GA5984@coredump.intra.peff.net","threadId":"11840","inReplyTo":"20080203030054.GA18654@coredump.intra.peff.net","subject":"[RFH] revision limiting sometimes ignored","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-02-03T04:33:10Z","receivedAt":"2008-02-03T04:33:10Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 02, 2008 at 10:00:54PM -0500, Jeff King wrote:\n\n> That being said, the commit in your 'master' branch _is_ part of\n> 1dd567d5, and should be culled. So I'm not clear on why it shows up only\n> when you ask to see both branches, and that may be a bug.\n\nOK, there is definitely a bug here, but I'm having some trouble figuring\nout the correct fix. It's in the revision walker, so I have cc'd those\nwho are more clueful than I.\n\nYou can recreate a problematic repo using this script:\n\n-- >8 --\nmkdir repo && cd repo\ngit init\n\ntouch file && git add file\ncommit() {\n  echo $1 >file && git commit -a -m $1 && git tag $1\n}\n\ncommit one\ncommit two\ncommit three\ngit checkout -b other two\ncommit alt-three\ngit checkout master\ngit merge other || true\ncommit merged\ncommit four\n-- 8< --\n\nSo a fairly simple repo, but with the key element that it contains a\nmerge. Now try this:\n\n  git log one --not four\n\nYou get the 'one' commit, even though it should be removed by \"--not\nfour\". But if you try this:\n\n  git log one --not two\n\nyou correctly get no output.\n\nIt seems that in limit_list, we do two things:\n  - first add the 'one' commit to the new list (since we process it\n    before it gets marked uninteresting)\n  - then traverse from 'four', marking commits and their parents as\n    uninteresting as we go\n\nHowever, the traversal seems to have trouble going over the merge. We\nadd the parents, but we end up marking them all as uninteresting, and\nthe everybody_uninteresting() optimization triggers, quitting the limit\nbefore we have a chance to reach back to 'one' and mark it. The patch\nbelow fixes it, but I'm very uncertain whether there is something else\ngoing on that I'm missing that should be handling this case.\n\n---\ndiff --git a/revision.c b/revision.c\nindex 6e85aaa..7d91ca1 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -579,8 +579,6 @@ static int limit_list(struct rev_info *revs)\n \t\t\treturn -1;\n \t\tif (obj->flags & UNINTERESTING) {\n \t\t\tmark_parents_uninteresting(commit);\n-\t\t\tif (everybody_uninteresting(list))\n-\t\t\t\tbreak;\n \t\t\tcontinue;\n \t\t}\n \t\tif (revs->min_age != -1 && (commit->date > revs->min_age))\n"},{"id":"67187","messageId":"7vwspmzhf2.fsf@gitster.siamese.dyndns.org","threadId":"11840","inReplyTo":"20080203043310.GA5984@coredump.intra.peff.net","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-03T06:24:17Z","receivedAt":"2008-02-03T06:24:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sat, Feb 02, 2008 at 10:00:54PM -0500, Jeff King wrote:\n>\n>> That being said, the commit in your 'master' branch _is_ part of\n>> 1dd567d5, and should be culled. So I'm not clear on why it shows up only\n>> when you ask to see both branches, and that may be a bug.\n>\n> OK, there is definitely a bug here, but I'm having some trouble figuring\n> out the correct fix. It's in the revision walker, so I have cc'd those\n> who are more clueful than I.\n\nI am officially in semi-vacation-post-release mode, but I\nnoticed this is reproducible even with a prehistoric git\n(v1.0.0).\n"},{"id":"67188","messageId":"7vr6fuzgq1.fsf@gitster.siamese.dyndns.org","threadId":"11840","inReplyTo":"20080203043310.GA5984@coredump.intra.peff.net","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-03T06:39:18Z","receivedAt":"2008-02-03T06:39:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sat, Feb 02, 2008 at 10:00:54PM -0500, Jeff King wrote:\n>\n>> That being said, the commit in your 'master' branch _is_ part of\n>> 1dd567d5, and should be culled. So I'm not clear on why it shows up only\n>> when you ask to see both branches, and that may be a bug.\n>\n> OK, there is definitely a bug here, but I'm having some trouble figuring\n> out the correct fix. It's in the revision walker, so I have cc'd those\n> who are more clueful than I.\n>\n> You can recreate a problematic repo using this script:\n>\n> -- >8 --\n> mkdir repo && cd repo\n> git init\n>\n> touch file && git add file\n> commit() {\n>   echo $1 >file && git commit -a -m $1 && git tag $1\n> }\n>\n> commit one\n> commit two\n> commit three\n> git checkout -b other two\n> commit alt-three\n> git checkout master\n> git merge other || true\n> commit merged\n> commit four\n> -- 8< --\n>\n> So a fairly simple repo, but with the key element that it contains a\n> merge.\n\nIt is not so simple, it appears.  If I add for reproducibility\n\"test_tick\" like this:\n\n        commit () {\n                test_tick &&\n                echo $1 >file &&\n                git commit -a -m $1 &&\n                git tag $1\n        }\n\nthe problem goes away.\n\nSo there is some interaction with \"insert_by_date()\".  Still\ndigging.\n\n t/t6009-rev-list-parent.sh |   43 +++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 43 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t6009-rev-list-parent.sh b/t/t6009-rev-list-parent.sh\nnew file mode 100755\nindex 0000000..66164e9\n--- /dev/null\n+++ b/t/t6009-rev-list-parent.sh\n@@ -0,0 +1,43 @@\n+#!/bin/sh\n+\n+test_description='properly cull all ancestors'\n+\n+. ./test-lib.sh\n+\n+commit () {\n+\t: test_tick &&\n+\techo $1 >file &&\n+\tgit commit -a -m $1 &&\n+\tgit tag $1\n+}\n+\n+test_expect_success setup '\n+\n+\ttouch file &&\n+\tgit add file &&\n+\n+\tcommit one &&\n+\tcommit two &&\n+\tcommit three &&\n+\n+\tgit checkout -b other two &&\n+\tcommit alt-three &&\n+\n+\tgit checkout master &&\n+\n+\tgit merge -s ours other &&\n+\n+\tcommit merged &&\n+\tcommit four &&\n+\n+\tgit -p show-branch --more=999\n+\n+'\n+\n+test_expect_failure 'one is ancestor of others and should not be shown' '\n+\n+\tgit rev-list one --not four >result &&\n+\t>expect &&\n+\tdiff -u expect result \n+\n+'\n"},{"id":"67189","messageId":"20080203071318.GA13849@coredump.intra.peff.net","threadId":"11840","inReplyTo":"7vr6fuzgq1.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-02-03T07:13:18Z","receivedAt":"2008-02-03T07:13:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 02, 2008 at 10:39:18PM -0800, Junio C Hamano wrote:\n\n> It is not so simple, it appears.  If I add for reproducibility\n> \"test_tick\" like this:\n> \n>         commit () {\n>                 test_tick &&\n>                 echo $1 >file &&\n>                 git commit -a -m $1 &&\n>                 git tag $1\n>         }\n\nAh. I think what is happening is something like this:\n\n  - when we add 'four' as uninteresting, we mark its parents as\n    uninteresting in handle_commit\n  - we don't recursively follow all of its parents because we haven't\n    parsed them yet\n  - when we get to limit_list, we call mark_parents_uninteresting again.\n    But we have already marked four^ as uninteresting, and therefore we\n    do not recurse in marking\n  - we add the parents to the list, but they are not interesting, and\n    therefore we quit\n\nThe reason it works with test_tick is that it changes the order we deal\nwith the commits in limit_list. We deal with 'one' _after_ dealing with\nthe uninteresting parents, so we never bail with\neverybody_uninteresting.\n\n> +test_expect_failure 'one is ancestor of others and should not be shown' '\n> +\n> +\tgit rev-list one --not four >result &&\n> +\t>expect &&\n> +\tdiff -u expect result \n> +\n> +'\n\nHooray, test_expect_failure is used properly. But you are still\nmissing a test_done. :)\n\n-Peff\n"},{"id":"67190","messageId":"20080203071833.GA16273@coredump.intra.peff.net","threadId":"11840","inReplyTo":"20080203071318.GA13849@coredump.intra.peff.net","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-02-03T07:18:33Z","receivedAt":"2008-02-03T07:18:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 03, 2008 at 02:13:18AM -0500, Jeff King wrote:\n\n> Ah. I think what is happening is something like this:\n> \n>   - when we add 'four' as uninteresting, we mark its parents as\n>     uninteresting in handle_commit\n>   - we don't recursively follow all of its parents because we haven't\n>     parsed them yet\n>   - when we get to limit_list, we call mark_parents_uninteresting again.\n>     But we have already marked four^ as uninteresting, and therefore we\n>     do not recurse in marking\n>   - we add the parents to the list, but they are not interesting, and\n>     therefore we quit\n\nSo the \"fix\" I posted before was to stop bailing on\neverybody_uninteresting; clearly it is possible that although those\ncommits are uninteresting, we still have work to do on their ancestors.\nThere is probably a performance impact since we will end up traversing\nthe whole commit chain just to mark them all uninteresting.\n\nWe could also always recurse in make_parents_uninteresting; I think this\nhas the same performance problem, since we have to parse the parents for\neach commit.\n\nWe could topologically order the commits going into limit_list (it just\nworks most of the time because the date ordering is _mostly_ right).\nThis guarantees that we deal with 'four' before 'one'. But topo sorting\nis expensive.\n\n-Peff\n"},{"id":"67192","messageId":"7vbq6yzdvr.fsf@gitster.siamese.dyndns.org","threadId":"11840","inReplyTo":"20080203071833.GA16273@coredump.intra.peff.net","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-03T07:40:40Z","receivedAt":"2008-02-03T07:40:40Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> We could topologically order the commits going into limit_list (it just\n> works most of the time because the date ordering is _mostly_ right).\n> This guarantees that we deal with 'four' before 'one'. But topo sorting\n> is expensive.\n\nI recall we did a rather clever optimization in merge-base.  I\nam starting to suspect that we would need a similar trick there.\n\nThe issue is:\n\n * We have pushed \"one\" out already to \"newlist\", but we haven't\n   given UNINTERESTING bit to it yet.\n\n * We are responsible to mark \"one\" UNINTERESTING, if it can be\n   reached from a commit that is UNINTERESTING.  We expect\n   further looping of the \"while (list)\" and\n   mark_parents_uninteresting() in that loop will eventually\n   smudge it.\n\n * We can obviously prove that we marked all UNINTERESTING\n   commits that matters by traversing _all_ history (i.e. make\n   sure mark_parents_uninteresting() recurses, and wait until\n   \"list\" truly becomes empty), but we would want to somehow\n   optimize it.  The everybody_uninteresting() check was\n   introduced for that purpose, but that is not a right\n   optimization if commit timestamps are skewed like this.\n\nThe right optimization is probably:\n\n * Wait until everybody on \"list\" is UNINTERESTING.  IOW, keep\n   the \"everybody_uninteresting()\" check with break as is.\n\n   At that point \"newlist\" will contain all the commits that we\n   might be interested in (e.g. \"one\").  The issue is reduced\n   from \"mark _all_ commits that can be reached from known\n   UNINTERESTING ones\" to \"make sure the commits on the newlist\n   that are reachable from UNINTERESTING ones in the \"list\" are\n   marked as UNINTERESTING (e.g. \"one\" should be checked for\n   reachability from the remaining UNINTERESTING commits in\n   \"list\", we do not have to check for anything else).\n\n * After the loop exits, traverse from all non UNINTERESTING\n   commits on the \"newlist\" and all remaining commits on the\n   \"list\" (by definition, the latter are UNINTERESTING) down to\n   their common merge base, propagating UNINTERESTING bit down.\n\n   Once we do that, we have proven that \"one\" is reachable from\n   any of the UNINTERESTING commit.\n"},{"id":"67193","messageId":"7v4pcqzdkl.fsf@gitster.siamese.dyndns.org","threadId":"11840","inReplyTo":"7vbq6yzdvr.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-03T07:47:22Z","receivedAt":"2008-02-03T07:47:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"By the way, the issue does not have anything to do with merges.\n\nThis is the absolute minimum (and reliable) reproduction recipe.\nYou need four commits.  If you remove \"commit three &&\", the\ntraversal will contaminate \"one\".\n\n---\n\n t/t6009-rev-list-parent.sh |   38 ++++++++++++++++++++++++++++++++++++++\n 1 files changed, 38 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t6009-rev-list-parent.sh b/t/t6009-rev-list-parent.sh\nnew file mode 100755\nindex 0000000..1da5dbb\n--- /dev/null\n+++ b/t/t6009-rev-list-parent.sh\n@@ -0,0 +1,38 @@\n+#!/bin/sh\n+\n+test_description='properly cull all ancestors'\n+\n+. ./test-lib.sh\n+\n+commit () {\n+\ttest_tick &&\n+\techo $1 >file &&\n+\tgit commit -a -m $1 &&\n+\tgit tag $1\n+}\n+\n+test_expect_success setup '\n+\n+\ttouch file &&\n+\tgit add file &&\n+\n+\tcommit one &&\n+\n+\ttest_tick=$(($test_tick - 2400))\n+\n+\tcommit two &&\n+\tcommit three &&\n+\tcommit four &&\n+\n+\tgit log --pretty=oneline --abbrev-commit\n+'\n+\n+test_expect_failure 'one is ancestor of others and should not be shown' '\n+\n+\tgit rev-list one --not four >result &&\n+\t>expect &&\n+\tdiff -u expect result \n+\n+'\n+\n+test_done\n"},{"id":"67195","messageId":"7vir16xxkx.fsf@gitster.siamese.dyndns.org","threadId":"11840","inReplyTo":"20080203071833.GA16273@coredump.intra.peff.net","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-03T08:18:06Z","receivedAt":"2008-02-03T08:18:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> There is probably a performance impact since we will end up traversing\n> the whole commit chain just to mark them all uninteresting.\n\nYes.  \"tip..initial\" obviously need to traverse almost all\nhistory to find out that the initial is reachable from tip, but\nthere is no avoiding that.\n\nHowever, a typical usage is \"old..new\" and old and new are both\nfar from the initial commit, and forcing traversal down to the\ninitial commit in such a usual case is unacceptable.\n\nI think we only need to traverse down to their merge bases to\nprove that new cannot be reachable from old, and we can find out\nall of their merge bases without traversing down to root.\n"},{"id":"67388","messageId":"alpine.LFD.1.00.0802040922480.3034@hp.linux-foundation.org","threadId":"11840","inReplyTo":"20080203043310.GA5984@coredump.intra.peff.net","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-02-04T17:32:15Z","receivedAt":"2008-02-04T17:32:15Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 2 Feb 2008, Jeff King wrote:\n> \n> OK, there is definitely a bug here, but I'm having some trouble figuring\n> out the correct fix. It's in the revision walker, so I have cc'd those\n> who are more clueful than I.\n\nOk, I agree that there is a bug, and your two-liner fix is a \"fix\" in that \nit works, but I think it's absolutely the wrogn fix because it is totally \nunacceptable from a performance angle. We obviously need to break out of \nthe loop before we have walked the whole commit chain.\n\n>  \t\tif (obj->flags & UNINTERESTING) {\n>  \t\t\tmark_parents_uninteresting(commit);\n> -\t\t\tif (everybody_uninteresting(list))\n> -\t\t\t\tbreak;\n>  \t\t\tcontinue;\n>  \t\t}\n\nSo I think the real problem here is not that the logic is wrong in \ngeneral, but that there is one *special* case where the logic to break out \nis wrong.\n\nAnd that special case is when we hit the root commit which isn't negative.\n\nThat case is special because *normally*, if we have a positive commit, we \nwill always continue to walk the parents of that positive commit, so the \n\"everybody_interesting()\" check will not trigger. BUT! If we hit a root \ncommit and it is positive, that won't happen (since, by definition, it has \nno parents to keep the list populated with), and now we break out early.\n\nSo I think your fix is wrong, but it's \"close\" to right: I suspect that we \ncan fix it by marking the \"we hit the root commit\" case, and just \ndisabling it for that case.\n\nThis patch is untested and obviously won't even compile (I didn't actually \nadd the \"hit_root\" bitfield to the revision struct), but shows what I \n*think* should fix this issue, without the performance problem.\n\nBut maybe I haven't thought it entirely through, and there is some other \ncase that can trigger this bug.\n\nSo please somebody double-check my thinking.\n\n\t\t\tLinus\n\n---\ndiff --git a/revision.c b/revision.c\nindex 6e85aaa..0e90988 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -456,6 +456,9 @@ static int add_parents_to_list(struct rev_info *revs, struct commit *commit, str\n \n \tleft_flag = (commit->object.flags & SYMMETRIC_LEFT);\n \n+\tif (!commit->parents)\n+\t\trevs->hit_root = 1;\n+\n \trest = !revs->first_parent_only;\n \tfor (parent = commit->parents, add = 1; parent; add = rest) {\n \t\tstruct commit *p = parent->item;\n@@ -579,7 +582,7 @@ static int limit_list(struct rev_info *revs)\n \t\t\treturn -1;\n \t\tif (obj->flags & UNINTERESTING) {\n \t\t\tmark_parents_uninteresting(commit);\n-\t\t\tif (everybody_uninteresting(list))\n+\t\t\tif (!revs->hit_root && everybody_uninteresting(list))\n \t\t\t\tbreak;\n \t\t\tcontinue;\n \t\t}\n"},{"id":"67389","messageId":"alpine.LFD.1.00.0802040936080.3034@hp.linux-foundation.org","threadId":"11840","inReplyTo":"alpine.LFD.1.00.0802040922480.3034@hp.linux-foundation.org","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-02-04T17:37:32Z","receivedAt":"2008-02-04T17:37:32Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 4 Feb 2008, Linus Torvalds wrote:\n>\n> This patch is untested and obviously won't even compile (I didn't actually \n> add the \"hit_root\" bitfield to the revision struct), but shows what I \n> *think* should fix this issue, without the performance problem.\n\nOk, so I was lazy. Here's the updated patch that actually compiles and is \nalso now verified to fix Junio's test-case.\n\n(Same patch, just the added bitfield declaration, and the testing ;)\n\n\t\tLinus\n---\n revision.c |    5 ++++-\n revision.h |    3 ++-\n 2 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 6e85aaa..0e90988 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -456,6 +456,9 @@ static int add_parents_to_list(struct rev_info *revs, struct commit *commit, str\n \n \tleft_flag = (commit->object.flags & SYMMETRIC_LEFT);\n \n+\tif (!commit->parents)\n+\t\trevs->hit_root = 1;\n+\n \trest = !revs->first_parent_only;\n \tfor (parent = commit->parents, add = 1; parent; add = rest) {\n \t\tstruct commit *p = parent->item;\n@@ -579,7 +582,7 @@ static int limit_list(struct rev_info *revs)\n \t\t\treturn -1;\n \t\tif (obj->flags & UNINTERESTING) {\n \t\t\tmark_parents_uninteresting(commit);\n-\t\t\tif (everybody_uninteresting(list))\n+\t\t\tif (!revs->hit_root && everybody_uninteresting(list))\n \t\t\t\tbreak;\n \t\t\tcontinue;\n \t\t}\ndiff --git a/revision.h b/revision.h\nindex 8572315..5188a2f 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -48,7 +48,8 @@ struct rev_info {\n \t\t\tparents:1,\n \t\t\treverse:1,\n \t\t\tcherry_pick:1,\n-\t\t\tfirst_parent_only:1;\n+\t\t\tfirst_parent_only:1,\n+\t\t\thit_root:1;\n \n \t/* Diff flags */\n \tunsigned int\tdiff:1,\n"},{"id":"67408","messageId":"7vr6fsk08w.fsf@gitster.siamese.dyndns.org","threadId":"11840","inReplyTo":"alpine.LFD.1.00.0802040922480.3034@hp.linux-foundation.org","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-04T19:08:47Z","receivedAt":"2008-02-04T19:08:47Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> So I think the real problem here is not that the logic is wrong in \n> general, but that there is one *special* case where the logic to break out \n> is wrong.\n>\n> And that special case is when we hit the root commit which isn't negative.\n>\n> That case is special because *normally*, if we have a positive commit, we \n> will always continue to walk the parents of that positive commit, so the \n> \"everybody_interesting()\" check will not trigger. BUT! If we hit a root \n> commit and it is positive, that won't happen (since, by definition, it has \n> no parents to keep the list populated with), and now we break out early.\n>\n> So I think your fix is wrong, but it's \"close\" to right: I suspect that we \n> can fix it by marking the \"we hit the root commit\" case, and just \n> disabling it for that case.\n\nAhh, I was preparing a response that begins with \"Wow, a joy of\nworking in a mailing list with people more clever than me!  It's\nso obvious but I did not think of it.\"  I've written something\nlike that more than a few times on this list responding to\nseveral people, I think.\n\nHowever, I am afraid that is not quite enough.  It is not just\n\"when we hit the root\".\n\nConsider the same topology in the small test (1-2-3-4) but with\nthree additional commits:\n\n         B---C\n        /\n    ---A---1---2---3---4\n\nAgain, 2-3-4 are in nice chronological order, but 1 has the\nyounguest timestamp, and A-B-C are all younger than 1.\n\n\t$ rev-list 1 ^4 ^A\n        $ rev-list 1 ^4 ^B\n\nThese two would both mark A as uninteresting while processing\nthe command line (revision.c::handle_commit()).  When we pop 1\noff, the call to add_parents_to_list() for it will not add\nanything positive back.\n\n\t$ rev-list 1 ^4 ^C\n\nThis would not mark A as uninteresting immediately, but by the\ntime 1 gets its turn, A is marked uninteresting.\n\nSo I think the rule to notice this situation with \"hit-root\"\nflag is something like:\n\n    when we pop a positive commit that does not have any\n    positive parent left (root is a special case of this), and\n    the negative parents were contaminated either by:\n\n        (1) being listed as negative on the command line or being a\n            direct parent of a negative commit listed on the\n            command line; or by\n\n        (2) traversing the list of negative commits who are all\n            younger than the positive commit in question.\n\n---\n\n t/t6009-rev-list-parent.sh |   36 ++++++++++++++++++++++++++++++++++--\n 1 files changed, 34 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t6009-rev-list-parent.sh b/t/t6009-rev-list-parent.sh\nindex be3d238..0bb5ac4 100755\n--- a/t/t6009-rev-list-parent.sh\n+++ b/t/t6009-rev-list-parent.sh\n@@ -16,6 +16,14 @@ test_expect_success setup '\n \ttouch file &&\n \tgit add file &&\n \n+\tcommit zero &&\n+\tcommit A &&\n+\tcommit B &&\n+\tcommit C &&\n+\n+\tgit reset --hard A &&\n+\n+\ttest_tick=$(($test_tick - 1200))\n \tcommit one &&\n \n \ttest_tick=$(($test_tick - 2400))\n@@ -27,9 +35,33 @@ test_expect_success setup '\n \tgit log --pretty=oneline --abbrev-commit\n '\n \n-test_expect_failure 'one is ancestor of others and should not be shown' '\n+test_expect_failure '\"zero ^four\" should be empty' '\n+\n+\tgit rev-list zero --not four >result &&\n+\t>expect &&\n+\tdiff -u expect result\n+\n+'\n+\n+test_expect_failure '\"one ^four ^A\" should be empty' '\n+\n+\tgit rev-list one --not four A >result &&\n+\t>expect &&\n+\tdiff -u expect result\n+\n+'\n+\n+test_expect_failure '\"one ^four ^B should be empty' '\n+\n+\tgit rev-list one --not four B >result &&\n+\t>expect &&\n+\tdiff -u expect result\n+\n+'\n+\n+test_expect_failure '\"one ^four ^C should be empty' '\n \n-\tgit rev-list one --not four >result &&\n+\tgit rev-list one --not four C >result &&\n \t>expect &&\n \tdiff -u expect result\n \n"},{"id":"67410","messageId":"alpine.LFD.1.00.0802041146060.3034@hp.linux-foundation.org","threadId":"11840","inReplyTo":"7vr6fsk08w.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-02-04T20:03:42Z","receivedAt":"2008-02-04T20:03:42Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 4 Feb 2008, Junio C Hamano wrote:\n> \n> However, I am afraid that is not quite enough.  It is not just\n> \"when we hit the root\".\n\nYou're right. The proper test is actually \"is the list of commits \ndisconnected\".\n\nWhich is not an entirely trivial thing to test for efficiently.\n\nI suspect that we can solve it by not even bothering to be efficient, and \ninstead just ask that question when we hit the \"are all commits negative\" \nquery.\n\nGaah. This is that stupid apporach. The smarter one might be harder, \nespecially for the special cases where you truly have two totally \nunconnected trees, ie:\n\n\ta -> b -> c\n\n\td -> e -> f\n\nand do\n\n\tgit rev-list c f ^b ^e\n\nwere you actually do not _have_ a single connected graph at all, but you \nknow the result should be \"c\" and \"f\" because they are *individually* \nconnected to what we already know is uninteresting.\n\nNot really tested at all, not really thought through. And that recursion \navoidance could be smarter.\n\n\t\t\tLinus\n\n---\n revision.c |   37 ++++++++++++++++++++++++++++++++++++-\n 1 files changed, 36 insertions(+), 1 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 6e85aaa..26b2343 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -558,6 +558,41 @@ static void cherry_pick_list(struct commit_list *list, struct rev_info *revs)\n \tfree_patch_ids(&ids);\n }\n \n+static inline int commit_is_connected(struct commit *commit)\n+{\n+\tstruct commit_list *parents;\n+\tfor (;;) {\n+\t\tif (commit->object.flags & UNINTERESTING)\n+\t\t\treturn 1;\n+\t\tparents = commit->parents;\n+\t\tif (!parents)\n+\t\t\treturn 0;\n+\t\tif (parents->next)\n+\t\t\tbreak;\n+\t\tcommit = parents->item;\n+\t}\n+\n+\tdo {\n+\t\tif (!commit_is_connected(parents->item))\n+\t\t\treturn 0;\n+\t\tparents = parents->next;\n+\t} while (parents);\n+\treturn 1;\n+}\n+\n+/* Check that the positive list is connected to the negative one.. */\n+static int is_connected(struct commit_list *list)\n+{\n+\twhile (list) {\n+\t\tstruct commit *commit = list->item;\n+\n+\t\tlist = list->next;\n+\t\tif (!commit_is_connected(commit))\n+\t\t\treturn 0;\n+\t}\n+\treturn 1;\n+}\n+\n static int limit_list(struct rev_info *revs)\n {\n \tstruct commit_list *list = revs->commits;\n@@ -579,7 +614,7 @@ static int limit_list(struct rev_info *revs)\n \t\t\treturn -1;\n \t\tif (obj->flags & UNINTERESTING) {\n \t\t\tmark_parents_uninteresting(commit);\n-\t\t\tif (everybody_uninteresting(list))\n+\t\t\tif (everybody_uninteresting(list) && is_connected(newlist))\n \t\t\t\tbreak;\n \t\t\tcontinue;\n \t\t}\n"},{"id":"67411","messageId":"alpine.LFD.1.00.0802041203510.3034@hp.linux-foundation.org","threadId":"11840","inReplyTo":"alpine.LFD.1.00.0802041146060.3034@hp.linux-foundation.org","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-02-04T20:06:16Z","receivedAt":"2008-02-04T20:06:16Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 4 Feb 2008, Linus Torvalds wrote:\n> \n> Not really tested at all, not really thought through. And that recursion \n> avoidance could be smarter.\n\n.. and by \"could be smarter\", I obviously mean \"really *really* should be \nsmarter\". Because this is O(2**n) in the number of merges, which is not \nacceptable even if the constant is really small.\n\nSo I really don't mean that we should do it this way. The right thing to \ndo would be to add a new object flag for that \"connected to UNINTERESTING\" \nproperty, and setting it as we traverse the graph in that \n\"commit_is_connected()\" logic. That should get rid of the exponential \nbehaviour.\n\n\t\t\tLinus\n"},{"id":"67419","messageId":"alpine.LFD.1.00.0802041223080.3034@hp.linux-foundation.org","threadId":"11840","inReplyTo":"alpine.LFD.1.00.0802041146060.3034@hp.linux-foundation.org","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-02-04T20:50:03Z","receivedAt":"2008-02-04T20:50:03Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 4 Feb 2008, Linus Torvalds wrote:\n> \n> Gaah. This is that stupid apporach.\n\n.. and it won't actually solve the problem you pointed to. It's not enough \nthat the positive commits should be connected to the negative ones, the \nproblem is that no negative ones could possibly connect to the positives. \n\nSo scratch that patch as broken too. \n\nReally annoying. It does look like we really want to check the *totally* \nconnected case, and we simply cannot do the \"two unconnected trees\" \ndecision case without traversing both trees fully (since we won't know \nthat they are *really* unconnected until we do).\n\nAnd that seems really quite expensive. I wonder if I've missed something \nagain.\n\n\t\tLinus\n"},{"id":"67487","messageId":"7vir13g9hx.fsf@gitster.siamese.dyndns.org","threadId":"11840","inReplyTo":"alpine.LFD.1.00.0802041223080.3034@hp.linux-foundation.org","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-05T07:14:50Z","receivedAt":"2008-02-05T07:14:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Mon, 4 Feb 2008, Linus Torvalds wrote:\n>> \n>> Gaah. This is that stupid apporach.\n>\n> .. and it won't actually solve the problem you pointed to. It's not enough \n> that the positive commits should be connected to the negative ones, the \n> problem is that no negative ones could possibly connect to the positives. \n>\n> So scratch that patch as broken too. \n>\n> Really annoying. It does look like we really want to check the *totally* \n> connected case, and we simply cannot do the \"two unconnected trees\" \n> decision case without traversing both trees fully (since we won't know \n> that they are *really* unconnected until we do).\n>\n> And that seems really quite expensive. I wonder if I've missed something \n> again.\n\nI tend to agree.  In a totally connected history, the upper\nbound we would need to traverse is down to the merge base of\nstill positive commits in the \"newlist\" and negative ones still\non the \"list\" when everybody on list becomes uninteresting.  And\nif there are two unrelated histories, that traversal will need\nto traverse down to respective roots.\n\nWhich sucks.\n"},{"id":"67557","messageId":"alpine.LFD.1.00.0802051300050.3110@woody.linux-foundation.org","threadId":"11840","inReplyTo":"7vir13g9hx.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-02-05T21:23:16Z","receivedAt":"2008-02-05T21:23:16Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 4 Feb 2008, Junio C Hamano wrote:\n> \n> I tend to agree.  In a totally connected history, the upper\n> bound we would need to traverse is down to the merge base of\n> still positive commits in the \"newlist\" and negative ones still\n> on the \"list\" when everybody on list becomes uninteresting.  And\n> if there are two unrelated histories, that traversal will need\n> to traverse down to respective roots.\n> \n> Which sucks.\n\nI really wonder if the right thing is not simply to admit that we consider \nthe commit time meaningful (within some fudge factor!), and then do:\n\n - make commit warn if any parent commit date is in the future from the \n   current commit date (allow a *small* fudge factor here, say 5 minutes).\n\n - teach fsck to complain about parent commits being in the future from \n   their children (allow the same small fudge factor).\n\n - make the revision walking code realize that if times are too close to \n   each other, it should walk a bit further back...\n\nbecause quite frankly, this bug only shows up when your time goes \nbackwards (or stays the same, but the fudge-factor should take care of \nthat too).\n\nSo the revision walking \"fudge factor\" could be as simple as the \nfollowing..\n\n\t\tLinus\n\n---\n revision.c |   11 +++++++++++\n 1 files changed, 11 insertions(+), 0 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 6e85aaa..e32e1e3 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -558,6 +558,8 @@ static void cherry_pick_list(struct commit_list *list, struct rev_info *revs)\n \tfree_patch_ids(&ids);\n }\n \n+#define FUDGE (60*60)\t/* 1 hour fudge-factor */\n+\n static int limit_list(struct rev_info *revs)\n {\n \tstruct commit_list *list = revs->commits;\n@@ -579,6 +581,15 @@ static int limit_list(struct rev_info *revs)\n \t\t\treturn -1;\n \t\tif (obj->flags & UNINTERESTING) {\n \t\t\tmark_parents_uninteresting(commit);\n+\n+\t\t\t/*\n+\t\t\t * If we have commits on the newlist, we don't\n+\t\t\t * want to do the \"everybody_uninteresting()\"\n+\t\t\t * test until we've hit a negative commit that\n+\t\t\t * is solidly in the past\n+\t\t\t */\n+\t\t\tif (newlist && newlist->item->date < commit->date + FUDGE)\n+\t\t\t\tcontinue;\n \t\t\tif (everybody_uninteresting(list))\n \t\t\t\tbreak;\n \t\t\tcontinue;\n"},{"id":"67569","messageId":"alpine.LSU.1.00.0802052228280.8543@racer.site","threadId":"11840","inReplyTo":"alpine.LFD.1.00.0802051300050.3110@woody.linux-foundation.org","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-05T22:34:42Z","receivedAt":"2008-02-05T22:34:42Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 5 Feb 2008, Linus Torvalds wrote:\n\n> I really wonder if the right thing is not simply to admit that we consider \n> the commit time meaningful (within some fudge factor!), and then do:\n> \n>  - make commit warn if any parent commit date is in the future from the \n>    current commit date (allow a *small* fudge factor here, say 5 minutes).\n\n5 minutes seems a little narrow to me.  I think we can even go with 86400 \nseconds.\n\n>  - teach fsck to complain about parent commits being in the future from \n>    their children (allow the same small fudge factor).\n> \n>  - make the revision walking code realize that if times are too close to \n>    each other, it should walk a bit further back...\n\nThere is one big problem, though.  Sometimes your clock is set wrong for \nall the wrong reasons.  Look at\n\n\thttp://article.gmane.org/gmane.comp.version-control.git/67848/raw\n\nfor example (look at the Original-Date).  If that happens without being \nnoticed (and it happened here!), and it is pushed, you have to live with \nit.\n\n_However_, I could imagine that you can get most of such errors by doing \nthe same as gmane: realise that the clock skew is into the future, and \njust take the sensible date.\n\nIn our case, this would mean that the revision walker should realise that \na child whose date is not older than its parent commit must be wrong.  And \njust take the parent's date instead (but maybe only for the purpose of \nlimiting).\n\nHmm?\n\nCiao,\nDscho\n"},{"id":"67589","messageId":"7vprvb6k9u.fsf@gitster.siamese.dyndns.org","threadId":"11840","inReplyTo":"alpine.LFD.1.00.0802051300050.3110@woody.linux-foundation.org","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-05T23:44:29Z","receivedAt":"2008-02-05T23:44:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> I really wonder if the right thing is not simply to admit that we consider \n> the commit time meaningful (within some fudge factor!), and then do:\n>\n>  - make commit warn if any parent commit date is in the future from the \n>    current commit date (allow a *small* fudge factor here, say 5 minutes).\n\nHmmm.  In other words, you are punished for trying to build on\ntop of somebody else who screwed up.  That sucks.\n\n>  - teach fsck to complain about parent commits being in the future from \n>    their children (allow the same small fudge factor).\n\nAnd this is even worse.\n\nBut I think these are sane thing to do regardless.  To prevent\nproblematic commits from contaminating other people, we could\nadd default post-receive hook and perhaps a new pre-merge hook\nto inspect the newly obtained history to warn and reject a merge\n(including fast-forward) if it contains commits far into the\nfuture.  We would also need a mechanism to force a merge with\nsuch a clock-skewed history if we did so.\n\n>  - make the revision walking code realize that if times are too close to \n>    each other, it should walk a bit further back...\n>\n> because quite frankly, this bug only shows up when your time goes \n> backwards (or stays the same, but the fudge-factor should take care of \n> that too).\n\n> @@ -579,6 +581,15 @@ static int limit_list(struct rev_info *revs)\n>  \t\t\treturn -1;\n>  \t\tif (obj->flags & UNINTERESTING) {\n>  \t\t\tmark_parents_uninteresting(commit);\n> +\n> +\t\t\t/*\n> +\t\t\t * If we have commits on the newlist, we don't\n> +\t\t\t * want to do the \"everybody_uninteresting()\"\n> +\t\t\t * test until we've hit a negative commit that\n> +\t\t\t * is solidly in the past\n> +\t\t\t */\n> +\t\t\tif (newlist && newlist->item->date < commit->date + FUDGE)\n> +\t\t\t\tcontinue;\n>  \t\t\tif (everybody_uninteresting(list))\n>  \t\t\t\tbreak;\n>  \t\t\tcontinue;\n\nWe picked up commit which we know is the youngest in the \"list\".\nBecause we push into newlist as we traverse, newlist is sorted\nby date, and the element pointed by it is the youngest in there.\nThat is something we have processed earlier, so it ought to be\nyounger than this commit.  Otherwise we have found a problematic\nclock skew.  Ok, I think it should work.\n\nThe clock skew t6009 artificially introduces needs to be\nshortened for this, though.\n"},{"id":"67583","messageId":"alpine.LFD.1.00.0802051539570.2967@woody.linux-foundation.org","threadId":"11840","inReplyTo":"alpine.LSU.1.00.0802052228280.8543@racer.site","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-02-05T23:59:43Z","receivedAt":"2008-02-05T23:59:43Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 5 Feb 2008, Johannes Schindelin wrote:\n> > \n> >  - make commit warn if any parent commit date is in the future from the \n> >    current commit date (allow a *small* fudge factor here, say 5 minutes).\n> \n> 5 minutes seems a little narrow to me.  I think we can even go with 86400 \n> seconds.\n\nWell, notice how I said *warn*. Not abort the commit. Not stop. Just make \npeople very aware of the fact that clocks are skewed.\n\nIn the case that actually triggered this whole discussion, the problem \nseems to sadly have been in the original CVS tree (or whatever it was \nimported from): the project started in 2006, had lots of regular commits \nup to October 2007, and then suddenly it had a commit that had a date in \n2002!\n\n[ For those interested in looking at this, the broken commit in that \n  Tilman's repo was commit 3a7340af2bd57488f832d7070b0ce96c4baa6b54, which \n  is from October 2002, and which is surrounded by commits from October \n  2007, so somebody was literally off by five years ]\n\nIn other words, the repo really was pretty broken, and the git behaviour \ncame from that breakage.\n\nOne way to work around this kind of thing is to flag broken dates, and \nyes, we can probably find most of these kinds of random breakages (in the \ncase of the broken repo, we had the parent of the broken commit already \nparsed, we could have seen that the date was bogus).\n\nBut yeah, I have to also admit that exactly *because* the bug came from \nsome import from somewhere else, the date requirement cannot work - I \ndon't want to change even obviously bogus data from an external import.\n\nI don't see a good way to find the breakage efficiently and generally, \nthough. In the particular case that this hit us, it's visible because the \nbreakage is entirely local (ie you can see the broken commit  by just \nlooking directly at its parents), but even if you have just *two* commits \nthat are broken in succession, the breakage is no longer locally obvious \nat the later one.\n\nNasty.\n\n\t\t\tLinus\n"},{"id":"67592","messageId":"alpine.LFD.1.00.0802051648410.2967@woody.linux-foundation.org","threadId":"11840","inReplyTo":"7vprvb6k9u.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-02-06T00:52:43Z","receivedAt":"2008-02-06T00:52:43Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 5 Feb 2008, Junio C Hamano wrote:\n> >\n> >  - make commit warn if any parent commit date is in the future from the \n> >    current commit date (allow a *small* fudge factor here, say 5 minutes).\n> \n> Hmmm.  In other words, you are punished for trying to build on\n> top of somebody else who screwed up.  That sucks.\n\nWell, I was actually thinking that the most reasonable thing to do is that \nif you pull from somebody, and you get this warning, you send an email \nsaying \"you suck, I will not pull your broken crap\".\n\nBut the real problem is that you might be importing it from some external \nlegacy SCM entity, and then you can't say \"you suck, I won't pull\", \nbecause the whole point is that external entity obviously *does* suck, and \nyou want to simply stop using it. And then the \"I won't pull\" isn't an \noption ;)\n\nSo yeah, I don't think the warnings really work, if only because of that \n\"import from crappy CVS repo\" issue.\n\nBut the revision.c change might be worth it, if only as a slight band-aid \nfor the current issue. It won't fix the original problem, though (because \nthat broken repo had a five *year* clock skew, not an hour :)\n\nI'll continue to think about whether I can come up with some sane \nheuristic that allows non-broken cases to not go all the way up to the \nroot.\n\n\t\tLinus\n"},{"id":"67600","messageId":"alpine.LFD.1.00.0802052021250.2732@xanadu.home","threadId":"11840","inReplyTo":"alpine.LSU.1.00.0802052228280.8543@racer.site","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-02-06T01:22:57Z","receivedAt":"2008-02-06T01:22:57Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 5 Feb 2008, Johannes Schindelin wrote:\n\n> In our case, this would mean that the revision walker should realise that \n> a child whose date is not older than its parent commit must be wrong.  And \n> just take the parent's date instead (but maybe only for the purpose of \n> limiting).\n\nOnly for the committer's date of course.  Author's date might be \ncompletely random.\n\n\nNicolas\n"},{"id":"67602","messageId":"7v7ihi7syj.fsf@gitster.siamese.dyndns.org","threadId":"11840","inReplyTo":"alpine.LSU.1.00.0802052228280.8543@racer.site","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-06T01:51:32Z","receivedAt":"2008-02-06T01:51:32Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> In our case, this would mean that the revision walker should realise that \n> a child whose date is not older than its parent commit must be wrong.  And \n> just take the parent's date instead (but maybe only for the purpose of \n> limiting).\n\nNo.\n\n\t1---2---3---4\n\nTimestamps are 2 < 3 < 4 < 1 and you ask:\n\n\t$ git rev-list 1 ^4\n\nWe push 1 and ^4 in \"list\".  We pick 1 and push it out to\n\"newlist\" (possible results, but the hope is they may later be\nmarked as UNINTERESTING as we traverse the remaining one still\non \"list\").  We pick ^4, mark 3 as UNINTERESTING and push ^3\ninto \"list\", and realize there is nobody that is still positive\n(i.e. without UNINTERESTING bit).  We have \"clever\" optimization\nthat stops in such a case.\n\nNowhere in this sequence we can notice that \"A child whose date\nis not older than its parent\".  We do not even get to commit 2\nduring the traversal.\n\nIn order to notice the problem, you need to make sure we will\nsee the link between 1 and 2 (i.e. the fact that 1 has a child\nthat is older than itself).  That would take traversing \"all the\nway down\".\n\nThe \"all the way down\" is not quite correct, though.  If we have\nother commits, like this:\n\n              B---C\n             /\n     ---0---A---1---2---3---4\n\nwhere timestamps are 0 < A < B < C < 2 < 3 < 4 < 1, and if you\nask:\n\n\t$ git rev-list 1 ^4 ^A\n\t$ git rev-list 1 ^4 ^B\n\t$ git rev-list 1 ^4 ^C\n\nwe will have a similar issue.  We do not have to go down the\npotentially long history beyond A.  But we at least need to\ntraverse down to the merge base of negatives in \"list\" and\npositives in \"newlist\" when \"list\" becomes all UNINTERESTING (in\nthis case, traverse all paths as if we are trying to find out\nthe merge-base between 1 and 3.  That traversal will see 2 and\nwe will see your clock skew).\n\nBut the point is that the condition you mentioned cannot be\nfound out unless you traverse to 2, and at that point you have\ntraversed enough already.\n\nAs Linus earlier said, the question really is: for positive\ncommits in \"newlist\", have we not missed any its UNINTERESTING\ndescendants?\n\nFor a toy-scale graph, a parallel merge-base traversal like what\nshow-branch does may work, but for a real workload, newlist\nwould contain literally hundreds of commits, so using unaltered\n\"merge-base\" algorithm is probably not an option either.\n"},{"id":"67613","messageId":"7vwspi4poh.fsf@gitster.siamese.dyndns.org","threadId":"11840","inReplyTo":"alpine.LFD.1.00.0802051648410.2967@woody.linux-foundation.org","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-06T05:30:38Z","receivedAt":"2008-02-06T05:30:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> But the revision.c change might be worth it, if only as a slight band-aid \n> for the current issue. It won't fix the original problem, though (because \n> that broken repo had a five *year* clock skew, not an hour :)\n>\n> I'll continue to think about whether I can come up with some sane \n> heuristic that allows non-broken cases to not go all the way up to the \n> root.\n\nI really wish this was still May 2005.  Then I (actually, you)\ncould just decree:\n\n\tSorry guys, but you all need to run convert-objects to\n\tupdate your repo.  What it does is to add a \"generation\"\n\theader to each and every commit object.  Then upgrade\n\tyour git to this version, that maintains the\n\t\"generation\" number, defined as:\n\n        (1) parentless commit gets generation #0;\n\n        (2) otherwise the generation number of a commit is\n\t    max(its parents' generation number)+1.\n"},{"id":"67614","messageId":"7vhcgm4o1p.fsf@gitster.siamese.dyndns.org","threadId":"11840","inReplyTo":"7v7ihi7syj.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-06T06:05:54Z","receivedAt":"2008-02-06T06:05:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> As Linus earlier said, the question really is: for positive\n> commits in \"newlist\", have we not missed any its UNINTERESTING\n> descendants?\n>\n> For a toy-scale graph, a parallel merge-base traversal like what\n> show-branch does may work, but for a real workload, newlist\n> would contain literally hundreds of commits, so using unaltered\n> \"merge-base\" algorithm is probably not an option either.\n\nAfter exiting the while (list) we need to prove that each\npositive commit in \"newlist\" cannot be reached by any of the\nnegative commit still in \"list\".\n\nEven though \"newlist\" may have thousands of commits, we do not\nhave to inspect all of them.  In order to prove that we\ntraversed everything that matters, we will only need to look at\nthe ones whose ancestors are not in \"newlist\" (bottom commits)\nand see if each of them can be reached from the negative ones.\nIf a non-bottom commit is reachable from one of the negative\nones, then the bottom commit that is ancestor of that non-bottom\ncommit surely is reachable as well.\n\nWe can make one pass to mark everything on \"newlist\" with one\nbit from flags, and then another pass to mark the positive ones\nwhose parent has that bit set, so we would need two bits in\ntotal while finding out the set of bottom commits (we can reuse\nthese two bits after we know what they are).\n\nOnce we find the set of bottom commits in \"newlist\", we would\nneed to prove that none of them can be reached from any of the\nnegative commits still in \"list\".  We can do this traversal\nusing two bits from flags, exactly like commit.c::merge_bases()\n\n    for each bottom commit B {\n\tL = empty list\n\tB.flags |= PARENT2\n\tL.append(B)\n\tfor each negative commit C in \"list from limit_list()\"\n            C.flags |= PARENT1\n            L.append(C)\n\twhile (L) {\n\t    C = shift L;\n\t    flag = C.flags & (PARENT1|PARENT2);\n            if (flag ==  (PARENT1|PARENT2))\n \t\tcontinue; /* common */\n\t    for each parent P of commit C:\n\t\tpflag = P.flags & (PARENT1|PARENT2);\n\t\tif (pflag == flag)\n                    continue;\n\t\tP.flags |= flags;\n                L.append(P)\n\t}\n        if (B.flags & PARENT1)\n            we still need to traverse -- everybody_uninteresting()\n\t    in limit_list() main loop was not enough!\n    }\n"},{"id":"67615","messageId":"7vd4ra4nid.fsf@gitster.siamese.dyndns.org","threadId":"11840","inReplyTo":"7vhcgm4o1p.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-06T06:17:30Z","receivedAt":"2008-02-06T06:17:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Once we find the set of bottom commits in \"newlist\", we would\n> need to prove that none of them can be reached from any of the\n> negative commits still in \"list\".  We can do this traversal\n> using two bits from flags, exactly like commit.c::merge_bases()\n>\n>     for each bottom commit B {\n>       L = empty list\n>       B.flags |= PARENT2\n>       L.append(B)\n>       for each negative commit C in \"list from limit_list()\"\n>           C.flags |= PARENT1\n>           L.append(C)\n>       while (L) {\n>           C = shift L;\n>           flag = C.flags & (PARENT1|PARENT2);\n>             if (flag ==  (PARENT1|PARENT2))\n>               continue; /* common */\n>           for each parent P of commit C:\n>               pflag = P.flags & (PARENT1|PARENT2);\n>               if (pflag == flag)\n>                     continue;\n>               P.flags |= flags;\n>                 L.append(P)\n>       }\n>       if (B.flags & PARENT1)\n>           we still need to traverse -- everybody_uninteresting()\n>           in limit_list() main loop was not enough!\n>     }\n\nActually when we do this traversal, we can mark the positive\ncommits on \"newlist\" as negative when it is painted with\nPARENT1.  I was assuming that as soon as we find that we are in\nproblematic history with this procedure we exit the whole thing\nand resume to limit_list(), but we may be able to use this as\nthe postprocessing step to clean-up the result.  I just need to\nprove that this postprocessing step will smudge all the \"falsely\npositive but in reality negative\" ones...\n"},{"id":"67626","messageId":"20080206081641.GA19876@diana.vm.bytemark.co.uk","threadId":"11840","inReplyTo":"7vwspi4poh.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-02-06T08:16:41Z","receivedAt":"2008-02-06T08:16:41Z","isPatch":false,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2008-02-05 21:30:38 -0800, Junio C Hamano wrote:\n\n> I really wish this was still May 2005. Then I (actually, you) could\n> just decree:\n>\n>       Sorry guys, but you all need to run convert-objects to update\n>       your repo. What it does is to add a \"generation\" header to\n>       each and every commit object. Then upgrade your git to this\n>       version, that maintains the \"generation\" number, defined as:\n>\n>         (1) parentless commit gets generation #0;\n>\n>         (2) otherwise the generation number of a commit is max(its\n>             parents' generation number)+1.\n\nWould it be possible to start adding a generation header to new\ncommits, so that this problem (and others -- I recall hearing this\nsame wish a year or two ago regarding some gitk toposorting issue)\nwill eventually fade away?\n\nFor old commits without an embedded generation number, git could\nconceivably compute their generation number once and store them in a\n(local) file somewhere.\n\nI expect that this has been considered already, and I'd be interested\nin hearing why it doesn't work, if you (or someone else) have some\ntime to waste. :-)\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"67651","messageId":"alpine.LFD.1.00.0802060216510.2967@woody.linux-foundation.org","threadId":"11840","inReplyTo":"7vwspi4poh.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-02-06T10:34:21Z","receivedAt":"2008-02-06T10:34:21Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 5 Feb 2008, Junio C Hamano wrote:\n> \n> I really wish this was still May 2005.  Then I (actually, you)\n> could just decree:\n\nYeah, we really should have done that, when this came up last.\n\nWe could still decide it's a good idea to do, and simply decide that\n\n - within all-new ranges (that *do* have generation numbers) we can use \n   the generation number to give certain guarantees.\n\n - when any commits involved don't have generation numbers, we just fall \n   back on the not-strict-guarantees-use-commit-date-heuristics.\n\nbut here's something I whipped up because I woke up at 2AM and decided \nthat there is a simple heuristic that works *most* of the time.. We just \nadd a bit of slop, namely:\n\n - we always walk an extra SLOP commits from the source list even if we \n   decide that the source list is probably all done (unless the source is \n   entirely empty, of course, because then we really can't do anything at \n   all)\n\n - we keep track of the date of the last commit we added to the \n   destination list (this will *generally* be the oldest entry we've seen \n   so far)\n\n - we compare that with the youngest entry (the first one) of the source \n   list, and if the destination is older than the source, we know we want \n   to look at the source.\n\n - otherwise, do the \"everybody_uninteresting()\" test to see whether we're \n   still interested in the source.\n\nI dunno. It's really late (or early ;), and I'm having a headache. Maybe \nit doesn't really work. But the idea is that this should be able to handle \nthe few occasional incorrect timestamps. Maybe.\n\n\t\tLinus\n\n---\n revision.c |   35 ++++++++++++++++++++++++++++++++++-\n 1 files changed, 34 insertions(+), 1 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 6e85aaa..a50ae02 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -558,8 +558,39 @@ static void cherry_pick_list(struct commit_list *list, struct rev_info *revs)\n \tfree_patch_ids(&ids);\n }\n \n+/* How many extra uninteresting commits we want to see.. */\n+#define SLOP 5\n+\n+static int still_interesting(struct commit_list *src, unsigned long date, int slop)\n+{\n+\t/*\n+\t * No source list at all? We're definitely done..\n+\t */\n+\tif (!src)\n+\t\treturn 0;\n+\n+\t/*\n+\t * Does the destination list contain entries with a date\n+\t * before the source list? Definitely _not_ done.\n+\t */\n+\tif (date < src->item->date)\n+\t\treturn SLOP;\n+\n+\t/*\n+\t * Does the source list still have interesting commits in\n+\t * it? Definitely not done..\n+\t */\n+\tif (!everybody_uninteresting(src))\n+\t\treturn SLOP;\n+\n+\t/* Ok, we're closing in.. */\n+\treturn slop-1;\n+}\n+\n static int limit_list(struct rev_info *revs)\n {\n+\tint slop = SLOP;\n+\tunsigned long date = ~0ul;\n \tstruct commit_list *list = revs->commits;\n \tstruct commit_list *newlist = NULL;\n \tstruct commit_list **p = &newlist;\n@@ -579,12 +610,14 @@ static int limit_list(struct rev_info *revs)\n \t\t\treturn -1;\n \t\tif (obj->flags & UNINTERESTING) {\n \t\t\tmark_parents_uninteresting(commit);\n-\t\t\tif (everybody_uninteresting(list))\n+\t\t\tslop = still_interesting(list, date, slop);\n+\t\t\tif (!slop)\n \t\t\t\tbreak;\n \t\t\tcontinue;\n \t\t}\n \t\tif (revs->min_age != -1 && (commit->date > revs->min_age))\n \t\t\tcontinue;\n+\t\tdate = commit->date;\n \t\tp = &commit_list_insert(commit, p)->next;\n \n \t\tshow = show_early_output;\n"},{"id":"67669","messageId":"20080206164303.GA1255@code-monkey.de","threadId":"11840","inReplyTo":"alpine.LFD.1.00.0802051539570.2967@woody.linux-foundation.org","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Tilman Sauerbeck","fromEmail":"tilman@code-monkey.de","sentAt":"2008-02-06T16:43:04Z","receivedAt":"2008-02-06T16:43:04Z","isPatch":false,"sender":{"key":"tilman@code-monkey.de","avatar":null},"body":"Linus Torvalds [2008-02-05 15:59]:\n\nHi guys,\nthanks for looking into this.\n\n> On Tue, 5 Feb 2008, Johannes Schindelin wrote:\n> > > \n> > >  - make commit warn if any parent commit date is in the future from the \n> > >    current commit date (allow a *small* fudge factor here, say 5 minutes).\n> > \n> > 5 minutes seems a little narrow to me.  I think we can even go with 86400 \n> > seconds.\n> \n> Well, notice how I said *warn*. Not abort the commit. Not stop. Just make \n> people very aware of the fact that clocks are skewed.\n> \n> In the case that actually triggered this whole discussion, the problem \n> seems to sadly have been in the original CVS tree (or whatever it was \n> imported from): the project started in 2006, had lots of regular commits \n> up to October 2007, and then suddenly it had a commit that had a date in \n> 2002!\n> \n> [ For those interested in looking at this, the broken commit in that \n>   Tilman's repo was commit 3a7340af2bd57488f832d7070b0ce96c4baa6b54, which \n>   is from October 2002, and which is surrounded by commits from October \n>   2007, so somebody was literally off by five years ]\n\nI'm not sure whether this repository was import from another SCM, but I\ndoubt it. I'm fairly sure that 3a7340af2bd57488f832d7070b0ce96c4baa6b54\nwas created using git commit though. I guess the committer's clock just\nwas a little late at that point.\n\nRegards,\nTilman\n\n-- \nA: Because it messes up the order in which people normally read text.\nQ: Why is top-posting such a bad thing?\nA: Top-posting.\nQ: What is the most annoying thing on usenet and in e-mail?\n"},{"id":"67678","messageId":"alpine.LFD.1.00.0802061220590.2732@xanadu.home","threadId":"11840","inReplyTo":"20080206164303.GA1255@code-monkey.de","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-02-06T17:28:22Z","receivedAt":"2008-02-06T17:28:22Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 6 Feb 2008, Tilman Sauerbeck wrote:\n\n> Linus Torvalds [2008-02-05 15:59]:\n> \n> Hi guys,\n> thanks for looking into this.\n> \n> > On Tue, 5 Feb 2008, Johannes Schindelin wrote:\n> > > > \n> > > >  - make commit warn if any parent commit date is in the future from the \n> > > >    current commit date (allow a *small* fudge factor here, say 5 minutes).\n> > > \n> > > 5 minutes seems a little narrow to me.  I think we can even go with 86400 \n> > > seconds.\n> > \n> > Well, notice how I said *warn*. Not abort the commit. Not stop. Just make \n> > people very aware of the fact that clocks are skewed.\n> > \n> > In the case that actually triggered this whole discussion, the problem \n> > seems to sadly have been in the original CVS tree (or whatever it was \n> > imported from): the project started in 2006, had lots of regular commits \n> > up to October 2007, and then suddenly it had a commit that had a date in \n> > 2002!\n> > \n> > [ For those interested in looking at this, the broken commit in that \n> >   Tilman's repo was commit 3a7340af2bd57488f832d7070b0ce96c4baa6b54, which \n> >   is from October 2002, and which is surrounded by commits from October \n> >   2007, so somebody was literally off by five years ]\n> \n> I'm not sure whether this repository was import from another SCM, but I\n> doubt it. I'm fairly sure that 3a7340af2bd57488f832d7070b0ce96c4baa6b54\n> was created using git commit though. I guess the committer's clock just\n> was a little late at that point.\n\nAnyway, the author's date are not necessarily monotonic.\n\nIf I pick up patches on a mailing list and apply them in a different \norder than they were posted for whatever sensible reason, I expect Git \nto preserve the original date, even if that means my branch will end up \nwith author dates stepping back and forth.  I might even apply a patch \nthat was posted last month, or even last year -- that shouldn't matter.\n\n\nNicolas\n"},{"id":"67679","messageId":"alpine.LFD.1.00.0802060942020.2967@woody.linux-foundation.org","threadId":"11840","inReplyTo":"alpine.LFD.1.00.0802061220590.2732@xanadu.home","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-02-06T17:42:38Z","receivedAt":"2008-02-06T17:42:38Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 6 Feb 2008, Nicolas Pitre wrote:\n> \n> Anyway, the author's date are not necessarily monotonic.\n\nGit never even looks at the author date. Only the commit date matters.\n\n\t\tLinus\n"},{"id":"67680","messageId":"alpine.LFD.1.00.0802061248280.2732@xanadu.home","threadId":"11840","inReplyTo":"alpine.LFD.1.00.0802060942020.2967@woody.linux-foundation.org","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-02-06T17:48:55Z","receivedAt":"2008-02-06T17:48:55Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 6 Feb 2008, Linus Torvalds wrote:\n\n> \n> \n> On Wed, 6 Feb 2008, Nicolas Pitre wrote:\n> > \n> > Anyway, the author's date are not necessarily monotonic.\n> \n> Git never even looks at the author date. Only the commit date matters.\n\nOK good.\n\n\nNicolas\n"},{"id":"67686","messageId":"alpine.LFD.1.00.0802061124550.2967@woody.linux-foundation.org","threadId":"11840","inReplyTo":"20080206164303.GA1255@code-monkey.de","subject":"Re: [RFH] revision limiting sometimes ignored","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-02-06T19:26:50Z","receivedAt":"2008-02-06T19:26:50Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 6 Feb 2008, Tilman Sauerbeck wrote:\n> \n> I'm not sure whether this repository was import from another SCM, but I\n> doubt it. I'm fairly sure that 3a7340af2bd57488f832d7070b0ce96c4baa6b54\n> was created using git commit though. I guess the committer's clock just\n> was a little late at that point.\n\nHeh. I thought it was imported from the outside because the commit log \nlooks so damn nasty. I'm used to projects with good and readable logs, so \nI associate \"native\" git repos with logs that actually explain what's \ngoing on.\n\nBut yeah, it looks like your repo is a perfectly native git repo, it just \nhas really sucky log messages. I guess you can create crap logs in any \nSCM, even if the system is written to encourage good logs ;)\n\n\t\tLinus\n"}]}