{"thread":{"id":"51934","subject":"Possible bug in git describe, additional commits differs when cloned with --depth","startedAt":"2019-09-27T09:51:21Z","lastAt":"2019-10-08T12:32:03Z","messageCount":3,"participants":["Anders Janmyr","SZEDER Gábor"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"383057","messageId":"CA+UvoT7EBa6S6Fi7scYTo8mYKx=n1e=sPvxy5TRP3vG7gw97Xw@mail.gmail.com","threadId":"51934","inReplyTo":null,"subject":"Possible bug in git describe, additional commits differs when cloned with --depth","fromName":"Anders Janmyr","fromEmail":"anders@janmyr.com","sentAt":"2019-09-27T09:51:07Z","receivedAt":"2019-09-27T09:51:21Z","isPatch":false,"sender":{"key":"anders@janmyr.com","avatar":null},"body":"Hi,\n\nI'm not sure if this is a bug or not but `git describe` gives\ndifferent results when the repo has been cloned with `--depth` or not.\n\nIn the example below from the git repository the number of additional\ncommits since the\nlast tag differs 256 vs. 265.\n\n```\n$ git clone https://github.com/git/git\n$ cd git/\n$ git describe\nv2.23.0-256-g4c86140027\n$ git rev-list -n 1 HEAD\n4c86140027f4a0d2caaa3ab4bd8bfc5ce3c11c8a\n\n\n$ git clone --depth=50 https://github.com/git/git git-depth\n$ cd git-depth/\n$ git describe\nv2.23.0-265-g4c861400\n$ git rev-list -n 1 HEAD\n4c86140027f4a0d2caaa3ab4bd8bfc5ce3c11c8a\n```\n\nIn the example above the first version also gives additional digits\nfor the SHA,\ng4c86140027 vs. g4c861400, but that is not always the case.\n\nI came upon this discrepancy when using the value from `git describe`\nin a build pipeline\nwhere the system optimized the build by cloning the repo using `--depth`.\n\nKind regards\n--\nhttp://anders.janmyr.com/\n@andersjanmyr\n+46 733-99 03 60\n"},{"id":"383060","messageId":"20190927120201.GM2637@szeder.dev","threadId":"51934","inReplyTo":"CA+UvoT7EBa6S6Fi7scYTo8mYKx=n1e=sPvxy5TRP3vG7gw97Xw@mail.gmail.com","subject":"Re: Possible bug in git describe, additional commits differs when cloned with --depth","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-09-27T12:02:01Z","receivedAt":"2019-09-27T12:02:08Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Sep 27, 2019 at 11:51:07AM +0200, Anders Janmyr wrote:\n> Hi,\n> \n> I'm not sure if this is a bug or not but `git describe` gives\n> different results when the repo has been cloned with `--depth` or not.\n> \n> In the example below from the git repository the number of additional\n> commits since the\n> last tag differs 256 vs. 265.\n> \n> ```\n> $ git clone https://github.com/git/git\n> $ cd git/\n> $ git describe\n> v2.23.0-256-g4c86140027\n> $ git rev-list -n 1 HEAD\n> 4c86140027f4a0d2caaa3ab4bd8bfc5ce3c11c8a\n> \n> \n> $ git clone --depth=50 https://github.com/git/git git-depth\n> $ cd git-depth/\n> $ git describe\n> v2.23.0-265-g4c861400\n> $ git rev-list -n 1 HEAD\n> 4c86140027f4a0d2caaa3ab4bd8bfc5ce3c11c8a\n> ```\n\nI don't think this is a bug, but rather an inherent limitation of\nshallow histories with lots of merges, and it affects not only 'git\ndescribe', but any limited history traversal.\n\nIn the Git project new features are developed on their dedicated\nbranches, which are then eventually merged to 'master'.  Alas, we make\nmistakes, and sometimes we realize that a feature was buggy after it\nhas already been merged to 'master'.  In such cases the bugfix is\noften applied not on top of 'master', but on top of the feature\nbranch, so it can be merged to maintenance releases as well.\n\nThis results in a history like this:\n\n  M2     Merge the bugfix to 'master'\n  |  \\\n  |   \\\n v2.0  |\n  |    o  Bugfix for new feature\n CO2   |\n  |    |\n  M1  /  Merge 'new feature' to 'master'\n  | \\/\n  |  o   new feature\n  |  |\n  |  o\n  |  |\n  | CO1\n  |  |    \n  | /\n v1.0\n\nDescribing M2 in a full repository results in something like\nv2.0-2-gdeadbeef, because M2 contains only two commits that aren't in\nv2.0, (M2 and the bugfix).\n\nNow let's suppose that in a shallow repo the given '--depth=<N>'\nresulted in a cutoff at commits CO1 and CO2, meaning that the shallow\nrepo does not include commits M1 and v1.0.  Consequently, Git can't\npossibly see that the commits implementing the new feature are already\nmerged and thus reachable from v2.0, so it will count those commits as\nwell, resulting in v2.0-5-gabcdef.\n\nThere is a lot more going on in the Git repository, so it's not as\nsimple as above.  Case in point is the merge d1a251a1fa (Merge branch\n'en/checkout-mismerge-fix', 2019-09-09), which merges a fix to a bug\nthat happened before v2.22.0-rc0, but that bug was not introduced in\nthe feature branch, but while merging that branch to 'master'.  The\nresult is still the same, though: since there are a lot of commits on\nthe ancestry path between that buggy merge and v2.23.0, '--depth=50'\ndoesn't include them all in the shallow clone, so Git can't possibly\nknow that that merge is reachable from v2.23.0.\n\n  # same in both the full and shallow repos\n  $ git log --oneline v2.23.0..d1a251a1fa^ |wc -l\n  57\n\n  # in the full repo\n  $ git log --oneline v2.23.0..d1a251a1fa |wc -l\n  59\n\n  # in the shallow repo\n  $ git log --oneline v2.23.0..d1a251a1fa |wc -l\n  132\n\n> In the example above the first version also gives additional digits\n> for the SHA,\n> g4c86140027 vs. g4c861400, but that is not always the case.\n\nGit shows as many hexdigits as needed to form a unique object name\nwith a few additional digits worth of safety margin.  There are a lot\nmore objects in the full repository than in the shallow clone, which\nmeans more hexdigits in the abbreviated object name.\n\n\nThanks for letting us know.  I think this is worth a warning in\nthe documentation of 'git clone --depth'.\n\n"},{"id":"383678","messageId":"20191008123156.GG11529@szeder.dev","threadId":"51934","inReplyTo":"20190927120201.GM2637@szeder.dev","subject":"[BUG] a commit date-related bug in 'git describe' [was: Re: Possible bug in git describe, additional commits differs when cloned with --depth]","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-10-08T12:31:56Z","receivedAt":"2019-10-08T12:32:03Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Sep 27, 2019 at 02:02:01PM +0200, SZEDER Gábor wrote:\n> On Fri, Sep 27, 2019 at 11:51:07AM +0200, Anders Janmyr wrote:\n> > I'm not sure if this is a bug or not but `git describe` gives\n> > different results when the repo has been cloned with `--depth` or not.\n> > \n> > In the example below from the git repository the number of additional\n> > commits since the\n> > last tag differs 256 vs. 265.\n> > \n> > ```\n> > $ git clone https://github.com/git/git\n> > $ cd git/\n> > $ git describe\n> > v2.23.0-256-g4c86140027\n> > $ git rev-list -n 1 HEAD\n> > 4c86140027f4a0d2caaa3ab4bd8bfc5ce3c11c8a\n> > \n> > \n> > $ git clone --depth=50 https://github.com/git/git git-depth\n> > $ cd git-depth/\n> > $ git describe\n> > v2.23.0-265-g4c861400\n> > $ git rev-list -n 1 HEAD\n> > 4c86140027f4a0d2caaa3ab4bd8bfc5ce3c11c8a\n> > ```\n> \n> I don't think this is a bug, but rather an inherent limitation of\n> shallow histories with lots of merges, and it affects not only 'git\n> describe', but any limited history traversal.\n> \n> In the Git project new features are developed on their dedicated\n> branches, which are then eventually merged to 'master'.  Alas, we make\n> mistakes, and sometimes we realize that a feature was buggy after it\n> has already been merged to 'master'.  In such cases the bugfix is\n> often applied not on top of 'master', but on top of the feature\n> branch, so it can be merged to maintenance releases as well.\n> \n> This results in a history like this:\n> \n>   M2     Merge the bugfix to 'master'\n>   |  \\\n>   |   \\\n>  v2.0  |\n>   |    o  Bugfix for new feature\n>  CO2   |\n>   |    |\n>   M1  /  Merge 'new feature' to 'master'\n>   | \\/\n>   |  o   new feature\n>   |  |\n>   |  o\n>   |  |\n>   | CO1\n>   |  |    \n>   | /\n>  v1.0\n> \n> Describing M2 in a full repository results in something like\n> v2.0-2-gdeadbeef, because M2 contains only two commits that aren't in\n> v2.0, (M2 and the bugfix).\n> \n> Now let's suppose that in a shallow repo the given '--depth=<N>'\n> resulted in a cutoff at commits CO1 and CO2, meaning that the shallow\n> repo does not include commits M1 and v1.0.  Consequently, Git can't\n> possibly see that the commits implementing the new feature are already\n> merged and thus reachable from v2.0, so it will count those commits as\n> well, resulting in v2.0-5-gabcdef.\n> \n> There is a lot more going on in the Git repository, so it's not as\n> simple as above.  Case in point is the merge d1a251a1fa (Merge branch\n> 'en/checkout-mismerge-fix', 2019-09-09), which merges a fix to a bug\n> that happened before v2.22.0-rc0, but that bug was not introduced in\n> the feature branch, but while merging that branch to 'master'.  The\n> result is still the same, though: since there are a lot of commits on\n> the ancestry path between that buggy merge and v2.23.0, '--depth=50'\n> doesn't include them all in the shallow clone, so Git can't possibly\n> know that that merge is reachable from v2.23.0.\n> \n>   # same in both the full and shallow repos\n>   $ git log --oneline v2.23.0..d1a251a1fa^ |wc -l\n>   57\n> \n>   # in the full repo\n>   $ git log --oneline v2.23.0..d1a251a1fa |wc -l\n>   59\n> \n>   # in the shallow repo\n>   $ git log --oneline v2.23.0..d1a251a1fa |wc -l\n>   132\n\nOk, so above I explained why 'git describe' is not buggy in a shallow\nclone, but rather this is the best it can do under the circumstances.\n\nUnfortunately (or but of course?), 'git describe' is in fact buggy,\nbut in a different way, and curiously the above mentioned commit\nd1a251a1fa shows why.  Bear with me.\n\nFirst, the desired behavior according to the manpage of 'git\ndescribe':\n\n  ... it suffixes the tag name with the number of additional commits\n  on top of the tagged object ...\n\nThe Examples section in the manpage is even more explicit:\n\n  The number of additional commits is the number of commits which\n  would be displayed by \"git log <tag>..<described-commit>\".\n\nNow let's have a look at that problematic commit:\n\n  $ git describe d1a251a1fa\n  v2.23.0-135-gd1a251a1fa\n  $ git log --oneline v2.23.0..d1a251a1fa | wc -l\n  59\n\nUh-oh, 59 != 135.\n\nThis is happening because:\n\n  - Git is too fast ;) and the committer date has a one second\n    granularity, so scripts can easily create subsequent commits with\n    the same committer date.  Case in point are the two subsequent\n    merge commits f3c19f85c5 and 4a3ed2bec6 at the bottom of this\n    simplified history snippet (kind of a hand-edited 'git log --graph\n    --format=\"%h %cd %s\"'):\n\n    *   d1a251a1fa 2019-09-09 12:26:36 -0700 Merge branch 'en/checkout-mismerge-fix'\n    |\\\n    * | ... a big chunk of history simplified away ...\n    | * acb7da05ac 2019-08-16 09:58:00 -0700 checkout: remove duplicate code\n    * | a5e4be2f68 2019-04-25 16:41:15 +0900 Merge branch 'ab/commit-graph-fixes'\n    * | f3c19f85c5 2019-04-25 16:41:14 +0900 Merge branch 'ab/gc-reflog'\n    |/\n    *   4a3ed2bec6 2019-04-25 16:41:14 +0900 Merge branch 'nd/checkout-m'\n\n  - 'git describe' implements its own history traversal: it iterates\n    over all parents of a commit, adds any yet unseen parents to a\n    commit list ordered by date, and then continues with the first,\n    i.e. most recent commit from that list.  While doing so it uses\n    several bits in 'commit->object.flags' to track reachability\n    information from several candidate tags at once, and copies these\n    flags from each commit to its parents; this is important to\n    compute the correct number of additional commits.  Another\n    important thing is the implementation detail that\n    commit_list_insert_by_date() inserts a new commit after all other\n    commits with the same date that are already in the list.\n\nSo while traversing the above piece of history in date order\nacb7da05ac is visited before a5e4be2f68, which is OK in itself as they\nare on parallel running branches.  However, it means that 4a3ed2bec6\nis added to the commit list before f3c19f85c5, which in turn means\nthat 4a3ed2bec6 is visited before f3c19f85c5, even though the former\nis the parent of the latter.  Consequently, those reachability flags\nthat are important to compute the number of additional commits were\ncopied from 4a3ed2bec6 to its parents before those flags could be\nupdated with the values from f3c19f85c5, which makes many of the\nancestor commits incorrectly appear as unreachable from the candidate\ntags.  Ultimately this causes that 135 != 59 difference shown above.\n\nThere are several thousands commits in git.git where 'git describe'\nreports incorrect number of additional commits.\n\nNow, I suppose that this particular case could be fixed by modifying\ncommit_list_insert_by_date() (or use a new variant of it) to insert a\nnew commit before all other commits with the same date that are\nalready in the list.  However, I also suppose that a clock skew could\nlead to similar incorrect additional commit counts, and merely\ntweaking the details of commit_list_insert_by_date() could not help\nwith that.\n\nI think the proper way to fix this issue would be to make 'git\ndescribe' traverse the history in topographical order.  Alas, I'm\nafraid this would result in a noticable performance penalty on big\nhistories without a commit graph.\n\n"}]}