{"thread":{"id":"37046","subject":"Race condition in git push --mirror can cause silent ref rewinding","startedAt":"2014-07-02T21:10:13Z","lastAt":"2014-07-14T04:09:16Z","messageCount":4,"participants":["Alex Vandiver","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"245356","messageId":"53B47535.3020101@chmrr.net","threadId":"37046","inReplyTo":null,"subject":"Race condition in git push --mirror can cause silent ref rewinding","fromName":"Alex Vandiver","fromEmail":"alex@chmrr.net","sentAt":"2014-07-02T21:10:13Z","receivedAt":"2014-07-02T21:10:13Z","isPatch":false,"sender":{"key":"alex@chmrr.net","avatar":"https://avatars.githubusercontent.com/u/28347?v=4"},"body":"Heya,\n\nWe recently ran into a particularly troubling race condition, discovered\nin git 2.0.0. The setup for it is as follows:\n\nThe repository is a bare repository, which developers push to via ssh;\nit mirrors its changes out onto github. In its config:\n\n [remote \"github\"]\n url = git@github.com:bestpractical/rt.git\n fetch = +refs/*:refs/*\n mirror = yes\n\nIt has a post-receive hook which does:\n\n sudo -u git -H /usr/bin/git push github\n\n\nWe recently saw a situation where a push of a new branch caused a\nsimultaneous update of a different branch (by a different user) to be\nrewound. From the reflog of the created branch (4.2/html-gumbo-loading):\n\n 0000000000000000000000000000000000000000\n1aefd600fcbb5ded14376f77d77a14758668fb39 Wallace Reis\n 1404326443 -0400 push\n\nAnd the updated branch (4.2-trunk), which was rewound:\n\n 44dc8ad0e4603e3f674b7c00deacc122ca52707a\n1e743b6225d502ad1a265929fb873f4c0bf4f8a5 Kevin Falcone\n 1404326446 -0400 push\n 1e743b6225d502ad1a265929fb873f4c0bf4f8a5\n44dc8ad0e4603e3f674b7c00deacc122ca52707a git \n1404326446 -0400 update by push\n\nIt is my belief that this comes because the \"--mirror\" argument causes\nthe local refs to be treated as tracking refs -- and thus updates all of\nthem during the push. I believe the race condition is thus:\n\n 1. User A starts a push --mirror; git records the values of the refs\n\n 2. User B updates a ref, commit mail goes out, etc\n\n 3. User A's push completes, updates \"tracking\" branch to value at (1).\n\n\nNeedless to say, silently losing commits which appeared for all purposes\nto be pushed successfully (neither User A nor User B sees anything out\nof the ordinary) is extremely troubling.\n\n - Alex\n"},{"id":"245358","messageId":"xmqqfvijflnr.fsf@gitster.dls.corp.google.com","threadId":"37046","inReplyTo":"53B47535.3020101@chmrr.net","subject":"Re: Race condition in git push --mirror can cause silent ref rewinding","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-02T22:20:08Z","receivedAt":"2014-07-02T22:20:08Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Vandiver writes:\n\n> [remote \"github\"]\n> url = git@github.com:bestpractical/rt.git\n> fetch = +refs/*:refs/*\n> mirror = yes\n\n\"git push github master^:master\" must stay a usable way to update\nthe published repository to an arbitrary commit, so \"if set to\nmirror, do not pretend that a fetch in reverse has happened during\n'git push'\" will not be a solution to this issue.\n\nPerhaps removing remote.github.fetch would be one sane way forward.\nOtherwise, even if your \"git push\" does not pretend to immediately\nfetch from there (i.e. even if the reported behaviour was a bug,\nwithout doing anything to trigger it) somebody running \"git fetch\"\nin this repository can destroy what other person pushes into this\nrepository at the same time exactly the same way, I would think.\n"},{"id":"245364","messageId":"53B49173.4020001@chmrr.net","threadId":"37046","inReplyTo":"xmqqfvijflnr.fsf@gitster.dls.corp.google.com","subject":"Re: Race condition in git push --mirror can cause silent ref rewinding","fromName":"Alex Vandiver","fromEmail":"alex@chmrr.net","sentAt":"2014-07-02T23:10:43Z","receivedAt":"2014-07-02T23:10:43Z","isPatch":false,"sender":{"key":"alex@chmrr.net","avatar":"https://avatars.githubusercontent.com/u/28347?v=4"},"body":"On 07/02/2014 06:20 PM, Junio C Hamano wrote:\n> Alex Vandiver writes:\n> \n>> [remote \"github\"]\n>> url = git@github.com:bestpractical/rt.git\n>> fetch = +refs/*:refs/*\n>> mirror = yes\n> \n> \"git push github master^:master\" must stay a usable way to update\n> the published repository to an arbitrary commit, so \"if set to\n> mirror, do not pretend that a fetch in reverse has happened during\n> 'git push'\" will not be a solution to this issue.\n\nHm? I'm confused, as mirror isn't compatible with refspecs:\n\n$ git push github master^:master\nerror: --mirror can't be combined with refspecs\n\n> Perhaps removing remote.github.fetch would be one sane way forward.\n\nAhh -- I see. The repository predates a9f5a355, which split `git remote\nadd --mirror` into `--mirror=push` and `--mirror=fetch`, because of more\nor less this exact problem. Of course, there is nothing much that can\nbe done for existing repositories in this situation as it's a legitimate\ncombination.\n - Alex\n"},{"id":"245964","messageId":"53C357EC.8060300@chmrr.net","threadId":"37046","inReplyTo":"53B49173.4020001@chmrr.net","subject":"Re: Race condition in git push --mirror can cause silent ref rewinding","fromName":"Alex Vandiver","fromEmail":"alex@chmrr.net","sentAt":"2014-07-14T04:09:16Z","receivedAt":"2014-07-14T04:09:16Z","isPatch":false,"sender":{"key":"alex@chmrr.net","avatar":"https://avatars.githubusercontent.com/u/28347?v=4"},"body":"On 07/02/2014 07:10 PM, Alex Vandiver wrote:\n> On 07/02/2014 06:20 PM, Junio C Hamano wrote:\n>> Alex Vandiver writes:\n>>\n>>> [remote \"github\"]\n>>> url = git@github.com:bestpractical/rt.git\n>>> fetch = +refs/*:refs/*\n>>> mirror = yes\n>>\n>> \"git push github master^:master\" must stay a usable way to update\n>> the published repository to an arbitrary commit, so \"if set to\n>> mirror, do not pretend that a fetch in reverse has happened during\n>> 'git push'\" will not be a solution to this issue.\n> \n> Hm? I'm confused, as mirror isn't compatible with refspecs:\n> \n> $ git push github master^:master\n> error: --mirror can't be combined with refspecs\n\nJust following up on this -- can you clarify your statement about \"git\npush github master^:master\" in light of the fact that --mirror already\ndisallows such?\n - Alex\n"}]}