{"thread":{"id":"48369","subject":"branch --contains / tag --merged inconsistency","startedAt":"2018-04-27T14:48:03Z","lastAt":"2018-04-30T14:01:53Z","messageCount":4,"participants":["Ferenc Wágner","SZEDER Gábor","Derrick Stolee"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"345950","messageId":"87fu3g67ry.fsf@lant.ki.iif.hu","threadId":"48369","inReplyTo":null,"subject":"branch --contains / tag --merged inconsistency","fromName":"Ferenc Wágner","fromEmail":"wferi@niif.hu","sentAt":"2018-04-27T14:30:57Z","receivedAt":"2018-04-27T14:48:03Z","isPatch":false,"sender":{"key":"wferi@niif.hu","avatar":null},"body":"Hi,\n\nI'm moving the IRC discussion here, because this might be a bug report\nin the end.  So, kindly try these steps (103 MB free space required):\n\n$ git clone https://github.com/ClusterLabs/pacemaker.git && cd pacemaker\n[...]\n$ git branch --contains Pacemaker-0.6.1\n* master\n$ git tag --merged master | fgrep Pacemaker-0.6\nPacemaker-0.6.0\nPacemaker-0.6.2\nPacemaker-0.6.3\nPacemaker-0.6.4\nPacemaker-0.6.5\nPacemaker-0.6.6\n\nNotice that Pacemaker-0.6.1 is missing from the output.  Kind people on\nIRC didn't find a quick explanation, and we all had to go eventually.\nIs this expected behavior?  Reproduced with git 2.11.0 and 2.17.0.\n-- \nThanks,\nFeri\n"},{"id":"345951","messageId":"20180427160344.12724-1-szeder.dev@gmail.com","threadId":"48369","inReplyTo":"87fu3g67ry.fsf@lant.ki.iif.hu","subject":"Re: branch --contains / tag --merged inconsistency","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-04-27T16:03:44Z","receivedAt":"2018-04-27T16:04:15Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Szia Feri,\n\n> I'm moving the IRC discussion here, because this might be a bug report\n> in the end.  So, kindly try these steps (103 MB free space required):\n> \n> $ git clone https://github.com/ClusterLabs/pacemaker.git && cd pacemaker\n> [...]\n> $ git branch --contains Pacemaker-0.6.1\n> * master\n> $ git tag --merged master | fgrep Pacemaker-0.6\n> Pacemaker-0.6.0\n> Pacemaker-0.6.2\n> Pacemaker-0.6.3\n> Pacemaker-0.6.4\n> Pacemaker-0.6.5\n> Pacemaker-0.6.6\n> \n> Notice that Pacemaker-0.6.1 is missing from the output.  Kind people on\n> IRC didn't find a quick explanation, and we all had to go eventually.\n> Is this expected behavior?  Reproduced with git 2.11.0 and 2.17.0.\n\nThe commit pointed to by the tag Pacemaker-0.6.1 and its parent have a\nserious clock skew, i.e. they are a few months older than their parents:\n\n$ git log --format='%h %ad %cd%d%n    %s' --date=short Pacemaker-0.6.1^..47a8ef4c\n47a8ef4ce 2008-02-15 2008-02-15\n    Low: TE: Logging - display the op's magic field for unexpected and foreign events\nb9cfcd6b4 2007-12-10 2007-12-10 (tag: Pacemaker-0.6.2)\n    haresources2cib.py: set default-action-timeout to the default (20s)\n52e7793e0 2007-12-10 2007-12-10\n    haresources2cib.py: update ra parameters lists\ndea277271 2008-02-14 2008-02-14\n    Medium: Build: Turn on snmp support in rpm packages (patch from MATSUDA, Daiki)\nf418742fe 2008-02-14 2008-02-14\n    Low: Build: Update the .spec file with the one used by build service\nccfa716a5 2008-02-14 2008-02-14\n    Medium: SNMP: Allow the snmp subagent to be built (patch from MATSUDA, Daiki)\n50f0ade2d 2008-02-14 2008-02-14\n    Low: Build: Update last release number\n90f11667f 2008-02-14 2008-02-14\n    Medium: Tools: Make sure the autoconf variables in haresources2cib are expanded\n9d2383c46 2008-02-11 2008-02-11 (tag: Pacemaker-0.6.1)\n    High: cib: Ensure the archived file hits the disk before returning\n\n(branch|tag|describe|...) (--contains|--merged) use the commit timestamp\ninformation as a heuristic to avoid traversing parts of the history,\nwhich makes these operations, especially on big histories, an order of\nmagnitude or two faster.  Yeah, commit timestamps can't always be\ntrusted, but skewed commits are rare, and skewed commits with this much\nskew are even rarer.\n\nI'm not sure how (or if it's at all possible) to turn off this\ntimestamp-based optimisation.\n\nFWIW, much work is being done on a cached commit graph including commit\ngeneration numbers, which will solve this issue both correctly and more\nefficiently.  Perhaps it will already be included in the next release.\n\n"},{"id":"346023","messageId":"877eor6e2i.fsf@lant.ki.iif.hu","threadId":"48369","inReplyTo":"20180427160344.12724-1-szeder.dev@gmail.com","subject":"Re: branch --contains / tag --merged inconsistency","fromName":"Ferenc Wágner","fromEmail":"wferi@niif.hu","sentAt":"2018-04-28T06:27:17Z","receivedAt":"2018-04-28T06:27:28Z","isPatch":false,"sender":{"key":"wferi@niif.hu","avatar":null},"body":"SZEDER Gábor <szeder.dev@gmail.com> writes:\n\n> The commit pointed to by the tag Pacemaker-0.6.1 and its parent have a\n> serious clock skew, i.e. they are a few months older than their parents:\n> [...]\n> (branch|tag|describe|...) (--contains|--merged) use the commit timestamp\n> information as a heuristic to avoid traversing parts of the history,\n> which makes these operations, especially on big histories, an order of\n> magnitude or two faster.  Yeah, commit timestamps can't always be\n> trusted, but skewed commits are rare, and skewed commits with this much\n> skew are even rarer.\n\nSzia Gábor,\n\nThank you very much for the explanation!  Good to know there's no\nserious problem or misunderstanding here, but a conscious algorithmic\nchoice being fooled by clock skew.  I wonder if there's a way such clock\nskew can appear without actual committer clocks being off by months...\n\n> FWIW, much work is being done on a cached commit graph including commit\n> generation numbers, which will solve this issue both correctly and more\n> efficiently.  Perhaps it will already be included in the next release.\n\nWonderful news, thanks for sharing it!\n-- \nRegards,\nFeri\n"},{"id":"346147","messageId":"f73a956d-2782-11b3-f84b-67ce1e0ae047@gmail.com","threadId":"48369","inReplyTo":"20180427160344.12724-1-szeder.dev@gmail.com","subject":"Re: branch --contains / tag --merged inconsistency","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-04-30T14:01:47Z","receivedAt":"2018-04-30T14:01:53Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 4/27/2018 12:03 PM, SZEDER Gábor wrote:\n> Szia Feri,\n>\n>> I'm moving the IRC discussion here, because this might be a bug report\n>> in the end.  So, kindly try these steps (103 MB free space required):\n>>\n>> $ git clone https://github.com/ClusterLabs/pacemaker.git && cd pacemaker\n>> [...]\n>> $ git branch --contains Pacemaker-0.6.1\n>> * master\n>> $ git tag --merged master | fgrep Pacemaker-0.6\n>> Pacemaker-0.6.0\n>> Pacemaker-0.6.2\n>> Pacemaker-0.6.3\n>> Pacemaker-0.6.4\n>> Pacemaker-0.6.5\n>> Pacemaker-0.6.6\n>>\n>> Notice that Pacemaker-0.6.1 is missing from the output.  Kind people on\n>> IRC didn't find a quick explanation, and we all had to go eventually.\n>> Is this expected behavior?  Reproduced with git 2.11.0 and 2.17.0.\n> The commit pointed to by the tag Pacemaker-0.6.1 and its parent have a\n> serious clock skew, i.e. they are a few months older than their parents:\n>\n> $ git log --format='%h %ad %cd%d%n    %s' --date=short Pacemaker-0.6.1^..47a8ef4c\n> 47a8ef4ce 2008-02-15 2008-02-15\n>      Low: TE: Logging - display the op's magic field for unexpected and foreign events\n> b9cfcd6b4 2007-12-10 2007-12-10 (tag: Pacemaker-0.6.2)\n>      haresources2cib.py: set default-action-timeout to the default (20s)\n> 52e7793e0 2007-12-10 2007-12-10\n>      haresources2cib.py: update ra parameters lists\n> dea277271 2008-02-14 2008-02-14\n>      Medium: Build: Turn on snmp support in rpm packages (patch from MATSUDA, Daiki)\n> f418742fe 2008-02-14 2008-02-14\n>      Low: Build: Update the .spec file with the one used by build service\n> ccfa716a5 2008-02-14 2008-02-14\n>      Medium: SNMP: Allow the snmp subagent to be built (patch from MATSUDA, Daiki)\n> 50f0ade2d 2008-02-14 2008-02-14\n>      Low: Build: Update last release number\n> 90f11667f 2008-02-14 2008-02-14\n>      Medium: Tools: Make sure the autoconf variables in haresources2cib are expanded\n> 9d2383c46 2008-02-11 2008-02-11 (tag: Pacemaker-0.6.1)\n>      High: cib: Ensure the archived file hits the disk before returning\n>\n> (branch|tag|describe|...) (--contains|--merged) use the commit timestamp\n> information as a heuristic to avoid traversing parts of the history,\n> which makes these operations, especially on big histories, an order of\n> magnitude or two faster.  Yeah, commit timestamps can't always be\n> trusted, but skewed commits are rare, and skewed commits with this much\n> skew are even rarer.\n>\n> I'm not sure how (or if it's at all possible) to turn off this\n> timestamp-based optimisation.\n\nThis is actually a bit more complicated. The \"--merged\" check in 'git \ntag' uses a different mechanism to detect which tags are reachable. It \nuses a revision walk starting at the \"merge commit\" (master in your \ncase) and all tags with the \"limited\" option (to ensure the walk happens \nduring prepare_revision_walk()) but marks the merge commit as \nUNINTERESTING. The limit_list() method stops when all commits are marked \nUNINTERESTING - minus some \"slop\" related to the commits that start the \nwalk.\n\nOne important note: the set of tags is important here. If you add a new \ntag to the root commit (git tag MyTag a2d71961f) then the walk succeeds \nby ensuring it walks until MyTag. This gets around the clock skew issue. \nThere may be other more-recent tags with a clock-skew issue, but since \nPacemaker-0.6.0 is the oldest tag, that requires the walk to continue \nuntil at least that date.\n\nThe commit-walk machinery in revision.c is rather complicated, and is \nused for a lot of different reasons, such as \"git log\" and this \napplication in \"git tag\". It is on my list to refactor this code to use \nthe commit-graph and generation numbers, but as we can see by this \nexample, it is not easy to tease out what is happening in the code.\n\nIn a world where generation numbers are expected to be available, we \ncould rewrite do_merge_filter() in ref-filter.c to call into \npaint_down_to_common() in commit.c using the new \"min_generation\" \nmarker. By assigning the tags to be in the \"twos\" list and the merge \ncommit in the \"one\" commit, we can check if the tags have the PARENT1 \nflag after the walk in paint_down_to_common(). Due to the static nature \nof paint_down_to_common(), we will likely want to abstract this into an \nexternal method in commit.c, say can_reach_many(struct commit *from, \nstruct commit_list *to).\n\n> FWIW, much work is being done on a cached commit graph including commit\n> generation numbers, which will solve this issue both correctly and more\n> efficiently.  Perhaps it will already be included in the next release.\n\nThe work in ds/generation-numbers is focused on the \"git tag --contains\" \nmethod, which does return correctly here (it is the reverse of the \n--merged condition):\n\nWhich tags can reach Pacemaker-0.6.1?\n\n$ git tag --contains Pacemaker-0.6.1\n(returns a big list)\n\nThis is the actual reverse lookup (which branches contain this tag?)\n\n$ git branch --contains Pacemaker-0.6.1 | grep master\n* master\n\nThese commands work despite clock skew. The commit-graph feature makes \nthem faster.\n\nThanks,\n-Stolee\n\n"}]}