{"thread":{"id":"34329","subject":"[RFD] Making \"git push [--force/--delete]\" safer?","startedAt":"2013-07-02T20:57:53Z","lastAt":"2013-07-17T17:09:42Z","messageCount":62,"participants":["Junio C Hamano","Johan Herland","Michael Haggerty","Jonathan del Strother","Johannes Sixt","Aaron Schrab","Marc Branchaud","John Keeping","Jonathan Nieder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"222403","messageId":"7vfvvwk7ce.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":null,"subject":"[RFD] Making \"git push [--force/--delete]\" safer?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-02T20:57:53Z","receivedAt":"2013-07-02T20:57:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Consider these two scenarios.\n\n1. If you are collaborating with others and you have arranged with\n the participants to rewind a shared branch, you would do\n something like this:\n\n $ git fetch origin branch\n ... fetch everything so that we won't lose anything ...\n $ git checkout -b rebase-work FETCH_HEAD\n $ git rebase -i\n $ git push origin +HEAD:branch\n\n The last step has to be \"--force\", as you are deliberately\n pushing a history that does not fast-forward.\n\n2. If you know a branch you pushed over there has now been fully\n merged to the \"trunk\" and want to remove it, you would do this:\n\n $ git fetch origin branch:tmp-branch trunk:tmp-trunk\n ... double check to make sure branch is fully merged ...\n $ git merge-base --is-ancestor tmp-branch tmp-trunk; echo $?\n 0\n ... good, branch is part of trunk ...\n $ git push origin --delete branch\n\n The last step would delete the branch, but you made sure it\n has been merged to the trunk, not to lose anybody's work.\n\nBut in either of these cases, if something happens at 'origin' to\nthe branch you are forcing or deleting since you fetched to inspect\nit, you may end up losing other people's work.\n\n - In the first scenario, somebody who is unaware of the decision to\n rewind and rebuild the branch may attempt to push to the branch\n between the time you fetched to rebase it and the time you pushed\n to replace it with the result of the rebasing.\n\n - In the second scenario, somebody may have pushed a new change to\n the branch since you fetched to inspect.\n\nWe can make these pushes safer by optionally allowing the user to\ntell \"git push\" this:\n\n I am forcing/deleting, based on the assumption that the\n value of 'branch' is still at this object. If that\n assumption no longer holds, i.e. if something happened to\n the branch since I started preparing for this push, please\n do not proceed and fail this push.\n\nWith such a mechanism, the first example would say \"'branch' must be\nat $(git rev-parse --verify FETCH_HEAD)\", and the second example\nwould say \"'branch' must be at $(git rev-parse --verify tmp-branch)\".\n\nThe network protocol of \"git push\" conveys enough information to\nmake this possible. An early part of the exchange goes like this:\n\n receiver -> sender\n list of \n sender -> receiver\n list of \n sender -> receiver\n packfile payload\n ...\n\nWhen the \"git push\" at the last step of the above two examples\ncontact the other end, we would immediately know what the current\nvalue of 'branch' is. We can locally fail the command if the value\nis different from what we expect.\n\nNow at the syntax level, I see three possibilities to let the user\nexpress this new constraints:\n\n (1) Extend \"refspec\" syntax to \"src:dst:expect\", e.g.\n\n $ git push there HEAD:branch:deadbabecafe\n\n would say \"update 'branch' with the object at my HEAD, only if\n the current value of 'branch' is deadbabecafe\".\n\n An reservation I have against this syntax is that it does not\n mesh well with the \"update the upstream of the currrent\n branch\" and other modes, and instead you have to always spell\n three components out. But perhaps requiring the precondition\n is rare enough that it may be acceptable.\n\n (2) Add --compare-and-swap=dst:expect parameters, e.g.\n\n $ git push --cas=master:deadbabecafe --cas=next:cafebabe \":\"\n\n This removes the \"reservation\" I expressed against (1) above\n (i.e. we are doing a \"matching\" push in this example, but we\n will fail if 'master' and 'next' are not pointing at the\n expected objects).\n\n (3) Add a mechanism to call a custom validation script after \"git\n push\" reads the list of tuples,\n but before responding with the proposed update. The script\n would be fed a list of tuples (i.e. what the sender _would_ tell the\n receiving end if there weren't this mechanism), and can tell\n \"git push\" to fail with its exit status.\n\n This would be the most flexible in that the validation does\n not have to be limited to \"the ref must be still pointing at\n the object we expect\" (aka compare-and-swap); the script could\n implement other semantics (e.g. \"the ref must be pointing at\n the object or its ancestor\").\n\n But it may be cumbersome to use and the added flexibility may\n not be worth it.\n\n - The way to specify the validation script could be an\n in-repository hook but then there will need a way to pass\n additional per-invocation parameters (in the earlier sample\n scenarios, values of FETCH_HEAD and tmp-branch).\n\n - Or it could be a \"--validate-script=check.sh\" option, and it\n is up to the caller how to tailor that check.sh script\n customized for this particular invocation (i.e. embedding\n the values of FETCH_HEAD and tmp-branch in the script in the\n earlier sample scenarios).\n\nI am inclined to say, if we were to do this, we should do (2) among\nthe above three.\n\nBut of course, others may have better ideas ;-).\n"},{"id":"222417","messageId":"CALKQrgenpqKUxOZ+p79NsaQD9M2-q4h93ZqN0oencVo-QZF=zg@mail.gmail.com","threadId":"34329","inReplyTo":"7vfvvwk7ce.fsf@alter.siamese.dyndns.org","subject":"Re: [RFD] Making \"git push [--force/--delete]\" safer?","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-07-02T22:55:34Z","receivedAt":"2013-07-02T22:55:34Z","isPatch":false,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Tue, Jul 2, 2013 at 10:57 PM, Junio C Hamano wrote:\n\n[...]\n\n> (2) Add --compare-and-swap=dst:expect parameters, e.g.\n>\n> $ git push --cas=master:deadbabecafe --cas=next:cafebabe \":\"\n>\n> This removes the \"reservation\" I expressed against (1) above\n> (i.e. we are doing a \"matching\" push in this example, but we\n> will fail if 'master' and 'next' are not pointing at the\n> expected objects).\n\nI still think this is too long/verbose for the average user to\nremember, and type out. Also, I don't like the name, as it is too\n'technical', and describes the nature of the implementation (i.e. the\n\"how\") rather than the purpose of using it (i.e. the \"why\" or \"what\").\n\n> (3) Add a mechanism to call a custom validation script after \"git\n> push\" reads the list of tuples,\n> but before responding with the proposed update. The script\n> would be fed a list of name, refname> tuples (i.e. what the sender _would_ tell the\n> receiving end if there weren't this mechanism), and can tell\n> \"git push\" to fail with its exit status.\n>\n> This would be the most flexible in that the validation does\n> not have to be limited to \"the ref must be still pointing at\n> the object we expect\" (aka compare-and-swap); the script could\n> implement other semantics (e.g. \"the ref must be pointing at\n> the object or its ancestor\").\n\nWith this, I guess --dry-run could be reformulated as a trivial\nvalidation script that always returns a non-zero exit code (although\nit should still cause 'push' to return zero).\n\n[...]\n\n> I am inclined to say, if we were to do this, we should do (2) among\n> the above three.\n>\n> But of course, others may have better ideas ;-).\n\nI assume that in most cases the expected value of the remote ref would\nequal the current value of the corresponding remote-tracking ref in\nthe user's repo, so why not use that as the default expected value?\nE.g.:\n\n $ git config push.default simple\n $ git checkout -b foo -t origin/foo\n # prepare non-ff update\n $ git push --force-if-expected\n # the above validates foo @ origin != origin/foo before pushing\n\nAnd if the users expects a different value, (s)he can pass that to the\nsame option:\n\n $ git push --force-if-expected=refs/original/foo my_remote HEAD:foo\n # the above fails if foo @ origin != refs/original/foo\n\nThe option name probably needs a little work, but as long as it\nproperly communicates the user's _intent_ I'm fine with whatever we\ncall it.\n\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"222427","messageId":"CALKQrgdovWTd50LVDnNR+BhurWgSCKkhr88wCo01VZF3sd5PNg@mail.gmail.com","threadId":"34329","inReplyTo":"CALKQrgenpqKUxOZ+p79NsaQD9M2-q4h93ZqN0oencVo-QZF=zg@mail.gmail.com","subject":"Re: [RFD] Making \"git push [--force/--delete]\" safer?","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-07-03T06:34:25Z","receivedAt":"2013-07-03T06:34:25Z","isPatch":false,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wed, Jul 3, 2013 at 12:55 AM, Johan Herland wrote:\n> I assume that in most cases the expected value of the remote ref would\n> equal the current value of the corresponding remote-tracking ref in\n> the user's repo, so why not use that as the default expected value?\n> E.g.:\n>\n> $ git config push.default simple\n> $ git checkout -b foo -t origin/foo\n> # prepare non-ff update\n> $ git push --force-if-expected\n> # the above validates foo @ origin != origin/foo before pushing\n\nOops, typo: s/!=/==/\n\n>\n> And if the users expects a different value, (s)he can pass that to the\n> same option:\n>\n> $ git push --force-if-expected=refs/original/foo my_remote HEAD:foo\n> # the above fails if foo @ origin != refs/original/foo\n>\n> The option name probably needs a little work, but as long as it\n> properly communicates the user's _intent_ I'm fine with whatever we\n> call it.\n\nOvernight, it occured to me that --force-if-expected could be\nsimplified by leveraging the existing --force option; for the above\ntwo examples, respectively:\n\n $ git push --force --expect\n # validate foo @ origin == @{upstream} before pushing\n\nand\n\n $ git push --force --expect=refs/original/foo my_remote HEAD:foo\n # validate foo @ my_remote == refs/original/foo before pushing\n\nIn other words, the --expect option becomes a modifier on the --force\nbehaviour: If --expect is given, and the remote ref is not as\nexpected, then the push will still fail, even when --force is given.\nFurthermore, this could be fleshed out by allowing the user to\nconfigure push.expect = True, in which case --expect will be assumed\nwhenever --force is used, and the user can override with --no-expect.\n\nIf push.expect == True (or if --expect is given on command-line\nwithout a parameter), we default to using @{upstream} as the expected\nvalue, and we complain to the user if the current branch has no\nupstream. This way, you can still enable push.expect even when you do\nnot configure @{upstream}, but it compels you to always supply\n--expect=$something (or --no-expect) when you use --force.\n\n\n...Johan\n\nPS: I'm still unsure about the option naming. Maybe --validate would\nbe better than --expect, but I feel it should convey more strongly\nthat we're doing _pre_-validation, as opposed to (post-)validating the\n_result_ of the push, whatever that would look like.\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"222442","messageId":"7vli5ogh8r.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"CALKQrgdovWTd50LVDnNR+BhurWgSCKkhr88wCo01VZF3sd5PNg@mail.gmail.com","subject":"Re: [RFD] Making \"git push [--force/--delete]\" safer?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-03T08:49:56Z","receivedAt":"2013-07-03T08:49:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland writes:\n\n> Overnight, it occured to me that --force-if-expected could be\n> simplified by leveraging the existing --force option; for the above\n> two examples, respectively:\n>\n> $ git push --force --expect\n> # validate foo @ origin == @{upstream} before pushing\n>\n> and\n>\n> $ git push --force --expect=refs/original/foo my_remote HEAD:foo\n> # validate foo @ my_remote == refs/original/foo before pushing\n\nFirst, on the name.\n\nI do not think either \"--validate\" or \"--expect\" is particularly a\ngood one. The former lets this feature squat on a good name that\ncovers a much broader spectrum, forbidding people from adding other\nkinds of validation later. \"--expect\" is slightly less bad in that\nsense; saying \"we expect this\" does imply \"otherwise it is an\nunexpected situation and we would fail\", but the name still does not\nfeel ideal.\n\nWhat is the essense of compare-and-swap? Perhaps we can find a good\nword by thinking that question through. \n\nTo me, it is a way to implement a \"lock\" on the remote ref without\nactually taking a lock (which would leave us open for a stale lock),\nand this \"lock\"-ness is what we want in order to guarantee safety.\n\nSo we could perhaps call it \"--lockref\"?\n\nI'll leave the name open but tentatively use this name in the\nfollowing, primarily to see how well it sits on the command line\nexamples.\n\nThen on the semantics/substance.\n\nI had quite a similar thought as you had while reading your initial\nresponse. In the most generic form, we would want to be able to\npass necessary information fully via the option, i.e.\n\n\t--lockref=theirRefName:expectedValue\n\nbut when the option is spelled without details, we could fill in the\ndefault values by making a reasonable guess of what the user could\nhave meant. If we only have --lockref without refname nor value,\nthen we will enable the safety for _all_ refs that we are going to\nupdate during this push. If we have --lockref=theirRefName without\nthe expected value for that ref, we will enable the safety only for\nthe ref (you can give more than one --lockref=theirRefName), and\nguess what value we should expect. If we have a fully specified\noption, we do not have to guess the value.\n\nAnd for the expected value, when we have a tracking branch for the\nbranch at the remote we are trying to update, its value is a very\ngood guess of what the user meant.\n\nNote, however, that this is very different from @{upstream}.\n\nYou could be pushing a branch \"frotz\", that is configured to\nintegrate with \"master\" taken from \"origin\", but\n\n (1) to a branch different from \"master\" of \"origin\", e.g.\n\n\t$ git push --lockref origin frotz:nitfol\n\t$ git push --lockref origin :nitfol\t;# deleting\n\n (2) even to a branch of a remote that is different from \"origin\",\n e.g.\n\n\t$ git push --lockref xyzzy frotz:nitfol\n\t$ git push --lockref xyzzy :nitfol\t;# deleting\n\nEven in these case, if you have a remote tracking branch for the\ndestination (i.e. you have refs/remotes/origin/nitfol in case (1) or\nrefs/remotes/xyzzy/nitfol in case (2) to be updated by fetching from\norigin or xyzzy), we can and should use that value as the default.\n\nThere is no room for frotz@{upstream} (or @{upstream} of the current\nbranch) to get in the picture.\n\nExcept when you happen to be pushing with \"push.default = upstream\",\nthat is. But that is a natural consequence of the more generic\ncheck with \"our remote tracking branch of the branch we are updating\nat the remote\" rule.\n"},{"id":"222452","messageId":"CALKQrge_REZKfds0T-owJOn2BvfLmHpk7yQeSog=yvofE_zKJQ@mail.gmail.com","threadId":"34329","inReplyTo":"7vli5ogh8r.fsf@alter.siamese.dyndns.org","subject":"Re: [RFD] Making \"git push [--force/--delete]\" safer?","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-07-03T10:00:01Z","receivedAt":"2013-07-03T10:00:01Z","isPatch":false,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wed, Jul 3, 2013 at 10:49 AM, Junio C Hamano wrote:\n> Johan Herland writes:\n>> Overnight, it occured to me that --force-if-expected could be\n>> simplified by leveraging the existing --force option; for the above\n>> two examples, respectively:\n>>\n>> $ git push --force --expect\n>> # validate foo @ origin == @{upstream} before pushing\n>>\n>> and\n>>\n>> $ git push --force --expect=refs/original/foo my_remote HEAD:foo\n>> # validate foo @ my_remote == refs/original/foo before pushing\n>\n> First, on the name.\n>\n> I do not think either \"--validate\" or \"--expect\" is particularly a\n> good one. The former lets this feature squat on a good name that\n> covers a much broader spectrum, forbidding people from adding other\n> kinds of validation later. \"--expect\" is slightly less bad in that\n> sense; saying \"we expect this\" does imply \"otherwise it is an\n> unexpected situation and we would fail\", but the name still does not\n> feel ideal.\n>\n> What is the essense of compare-and-swap? Perhaps we can find a good\n> word by thinking that question through.\n>\n> To me, it is a way to implement a \"lock\" on the remote ref without\n> actually taking a lock (which would leave us open for a stale lock),\n> and this \"lock\"-ness is what we want in order to guarantee safety.\n>\n> So we could perhaps call it \"--lockref\"?\n>\n> I'll leave the name open but tentatively use this name in the\n> following, primarily to see how well it sits on the command line\n> examples.\n\nI agree that neither --expect nor --validate are very good. I also\ndon't like --lockref, mostly because there is no locking involved, and\nI think most users will jump to an incorrect conclusion about what\nthis option does, unless they read the documentation.\n\nSome other suggestions:\n\na) --update-if. I think this reads quite nicely in the fully specified\nvariant: --update-if=theirRefName:expectedValue, but it becomes more\ncryptic when defaults are assumed (i.e. --update-if without any\narguments).\n\nb) --precond. This makes it clear that we're specifying a precondition\non the push. Again, I think the fully specified version reads nicely,\nbut it might seem a little cryptic when no arguments are given.\n\nc) --pre-verify, --pre-check are merely variations on (b), other\nvariations include --pre-verify-ref or --pre-check-ref, making things\nmore explicit at the cost of option name length.\n\n> Then on the semantics/substance.\n>\n> I had quite a similar thought as you had while reading your initial\n> response. In the most generic form, we would want to be able to\n> pass necessary information fully via the option, i.e.\n>\n> --lockref=theirRefName:expectedValue\n>\n> but when the option is spelled without details, we could fill in the\n> default values by making a reasonable guess of what the user could\n> have meant. If we only have --lockref without refname nor value,\n> then we will enable the safety for _all_ refs that we are going to\n> update during this push. If we have --lockref=theirRefName without\n> the expected value for that ref, we will enable the safety only for\n> the ref (you can give more than one --lockref=theirRefName), and\n> guess what value we should expect. If we have a fully specified\n> option, we do not have to guess the value.\n>\n> And for the expected value, when we have a tracking branch for the\n> branch at the remote we are trying to update, its value is a very\n> good guess of what the user meant.\n>\n> Note, however, that this is very different from @{upstream}.\n>\n> You could be pushing a branch \"frotz\", that is configured to\n> integrate with \"master\" taken from \"origin\", but\n>\n> (1) to a branch different from \"master\" of \"origin\", e.g.\n>\n> $ git push --lockref origin frotz:nitfol\n> $ git push --lockref origin :nitfol ;# deleting\n>\n> (2) even to a branch of a remote that is different from \"origin\",\n> e.g.\n>\n> $ git push --lockref xyzzy frotz:nitfol\n> $ git push --lockref xyzzy :nitfol ;# deleting\n>\n> Even in these case, if you have a remote tracking branch for the\n> destination (i.e. you have refs/remotes/origin/nitfol in case (1) or\n> refs/remotes/xyzzy/nitfol in case (2) to be updated by fetching from\n> origin or xyzzy), we can and should use that value as the default.\n>\n> There is no room for frotz@{upstream} (or @{upstream} of the current\n> branch) to get in the picture.\n>\n> Except when you happen to be pushing with \"push.default = upstream\",\n> that is. But that is a natural consequence of the more generic\n> check with \"our remote tracking branch of the branch we are updating\n> at the remote\" rule.\n\nFully agree with all of the above. I've been living under the\n\"push.default = upstream\" rock for too long... ;)\n\nSo, how do we deal with the various corner cases?\n\nIf we don't have a tracking branch for the branch at the remote we are\ntrying to update (and an expected value is not specified on the\ncommand line) we should obviously fail the push as we were asked to\nverify, but don't know what to verify against. AFAICS, there is no\nalternative fallbacks for the expected value of the remote ref, and\neven if there were, I'm wary of adding too many fallbacks as it makes\nthe behaviour less transparent.\n\nSimilarly, if we're expecting a certain value for a ref, and that ref\ndoes not (yet) exist at the remote, we should also fail (even if the\nsame push without --force (and --lockref) would succeed), as we\nclearly expected the ref to already exist, and its non-existence might\npoint to a typo somewhere in our command.\n\nFor the case where we're pushing multiple refs, and have expected\nvalues for more than one of them, we need to determine if a failing\nexpectation should cause the entire push to fail, or merely stop that\none ref from being pushed (letting the others go through). I'd feel\nsafer if the _whole_ push failed, but I'm not a \"push.default =\nmatching\" user, so my opinion is of limited value. AFAICS, the current\nbehaviour of \"matching\" is that one failing (e.g. non-ff) ref does not\nabort the entire push, which I find somewhat unsafe...\n\n...Johan\n\n--\nJohan Herland, \nwww.herland.net\n"},{"id":"222465","messageId":"CAF5DW8++sc2VYmdJEjbD_ue_wtDFj21vcyFzNWU0M+rAm2X0sQ@mail.gmail.com","threadId":"34329","inReplyTo":"CALKQrge_REZKfds0T-owJOn2BvfLmHpk7yQeSog=yvofE_zKJQ@mail.gmail.com","subject":"Re: [RFD] Making \"git push [--force/--delete]\" safer?","fromName":"Jonathan del Strother","fromEmail":"maillist@steelskies.com","sentAt":"2013-07-03T10:06:26Z","receivedAt":"2013-07-03T10:06:26Z","isPatch":false,"sender":{"key":"jon.delstrother@bestbefore.tv","avatar":"https://gravatar.com/avatar/754e21ab701c00e2d21fc261187254c34b2a1c0b959d9ee5be1a295990be3081?d=mp&s=160"},"body":"On 3 July 2013 11:00, Johan Herland wrote:\n> On Wed, Jul 3, 2013 at 10:49 AM, Junio C Hamano wrote:\n>> Johan Herland writes:\n>>> Overnight, it occured to me that --force-if-expected could be\n>>> simplified by leveraging the existing --force option; for the above\n>>> two examples, respectively:\n>>>\n>>> $ git push --force --expect\n>>> # validate foo @ origin == @{upstream} before pushing\n>>>\n>>> and\n>>>\n>>> $ git push --force --expect=refs/original/foo my_remote HEAD:foo\n>>> # validate foo @ my_remote == refs/original/foo before pushing\n>>\n>> First, on the name.\n>>\n>> I do not think either \"--validate\" or \"--expect\" is particularly a\n>> good one. The former lets this feature squat on a good name that\n>> covers a much broader spectrum, forbidding people from adding other\n>> kinds of validation later. \"--expect\" is slightly less bad in that\n>> sense; saying \"we expect this\" does imply \"otherwise it is an\n>> unexpected situation and we would fail\", but the name still does not\n>> feel ideal.\n>>\n>> What is the essense of compare-and-swap? Perhaps we can find a good\n>> word by thinking that question through.\n>>\n>> To me, it is a way to implement a \"lock\" on the remote ref without\n>> actually taking a lock (which would leave us open for a stale lock),\n>> and this \"lock\"-ness is what we want in order to guarantee safety.\n>>\n>> So we could perhaps call it \"--lockref\"?\n>>\n>> I'll leave the name open but tentatively use this name in the\n>> following, primarily to see how well it sits on the command line\n>> examples.\n>\n> I agree that neither --expect nor --validate are very good. I also\n> don't like --lockref, mostly because there is no locking involved, and\n> I think most users will jump to an incorrect conclusion about what\n> this option does, unless they read the documentation.\n>\n> Some other suggestions:\n>\n> a) --update-if. I think this reads quite nicely in the fully specified\n> variant: --update-if=theirRefName:expectedValue, but it becomes more\n> cryptic when defaults are assumed (i.e. --update-if without any\n> arguments).\n>\n> b) --precond. This makes it clear that we're specifying a precondition\n> on the push. Again, I think the fully specified version reads nicely,\n> but it might seem a little cryptic when no arguments are given.\n>\n> c) --pre-verify, --pre-check are merely variations on (b), other\n> variations include --pre-verify-ref or --pre-check-ref, making things\n> more explicit at the cost of option name length.\n\nI'm struggling to think of instances where I wouldn't want this\nCAS-like behaviour. Wouldn't it be better to make it the default when\npushing, and allowing the current behaviour with \"git push\n--blind-force\" or something?\n"},{"id":"222456","messageId":"CALKQrgfQhVVC1NxizjCQdDmNfihfyEgypYddWB0CMTPqW9Mxtg@mail.gmail.com","threadId":"34329","inReplyTo":"CAF5DW8++sc2VYmdJEjbD_ue_wtDFj21vcyFzNWU0M+rAm2X0sQ@mail.gmail.com","subject":"Re: [RFD] Making \"git push [--force/--delete]\" safer?","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-07-03T10:11:57Z","receivedAt":"2013-07-03T10:11:57Z","isPatch":false,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wed, Jul 3, 2013 at 12:06 PM, Jonathan del Strother\n wrote:\n> I'm struggling to think of instances where I wouldn't want this\n> CAS-like behaviour. Wouldn't it be better to make it the default when\n> pushing, and allowing the current behaviour with \"git push\n> --blind-force\" or something?\n\nI believe I agree with you. I guess the reason this hasn't come up\nbefore is that by far most of the pushes we do are either\nfast-forwarding, or pushing into a non-shared repo (e.g. my own public\nrepo), and this safety only really applies when we're forcing a\nnon-fast-forward push into a shared repo...\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"222461","messageId":"51D40203.1010100@alum.mit.edu","threadId":"34329","inReplyTo":"CALKQrgfQhVVC1NxizjCQdDmNfihfyEgypYddWB0CMTPqW9Mxtg@mail.gmail.com","subject":"Re: [RFD] Making \"git push [--force/--delete]\" safer?","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-07-03T10:50:43Z","receivedAt":"2013-07-03T10:50:43Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 07/03/2013 12:11 PM, Johan Herland wrote:\n> On Wed, Jul 3, 2013 at 12:06 PM, Jonathan del Strother\n> wrote:\n>> I'm struggling to think of instances where I wouldn't want this\n>> CAS-like behaviour. Wouldn't it be better to make it the default when\n>> pushing, and allowing the current behaviour with \"git push\n>> --blind-force\" or something?\n> \n> I believe I agree with you. I guess the reason this hasn't come up\n> before is that by far most of the pushes we do are either\n> fast-forwarding, or pushing into a non-shared repo (e.g. my own public\n> repo), and this safety only really applies when we're forcing a\n> non-fast-forward push into a shared repo...\n\nI didn't see Jonathan's original email but I was having exactly the same\nthough as him (and was even going to propose the same option name).\n\nNon-ff pushing without knowing what you are going to overwrite is\nirresponsible in most scenarios, and (if backwards-compatibility\nconcerns can be overcome) I think it would be quite prudent to forbid a\nnon-ff push if there is no local remote-tracking branch that is\nup-to-date at the time of the push. Circumventing that check should\nrequire some extra-super-force option.\n\nSo yes, I very much like the general idea of the RFD and personally\nwould lean towards making it stronger and default, at the 2.0 transition\nif necessary.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"222466","messageId":"51D413BA.6080709@viscovery.net","threadId":"34329","inReplyTo":"51D40203.1010100@alum.mit.edu","subject":"Re: [RFD] Making \"git push [--force/--delete]\" safer?","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2013-07-03T12:06:18Z","receivedAt":"2013-07-03T12:06:18Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 7/3/2013 12:50, schrieb Michael Haggerty:\n> On 07/03/2013 12:11 PM, Johan Herland wrote:\n>> On Wed, Jul 3, 2013 at 12:06 PM, Jonathan del Strother\n>> wrote:\n>>> I'm struggling to think of instances where I wouldn't want this\n>>> CAS-like behaviour. Wouldn't it be better to make it the default when\n>>> pushing, and allowing the current behaviour with \"git push\n>>> --blind-force\" or something?\n>>\n>> I believe I agree with you. I guess the reason this hasn't come up\n>> before is that by far most of the pushes we do are either\n>> fast-forwarding, or pushing into a non-shared repo (e.g. my own public\n>> repo), and this safety only really applies when we're forcing a\n>> non-fast-forward push into a shared repo...\n> \n> I didn't see Jonathan's original email but I was having exactly the same\n> though as him (and was even going to propose the same option name).\n> \n> Non-ff pushing without knowing what you are going to overwrite is\n> irresponsible in most scenarios, and (if backwards-compatibility\n> concerns can be overcome) I think it would be quite prudent to forbid a\n> non-ff push if there is no local remote-tracking branch that is\n> up-to-date at the time of the push. Circumventing that check should\n> require some extra-super-force option.\n\nI don't think that is necessary. We already have *two* options to\nforce-push a ref: the + in front of refspec, and --force.\n\nIMO, the meaning of + should be changed to \"force-push with safety\", and\n--force can then be used to override if the safety triggers (i.e., --force\nis your extra-super-force option).\n\n-- Hannes\n"},{"id":"222486","messageId":"7vvc4re86g.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"CALKQrge_REZKfds0T-owJOn2BvfLmHpk7yQeSog=yvofE_zKJQ@mail.gmail.com","subject":"Re: [RFD] Making \"git push [--force/--delete]\" safer?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-03T19:48:39Z","receivedAt":"2013-07-03T19:48:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland writes:\n\n>> I'll leave the name open but tentatively use this name in the\n>> following, primarily to see how well it sits on the command line\n>> examples.\n>\n> I agree that neither --expect nor --validate are very good. I also\n> don't like --lockref, mostly because there is no locking involved, and\n\nYes and no.\n\nThis is not compare-and-swap but is \"store-conditional\" step in\nll/sc. It is letting other people's activities to break your lock\nto prevent you from making an undesirable update. So in that sense,\nthis mechanism is very much a lock.\n\n> Some other suggestions:\n>\n> a) --update-if. I think this reads quite nicely in the fully specified\n> variant: --update-if=theirRefName:expectedValue, but it becomes more\n> cryptic when defaults are assumed (i.e. --update-if without any\n> arguments).\n\nThis name is in line with the \"store conditional\" aspect of the\noperation, but it, together with your --precond and --pre-verify,\nshare the same problem as your --validate. This is only to check\none specific precondition \"The remote ref being updated must point\nat this object\", but all the names you suggested are too broad.\n\nIf we were to go in the direction (3) I suggested in the original\nmessage to let you specify an arbitrary script that reads the list\nof proposed updates and decide to allow them, --update-if=script.sh\nwould be the ideal name for that option to specify the script to be\nrun, though. That mechanism is broad enough to deserve such a broad\nname, if we were to go in that direction.\n\n> b) --precond. This makes it clear that we're specifying a precondition\n> on the push. Again, I think the fully specified version reads nicely,\n> but it might seem a little cryptic when no arguments are given.\n\nSee above.\n\n> c) --pre-verify, --pre-check are merely variations on (b), other\n> variations include --pre-verify-ref or --pre-check-ref, making things\n> more explicit at the cost of option name length.\n\nSee above.\n\n> So, how do we deal with the various corner cases?\n\nI thought I spelled out everything, but apparently I didn't. Here\nis what I had in mind.\n\n (1) A bare \"--lockref\" exists on the command line. E.g.\n\n $ git push --lockref [remote [refspec]...] ;# nothing else about lockref\n\n This will apply to updates of _all_ refs to be updated (e.g.\n with \"remote.origin.push = +refs/heads/pu:refs/heads/pu\", the\n update of 'pu' at the origin will be rejected if 'pu' fails to\n pass the test) with this push. We make sure\n\n - we have remote-tracking branch for the updated ref; if we do\n not have any, we *fail* the update.\n\n - the value of that remote-tracking branch is the same as what\n the remote advertises to \"git push\"; if they do not match, we\n *fail* the update. This includes the case where there is no\n such ref at the remote (may have deleted while we are looking\n the other way).\n\n (2) Remote ref specified on one of the --lockref option(s). E.g.\n\n $ git push --lockref=theirRef[:value] [remote [refspec]...]\n\n This will apply to updates of _only_ the refs given. refs not\n covered by --lockref will follow the usual rule (i.e. with\n --force, anything goes, without --force, only fast-forward is\n allowed). If \":value\" is given, we will use it, otherwise we\n will try to find the remote tracking branch for the updated\n ref, just like a non-specific case as above.\n\n A --lockref=theirRef[:value] that specifies theirRef that is\n not being pushed will be _ignored_ and not checked, so that you\n could say\n\n\t[alias]\n \tsafepush = push --lockref=next\n\t[remote \"origin\"]\n \tpush = refs/heads/maint:refs/heads/maint\n \tpush = refs/heads/master:refs/heads/master\n \tpush = refs/heads/next:refs/heads/next\n \tpush = +refs/heads/pu:refs/heads/pu\n\n and then do\n\n\t$ git safepush origin +next\n\n after a major version bump to rewind 'next' but still do so\n with safety, while still allowing you to say\n\n\t$ git safepush origin maint\n\n to push out 'maint' without having to worry about --lockref=next\n getting in the way.\n\n (3) Mixing --lockref and --lockref=theirRef[:value].\n\n Apply (2) for refs we do have remote ref specified on\n --lockref, and apply (1) for other refs we are going to update.\n\nIn any case, this check happens after we learn the current value of\nremote refs but before we propose what the updated values would be,\nso we can afford to fail the entire push atomically. We could only\nfail the ones that do not pass the check and let others go, but I do\nnot think it is a good idea.\n\nSo in short, I think I agree with you on the semantics.\n"},{"id":"222487","messageId":"7vr4ffe83n.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"CAF5DW8++sc2VYmdJEjbD_ue_wtDFj21vcyFzNWU0M+rAm2X0sQ@mail.gmail.com","subject":"Re: [RFD] Making \"git push [--force/--delete]\" safer?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-03T19:50:20Z","receivedAt":"2013-07-03T19:50:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan del Strother writes:\n\n> I'm struggling to think of instances where I wouldn't want this\n> CAS-like behaviour. Wouldn't it be better to make it the default when\n> pushing, and allowing the current behaviour with \"git push\n> --blind-force\" or something?\n\nNot until we run this in the wild for a while and the mechanism\nproves to be useful without being too cumbersome to some population.\n\nThen at a major version bump, we can start talking about enabling it\nby default, allowing people to selectively disable it.\n"},{"id":"222488","messageId":"7vmwq3e7xy.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"51D413BA.6080709@viscovery.net","subject":"Re: [RFD] Making \"git push [--force/--delete]\" safer?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-03T19:53:45Z","receivedAt":"2013-07-03T19:53:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> I don't think that is necessary. We already have *two* options to\n> force-push a ref: the + in front of refspec, and --force.\n\nThey mean exactly the same thing; the only difference being that \"+\"\nprefix is per target ref, while \"--force\" covers everything, acting\nas a mere short-hand to add \"+\" to everything you push.\n\nIf the \"--lockref/--update-only-if-ref-is-still-there\" option\ndefeats \"--force\", it should defeat \"+src:dst\" exactly the same way.\n"},{"id":"222493","messageId":"7vbo6je6tb.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"7vr4ffe83n.fsf@alter.siamese.dyndns.org","subject":"Re: [RFD] Making \"git push [--force/--delete]\" safer?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-03T20:18:08Z","receivedAt":"2013-07-03T20:18:08Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jonathan del Strother writes:\n>\n>> I'm struggling to think of instances where I wouldn't want this\n>> CAS-like behaviour. Wouldn't it be better to make it the default when\n>> pushing, and allowing the current behaviour with \"git push\n>> --blind-force\" or something?\n>\n> Not until we run this in the wild for a while and the mechanism\n> proves to be useful without being too cumbersome to some population.\n>\n> Then at a major version bump, we can start talking about enabling it\n> by default, allowing people to selectively disable it.\n\nIf we enable this by default, we would need to be a lot more careful\ndesigning what should happen when there is no remote-tracking branch\nthe corresponds to what we are updating/deleting.\n\nThe proposed behaviour so far is to fail, and that is justifiable\nbecause \"the user asked us to check, but did not say what to check\nagainst, and we tried to check with a remote-tracking branch and\nfound none. We cannot satisfy the user's request to check, hence we\nfail\".\n\nEnabling the check by default will change the picture somewhat; that\njustification no longer holds.\n\nIf you are pushing to more than one publishing branches of your own,\nthere is no reason to have remote-tracking branches for the\nsecondary locations, because you always push to all your publishing\nrepositories at the same time, and you only need to keep remote\ntracking for one of them to remember what you pushed out. Making a\npush to secondaries fail in such a case is bad, and forcing the user\nto disable the feature for each secondary remotes is unnice, too.\n"},{"id":"222526","messageId":"51D50A1E.2030904@viscovery.net","threadId":"34329","inReplyTo":"7vmwq3e7xy.fsf@alter.siamese.dyndns.org","subject":"Re: [RFD] Making \"git push [--force/--delete]\" safer?","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2013-07-04T05:37:34Z","receivedAt":"2013-07-04T05:37:34Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 7/3/2013 21:53, schrieb Junio C Hamano:\n> Johannes Sixt writes:\n> \n>> I don't think that is necessary. We already have *two* options to\n>> force-push a ref: the + in front of refspec, and --force.\n> \n> They mean exactly the same thing; the only difference being that \"+\"\n> prefix is per target ref, while \"--force\" covers everything, acting\n> as a mere short-hand to add \"+\" to everything you push.\n\nI know, and I'm saying that we do not have to keep this duplicity.\n\n> If the \"--lockref/--update-only-if-ref-is-still-there\" option\n> defeats \"--force\", it should defeat \"+src:dst\" exactly the same way.\n\nThis logic is backwards. If anything, then \"--force\" must defeat the\nsafety that \"--lockref\" gives.\n\n-- Hannes\n"},{"id":"222527","messageId":"7va9m2dgid.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"51D50A1E.2030904@viscovery.net","subject":"Re: [RFD] Making \"git push [--force/--delete]\" safer?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-04T05:46:18Z","receivedAt":"2013-07-04T05:46:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 7/3/2013 21:53, schrieb Junio C Hamano:\n>> Johannes Sixt writes:\n>> \n>>> I don't think that is necessary. We already have *two* options to\n>>> force-push a ref: the + in front of refspec, and --force.\n>> \n>> They mean exactly the same thing; the only difference being that \"+\"\n>> prefix is per target ref, while \"--force\" covers everything, acting\n>> as a mere short-hand to add \"+\" to everything you push.\n>\n> I know, and I'm saying that we do not have to keep this duplicity.\n\nOf course we do. If you change + prefix to \"push but always require\ntracking ref in the opposite direction\", you will break existing\nsetup by (1) making it impossible to loosen the restriction per ref,\nand (2) forcing people to have reverse tracking ref.\n\nIt is OK to introduce another prefix that mean a new thing. It is\nabsolutely not OK to change the semantics of only one and not the\nother.\n"},{"id":"222956","messageId":"1373399610-8588-1-git-send-email-gitster@pobox.com","threadId":"34329","inReplyTo":"7vfvvwk7ce.fsf@alter.siamese.dyndns.org","subject":"[PATCH 0/7] safer \"push --force\" with compare-and-swap","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-09T19:53:23Z","receivedAt":"2013-07-09T19:53:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When you have to replace an already published branch with its\nrebased version, you would have to --force, but then you risk losing\nwork, if any, that was pushed by somebody else while you are working\non rebasing, as your earlier decision that replacing the old one\nwith its rebase is OK was based on the assumption that there is\nnothing else going on. Unfortunately, --force is blind, and did not\noffer this \"... but fail if the tip has moved from what I expect\".\n\nAnd here is a series to remedy it. It lets you specify what the\nexpected current values of refs you are attempting to update, and if\nyou are lazy, the value is taken from your remote tracking branch\nfor the ref you are attempting to update.\n\nI am not married to the \"lockref\" name, but I think the semantics\nimplemented here is what we discussed to do in the earlier\ndiscussion.\n\ncf. http://thread.gmane.org/gmane.comp.version-control.git/229430\n\nThis may still be rough at edges, but a basic testset seems to pass.\n\nI haven't bothered to check the smart-http transport; help is\ngreatly appreciated.\n\nJunio C Hamano (7):\n cache.h: move remote/connect API out of it\n builtin/push.c: use OPT_BOOL, not OPT_BOOLEAN\n push: beginning of compare-and-swap \"force/delete safety\"\n remote.c: add command line option parser for --lockref\n push --lockref: implement logic to populate old_sha1_expect[]\n t5533: test \"push --lockref\"\n push: document --lockref\n\n Documentation/git-push.txt | 26 +++++++\n builtin/fetch-pack.c | 2 +\n builtin/push.c | 18 ++++-\n builtin/receive-pack.c | 1 +\n builtin/send-pack.c | 26 +++++++\n cache.h | 62 ----------------\n connect.c | 1 +\n connect.h | 13 ++++\n fetch-pack.c | 1 +\n fetch-pack.h | 1 +\n refs.c | 8 ---\n remote.c | 149 +++++++++++++++++++++++++++++++++++++-\n remote.h | 83 +++++++++++++++++++++\n send-pack.c | 2 +\n t/t5533-push-cas.sh | 176 +++++++++++++++++++++++++++++++++++++++++++++\n transport-helper.c | 6 ++\n transport.c | 13 ++++\n transport.h | 5 ++\n upload-pack.c | 1 +\n 19 files changed, 519 insertions(+), 75 deletions(-)\n create mode 100644 connect.h\n create mode 100755 t/t5533-push-cas.sh\n\n-- \n1.8.3.2-875-g76c723c\n"},{"id":"222958","messageId":"1373399610-8588-2-git-send-email-gitster@pobox.com","threadId":"34329","inReplyTo":"1373399610-8588-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/7] cache.h: move remote/connect API out of it","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-09T19:53:24Z","receivedAt":"2013-07-09T19:53:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The definition of \"struct ref\" in \"cache.h\", a header file so\ncentral to the system, always confused me. This structure is not\nabout the local ref used by sha1-name API to name local objects.\n\nIt is what refspecs are expanded into, after finding out what refs\nthe other side has, to define what refs are updated after object\ntransfer succeeds to what values. It belongs to \"remote.h\" together\nwith \"struct refspec\".\n\nWhile we are at it, also move the types and functions related to the\nGit transport connection to a new header file connect.h\n\nSigned-off-by: Junio C Hamano \n---\n builtin/fetch-pack.c | 2 ++\n builtin/receive-pack.c | 1 +\n builtin/send-pack.c | 1 +\n cache.h | 62 --------------------------------------------------\n connect.c | 1 +\n connect.h | 13 +++++++++++\n fetch-pack.c | 1 +\n fetch-pack.h | 1 +\n refs.c | 8 -------\n remote.c | 8 +++++++\n remote.h | 54 +++++++++++++++++++++++++++++++++++++++++++\n send-pack.c | 1 +\n transport.c | 2 ++\n transport.h | 1 +\n upload-pack.c | 1 +\n 15 files changed, 87 insertions(+), 70 deletions(-)\n create mode 100644 connect.h\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex aba4465..c6888c6 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -1,6 +1,8 @@\n #include \"builtin.h\"\n #include \"pkt-line.h\"\n #include \"fetch-pack.h\"\n+#include \"remote.h\"\n+#include \"connect.h\"\n \n static const char fetch_pack_usage[] =\n \"git fetch-pack [--all] [--stdin] [--quiet|-q] [--keep|-k] [--thin] \"\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex e3eb5fc..7434d9b 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -8,6 +8,7 @@\n #include \"commit.h\"\n #include \"object.h\"\n #include \"remote.h\"\n+#include \"connect.h\"\n #include \"transport.h\"\n #include \"string-list.h\"\n #include \"sha1-array.h\"\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 152c4ea..e86d3b5 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -5,6 +5,7 @@\n #include \"sideband.h\"\n #include \"run-command.h\"\n #include \"remote.h\"\n+#include \"connect.h\"\n #include \"send-pack.h\"\n #include \"quote.h\"\n #include \"transport.h\"\ndiff --git a/cache.h b/cache.h\nindex dd0fb33..cb2891d 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1035,68 +1035,6 @@ struct pack_entry {\n \tstruct packed_git *p;\n };\n \n-struct ref {\n-\tstruct ref *next;\n-\tunsigned char old_sha1[20];\n-\tunsigned char new_sha1[20];\n-\tchar *symref;\n-\tunsigned int\n-\t\tforce:1,\n-\t\tforced_update:1,\n-\t\tdeletion:1,\n-\t\tmatched:1;\n-\n-\t/*\n-\t * Order is important here, as we write to FETCH_HEAD\n-\t * in numeric order. And the default NOT_FOR_MERGE\n-\t * should be 0, so that xcalloc'd structures get it\n-\t * by default.\n-\t */\n-\tenum {\n-\t\tFETCH_HEAD_MERGE = -1,\n-\t\tFETCH_HEAD_NOT_FOR_MERGE = 0,\n-\t\tFETCH_HEAD_IGNORE = 1\n-\t} fetch_head_status;\n-\n-\tenum {\n-\t\tREF_STATUS_NONE = 0,\n-\t\tREF_STATUS_OK,\n-\t\tREF_STATUS_REJECT_NONFASTFORWARD,\n-\t\tREF_STATUS_REJECT_ALREADY_EXISTS,\n-\t\tREF_STATUS_REJECT_NODELETE,\n-\t\tREF_STATUS_REJECT_FETCH_FIRST,\n-\t\tREF_STATUS_REJECT_NEEDS_FORCE,\n-\t\tREF_STATUS_UPTODATE,\n-\t\tREF_STATUS_REMOTE_REJECT,\n-\t\tREF_STATUS_EXPECTING_REPORT\n-\t} status;\n-\tchar *remote_status;\n-\tstruct ref *peer_ref; /* when renaming */\n-\tchar name[FLEX_ARRAY]; /* more */\n-};\n-\n-#define REF_NORMAL\t(1u << 0)\n-#define REF_HEADS\t(1u << 1)\n-#define REF_TAGS\t(1u << 2)\n-\n-extern struct ref *find_ref_by_name(const struct ref *list, const char *name);\n-\n-#define CONNECT_VERBOSE (1u << 0)\n-extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);\n-extern int finish_connect(struct child_process *conn);\n-extern int git_connection_is_socket(struct child_process *conn);\n-struct extra_have_objects {\n-\tint nr, alloc;\n-\tunsigned char (*array)[20];\n-};\n-extern struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n-\t\t\t\t struct ref **list, unsigned int flags,\n-\t\t\t\t struct extra_have_objects *);\n-extern int server_supports(const char *feature);\n-extern int parse_feature_request(const char *features, const char *feature);\n-extern const char *server_feature_value(const char *feature, int *len_ret);\n-extern const char *parse_feature_value(const char *feature_list, const char *feature, int *len_ret);\n-\n extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path);\n \n /* A hook for count-objects to report invalid files in pack directory */\ndiff --git a/connect.c b/connect.c\nindex a0783d4..a80ebd3 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -5,6 +5,7 @@\n #include \"refs.h\"\n #include \"run-command.h\"\n #include \"remote.h\"\n+#include \"connect.h\"\n #include \"url.h\"\n \n static char *server_capabilities;\ndiff --git a/connect.h b/connect.h\nnew file mode 100644\nindex 0000000..9dff25c\n--- /dev/null\n+++ b/connect.h\n@@ -0,0 +1,13 @@\n+#ifndef CONNECT_H\n+#define CONNECT_H\n+\n+#define CONNECT_VERBOSE (1u << 0)\n+extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);\n+extern int finish_connect(struct child_process *conn);\n+extern int git_connection_is_socket(struct child_process *conn);\n+extern int server_supports(const char *feature);\n+extern int parse_feature_request(const char *features, const char *feature);\n+extern const char *server_feature_value(const char *feature, int *len_ret);\n+extern const char *parse_feature_value(const char *feature_list, const char *feature, int *len_ret);\n+\n+#endif\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex abe5ffb..c2bab42 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -9,6 +9,7 @@\n #include \"fetch-pack.h\"\n #include \"remote.h\"\n #include \"run-command.h\"\n+#include \"connect.h\"\n #include \"transport.h\"\n #include \"version.h\"\n \ndiff --git a/fetch-pack.h b/fetch-pack.h\nindex 40f08ba..461cbf3 100644\n--- a/fetch-pack.h\n+++ b/fetch-pack.h\n@@ -2,6 +2,7 @@\n #define FETCH_PACK_H\n \n #include \"string-list.h\"\n+#include \"run-command.h\"\n \n struct fetch_pack_args {\n \tconst char *uploadpack;\ndiff --git a/refs.c b/refs.c\nindex 4302206..330060c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3193,14 +3193,6 @@ int update_ref(const char *action, const char *refname,\n \treturn 0;\n }\n \n-struct ref *find_ref_by_name(const struct ref *list, const char *name)\n-{\n-\tfor ( ; list; list = list->next)\n-\t\tif (!strcmp(list->name, name))\n-\t\t\treturn (struct ref *)list;\n-\treturn NULL;\n-}\n-\n /*\n * generate a format suitable for scanf from a ref_rev_parse_rules\n * rule, that is replace the \"%.*s\" spec with a \"%s\" spec\ndiff --git a/remote.c b/remote.c\nindex 6f57830..b1ff7a2 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1302,6 +1302,14 @@ static void add_missing_tags(struct ref *src, struct ref **dst, struct ref ***ds\n \tfree(sent_tips.tip);\n }\n \n+struct ref *find_ref_by_name(const struct ref *list, const char *name)\n+{\n+\tfor ( ; list; list = list->next)\n+\t\tif (!strcmp(list->name, name))\n+\t\t\treturn (struct ref *)list;\n+\treturn NULL;\n+}\n+\n /*\n * Given the set of refs the local repository has, the set of refs the\n * remote repository has, and the refspec used for push, determine\ndiff --git a/remote.h b/remote.h\nindex cf56724..a850059 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -71,6 +71,52 @@ struct refspec {\n \n extern const struct refspec *tag_refspec;\n \n+struct ref {\n+\tstruct ref *next;\n+\tunsigned char old_sha1[20];\n+\tunsigned char new_sha1[20];\n+\tchar *symref;\n+\tunsigned int\n+\t\tforce:1,\n+\t\tforced_update:1,\n+\t\tdeletion:1,\n+\t\tmatched:1;\n+\n+\t/*\n+\t * Order is important here, as we write to FETCH_HEAD\n+\t * in numeric order. And the default NOT_FOR_MERGE\n+\t * should be 0, so that xcalloc'd structures get it\n+\t * by default.\n+\t */\n+\tenum {\n+\t\tFETCH_HEAD_MERGE = -1,\n+\t\tFETCH_HEAD_NOT_FOR_MERGE = 0,\n+\t\tFETCH_HEAD_IGNORE = 1\n+\t} fetch_head_status;\n+\n+\tenum {\n+\t\tREF_STATUS_NONE = 0,\n+\t\tREF_STATUS_OK,\n+\t\tREF_STATUS_REJECT_NONFASTFORWARD,\n+\t\tREF_STATUS_REJECT_ALREADY_EXISTS,\n+\t\tREF_STATUS_REJECT_NODELETE,\n+\t\tREF_STATUS_REJECT_FETCH_FIRST,\n+\t\tREF_STATUS_REJECT_NEEDS_FORCE,\n+\t\tREF_STATUS_UPTODATE,\n+\t\tREF_STATUS_REMOTE_REJECT,\n+\t\tREF_STATUS_EXPECTING_REPORT\n+\t} status;\n+\tchar *remote_status;\n+\tstruct ref *peer_ref; /* when renaming */\n+\tchar name[FLEX_ARRAY]; /* more */\n+};\n+\n+#define REF_NORMAL\t(1u << 0)\n+#define REF_HEADS\t(1u << 1)\n+#define REF_TAGS\t(1u << 2)\n+\n+extern struct ref *find_ref_by_name(const struct ref *list, const char *name);\n+\n struct ref *alloc_ref(const char *name);\n struct ref *copy_ref(const struct ref *ref);\n struct ref *copy_ref_list(const struct ref *ref);\n@@ -84,6 +130,14 @@ int check_ref_type(const struct ref *ref, int flags);\n */\n void free_refs(struct ref *ref);\n \n+struct extra_have_objects {\n+\tint nr, alloc;\n+\tunsigned char (*array)[20];\n+};\n+extern struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n+\t\t\t\t struct ref **list, unsigned int flags,\n+\t\t\t\t struct extra_have_objects *);\n+\n int resolve_remote_symref(struct ref *ref, struct ref *list);\n int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1);\n \ndiff --git a/send-pack.c b/send-pack.c\nindex 7d172ef..9a9908c 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -5,6 +5,7 @@\n #include \"sideband.h\"\n #include \"run-command.h\"\n #include \"remote.h\"\n+#include \"connect.h\"\n #include \"send-pack.h\"\n #include \"quote.h\"\n #include \"transport.h\"\ndiff --git a/transport.c b/transport.c\nindex e15db98..b84dbf0 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -3,6 +3,8 @@\n #include \"run-command.h\"\n #include \"pkt-line.h\"\n #include \"fetch-pack.h\"\n+#include \"remote.h\"\n+#include \"connect.h\"\n #include \"send-pack.h\"\n #include \"walker.h\"\n #include \"bundle.h\"\ndiff --git a/transport.h b/transport.h\nindex ea70ea7..b551f99 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -2,6 +2,7 @@\n #define TRANSPORT_H\n \n #include \"cache.h\"\n+#include \"run-command.h\"\n #include \"remote.h\"\n \n struct git_transport_options {\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 127e59a..b03492e 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -10,6 +10,7 @@\n #include \"revision.h\"\n #include \"list-objects.h\"\n #include \"run-command.h\"\n+#include \"connect.h\"\n #include \"sigchain.h\"\n #include \"version.h\"\n #include \"string-list.h\"\n-- \n1.8.3.2-875-g76c723c\n"},{"id":"222959","messageId":"1373399610-8588-3-git-send-email-gitster@pobox.com","threadId":"34329","inReplyTo":"1373399610-8588-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/7] builtin/push.c: use OPT_BOOL, not OPT_BOOLEAN","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-09T19:53:25Z","receivedAt":"2013-07-09T19:53:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The command line parser of \"git push\" for \"--tags\", \"--delete\", and\n\"--thin\" options still used outdated OPT_BOOLEAN. Because these\noptions do not give escalating levels when given multiple times,\nthey should use OPT_BOOL.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/push.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 2d84d10..342d792 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -427,15 +427,15 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT( 0 , \"all\", &flags, N_(\"push all refs\"), TRANSPORT_PUSH_ALL),\n \t\tOPT_BIT( 0 , \"mirror\", &flags, N_(\"mirror all refs\"),\n \t\t\t (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE)),\n-\t\tOPT_BOOLEAN( 0, \"delete\", &deleterefs, N_(\"delete refs\")),\n-\t\tOPT_BOOLEAN( 0 , \"tags\", &tags, N_(\"push tags (can't be used with --all or --mirror)\")),\n+\t\tOPT_BOOL( 0, \"delete\", &deleterefs, N_(\"delete refs\")),\n+\t\tOPT_BOOL( 0 , \"tags\", &tags, N_(\"push tags (can't be used with --all or --mirror)\")),\n \t\tOPT_BIT('n' , \"dry-run\", &flags, N_(\"dry run\"), TRANSPORT_PUSH_DRY_RUN),\n \t\tOPT_BIT( 0, \"porcelain\", &flags, N_(\"machine-readable output\"), TRANSPORT_PUSH_PORCELAIN),\n \t\tOPT_BIT('f', \"force\", &flags, N_(\"force updates\"), TRANSPORT_PUSH_FORCE),\n \t\t{ OPTION_CALLBACK, 0, \"recurse-submodules\", &flags, N_(\"check\"),\n \t\t\tN_(\"control recursive pushing of submodules\"),\n \t\t\tPARSE_OPT_OPTARG, option_parse_recurse_submodules },\n-\t\tOPT_BOOLEAN( 0 , \"thin\", &thin, N_(\"use thin pack\")),\n+\t\tOPT_BOOL( 0 , \"thin\", &thin, N_(\"use thin pack\")),\n \t\tOPT_STRING( 0 , \"receive-pack\", &receivepack, \"receive-pack\", N_(\"receive pack program\")),\n \t\tOPT_STRING( 0 , \"exec\", &receivepack, \"receive-pack\", N_(\"receive pack program\")),\n \t\tOPT_BIT('u', \"set-upstream\", &flags, N_(\"set upstream for git pull/status\"),\n-- \n1.8.3.2-875-g76c723c\n"},{"id":"222960","messageId":"1373399610-8588-4-git-send-email-gitster@pobox.com","threadId":"34329","inReplyTo":"1373399610-8588-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/7] push: beginning of compare-and-swap \"force/delete safety\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-09T19:53:26Z","receivedAt":"2013-07-09T19:53:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This teaches the deepest part of the callchain for \"git push\" (and\n\"git send-pack\") to optionally allow \"the old value of the ref must\nbe this, otherwise fail this push\" we discussed earlier.\n\nNobody sets the new \"expect_old_sha1\" and \"expect_old_no_trackback\"\nbitfields yet, so this is still a no-op.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/send-pack.c | 5 +++++\n remote.c | 21 +++++++++++++++++++--\n remote.h | 4 ++++\n send-pack.c | 1 +\n transport-helper.c | 6 ++++++\n transport.c | 5 +++++\n 6 files changed, 40 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex e86d3b5..c86c556 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -55,6 +55,11 @@ static void print_helper_status(struct ref *ref)\n \t\t\tmsg = \"needs force\";\n \t\t\tbreak;\n \n+\t\tcase REF_STATUS_REJECT_STALE:\n+\t\t\tres = \"error\";\n+\t\t\tmsg = \"stale info\";\n+\t\t\tbreak;\n+\n \t\tcase REF_STATUS_REJECT_ALREADY_EXISTS:\n \t\t\tres = \"error\";\n \t\t\tmsg = \"already exists\";\ndiff --git a/remote.c b/remote.c\nindex b1ff7a2..81bc876 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1416,13 +1416,30 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,\n \t\t}\n \n \t\t/*\n+\t\t * If we know what the old value of the remote ref\n+\t\t * should be, reject any push, even forced ones,\n+\t\t * if they do not match.\n+\t\t *\n+\t\t * It also is an error if the user told us to check\n+\t\t * with the remote-tracking branch to find the value\n+\t\t * to expect, but we did not have such a tracking\n+\t\t * branch.\n+\t\t */\n+\t\tif (ref->expect_old_sha1 &&\n+\t\t (ref->expect_old_no_trackback ||\n+\t\t hashcmp(ref->old_sha1, ref->old_sha1_expect))) {\n+\t\t\tref->status = REF_STATUS_REJECT_STALE;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t/*\n \t\t * Decide whether an individual refspec A:B can be\n \t\t * pushed. The push will succeed if any of the\n \t\t * following are true:\n \t\t *\n-\t\t * (1) the remote reference B does not exist\n+\t\t * (1) the remote reference B does not exist (i.e. create)\n \t\t *\n-\t\t * (2) the remote reference B is being removed (i.e.,\n+\t\t * (2) the remote reference B is being removed (i.e. delete;\n \t\t * pushing :B where no source is specified)\n \t\t *\n \t\t * (3) the destination is not under refs/tags/, and\ndiff --git a/remote.h b/remote.h\nindex a850059..7ad37e6 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -75,10 +75,13 @@ struct ref {\n \tstruct ref *next;\n \tunsigned char old_sha1[20];\n \tunsigned char new_sha1[20];\n+\tunsigned char old_sha1_expect[20]; /* used by expect-old */\n \tchar *symref;\n \tunsigned int\n \t\tforce:1,\n \t\tforced_update:1,\n+\t\texpect_old_sha1:1,\n+\t\texpect_old_no_trackback:1,\n \t\tdeletion:1,\n \t\tmatched:1;\n \n@@ -102,6 +105,7 @@ struct ref {\n \t\tREF_STATUS_REJECT_NODELETE,\n \t\tREF_STATUS_REJECT_FETCH_FIRST,\n \t\tREF_STATUS_REJECT_NEEDS_FORCE,\n+\t\tREF_STATUS_REJECT_STALE,\n \t\tREF_STATUS_UPTODATE,\n \t\tREF_STATUS_REMOTE_REJECT,\n \t\tREF_STATUS_EXPECTING_REPORT\ndiff --git a/send-pack.c b/send-pack.c\nindex 9a9908c..b228d65 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -227,6 +227,7 @@ int send_pack(struct send_pack_args *args,\n \t\tcase REF_STATUS_REJECT_ALREADY_EXISTS:\n \t\tcase REF_STATUS_REJECT_FETCH_FIRST:\n \t\tcase REF_STATUS_REJECT_NEEDS_FORCE:\n+\t\tcase REF_STATUS_REJECT_STALE:\n \t\tcase REF_STATUS_UPTODATE:\n \t\t\tcontinue;\n \t\tdefault:\ndiff --git a/transport-helper.c b/transport-helper.c\nindex db9bd18..95d22f8 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -683,6 +683,11 @@ static int push_update_ref_status(struct strbuf *buf,\n \t\t\tfree(msg);\n \t\t\tmsg = NULL;\n \t\t}\n+\t\telse if (!strcmp(msg, \"stale info\")) {\n+\t\t\tstatus = REF_STATUS_REJECT_STALE;\n+\t\t\tfree(msg);\n+\t\t\tmsg = NULL;\n+\t\t}\n \t}\n \n \tif (*ref)\n@@ -756,6 +761,7 @@ static int push_refs_with_push(struct transport *transport,\n \t\t/* Check for statuses set by set_ref_status_for_push() */\n \t\tswitch (ref->status) {\n \t\tcase REF_STATUS_REJECT_NONFASTFORWARD:\n+\t\tcase REF_STATUS_REJECT_STALE:\n \t\tcase REF_STATUS_REJECT_ALREADY_EXISTS:\n \t\tcase REF_STATUS_UPTODATE:\n \t\t\tcontinue;\ndiff --git a/transport.c b/transport.c\nindex b84dbf0..98f5270 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -709,6 +709,10 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i\n \t\tprint_ref_status('!', \"[rejected]\", ref, ref->peer_ref,\n \t\t\t\t\t\t \"needs force\", porcelain);\n \t\tbreak;\n+\tcase REF_STATUS_REJECT_STALE:\n+\t\tprint_ref_status('!', \"[rejected]\", ref, ref->peer_ref,\n+\t\t\t\t\t\t \"stale info\", porcelain);\n+\t\tbreak;\n \tcase REF_STATUS_REMOTE_REJECT:\n \t\tprint_ref_status('!', \"[remote rejected]\", ref,\n \t\t\t\t\t\t ref->deletion ? NULL : ref->peer_ref,\n@@ -1078,6 +1082,7 @@ static int run_pre_push_hook(struct transport *transport,\n \tfor (r = remote_refs; r; r = r->next) {\n \t\tif (!r->peer_ref) continue;\n \t\tif (r->status == REF_STATUS_REJECT_NONFASTFORWARD) continue;\n+\t\tif (r->status == REF_STATUS_REJECT_STALE) continue;\n \t\tif (r->status == REF_STATUS_UPTODATE) continue;\n \n \t\tstrbuf_reset(&buf);\n-- \n1.8.3.2-875-g76c723c\n"},{"id":"222957","messageId":"1373399610-8588-5-git-send-email-gitster@pobox.com","threadId":"34329","inReplyTo":"1373399610-8588-1-git-send-email-gitster@pobox.com","subject":"[PATCH 4/7] remote.c: add command line option parser for --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-09T19:53:27Z","receivedAt":"2013-07-09T19:53:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Update \"git push\" and \"git send-pack\" to parse this commnd line\noption.\n\nThe intended sematics is:\n\n * \"--lockref\" alone, without specifying the details, will protect\n _all_ remote refs that are going to be updated by requiring their\n current value to be the same as the remote-tracking branch we\n have for them, unless otherwise specified;\n\n * \"--lockref=refname\", without specifying the expected value, will\n protect that refname, if it is going to be updated, by requiring\n its current value to be the same as the remote-tracking branch we\n have for it;\n\n * \"--lockref=refname:value\" will protect that refname, if it is\n going to be updated, by requiring its current value to be the\n same as the specified value (which is allowed to be different\n from the remote-tracking branch we have for the refname, or we do\n not even have to have such a remote-tracking branch when this\n form is used);\n\n * \"--lockref=refname:\" (empty value) is to expect that refname does\n not exist (yet); and\n\n * \"--no-lockref\" will cancel all the previous --lockref on the\n command line.\n\nIn any of the forms, when we try to use a remote-tracking branch for\nthe remote ref being updated, it is an error if there is no such\nremote-tracking branch on our end.\n\nBecause the command line options are parsed _before_ we know which\nremote we are pushing to, there needs further processing to the\nparsed data after we instantiate the transport object to:\n\n * expand \"refname\" given by the user to a full refname to be\n matched with the list of \"struct ref\" used in match_push_refs()\n and set_ref_status_for_push(); and\n\n * learning the actual local ref that is the remote-tracking branch\n for the specified remote ref.\n\nFurther, some processing need to be deferred until we find the set\nof remote refs and match_push_refs() returns in order to find the\nones that need to be checked after explicit ones have been processed\nfor \"--lockref\" (no specific details).\n\nThese post-processing will be the topic of the next patch.\n\nOh, of course, the option name is still not cast in stone.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/push.c | 6 ++++++\n builtin/send-pack.c | 17 +++++++++++++++\n remote.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n remote.h | 22 ++++++++++++++++++++\n 4 files changed, 104 insertions(+)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 342d792..31a5ba0 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -21,6 +21,8 @@ static const char *receivepack;\n static int verbosity;\n static int progress = -1;\n \n+static struct push_cas_option cas;\n+\n static const char **refspec;\n static int refspec_nr;\n static int refspec_alloc;\n@@ -432,6 +434,10 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT('n' , \"dry-run\", &flags, N_(\"dry run\"), TRANSPORT_PUSH_DRY_RUN),\n \t\tOPT_BIT( 0, \"porcelain\", &flags, N_(\"machine-readable output\"), TRANSPORT_PUSH_PORCELAIN),\n \t\tOPT_BIT('f', \"force\", &flags, N_(\"force updates\"), TRANSPORT_PUSH_FORCE),\n+\t\t{ OPTION_CALLBACK,\n+\t\t 0, CAS_OPT_NAME, &cas, N_(\"refname>:use_tracking = 1;\n+\telse if (!colon[1])\n+\t\thashclr(entry->expect);\n+\telse if (get_sha1(colon + 1, entry->expect))\n+\t\treturn error(\"cannot parse expected object name '%s'\", colon + 1);\n+\treturn 0;\n+}\ndiff --git a/remote.h b/remote.h\nindex 7ad37e6..28eb6a3 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -1,6 +1,8 @@\n #ifndef REMOTE_H\n #define REMOTE_H\n \n+#include \"parse-options.h\"\n+\n enum {\n \tREMOTE_CONFIG,\n \tREMOTE_REMOTES,\n@@ -230,4 +232,24 @@ struct ref *guess_remote_head(const struct ref *head,\n /* Return refs which no longer exist on remote */\n struct ref *get_stale_heads(struct refspec *refs, int ref_count, struct ref *fetch_map);\n \n+/*\n+ * Lockref aka CAS\n+ */\n+#define CAS_OPT_NAME \"lockref\"\n+\n+struct push_cas_option {\n+\tunsigned use_tracking_for_rest:1;\n+\tstruct push_cas {\n+\t\tunsigned char expect[20];\n+\t\tunsigned use_tracking:1;\n+\t\tchar *refname;\n+\t} *entry;\n+\tint nr;\n+\tint alloc;\n+};\n+\n+extern int parseopt_push_cas_option(const struct option *, const char *arg, int unset);\n+extern int parse_push_cas_option(struct push_cas_option *, const char *arg, int unset);\n+extern void clear_cas_option(struct push_cas_option *);\n+\n #endif\n-- \n1.8.3.2-875-g76c723c\n"},{"id":"222961","messageId":"1373399610-8588-6-git-send-email-gitster@pobox.com","threadId":"34329","inReplyTo":"1373399610-8588-1-git-send-email-gitster@pobox.com","subject":"[PATCH 5/7] push --lockref: implement logic to populate old_sha1_expect[]","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-09T19:53:28Z","receivedAt":"2013-07-09T19:53:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This plugs the push_cas_option data collected by the command line\noption parser to the transport system with a new function\napply_push_cas(), which is called after match_push_refs() has\nalready been called. At this point, we know which remote we are\ntalking to, and what remote refs we are going to update, so we can\nfill in the details that may have been missing from the command\nline, such as\n\n (1) what abbreviated refname the user gave us matches the actual\n refname at the remote; and\n\n (2) which remote tracking branch in our local repository to read the\n value of the object to expect at the remote.\n\nto populate the old_sha1_expect[] field of each of the remote ref.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/push.c | 6 ++++++\n builtin/send-pack.c | 3 +++\n remote.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n remote.h | 3 +++\n transport.c | 6 ++++++\n transport.h | 4 ++++\n 6 files changed, 83 insertions(+)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 31a5ba0..b0e3691 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -299,6 +299,12 @@ static int push_with_options(struct transport *transport, int flags)\n \tif (thin)\n \t\ttransport_set_option(transport, TRANS_OPT_THIN, \"yes\");\n \n+\tif (!is_empty_cas(&cas)) {\n+\t\tif (!transport->smart_options)\n+\t\t\tdie(\"underlying transport does not support --lockref option\");\n+\t\ttransport->smart_options->cas = &cas;\n+\t}\n+\n \tif (verbosity > 0)\n \t\tfprintf(stderr, _(\"Pushing to %s\\n\"), transport->url);\n \terr = transport_push(transport, refspec_nr, refspec, flags,\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 061e2b2..41dc512 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -247,6 +247,9 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \tif (match_push_refs(local_refs, &remote_refs, nr_refspecs, refspecs, flags))\n \t\treturn -1;\n \n+\tif (!is_empty_cas(&cas))\n+\t\tapply_push_cas(&cas, remote, remote_refs);\n+\n \tset_ref_status_for_push(remote_refs, args.send_mirror,\n \t\targs.force_update);\n \ndiff --git a/remote.c b/remote.c\nindex e9b423a..db418ff 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1997,3 +1997,64 @@ int parse_push_cas_option(struct push_cas_option *cas, const char *arg, int unse\n \t\treturn error(\"cannot parse expected object name '%s'\", colon + 1);\n \treturn 0;\n }\n+\n+int is_empty_cas(const struct push_cas_option *cas)\n+{\n+\treturn !cas->use_tracking_for_rest && !cas->nr;\n+}\n+\n+/*\n+ * Look at remote.fetch refspec and see if we have a remote\n+ * tracking branch for the refname there. Fill its current\n+ * value in sha1[].\n+ * If we cannot do so, return negative to signal an error.\n+ */\n+static int remote_tracking(struct remote *remote, const char *refname,\n+\t\t\t unsigned char sha1[20])\n+{\n+\tchar *dst;\n+\n+\tdst = apply_refspecs(remote->fetch, remote->fetch_refspec_nr, refname);\n+\tif (!dst)\n+\t\treturn -1; /* no tracking ref for refname at remote */\n+\tif (read_ref(dst, sha1))\n+\t\treturn -1; /* we know what the tracking ref is but we cannot read it */\n+\treturn 0;\n+}\n+\n+static void apply_cas(struct push_cas_option *cas,\n+\t\t struct remote *remote,\n+\t\t struct ref *ref)\n+{\n+\tint i;\n+\n+\t/* Find an explicit --lockref=[:] entry */\n+\tfor (i = 0; i < cas->nr; i++) {\n+\t\tstruct push_cas *entry = &cas->entry[i];\n+\t\tif (!refname_match(entry->refname, ref->name, ref_rev_parse_rules))\n+\t\t\tcontinue;\n+\t\tref->expect_old_sha1 = 1;\n+\t\tif (!entry->use_tracking)\n+\t\t\thashcpy(ref->old_sha1_expect, cas->entry[i].expect);\n+\t\telse if (remote_tracking(remote, ref->name, ref->old_sha1_expect))\n+\t\t\tref->expect_old_no_trackback = 1;\n+\t\treturn;\n+\t}\n+\n+\t/* Are we using \"--lockref\" to cover all? */\n+\tif (!cas->use_tracking_for_rest)\n+\t\treturn;\n+\n+\tref->expect_old_sha1 = 1;\n+\tif (remote_tracking(remote, ref->name, ref->old_sha1_expect))\n+\t\tref->expect_old_no_trackback = 1;\n+}\n+\n+void apply_push_cas(struct push_cas_option *cas,\n+\t\t struct remote *remote,\n+\t\t struct ref *remote_refs)\n+{\n+\tstruct ref *ref;\n+\tfor (ref = remote_refs; ref; ref = ref->next)\n+\t\tapply_cas(cas, remote, ref);\n+}\ndiff --git a/remote.h b/remote.h\nindex 28eb6a3..baa1c68 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -252,4 +252,7 @@ extern int parseopt_push_cas_option(const struct option *, const char *arg, int\n extern int parse_push_cas_option(struct push_cas_option *, const char *arg, int unset);\n extern void clear_cas_option(struct push_cas_option *);\n \n+extern int is_empty_cas(const struct push_cas_option *);\n+void apply_push_cas(struct push_cas_option *, struct remote *, struct ref *);\n+\n #endif\ndiff --git a/transport.c b/transport.c\nindex 98f5270..b321d6a 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1147,6 +1147,12 @@ int transport_push(struct transport *transport,\n \t\t\treturn -1;\n \t\t}\n \n+\t\tif (transport->smart_options &&\n+\t\t transport->smart_options->cas &&\n+\t\t !is_empty_cas(transport->smart_options->cas))\n+\t\t\tapply_push_cas(transport->smart_options->cas,\n+\t\t\t\t transport->remote, remote_refs);\n+\n \t\tset_ref_status_for_push(remote_refs,\n \t\t\tflags & TRANSPORT_PUSH_MIRROR,\n \t\t\tflags & TRANSPORT_PUSH_FORCE);\ndiff --git a/transport.h b/transport.h\nindex b551f99..10f7556 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -14,6 +14,7 @@ struct git_transport_options {\n \tint depth;\n \tconst char *uploadpack;\n \tconst char *receivepack;\n+\tstruct push_cas_option *cas;\n };\n \n struct transport {\n@@ -127,6 +128,9 @@ struct transport *transport_get(struct remote *, const char *);\n /* Transfer the data as a thin pack if not null */\n #define TRANS_OPT_THIN \"thin\"\n \n+/* Check the current value of the remote ref */\n+#define TRANS_OPT_CAS \"cas\"\n+\n /* Keep the pack that was transferred if not null */\n #define TRANS_OPT_KEEP \"keep\"\n \n-- \n1.8.3.2-875-g76c723c\n"},{"id":"222963","messageId":"1373399610-8588-7-git-send-email-gitster@pobox.com","threadId":"34329","inReplyTo":"1373399610-8588-1-git-send-email-gitster@pobox.com","subject":"[PATCH 6/7] t5533: test \"push --lockref\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-09T19:53:29Z","receivedAt":"2013-07-09T19:53:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Prepare two repositories, src and dst, the latter of which is a\nclone of the former (with tracking branches), and push from the\nlatter into the former, using --lockref=name (using tracking ref for\n\"name\" when updating \"name\"), --lockref=name:value, --lockref=name:\n(i.e. check creation), and --lockref (using tracking ref for\nanything that we update).\n\nSigned-off-by: Junio C Hamano \n---\n t/t5533-push-cas.sh | 176 ++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 176 insertions(+)\n create mode 100755 t/t5533-push-cas.sh\n\ndiff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh\nnew file mode 100755\nindex 0000000..c080467\n--- /dev/null\n+++ b/t/t5533-push-cas.sh\n@@ -0,0 +1,176 @@\n+#!/bin/sh\n+\n+test_description='compare & swap push force/delete safety'\n+\n+. ./test-lib.sh\n+\n+setup_srcdst_basic () {\n+\trm -fr src dst &&\n+\tgit clone --no-local . src &&\n+\tgit clone --no-local src dst &&\n+\t(\n+\t\tcd src && git checkout HEAD^0\n+\t)\n+}\n+\n+test_expect_success setup '\n+\t: create template repository\n+\ttest_commit A &&\n+\ttest_commit B &&\n+\ttest_commit C\n+'\n+\n+test_expect_success 'push to create (protected)' '\n+\tsetup_srcdst_basic &&\n+\t(\n+\t\tcd dst &&\n+\t\ttest_commit D &&\n+\t\ttest_must_fail git push --lockref=master: origin master &&\n+\t\ttest_must_fail git push --force --lockref=master: origin master\n+\t) &&\n+\t>expect &&\n+\tgit ls-remote src refs/heads/naster >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'push to create (allowed)' '\n+\tsetup_srcdst_basic &&\n+\t(\n+\t\tcd dst &&\n+\t\ttest_commit D &&\n+\t\tgit push --lockref=naster: origin HEAD:naster\n+\t) &&\n+\tgit ls-remote dst refs/heads/master |\n+\tsed -e \"s/master/naster/\" >expect &&\n+\tgit ls-remote src refs/heads/naster >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'push to update (protected)' '\n+\tsetup_srcdst_basic &&\n+\t(\n+\t\tcd dst &&\n+\t\ttest_commit D &&\n+\t\ttest_must_fail git push --lockref=master:master origin master &&\n+\t\ttest_must_fail git push --force --lockref=master:master origin master\n+\t) &&\n+\tgit ls-remote . refs/heads/master >expect &&\n+\tgit ls-remote src refs/heads/master >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'push to update (protected, tracking)' '\n+\tsetup_srcdst_basic &&\n+\t(\n+\t\tcd src &&\n+\t\tgit checkout master &&\n+\t\ttest_commit D &&\n+\t\tgit checkout HEAD^0\n+\t) &&\n+\tgit ls-remote src refs/heads/master >expect &&\n+\t(\n+\t\tcd dst &&\n+\t\ttest_commit E &&\n+\t\tgit ls-remote . refs/remotes/origin/master >expect &&\n+\t\ttest_must_fail git push --lockref=master origin master &&\n+\t\ttest_must_fail git push --force --lockref=master origin master &&\n+\t\tgit ls-remote . refs/remotes/origin/master >actual &&\n+\t\ttest_cmp expect actual\n+\t) &&\n+\tgit ls-remote src refs/heads/master >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'push to update (allowed)' '\n+\tsetup_srcdst_basic &&\n+\t(\n+\t\tcd dst &&\n+\t\ttest_commit D &&\n+\t\tgit push --lockref=master:master^ origin master\n+\t) &&\n+\tgit ls-remote dst refs/heads/master >expect &&\n+\tgit ls-remote src refs/heads/master >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'push to update (allowed, tracking)' '\n+\tsetup_srcdst_basic &&\n+\t(\n+\t\tcd dst &&\n+\t\ttest_commit D &&\n+\t\tgit push --lockref=master origin master\n+\t) &&\n+\tgit ls-remote dst refs/heads/master >expect &&\n+\tgit ls-remote src refs/heads/master >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'push to update (still rejected with non-ff check)' '\n+\tsetup_srcdst_basic &&\n+\tgit ls-remote src refs/heads/master >expect &&\n+\t(\n+\t\tcd dst &&\n+\t\tgit reset --hard HEAD^ &&\n+\t\ttest_commit D &&\n+\t\ttest_must_fail git push --lockref=master origin master\n+\t) &&\n+\tgit ls-remote src refs/heads/master >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'push to delete (protected)' '\n+\tsetup_srcdst_basic &&\n+\tgit ls-remote src refs/heads/master >expect &&\n+\t(\n+\t\tcd dst &&\n+\t\ttest_must_fail git push --lockref=master:master^ origin :master &&\n+\t\ttest_must_fail git push --force --lockref=master:master^ origin :master\n+\t) &&\n+\tgit ls-remote src refs/heads/master >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'push to delete (allowed)' '\n+\tsetup_srcdst_basic &&\n+\t(\n+\t\tcd dst &&\n+\t\tgit push --lockref=master origin :master\n+\t) &&\n+\t>expect &&\n+\tgit ls-remote src refs/heads/master >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'cover everything with default lockref (protected)' '\n+\tsetup_srcdst_basic &&\n+\t(\n+\t\tcd src &&\n+\t\tgit branch naster master^\n+\t)\n+\tgit ls-remote src refs/heads/\\* >expect &&\n+\t(\n+\t\tcd dst &&\n+\t\ttest_must_fail git push --lockref origin master master:naster\n+\t) &&\n+\tgit ls-remote src refs/heads/\\* >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'cover everything with default lockref (allowed)' '\n+\tsetup_srcdst_basic &&\n+\t(\n+\t\tcd src &&\n+\t\tgit branch naster master^\n+\t)\n+\t(\n+\t\tcd dst &&\n+\t\tgit fetch &&\n+\t\tgit push --lockref origin master master:naster\n+\t) &&\n+\tgit ls-remote dst refs/heads/master |\n+\tsed -e \"s/master/naster/\" >expect &&\n+\tgit ls-remote src refs/heads/naster >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \n1.8.3.2-875-g76c723c\n"},{"id":"222962","messageId":"1373399610-8588-8-git-send-email-gitster@pobox.com","threadId":"34329","inReplyTo":"1373399610-8588-1-git-send-email-gitster@pobox.com","subject":"[PATCH 7/7] push: document --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-09T19:53:30Z","receivedAt":"2013-07-09T19:53:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano \n---\n Documentation/git-push.txt | 26 ++++++++++++++++++++++++++\n 1 file changed, 26 insertions(+)\n\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex f7dfe48..e7c8bd6 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -11,6 +11,7 @@ SYNOPSIS\n [verse]\n 'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run] [--receive-pack=]\n \t [--repo=] [-f | --force] [--prune] [-v | --verbose] [-u | --set-upstream]\n+\t [--lockref[=[:[]]]]\n \t [--no-verify] [ [...]]\n \n DESCRIPTION\n@@ -146,6 +147,31 @@ already exists on the remote side.\n \tto the `master`\tbranch). See the `...` section above\n \tfor details.\n \n+--lockref::\n+--lockref=::\n+--lockref=:::\n+\tWhen updating at the remote, make sure that the\n+\tref currently points at (an object name), and else\n+\tfail the push, even if `--force` is specified. If only\n+\t is given, the expected value is taken from the\n+\tremote-tracking branch that holds the last-observed value of\n+\tthe . given as an empty string means the\n+\t should not exist and this push must be creating\n+\tit. If `--lockref` (without any value) is given, make sure\n+\teach ref this push is going to update points at the object\n+\tour remote-tracking branch for it points at.\n++\n+This is meant to make `--force` safer to use. Imagine that you have\n+to rebase what you have already published. You will have to\n+`--force` the push to replace the history you originally published\n+with the rebased history. If somebody else built on top of your\n+original history while you are rebasing, the tip of the branch at\n+the remote may advance with her commit, and blindly pushing with\n+`--force` will lose her work. By using this option to specify that\n+you expect the history you are updating is what you rebased and want\n+to replace, you can make sure other people's work will not be losed\n+by a forced push. in such a case.\n+\n --repo=::\n \tThis option is only relevant if no argument is\n \tpassed in the invocation. In this case, 'git push' derives the\n-- \n1.8.3.2-875-g76c723c\n"},{"id":"222969","messageId":"20130709201724.GI4604@pug.qqx.org","threadId":"34329","inReplyTo":"1373399610-8588-8-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-07-09T20:17:24Z","receivedAt":"2013-07-09T20:17:24Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"At 12:53 -0700 09 Jul 2013, Junio C Hamano wrote:\n>+This is meant to make `--force` safer to use. Imagine that you have\n>+to rebase what you have already published. You will have to\n>+`--force` the push to replace the history you originally published\n>+with the rebased history. If somebody else built on top of your\n>+original history while you are rebasing, the tip of the branch at\n>+the remote may advance with her commit, and blindly pushing with\n>+`--force` will lose her work. By using this option to specify that\n>+you expect the history you are updating is what you rebased and want\n>+to replace, you can make sure other people's work will not be losed\n>+by a forced push. in such a case.\n\ns/losed/lost/\n\nHow does this behave if --force is not used? I think it would be best \nif it was a no-op in that case to make it easy to add a config option to \nturn this on by default.\n"},{"id":"222968","messageId":"51DC7199.2050302@kdbg.org","threadId":"34329","inReplyTo":"1373399610-8588-8-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-07-09T20:24:57Z","receivedAt":"2013-07-09T20:24:57Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 09.07.2013 21:53, schrieb Junio C Hamano:\n> +--lockref::\n> +--lockref=::\n> +--lockref=:::\n> ...\n> +This is meant to make `--force` safer to use.\n\nThis is a contradiction. \"--force\" means \"I mean it, dude\", and not \"I\nmean it sometimes\". It would make sense if this sentence were \"This is\nmeant to make `+refspec` safer to use.\"\n\nDo you intend to require users to opt in to safety by saying --lockref\nuntil the end of time? Which makes it actually usable only for scripts\nand aliases. How do you override when the safety triggers, e.g., in an\nalias that uses --force --lockref? Add --i-really-mean-it?\n\nOr do we want to make --lockref the default at least for cases where\nnecessary ingredients can be derived automatically, perhaps in Git 3.0?\nThen, how do you override when the safety triggers? Add --i-really-mean-it?\n\nIMO, the way forward is:\n\n1. Teach users to use +refspec to force-push. Do not encourage 'push\n--force'.\n\n2. Add --lockref as an opt-in for +refspec. Do not apply the safety to\n'push --force'. (Current users and scripts do not see a behavior change\nbecause they do not use --lockref, either.)\n\n3. Make --lockref behavior the default at least for +refspec. Then 'push\n--force' is still able to override the safety.\n\n-- Hannes\n"},{"id":"222970","messageId":"51DC723F.2000602@alum.mit.edu","threadId":"34329","inReplyTo":"1373399610-8588-8-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-07-09T20:27:43Z","receivedAt":"2013-07-09T20:27:43Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 07/09/2013 09:53 PM, Junio C Hamano wrote:\n> Signed-off-by: Junio C Hamano \n> ---\n> Documentation/git-push.txt | 26 ++++++++++++++++++++++++++\n> 1 file changed, 26 insertions(+)\n> \n> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\n> index f7dfe48..e7c8bd6 100644\n> --- a/Documentation/git-push.txt\n> +++ b/Documentation/git-push.txt\n> @@ -11,6 +11,7 @@ SYNOPSIS\n> [verse]\n> 'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run] [--receive-pack=]\n> \t [--repo=] [-f | --force] [--prune] [-v | --verbose] [-u | --set-upstream]\n> +\t [--lockref[=[:[]]]]\n> \t [--no-verify] [ [...]]\n> \n> DESCRIPTION\n> @@ -146,6 +147,31 @@ already exists on the remote side.\n> \tto the `master`\tbranch). See the `...` section above\n> \tfor details.\n> \n> +--lockref::\n> +--lockref=::\n> +--lockref=:::\n> +\tWhen updating at the remote, make sure that the\n> +\tref currently points at (an object name), and else\n> +\tfail the push, even if `--force` is specified. If only\n> +\t is given, the expected value is taken from the\n> +\tremote-tracking branch that holds the last-observed value of\n> +\tthe . given as an empty string means the\n> +\t should not exist and this push must be creating\n> +\tit. If `--lockref` (without any value) is given, make sure\n> +\teach ref this push is going to update points at the object\n> +\tour remote-tracking branch for it points at.\n\nI thought that the explanation in your patch 4/7 log message was\nclearer. In particular, I think that documenting the forms separately,\nas you did in the log message, makes it unambiguous, whereas for example\nthe distinction in prose between \"If only is given\" and\n\" given as an empty string\" is easy to miss.\n\nDoes \"--lockref\" only apply to references that need non-ff updates, or\nto all references that are being pushed? This is mostly interesting for\nthe zero-argument form (especially if a config option is invented to\nmake this the default), but the question should also be answered for the\nother forms.\n\n> +This is meant to make `--force` safer to use. Imagine that you have\n> +to rebase what you have already published. You will have to\n> +`--force` the push to replace the history you originally published\n> +with the rebased history. If somebody else built on top of your\n> +original history while you are rebasing, the tip of the branch at\n\ns/are/were/\n\n> +the remote may advance with her commit, and blindly pushing with\n\ns/advance/have advanced/\n\n> +`--force` will lose her work. By using this option to specify that\n> +you expect the history you are updating is what you rebased and want\n> +to replace, you can make sure other people's work will not be losed\n\ns/losed/lost/\n\n> +by a forced push. in such a case.\n\ns/push./push/ or s/in such a case.//\n\n> +\n> --repo=::\n> \tThis option is only relevant if no argument is\n> \tpassed in the invocation. In this case, 'git push' derives the\n> \n\nAnother minor point: \"git update-ref\" allows either 40 \"0\" or the empty\nstring to check that the ref doesn't already exist. For consistency it\nmight be nice to accept 40 \"0\" here as well.\n\nI still really like the idea of the feature.\n\n\nThe name \"--lockref\" is OK, but for me it's less a question of\n\"locking\", because as far as the user is concerned the push is an atomic\noperation so there is no sense of a \"lock\" that is being held for a\nfinite period of time. For me it is more a question of \"checking\" or\n\"verifying\". I see that the word \"verify\" already has a meaning for\nthis command, so maybe \"--checkref\" or \"--checkold\" or \"--checkoldref\"?\n\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"222971","messageId":"7vhag3v59o.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"51DC7199.2050302@kdbg.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-09T20:37:39Z","receivedAt":"2013-07-09T20:37:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 09.07.2013 21:53, schrieb Junio C Hamano:\n>> +--lockref::\n>> +--lockref=::\n>> +--lockref=:::\n>> ...\n>> +This is meant to make `--force` safer to use.\n>\n> This is a contradiction. \"--force\" means \"I mean it, dude\", and not \"I\n> mean it sometimes\". It would make sense if this sentence were \"This is\n> meant to make `+refspec` safer to use.\"\n\nNo, this *IS* making --force safer by letting you to say in addition\nto --force alone which is blind, add --lockref to defeat it.\n\nI do not see any good reason to change the samentics of \"+refspec\"\nfor something like this. \"+refspec\" and \"--force refspec\" have\nmeant the same thing forever. If --lockref adds safety to +refspec,\nthe same safety should apply to \"--force refspec\".\n\n> Do you intend to require users to opt in to safety by saying --lockref\n> until the end of time?\n\nFor normal users this is *NOT* necessary. I do not know where\npeople are getting the idea of making it default.\n\nRewinding a branch, needing to --force, is an exceptional case.\n\n> Which makes it actually usable only for scripts\n> and aliases. How do you override when the safety triggers, e.g., in an\n> alias that uses --force --lockref?\n\nThe original request for this feature did come from script writers,\nwho want to spin\n\n\tuntil\n git fetch &&\n ... magic integrate of the ongoing work ... &&\n git push --lockref\n\tdo\n \t: spin\n\tdone\n"},{"id":"222972","messageId":"7vd2qrv56p.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"20130709201724.GI4604@pug.qqx.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-09T20:39:26Z","receivedAt":"2013-07-09T20:39:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Schrab writes:\n\n> How does this behave if --force is not used?\n\nBoth the usual \"must fast-forward\" safety and the \"ref should not\nhave moved\" safety apply.\n"},{"id":"222973","messageId":"7v8v1fv52c.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"51DC723F.2000602@alum.mit.edu","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-09T20:42:03Z","receivedAt":"2013-07-09T20:42:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty writes:\n\n> \n> The name \"--lockref\" is OK, but for me it's less a question of\n> \"locking\", because as far as the user is concerned the push is an atomic\n> operation so there is no sense of a \"lock\" that is being held for a\n> finite period of time.\n\nYeah, I think this is more like \"taking a lease\".\n"},{"id":"222976","messageId":"51DC78C0.9030202@kdbg.org","threadId":"34329","inReplyTo":"7vhag3v59o.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-07-09T20:55:28Z","receivedAt":"2013-07-09T20:55:28Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 09.07.2013 22:37, schrieb Junio C Hamano:\n> Johannes Sixt writes:\n> \n>> Am 09.07.2013 21:53, schrieb Junio C Hamano:\n>>> +--lockref::\n>>> +--lockref=::\n>>> +--lockref=:::\n>>> ...\n>>> +This is meant to make `--force` safer to use.\n>>\n>> This is a contradiction. \"--force\" means \"I mean it, dude\", and not \"I\n>> mean it sometimes\". It would make sense if this sentence were \"This is\n>> meant to make `+refspec` safer to use.\"\n> \n> No, this *IS* making --force safer by letting you to say in addition\n> to --force alone which is blind, add --lockref to defeat it.\n> \n> I do not see any good reason to change the samentics of \"+refspec\"\n> for something like this. \"+refspec\" and \"--force refspec\" have\n> meant the same thing forever.\n\nSo what? They still mean the same thing as long as --lockref is not used.\n\n> If --lockref adds safety to +refspec,\n> the same safety should apply to \"--force refspec\".\n\nNo. --force means \"I know what I am doing, no safety needed, thank you\".\n\nBy applying the safety to --force as well, you lose it as the obvious\ntool that overrides the safety.\n\n-- Hannes\n"},{"id":"222978","messageId":"51DC82A2.8020203@xiplink.com","threadId":"34329","inReplyTo":"7vhag3v59o.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Marc Branchaud","fromEmail":"mbranchaud@xiplink.com","sentAt":"2013-07-09T21:37:38Z","receivedAt":"2013-07-09T21:37:38Z","isPatch":true,"sender":{"key":"mbranchaud@xiplink.com","avatar":null},"body":"On 13-07-09 04:37 PM, Junio C Hamano wrote:\n> Johannes Sixt writes:\n> \n>> Am 09.07.2013 21:53, schrieb Junio C Hamano:\n>>> +--lockref::\n>>> +--lockref=::\n>>> +--lockref=:::\n>>> ...\n>>> +This is meant to make `--force` safer to use.\n>>\n>> This is a contradiction. \"--force\" means \"I mean it, dude\", and not \"I\n>> mean it sometimes\". It would make sense if this sentence were \"This is\n>> meant to make `+refspec` safer to use.\"\n> \n> No, this *IS* making --force safer by letting you to say in addition\n> to --force alone which is blind, add --lockref to defeat it.\n> \n> I do not see any good reason to change the samentics of \"+refspec\"\n> for something like this. \"+refspec\" and \"--force refspec\" have\n> meant the same thing forever. If --lockref adds safety to +refspec,\n> the same safety should apply to \"--force refspec\".\n> \n>> Do you intend to require users to opt in to safety by saying --lockref\n>> until the end of time?\n> \n> For normal users this is *NOT* necessary. I do not know where\n> people are getting the idea of making it default.\n> \n> Rewinding a branch, needing to --force, is an exceptional case.\n\nYes, rewinding is exceptional.\n\nHowever, when a rewind has to happen, I think most users would want to have\nthis feature most of the time. I think anyone who rewinds a shared branch\nwould hate to inadvertently throw away someone else's work. Rare is the\nperson who really won't care about that.\n\nSo I agree with those who say that this would be nice default behaviour. I\nalso don't think we need to make --force different from +refspec, mainly\nbecause if the rewound ref turns out to have moved a simple \"git fetch\" will\nupdate it and likely allow the next rewind attempt to succeed. A helpful\nerror message would make this plain.\n\nI also appreciate the desire to let this stew a while before making it the\ndefault. However, I don't think that leaving it as an option of push will\ngive it enough exposure. I myself want this feature, and I do rewind or\ndelete a branch every few months or so, but I'm almost certainly going to\nforget to use this option the next time the need arises.\n\nBut if it was instead/also a configurable option I could just turn on, that\nwould be awesome.\n\n\nFor the option name, how about --match-baseref ?\n\n\n\t\tM.\n"},{"id":"222979","messageId":"7v38rnv0zt.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"51DC78C0.9030202@kdbg.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-09T22:09:58Z","receivedAt":"2013-07-09T22:09:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> No. --force means \"I know what I am doing, no safety needed, thank you\".\n\nI sympathize the desire to keep a big red button to override\neverything, but it is still not clear how these two independent\nsafety should work together and should possibly seletively be\noverriden.\n\nA proposed ref update can be in one of the four:\n\n 1. The update fast-forwards, and the ref to be updated is at the\n expected place (or you simply do not care what the current value\n is);\n\n 2. The update does not fast-forward, and the ref to be updated is\n at the expected place (or you simply do not care what the\n current value is);\n\n 3. The update fast-forwards, but the ref to be updated is not at the\n expected place; or\n\n 4. The update does not fast-forward, and the ref to be updated is\n not at the expected place.\n\nSo far we had only 1. and 2. because we did not have this \"old value\nhas to be at X\". And --force has been the way to allow 2. to go\nthrough.\n\nNow we are adding 3. and 4. to the mix.\n\nIf --force were the big red button that allows all four, is that\nsufficient to cover the necessary cases, especially given that some\npeople seem to want to make the --lockref on by default (implying\nthat 3. and 4. will both fail by default unless forced in some way)?\nFor example, would there be a case where we want to allow 3. but not\n4. (or vice versa)?\n\nYou _could_ structure the safety into hierarchies:\n\n * safest: no-ff will be rejected, and current value at an\n unexpected place is also rejected. That would be:\n\n $ git push --lockref\n\n * --lockref only: no-ff is not even checked, but current value\n must be at an expected place. How would that be spelled???\n\n $ git push --lockref ???\n\n * --force: anything goes.\n\n $ git push --force --no-lockref\n\nWhere does \"ff-check only\" fit in the hierarchy?\n\nThis is one of the reasons why the original design of \"--lockref\"\nwas to even countermand \"allow non-fast-forward\" (which is the\noriginal meaning of \"--force\").\n\nI _think_ I am OK if we introduced \"--allow-no-ff\" that means the\ncurrent \"--force\" (i.e. \"rewinding is OK\"), that does not defeat the\n\"--lockref\" safety. That is the intended application (you know that\npush does not fast-forward because you rebased, but you also want to\nmake sure there is nothing you are losing by enforcing --lockref\nsafety).\n\nIf that is what happens, then I think \"--force\" that means \"anything\ngoes\" makes sense.\n\nWith the posted series, adding \"--force --no-lockref\" to the command\nline is how to spell that big red button.\n"},{"id":"222981","messageId":"7vvc4jtjqa.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"7v38rnv0zt.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-09T23:08:13Z","receivedAt":"2013-07-09T23:08:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I _think_ I am OK if we introduced \"--allow-no-ff\" that means the\n> current \"--force\" (i.e. \"rewinding is OK\"), that does not defeat the\n> \"--lockref\" safety. That is the intended application (you know that\n> push does not fast-forward because you rebased, but you also want to\n> make sure there is nothing you are losing by enforcing --lockref\n> safety).\n>\n> If that is what happens, then I think \"--force\" that means \"anything\n> goes\" makes sense.\n\nOr perhaps you were implicitly assuming that \"--lockref\" would\nautomatically mean \"I know I am rewinding, so as soon as I say\n--lockref, I mean --allow-no-ff\", and I did not realize that.\n\nIf that is the semantics you are proposing, then I think it makes\nsense to make \"--force\" the big red button that lets anything go.\n\nI was considering \"--lockref\" to be orthogonal to the traditional\n\"ff only check\", and rejecting a push when the updated ref's current\nvalue is expected (i.e. --lockref satisfied) but the update does not\nfast-forward, and that was where my resistance to allow \"--force\" to\noverride \"--lockref\" comes from (because otherwise there is no way\nto say \"I know I want to bypass 'ff-only' check, but instead make\nsure the current value is this\").\n"},{"id":"223095","messageId":"51DF1F56.9000705@kdbg.org","threadId":"34329","inReplyTo":"7vvc4jtjqa.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-07-11T21:10:46Z","receivedAt":"2013-07-11T21:10:46Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 10.07.2013 01:08, schrieb Junio C Hamano:\n> Junio C Hamano writes:\n> \n>> I _think_ I am OK if we introduced \"--allow-no-ff\" that means the\n>> current \"--force\" (i.e. \"rewinding is OK\"), that does not defeat the\n>> \"--lockref\" safety. That is the intended application (you know that\n>> push does not fast-forward because you rebased, but you also want to\n>> make sure there is nothing you are losing by enforcing --lockref\n>> safety).\n>>\n>> If that is what happens, then I think \"--force\" that means \"anything\n>> goes\" makes sense.\n> \n> Or perhaps you were implicitly assuming that \"--lockref\" would\n> automatically mean \"I know I am rewinding, so as soon as I say\n> --lockref, I mean --allow-no-ff\", and I did not realize that.\n\nThat's what I mean, sort of. Because of your 4 cases of a ref update, I\ndo not think that\n\n> 3. The update fast-forwards, but the ref to be updated is not at the\n> expected place; or\n\nis important to consider. The point of --lockref is to avoid data loss,\nbut if the push is fast-forward, there is no data loss.\n\n> If that is the semantics you are proposing, then I think it makes\n> sense to make \"--force\" the big red button that lets anything go.\n> \n> I was considering \"--lockref\" to be orthogonal to the traditional\n> \"ff only check\", and rejecting a push when the updated ref's current\n> value is expected (i.e. --lockref satisfied) but the update does not\n> fast-forward, and that was where my resistance to allow \"--force\" to\n> override \"--lockref\" comes from (because otherwise there is no way\n> to say \"I know I want to bypass 'ff-only' check, but instead make\n> sure the current value is this\").\n\nAgain: Why not just define +refspec as the way to achieve this check?\n\n-- Hannes\n"},{"id":"223099","messageId":"7v8v1crc84.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"51DF1F56.9000705@kdbg.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-11T21:57:47Z","receivedAt":"2013-07-11T21:57:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n>> Or perhaps you were implicitly assuming that \"--lockref\" would\n>> automatically mean \"I know I am rewinding, so as soon as I say\n>> --lockref, I mean --allow-no-ff\", and I did not realize that.\n>\n> That's what I mean, sort of. Because of your 4 cases of a ref update, I\n> do not think that\n>\n>> 3. The update fast-forwards, but the ref to be updated is not at the\n>> expected place; or\n>\n> is important to consider. The point of --lockref is to avoid data loss,\n> but if the push is fast-forward, there is no data loss.\n>\n>> If that is the semantics you are proposing, then I think it makes\n>> sense to make \"--force\" the big red button that lets anything go.\n\nI have a reroll that goes in that direction.\n"},{"id":"223101","messageId":"7vzjtspwvo.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"51DF1F56.9000705@kdbg.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-11T22:14:35Z","receivedAt":"2013-07-11T22:14:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Again: Why not just define +refspec as the way to achieve this check?\n\nWhat justification do we have to break existing people's\nconfiguration that says something like:\n\n\t[remote \"ko\"]\n\t\turl = kernel.org:/pub/scm/git/git.git\n push = master\n push = next\n push = +pu\n push = maint\n\nby adding a _new_ requirement they may not be able to satisify?\nNotice that the above is a typical \"push only\" publishing point,\nwhere you do not need any remote tracking branches.\n\nI am not opposed if your proposal were to introduce a new syntax\nelement that calls for this new feature, e.g.\n\n\t[remote \"ko\"]\n\t\turl = kernel.org:/pub/scm/git/git.git\n push = *pu\n fetch = +refs/heads/*:refs/remotes/ko/*\n\nbut changing what \"+\" means to something new will simply not fly.\n"},{"id":"223172","messageId":"51E03B18.5040502@kdbg.org","threadId":"34329","inReplyTo":"7vzjtspwvo.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-07-12T17:21:28Z","receivedAt":"2013-07-12T17:21:28Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 12.07.2013 00:14, schrieb Junio C Hamano:\n> Johannes Sixt writes:\n> \n>> Again: Why not just define +refspec as the way to achieve this check?\n> \n> What justification do we have to break existing people's\n> configuration that says something like:\n> \n> \t[remote \"ko\"]\n> \t\turl = kernel.org:/pub/scm/git/git.git\n> push = master\n> push = next\n> push = +pu\n> push = maint\n> \n> by adding a _new_ requirement they may not be able to satisify?\n> Notice that the above is a typical \"push only\" publishing point,\n> where you do not need any remote tracking branches.\n\nWhy would it break? When you do not specify --lockref, there is no\nchange whatsoever.\n\nTo achieve any safety at all for these push-only refs, you have to be\nvery explicit by saying --lockref=pu:$myoldpu\n--lockref=master:$myoldmaster etc, and what is wrong if in this case\n--lockref semantics are applied, but only pu is allowed to be no-ff?\n\n> I am not opposed if your proposal were to introduce a new syntax\n> element that calls for this new feature, e.g.\n> \n> \t[remote \"ko\"]\n> \t\turl = kernel.org:/pub/scm/git/git.git\n> push = *pu\n> fetch = +refs/heads/*:refs/remotes/ko/*\n> \n> but changing what \"+\" means to something new will simply not fly.\n\nI still do not see why we need two different kinds of ways to spell the\nsame strong kind of override (--force and +refspec) under the presence\nof --lockref, and why we need a third one (--allow-no-ff) to give a\nweaker kind of override (that makes sense only when --lockref was given\nin the first place).\n\n-- Hannes\n"},{"id":"223197","messageId":"7vli5bllsd.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"51E03B18.5040502@kdbg.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-12T17:40:02Z","receivedAt":"2013-07-12T17:40:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 12.07.2013 00:14, schrieb Junio C Hamano:\n>> Johannes Sixt writes:\n>> \n>>> Again: Why not just define +refspec as the way to achieve this check?\n>> \n>> What justification do we have to break existing people's\n>> configuration that says something like:\n>> \n>> \t[remote \"ko\"]\n>> \t\turl = kernel.org:/pub/scm/git/git.git\n>> push = master\n>> push = next\n>> push = +pu\n>> push = maint\n>> \n>> by adding a _new_ requirement they may not be able to satisify?\n>> Notice that the above is a typical \"push only\" publishing point,\n>> where you do not need any remote tracking branches.\n>\n> Why would it break? When you do not specify --lockref, there is no\n> change whatsoever.\n\nI thought your suggestion \"Why not just define +pu as the way to\nachieve _THIS_ check?\" was to make +pu to mean\n\n\tgit push ko --lockref pu\n\nwhich would mean \"check refs/remotes/ko/pu and make sure the remote\nside still is at that commit\".\n\nIf that is not what you meant, please clarify what _THIS_ is.\n"},{"id":"223210","messageId":"51E0605E.9020902@kdbg.org","threadId":"34329","inReplyTo":"7vli5bllsd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-07-12T20:00:30Z","receivedAt":"2013-07-12T20:00:30Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 12.07.2013 19:40, schrieb Junio C Hamano:\n> Johannes Sixt writes:\n> \n>> Am 12.07.2013 00:14, schrieb Junio C Hamano:\n>>> Johannes Sixt writes:\n>>>\n>>>> Again: Why not just define +refspec as the way to achieve this check?\n>>>\n>>> What justification do we have to break existing people's\n>>> configuration that says something like:\n>>>\n>>> \t[remote \"ko\"]\n>>> \t\turl = kernel.org:/pub/scm/git/git.git\n>>> push = master\n>>> push = next\n>>> push = +pu\n>>> push = maint\n>>>\n>>> by adding a _new_ requirement they may not be able to satisify?\n>>> Notice that the above is a typical \"push only\" publishing point,\n>>> where you do not need any remote tracking branches.\n>>\n>> Why would it break? When you do not specify --lockref, there is no\n>> change whatsoever.\n> \n> I thought your suggestion \"Why not just define +pu as the way to\n> achieve _THIS_ check?\" was to make +pu to mean\n> \n> \tgit push ko --lockref pu\n> \n> which would mean \"check refs/remotes/ko/pu and make sure the remote\n> side still is at that commit\".\n> \n> If that is not what you meant, please clarify what _THIS_ is.\n\nWe have three independent options that the user can choose in any\ncombination:\n\n o --force given or not;\n\n o --lockref semantics enabled or not;\n\n o refspec with or without +;\n\nand these two orthogonal preconditions of the push\n\n o push is fast-forward or it is not (\"ff\", \"noff\");\n\n o the branch at the remote is at the expected rev or it is not\n (\"match\", \"mismatch\").\n\nHere is a table with the expected outcome. \"ok\" means that the push is\nallowed(*), \"fail\" means that the push is denied. (Four more lines with\n--force are omitted because they have \"ok\" in all spots.)\n\n ff noff ff noff\n match match mismatch mismatch\n\n--lockref +refspec ok ok denied denied\n--lockref refspec ok denied denied denied\n +refspec ok ok ok ok\n refspec ok denied ok denied\n\nNotice that without --lockref semantics enabled, +refspec and refspec\nkeep the current behavior.\n\n(*) As we are talking only about the client-side of the push here, I'm\nsaying \"allowed\" instead of \"succeeds\" because the server can have\nadditional restrictions that can make the push fail.\n\n-- Hannes\n"},{"id":"223221","messageId":"7vy59biih4.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"51E0605E.9020902@kdbg.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-12T21:19:51Z","receivedAt":"2013-07-12T21:19:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> We have three independent options that the user can choose in any\n> combination:\n>\n> o --force given or not;\n>\n> o --lockref semantics enabled or not;\n>\n> o refspec with or without +;\n>\n> and these two orthogonal preconditions of the push\n>\n> o push is fast-forward or it is not (\"ff\", \"noff\");\n>\n> o the branch at the remote is at the expected rev or it is not\n> (\"match\", \"mismatch\").\n>\n> Here is a table with the expected outcome. \"ok\" means that the push is\n> allowed(*), \"fail\" means that the push is denied. (Four more lines with\n> --force are omitted because they have \"ok\" in all spots.)\n>\n> ff noff ff noff\n> match match mismatch mismatch\n>\n> --lockref +refspec ok ok denied denied\n> --lockref refspec ok denied denied denied\n\nI am confused with these. The latter is the most typical:\n\n\tgit fetch\n git checkout topic\n git rebase topic\n\tgit push --lockref topic\n\nwhere we know it is \"noff\" already, and we just want to make sure\nthat nobody mucked with our remote while we are rebasing.\n\nIf nobody updated the remote, why should this push be denied? And in\norder to make it succeed, you need to force with +refspec or --force,\nbut that would bypass match/mismatch safety, which makes the whole\n\"make sure the other end is unchanged\" safety meaningless, no?\n\n> +refspec ok ok ok ok\n\nThis is traditional --force.\n\n> refspec ok denied ok denied\n\nWe are not asking for --lockref, so match/mismatch does not affect\nthe outcome.\n\n> Notice that without --lockref semantics enabled, +refspec and refspec\n> keep the current behavior.\n\nBut I do not think the above table with --lockref makes much sense.\n\nLet's look at noff/match case. That is the only interesting one.\n\nThis should fail:\n\n\tgit push topic\n\ndue to no-ff.\n\nYour table above makes this fail:\n\n git push --lockref topic\n\nand the user has to force it, like this?\n\n\tgit push --lockref --force topic ;# or alternatively\n git push --lockref +topic\n\nWhy is it even necessary?\n\nIf you make\n\n\tgit push --lockref topic\n\nsucceed in noff/match case, everything makes more sense to me.\n\nThe --lockref option is merely a weaker form of --force but still a\nway to override the noff check. If the user wants to keep noff\ncheck, the user can simply choose not to use the option.\n\nOf course, that form should fail if \"mismatch\". And then you can\nforce it,\n\n\tgit push --force [--lockref] topic\n\nAs \"--force\" is \"anything goes\", it does not matter if you give the\nother option on the command line.\n\n> (*) As we are talking only about the client-side of the push here, I'm\n> saying \"allowed\" instead of \"succeeds\" because the server can have\n> additional restrictions that can make the push fail.\n\nYes, you and I have known from the beginning that we are in\nagreement on that, but it is a good idea to explicitly say so for\nthe sake of bystanders.\n"},{"id":"223241","messageId":"51E0F93A.8050201@kdbg.org","threadId":"34329","inReplyTo":"7vy59biih4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-07-13T06:52:42Z","receivedAt":"2013-07-13T06:52:42Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 12.07.2013 23:19, schrieb Junio C Hamano:\n> Johannes Sixt writes:\n> \n>> We have three independent options that the user can choose in any\n>> combination:\n>>\n>> o --force given or not;\n>>\n>> o --lockref semantics enabled or not;\n>>\n>> o refspec with or without +;\n>>\n>> and these two orthogonal preconditions of the push\n>>\n>> o push is fast-forward or it is not (\"ff\", \"noff\");\n>>\n>> o the branch at the remote is at the expected rev or it is not\n>> (\"match\", \"mismatch\").\n>>\n>> Here is a table with the expected outcome. \"ok\" means that the push is\n>> allowed(*), \"fail\" means that the push is denied. (Four more lines with\n>> --force are omitted because they have \"ok\" in all spots.)\n>>\n>> ff noff ff noff\n>> match match mismatch mismatch\n>>\n>> --lockref +refspec ok ok denied denied\n>> --lockref refspec ok denied denied denied\n> \n> I am confused with these. The latter is the most typical:\n> \n> \tgit fetch\n> git checkout topic\n> git rebase topic\n> \tgit push --lockref topic\n> \n> where we know it is \"noff\" already, and we just want to make sure\n> that nobody mucked with our remote while we are rebasing.\n\nToday (without --lockref), the above sequence would fail to push.\n(Because there is no + and no --force.)\n\n> If nobody updated the remote, why should this push be denied? And in\n> order to make it succeed, you need to force with +refspec or --force,\n> but that would bypass match/mismatch safety, which makes the whole\n> \"make sure the other end is unchanged\" safety meaningless, no?\n\nI am suggesting that +refspec would *not* override the match/mismatch\nsafety, but --force would.\n\n> \n>> +refspec ok ok ok ok\n> \n> This is traditional --force.\n> \n>> refspec ok denied ok denied\n> \n> We are not asking for --lockref, so match/mismatch does not affect\n> the outcome.\n\nI think you are worried that a deviation from the principle that\n+refspec == --force hurts current users. But I am arguing that this is\nnot the case because \"current\" users do not use --lockref. As you have\nseen from the table, without --lockref there is *no change* in behavior.\n\nI still have not seen an example where +refspec != --force would have\nunexpected consequences. (The inequality is merely that +refspec fails\non mismatch when --lockref was also given while --force does not.)\n\n>> Notice that without --lockref semantics enabled, +refspec and refspec\n>> keep the current behavior.\n> \n> But I do not think the above table with --lockref makes much sense.\n> \n> Let's look at noff/match case. That is the only interesting one.\n> \n> This should fail:\n> \n> \tgit push topic\n> \n> due to no-ff.\n\nYes.\n\n> Your table above makes this fail:\n> \n> git push --lockref topic\n> \n> and the user has to force it,\n\nOf course.\n\n> like this?\n> \n> \tgit push --lockref --force topic ;# or alternatively\n> git push --lockref +topic\n> \n> Why is it even necessary?\n\nBecause it is no-ff. How do you achieve the push today (without\n--lockref)? You use one of these two options. It does not change with\n--lockref.\n\n> If you make\n> \n> \tgit push --lockref topic\n> \n> succeed in noff/match case, everything makes more sense to me.\n\nNot to me, obviously ;)\n\n> The --lockref option is merely a weaker form of --force but still a\n> way to override the noff check.\n\nNo; --lockref only adds the check that the destination is at the\nexpected revision, but does *NOT* override the no-ff check. Why should\nit? (This is not a rethoric question.)\n\n(I think I said differently in an earlier messages, but back then things\nwere still blurry. The table in my previous message is what I mean.)\n\n> If the user wants to keep noff\n> check, the user can simply choose not to use the option.\n\nNo. If the user wants to keep the no-ff check, she does not use the + in\nthe refspec and does not use --force. (Just like today.)\n\n> Of course, that form should fail if \"mismatch\". And then you can\n> force it,\n> \n> \tgit push --force [--lockref] topic\n> \n> As \"--force\" is \"anything goes\", it does not matter if you give the\n> other option on the command line.\n\n... or the + in the refsepc.\n\n-- Hannes\n"},{"id":"223249","messageId":"7vwqougwec.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"51E0F93A.8050201@kdbg.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-13T18:14:19Z","receivedAt":"2013-07-13T18:14:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> I am suggesting that +refspec would *not* override the match/mismatch\n> safety, but --force would.\n\nOK.\n\nI earlier did not read from your message that you wanted to change\n\"+refspec\" to mean \"allow non-ff push\", so the two entries in your\ntable:\n\n> ff noff ff noff\n> match match mismatch mismatch\n>\n> --lockref +refspec ok ok denied denied\n> --lockref refspec ok denied denied denied\n\ndid not make sense to me. If you are making \"+refspec\" to mean\n\"--allow-no-ff refspec\", then above is at least internally\nconsistent.\n\n>> Let's look at noff/match case. That is the only interesting one.\n>> \n>> This should fail:\n>> \n>> \tgit push topic\n>> \n>> due to no-ff.\n>\n> Yes.\n>\n>> Your table above makes this fail:\n>> \n>> git push --lockref topic\n>> \n>> and the user has to force it,\n>\n> Of course.\n>\n>> like this?\n>> \n>> \tgit push --lockref --force topic ;# or alternatively\n>> git push --lockref +topic\n>> \n>> Why is it even necessary?\n\n> Because it is no-ff. How do you achieve the push today (without\n> --lockref)? You use one of these two options. It does not change with\n> --lockref.\n\nBut by going that route, you are making --lockref _less_ useful, no?\n\n\"git push topic\" in no-ff/match case fails as it should. The whole\npurpose of \"--lockref\" is to make this case easier and safer than\nthe today's system, where the anything-goes \"--force\" is the only\nway to make this push. We want to give a user who\n\n - rebased the topic, and\n\n - knows where the topic at the remote should be\n\na way to say \"I know I am pushing a no-ff, and I want to make sure\nthe current value is this\" in order to avoid losing somebody else's\nwork queued on top of the topic at the remote while he was rebasing.\n\nYou _CAN_ introduce a new --allow-no-ff at the same time and fail a\nno-ff/match push:\n\n\tgit push --lockref topic\n\nand then allow it back with:\n\n\tgit push --lockref --allow-no-ff topic\n\tgit push --lockref +topic ;# +topic is now --allow-no-ff topic\n\nbut why _SHOULD_ we? As soon as the user _says_ --lockref, the user\nis telling us he is pushing a no-ff. If that is not the case, the\nuser can push without --lockref in the first place.\n\nThe only potential thing you are gaining with such a change is that\nyou are allowing people to say \"this will fast-forward _and_ the I\nknow the current value; if either of these two assumptions is\nviolated, please fail this push\".\n\nIf \"--lockref\" automatically implies \"--allow-no-ff\" (the design in\nthe reposted patch), you cannot express that combination. But once\nyou use \"--lockref\" in such a situation , for the push to succeed,\nyou know that the push replaces not just _any_ ancestor of what you\nare pushing, but replaces the exact current value. So I do not think\nyour implicit introduction of --allow-no-ff via redefining the\nsemantics of the plus prefix is not adding much value (if any),\nwhile making the common case less easy to use.\n\n> No; --lockref only adds the check that the destination is at the\n> expected revision, but does *NOT* override the no-ff check.\n\nYou _could_ do it in that way, but that is less useful.\n"},{"id":"223252","messageId":"7vr4f2gr4m.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"7vwqougwec.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-13T20:08:09Z","receivedAt":"2013-07-13T20:08:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> If \"--lockref\" automatically implies \"--allow-no-ff\" (the design in\n> the reposted patch), you cannot express that combination. But once\n> you use \"--lockref\" in such a situation , for the push to succeed,\n> you know that the push replaces not just _any_ ancestor of what you\n> are pushing, but replaces the exact current value. So I do not think\n> your implicit introduction of --allow-no-ff via redefining the\n> semantics of the plus prefix is not adding much value (if any),\n> while making the common case less easy to use.\n>\n>> No; --lockref only adds the check that the destination is at the\n>> expected revision, but does *NOT* override the no-ff check.\n>\n> You _could_ do it in that way, but that is less useful.\n\nAnother issue I have with the proposal is that we close the door to\n\"force only this one\" convenience we have with \"+ref\" vs \"--force\nref\". Assuming that it is useful to require lockref while still\nmaking sure that the usual \"must fast-forward\" rule is followed (if\nthat is not the case, I do not see a reason why your proposal is any\nuseful---am I missing something?), I would prefer to allow users a\nway to decorate this basic syntax to say:\n\n git push --lockref master jch pu\n\nthings like\n\n (1) pu may not fast-forward and please override that \"must\n fast-forward\" check from it, while still keeping the lockref\n safety (e.g. \"+pu\" that does not --force, which is your\n proposal);\n\n (2) any of them may not fast-forward and please override that \"must\n fast-forward\" check from it, while still keeping the lockref\n safety (without adding \"--allow-no-ff\", I do not see how it is\n possible with your proposal, short of forcing user to add \"+\"\n everywhere);\n\n (3) I know jch does not fast-forward so please override the \"must\n fast-forward\", but still apply the lockref safety, pu may not\n even satisfy lockref safety so please force it (as the \"only\n force this one\" semantics is removed from \"+\", I do not see how\n it is possible with your proposal).\n\nSo I would understand if your proposal _were_ to\n\n * add \"--allow-no-ff\" option;\n\n * change the meaning of \"+ref\" to \"--allow-no-ff for only this\n ref\"; and\n\n * add a new \"*ref\" (or whatever new syntax) to still allow people\n to say \"--force only this ref\".\n\nbut we still need to assume that it makes sense to ask lockref but\nstill want to ensure the update fast-forwards. I personally do not\nthink it does [*1*].\n\nThe semantics the posted patch (rerolled to allow \"--force\" push\nanything) implements lets \"--lockref\" to imply \"--allow-no-ff\" and\nthat makes it much simpler; we do not have to deal with any of the\nabove complexity.\n\n\n[Footnote]\n\n *1* The assurance --lockref gives is a lot stronger than \"must\n fast-forward\". You may have fetched the topic whose tip was at\n commit X, and rebased it on top of X~4 to create a new history\n leading to Y.\n\n o----o----Y\n /\n o---o----o----o----o----X\n\tX~4\n\n When you \"git push --lockref=topic:X Y:X\", you are requiring\n their tip to be still at X. Other people's change cannot be to\n add something on top of X (which will be lost if we replace the\n tip of the topic with Y).\n\n If your change were not a rebase but to build one of you own:\n\n o---o----o----o----o----X---Y\n\n your \"git push --lockref=topic:X Y:X\" still requires the tip is\n at X. If somebody rewound the tip to X~2 in the meantime\n (because they decided the tip 2 commits were not good), your\n \"git push Y:X\" without the \"--lockref\" will lose their rewind,\n because Y will still be a fast-forward update of X~2.\n \"--lockref=topic:X\" will protect you in this case as well.\n\n So I think \"--lockref\" that automatically disables \"must\n fast-forward\" check is the right thing to do, as we are\n replacing the weaker \"must fast-forward\" with something\n stronger. I do not think we are getting anything from forcing\n the user to say \"--allow-no-ff\" with \"+ref\" syntax when the\n user says \"--lockref\". It is not making it safer, and it is\n making it less convenient.\n"},{"id":"223253","messageId":"51E1B5DB.9080904@kdbg.org","threadId":"34329","inReplyTo":"7vwqougwec.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-07-13T20:17:31Z","receivedAt":"2013-07-13T20:17:31Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 13.07.2013 20:14, schrieb Junio C Hamano:\n> Johannes Sixt writes:\n>>> Your table above makes this fail:\n>>>\n>>> git push --lockref topic\n>>>\n>>> and the user has to force it,\n>>\n>> Of course.\n>>\n>>> like this?\n>>>\n>>> \tgit push --lockref --force topic ;# or alternatively\n>>> git push --lockref +topic\n>>>\n>>> Why is it even necessary?\n> \n>> Because it is no-ff. How do you achieve the push today (without\n>> --lockref)? You use one of these two options. It does not change with\n>> --lockref.\n> \n> But by going that route, you are making --lockref _less_ useful, no?\n> \n> \"git push topic\" in no-ff/match case fails as it should. The whole\n> purpose of \"--lockref\" is to make this case easier and safer than\n> the today's system, where the anything-goes \"--force\" is the only\n> way to make this push. We want to give a user who\n> \n> - rebased the topic, and\n> \n> - knows where the topic at the remote should be\n> \n> a way to say \"I know I am pushing a no-ff, and I want to make sure\n> the current value is this\" in order to avoid losing somebody else's\n> work queued on top of the topic at the remote while he was rebasing.\n> \n> You _CAN_ introduce a new --allow-no-ff at the same time and fail a\n> no-ff/match push:\n> \n> \tgit push --lockref topic\n> \n> and then allow it back with:\n> \n> \tgit push --lockref --allow-no-ff topic\n> \tgit push --lockref +topic ;# +topic is now --allow-no-ff topic\n> \n> but why _SHOULD_ we? As soon as the user _says_ --lockref, the user\n> is telling us he is pushing a no-ff. If that is not the case, the\n> user can push without --lockref in the first place.\n> \n> The only potential thing you are gaining with such a change is that\n> you are allowing people to say \"this will fast-forward _and_ the I\n> know the current value; if either of these two assumptions is\n> violated, please fail this push\".\n> \n> If \"--lockref\" automatically implies \"--allow-no-ff\" (the design in\n> the reposted patch), you cannot express that combination. But once\n> you use \"--lockref\" in such a situation , for the push to succeed,\n> you know that the push replaces not just _any_ ancestor of what you\n> are pushing, but replaces the exact current value. So I do not think\n> your implicit introduction of --allow-no-ff via redefining the\n> semantics of the plus prefix is not adding much value (if any),\n> while making the common case less easy to use.\n> \n>> No; --lockref only adds the check that the destination is at the\n>> expected revision, but does *NOT* override the no-ff check.\n> \n> You _could_ do it in that way, but that is less useful.\n\nAll you have been saying is that you find your\n\n git push --lockref there topic\n\nis more useful than my\n\n git push --lockref there +topic\n\nYou are trading crystal clear semantics to save users ONE character to\ntype. IMO, it's a bad deal.\n\nThe crystal clear semantics would be:\n\n - to override no-ff safety, use +refspec;\n\n - to override \"mismatch\" safety, do not use --lockref/use --no-lockref;\n\n - do not use --force unless you know the consequences.\n\nI actually think that by implying allow-no-ff in --lockref, you are\nhurting users who have configured a push refspec without a + prefix:\nThey suddenly do not get the push denied when it is not a fast-forward\nanymore. For example, when you have\n\n [remote \"ko\"]\n push = master\n push = +pu\n\nand you accidentally rewound master before the point that is already\npublished, then\n\n git push --lockref ko\n\nwill happily push the rewound master.\n\n-- Hannes\n"},{"id":"223255","messageId":"51E1C27B.7070705@kdbg.org","threadId":"34329","inReplyTo":"7vr4f2gr4m.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-07-13T21:11:23Z","receivedAt":"2013-07-13T21:11:23Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 13.07.2013 22:08, schrieb Junio C Hamano:\n> Junio C Hamano writes:\n> \n>> If \"--lockref\" automatically implies \"--allow-no-ff\" (the design in\n>> the reposted patch), you cannot express that combination. But once\n>> you use \"--lockref\" in such a situation , for the push to succeed,\n>> you know that the push replaces not just _any_ ancestor of what you\n>> are pushing, but replaces the exact current value. So I do not think\n>> your implicit introduction of --allow-no-ff via redefining the\n>> semantics of the plus prefix is not adding much value (if any),\n>> while making the common case less easy to use.\n>>\n>>> No; --lockref only adds the check that the destination is at the\n>>> expected revision, but does *NOT* override the no-ff check.\n>>\n>> You _could_ do it in that way, but that is less useful.\n> \n> Another issue I have with the proposal is that we close the door to\n> \"force only this one\" convenience we have with \"+ref\" vs \"--force\n> ref\". Assuming that it is useful to require lockref while still\n> making sure that the usual \"must fast-forward\" rule is followed (if\n> that is not the case, I do not see a reason why your proposal is any\n> useful---am I missing something?),\n\nThe ability to express \"require both fast-forward and --lockref\" is just\nan artefact of the independence of fast-forward-ness and --lockref in my\nproposal. It is not something that I think is absolutely necessary.\n\n> I would prefer to allow users a\n> way to decorate this basic syntax to say:\n> \n> git push --lockref master jch pu\n> \n> things like\n> \n> (1) pu may not fast-forward and please override that \"must\n> fast-forward\" check from it, while still keeping the lockref\n> safety (e.g. \"+pu\" that does not --force, which is your\n> proposal);\n\nThat must be a misunderstanding. In my proposal\n\n git push --lockref +pu\n\nwould do what you need here. I don't know where you get the idea that\nthese two\n\n git push --lockref +pu\n git push +pu\n\nwould be different with regard to non-fast-forward-ness. The table\nentries were correct.\n\n[Please do not use the option name \"--force\" in the discussion unless\nyou mean \"all kinds of safety off\".]\n\n> (2) any of them may not fast-forward and please override that \"must\n> fast-forward\" check from it, while still keeping the lockref\n> safety (without adding \"--allow-no-ff\", I do not see how it is\n> possible with your proposal, short of forcing user to add \"+\"\n> everywhere);\n\nThe point of my proposal is to force users to add + when they want to\nallow non-fast-forward. Usually, this is shorter to type anyway than to\ninsert --force or --allow-no-ff in the command.\n\n> \n> (3) I know jch does not fast-forward so please override the \"must\n> fast-forward\", but still apply the lockref safety, pu may not\n> even satisfy lockref safety so please force it (as the \"only\n> force this one\" semantics is removed from \"+\", I do not see how\n> it is possible with your proposal).\n\nI think\n\n git push --lockref=jch +jch +pu\n\nwould do.\n\n> The semantics the posted patch (rerolled to allow \"--force\" push\n> anything) implements lets \"--lockref\" to imply \"--allow-no-ff\" and\n> that makes it much simpler; we do not have to deal with any of the\n> above complexity.\n\nBut see my other post, where this hurts users who have a fast-forward\npush refspec configured.\n\n> [Footnote]\n> \n> *1* The assurance --lockref gives is a lot stronger than \"must\n> fast-forward\".\n...\n> If your change were not a rebase but to build one of you own:\n> \n> o---o----o----o----o----X---Y\n> \n> your \"git push --lockref=topic:X Y:X\" still requires the tip is\n> at X. If somebody rewound the tip to X~2 in the meantime\n> (because they decided the tip 2 commits were not good), your\n> \"git push Y:X\" without the \"--lockref\" will lose their rewind,\n> because Y will still be a fast-forward update of X~2.\n> \"--lockref=topic:X\" will protect you in this case as well.\n\nGood point.\n\n> So I think \"--lockref\" that automatically disables \"must\n> fast-forward\" check is the right thing to do, as we are\n> replacing the weaker \"must fast-forward\" with something\n> stronger.\n\nBut I do not share this conclusion. My conclusion is that your proposal\nreplaces one kind of check with a very different kind of check.\n\n> I do not think we are getting anything from forcing\n> the user to say \"--allow-no-ff\" with \"+ref\" syntax when the\n> user says \"--lockref\".\n\nIs this the same misunderstanding? My proposal does not require\n--allow-no-ff with +ref syntax when --lockref is used.\n\n-- Hannes\n"},{"id":"223325","messageId":"20130714142806.GA2239@serenity.lan","threadId":"34329","inReplyTo":"7vr4f2gr4m.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-07-14T14:28:06Z","receivedAt":"2013-07-14T14:28:06Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Sat, Jul 13, 2013 at 01:08:09PM -0700, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n> > If \"--lockref\" automatically implies \"--allow-no-ff\" (the design in\n> > the reposted patch), you cannot express that combination. But once\n> > you use \"--lockref\" in such a situation , for the push to succeed,\n> > you know that the push replaces not just _any_ ancestor of what you\n> > are pushing, but replaces the exact current value. So I do not think\n> > your implicit introduction of --allow-no-ff via redefining the\n> > semantics of the plus prefix is not adding much value (if any),\n> > while making the common case less easy to use.\n> >\n> >> No; --lockref only adds the check that the destination is at the\n> >> expected revision, but does *NOT* override the no-ff check.\n> >\n> > You _could_ do it in that way, but that is less useful.\n> \n> Another issue I have with the proposal is that we close the door to\n> \"force only this one\" convenience we have with \"+ref\" vs \"--force\n> ref\". Assuming that it is useful to require lockref while still\n> making sure that the usual \"must fast-forward\" rule is followed (if\n> that is not the case, I do not see a reason why your proposal is any\n> useful---am I missing something?), I would prefer to allow users a\n> way to decorate this basic syntax to say:\n> \n> git push --lockref master jch pu\n> \n> things like\n> \n> (1) pu may not fast-forward and please override that \"must\n> fast-forward\" check from it, while still keeping the lockref\n> safety (e.g. \"+pu\" that does not --force, which is your\n> proposal);\n> \n> (2) any of them may not fast-forward and please override that \"must\n> fast-forward\" check from it, while still keeping the lockref\n> safety (without adding \"--allow-no-ff\", I do not see how it is\n> possible with your proposal, short of forcing user to add \"+\"\n> everywhere);\n> \n> (3) I know jch does not fast-forward so please override the \"must\n> fast-forward\", but still apply the lockref safety, pu may not\n> even satisfy lockref safety so please force it (as the \"only\n> force this one\" semantics is removed from \"+\", I do not see how\n> it is possible with your proposal).\n\nI haven't been following this thread too closely, but I was assuming\nthat the interface would be something like this:\n\n git push origin +master\n git push --force origin master\n\nmean the same thing and do what they do now.\n\n git push origin *master\n git push --lockref origin master\n\nboth mean the same thing: using the new compare-and-swap mode only\nupdate master if the remote side corresponds to remotes/origin/master\n[1].\n\n git push origin *master:refs/heads/master:@{1}\n\nmeans to push the local ref master to the remote ref refs/heads/master\nif it currently points at \"@{1}\".\n\nIn this scenario, giving both --lockref and --force should be an error\nbecause the user is probably confused (the obvious interpretation is\nthat --force wins, but I don't think that's sensible).\n\nI'm not sure what should happen with:\n\n git push --force origin *master\n\nwhere it appears that the user is asking for a compare-and-swap update\nof master but the --force is overriding this. I think we have to let\n--force win because when the refspec comes from remote..push we\nhave to let the command-line --force override the specified behaviour.\n\nI don't particularly like the name --lockref, the original --cas feels\nmore descriptive to me.\n\n\n[1] In fact, I suspect this would have to be \"the ref that\n refs/heads/master maps to using remote.origin.fetch\".\n"},{"id":"223344","messageId":"7v61wdgdd1.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"51E1B5DB.9080904@kdbg.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-14T19:17:46Z","receivedAt":"2013-07-14T19:17:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> All you have been saying is that you find your\n>\n> git push --lockref there topic\n>\n> is more useful than my\n>\n> git push --lockref there +topic\n>\n> You are trading crystal clear semantics to save users ONE character to\n> type. IMO, it's a bad deal.\n\nThink how you would explain the option in a tutorial for those who\nuse the push.default=simple semantics.\n\n\"\"\"\n\tYou usually do\n\n\t\t$ git pull [--rebase]\n\n\tto integrate with the shared branch and push it back with\n\n\t\t$ git push\n\n\tSometimes the project wants to rewind the tip of such a\n\tshared branch (perhaps a bad commit included inappropriate\n\tmaterial that should not be in the history). You cordinate\n\tthe decision to do such a rewinding with others in the\n\tproject, you \"git rebase [-i]\" to prepare a replacement\n\thistory, and then try to push tthe result out. However\n\n\t\t$ git push\n\n\twill fail, because this does not fast-forward. But you and\n\tyour colleagues agreed that the project wants this new\n\thistory!\n\n With older Git, the only way to make this push go through\n was to \"--force\" it. That will risk losing work of other\n people who were not aware of the collective decision to\n rewind this shared branch [discussion of lockref safety\n comes here]. Instead you can use\n\n\t\t$ git push --lockref\n\n\"\"\"\n\nHow does the last line look with your \"--lockref does not override\nmust-fast-forward\" proposal?\n\n\"\"\"\n\n\tIf your current branch is configured to push to update the\n\tbranch 'frotz' of the remote 'origin' (replace these two\n\tappropriately for your situation), you would say:\n\n\t\t$ git push --lockref origin +HEAD:frotz\n\n\"\"\"\n\nHow is that crystal clear? You are just making things more complex\nand harder to learn (I was tempted to add \"for no good reason\" here,\nbut I'd assume that probably you haven't explained your reasons well\nenough to be heard).\n\n> The crystal clear semantics would be:\n>\n> - to override no-ff safety, use +refspec;\n>\n> - to override \"mismatch\" safety, do not use --lockref/use --no-lockref;\n>\n> - do not use --force unless you know the consequences.\n\nAlternatively, this is also crystal clear\n\n - to use the full safety, do not use anything funky\n\n - to push a history that does not fast-forward safely, use\n --lockref\n\n - do not use --force unless you know the consequences.\n\nand that is what the patch does.\n\n> I actually think that by implying allow-no-ff in --lockref, you are\n> hurting users who have configured a push refspec without a + prefix:\n> They suddenly do not get the push denied when it is not a fast-forward\n> anymore.\n\nOf course, that is why you should not use --lockref when you do not\nhave to. It is a tool to loosen \"must fast-forward\" in a more\ncontrolled way than the traditional \"--force\".\n\n For example, when you have\n>\n> [remote \"ko\"]\n> push = master\n> push = +pu\n>\n> and you accidentally rewound master before the point that is already\n> published, then\n>\n> git push --lockref ko\n>\n> will happily push the rewound master.\n\nYes, and I am not (and I do expect nobody is) stupid to use --lockref\nin such a situation where there is no need to do so.\n"},{"id":"223348","messageId":"51E3084D.2040504@kdbg.org","threadId":"34329","inReplyTo":"7v61wdgdd1.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-07-14T20:21:33Z","receivedAt":"2013-07-14T20:21:33Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 14.07.2013 21:17, schrieb Junio C Hamano:\n> Johannes Sixt writes:\n>> I actually think that by implying allow-no-ff in --lockref, you are\n>> hurting users who have configured a push refspec without a + prefix:\n>> They suddenly do not get the push denied when it is not a fast-forward\n>> anymore.\n> \n> Of course, that is why you should not use --lockref when you do not\n> have to. It is a tool to loosen \"must fast-forward\" in a more\n> controlled way than the traditional \"--force\".\n\nSorry, IMO, this goes into a totally wrong direction, in particular, I\nthink that this is going to close to door to make --lockref the default\nsome day in a way that helps everyone.\n\nI think I have not understood your motivations for this feature, and I\nam not able spend more mindwidth on arguing back and forth to make it\nmore usable (again: IMO).\n\nSo, I bow out, and I appologize to have wasted so much of your time.\n\n-- Hannes\n"},{"id":"223349","messageId":"20130714203403.GE8564@google.com","threadId":"34329","inReplyTo":"51E3084D.2040504@kdbg.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-07-14T20:34:03Z","receivedAt":"2013-07-14T20:34:03Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Johannes Sixt wrote:\n\n> Sorry, IMO, this goes into a totally wrong direction, in particular, I\n> think that this is going to close to door to make --lockref the default\n> some day in a way that helps everyone.\n\nWould a '*' that acts like --lockref on a per ref basis address your\nconcerns?\n\nI realize that that design would hurt a project of making '+' use\nlockref automatically some day. I think that's ok, and that '+'\nmeaning \"push whatever I have, regardless of what's on the other end,\nand I mean it\" would be better semantics in the long term (which\ndoesn't match the current behavior either :/).\n\nJonathan\n"},{"id":"223350","messageId":"20130714204920.GF8564@google.com","threadId":"34329","inReplyTo":"20130714203403.GE8564@google.com","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-07-14T20:49:20Z","receivedAt":"2013-07-14T20:49:20Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n> Johannes Sixt wrote:\n\n>> Sorry, IMO, this goes into a totally wrong direction, in particular, I\n>> think that this is going to close to door to make --lockref the default\n>> some day in a way that helps everyone.\n>\n> Would a '*' that acts like --lockref on a per ref basis address your\n> concerns?\n\n(Aside: '*' is not a great character for that. * is already taken in\nrefspec syntax. There's no clash but the two uses would be confusing.\n\n\t*:\n\t*:*\n\nSome other single-character prefix could work, such as '.' or '~'.)\n"},{"id":"223351","messageId":"51E31131.3070005@kdbg.org","threadId":"34329","inReplyTo":"20130714203403.GE8564@google.com","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-07-14T20:59:29Z","receivedAt":"2013-07-14T20:59:29Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 14.07.2013 22:34, schrieb Jonathan Nieder:\n> Johannes Sixt wrote:\n> \n>> Sorry, IMO, this goes into a totally wrong direction, in particular, I\n>> think that this is going to close to door to make --lockref the default\n>> some day in a way that helps everyone.\n> \n> Would a '*' that acts like --lockref on a per ref basis address your\n> concerns?\n\nNo, because I think that new syntax is not necessary.\n\nBut admittedly, I haven't spent any time to think about push.default\nmodes other than 'matching'. In particular, I wonder how Junio's last\nexample with push.default=simple can work today:\n\n $ git pull --rebase # not a merge\n $ git push\n\nbecause it is not a fast-forward. I am assuming that a +refspec must be\nin the game somehow. Why would we then need that --lockref implies\nallow-no-ff when we already have +refspec that already means allow-no-ff?\n\nBut as I said, I'm not familiar with push.default other than matching\nand my assumption may be wrong.\n\n-- Hannes\n"},{"id":"223352","messageId":"20130714212800.GA11009@google.com","threadId":"34329","inReplyTo":"51E31131.3070005@kdbg.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-07-14T21:28:00Z","receivedAt":"2013-07-14T21:28:00Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Johannes Sixt wrote:\n> Am 14.07.2013 22:34, schrieb Jonathan Nieder:\n\n>> Would a '*' that acts like --lockref on a per ref basis address your\n>> concerns?\n>\n> No, because I think that new syntax is not necessary.\n>\n> But admittedly, I haven't spent any time to think about push.default\n> modes other than 'matching'. In particular, I wonder how Junio's last\n> example with push.default=simple can work today:\n>\n> $ git pull --rebase # not a merge\n> $ git push\n>\n> because it is not a fast-forward.\n\nRight, let's examine this example more closely.\n\nIf I run:\n\n\t(1) git pull --rebase\n\t(2) git push\n\nthen normally that push will be a fast-forward. My changes are\non top of the new upstream changes, just as though I used format-patch\nand send-email to submit the changes to a maintainer who would then\napply them.\n\nHowever, someone else might have pushed to the same branch between\nstep (1) and (2), causing the fast-forward-only push to fail.\n\nUsually that means other person made a valuable change and I can\nsimply repeat steps (1), and (2) and they will succeed.\n\nBut maybe that intervening push was a mistake. To distinguish that\npossibility I might do something like\n\n\t(3) git fetch origin\n\t(4) gitk @{u}@{1}..@{u}; # Is the change good?\n\n\t(5a) git pull --rebase; git push; # Yes, put my change on top of it\n\t(5b) git push --force; # No, my change is better!\n\nSo far so good. But what if yet another change is made upstream\nbetween step (3) and (5)?\n\nIf following approach (5a), that's fine. We notice the new\nintervening change and react accordingly, again. There is a\npossibility of starvation, but no other harm done.\n\nIn case (5b), it may be a serious problem. I don't know about the\nintervening change until I read the \"git push\" output, and in the\nusual case I just won't notice. The new lockref UI is meant to\naddress this problem. So in the new world order, in case (5b) it\nsounds like I should have instead used\n\n\t(5b') git push --allow-non-ff\n\nSuppose I am writing a script that is meant to set the remote\nrepository to a known state. Other contributors are only using\nfast-forward updates so once my change goes in they will act\nappropriately. I just need to get my ref update in, without being\nblocked by other ref updates.\n\nThen I will use\n\n\t(5c) git push --force\n\nwhich means not to use this new lockref trick that looks at my\nremote-tracking branch and instead to just force the ref update. This\nwould for example be the right semantics when pushing to a mirror from\na relay that also fetches from a canonical repository. It avoids\nneeding to fetch from the target repo before every push.\n\nOf course if ref updates are highly contended, even the current \"git\npush --force\" will sometimes fail, since it internally *does* use a\ncompare-and-swap against the result of an ls-remote. That's a (minor)\nbug, imho. Fixing it will require tweaking the protocol to make the\ncompare-and-swap optional.\n"},{"id":"223368","messageId":"7vd2qkfpm8.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"51E3084D.2040504@kdbg.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-15T03:50:39Z","receivedAt":"2013-07-15T03:50:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 14.07.2013 21:17, schrieb Junio C Hamano:\n>> Johannes Sixt writes:\n>>> I actually think that by implying allow-no-ff in --lockref, you are\n>>> hurting users who have configured a push refspec without a + prefix:\n>>> They suddenly do not get the push denied when it is not a fast-forward\n>>> anymore.\n>> \n>> Of course, that is why you should not use --lockref when you do not\n>> have to. It is a tool to loosen \"must fast-forward\" in a more\n>> controlled way than the traditional \"--force\".\n>\n> Sorry, IMO, this goes into a totally wrong direction, in particular, I\n> think that this is going to close to door to make --lockref the default\n> some day in a way that helps everyone.\n\nI would presume that you would force that \"reverse tracking\"\nshort-hand as the expected value, as \"default\" will not have other\nsources of information.\n\nI think the use of \"reverse tracking\" is way overrated. It is\nprobably the only default value that we could use, if the user is\ntoo lazy not to specify it, but I do not think it is particularly a\nsensible or safe default.\n\nThe following does not discuss \"should --lockref automatically\ndisable the 'must fast-forward' check?\". The problem highlighted is\nthe same, regardless of the answer to that question.\n\nAfter rebasing beyond what is already published, you try the\n\"lockref\" push, e.g. (we assume you work on master and push back to\nupdate master at your origin):\n\n\t$ git fetch\n $ git rebase -i @{u}~4 ;# rebase beyond what is there\n $ git push ;# of course this will not fast-forward\n $ git push --lockref\n\t... or with your \"must-fast-forward is independent\"\n\t$ git push --lockref origin +master\n ... or also with your \"--lockref is default\"\n\t$ git push origin +master\n\nIf somebody else pushed while you are working on the rebase, the\nlast step (one of the above push) will fail due to stale\nexpectation. What now?\n\nThe user would want to keep the updated tip, so the first thing that\nhappens will always be\n\n\t$ git fetch\n\t$ git log ..@{u} ;# what will we be losing?\n\nThe right thing to do at this point is to rebase your 'master' again\non top of @{u}\n\n\t$ git rebase -i @{u}\n\nbefore attempting to push back again. If you do that, then you can\ndo another \"lockref\" push.\n\nBut the thing is, a novice who does not know what he is doing will\nlikely to do this:\n\n $ git push --lockref\n\t... or with your \"must-fast-forward is independent\"\n\t$ git push --lockref origin +master\n ... or also with your \"--lockref is default\"\n\t$ git push origin +master\n\n\t... rejected due to stale expectation\n $ git fetch\n\nYou just have updated the lockref base, so if you did, without doing\nanything else, \n\n\t$ git push origin +master\n\nthen you will lose the updated contents.\n\nThe conclusion? It does not make sense to make \"lockref\" the\ndefault.\n\nThe --lockref mechanism is necessary _only_ when you want to break\nthe usual \"must fast-forward\" safety, and the user needs to be made\nvery aware of what he is doing. Making it default and making it\nappear easy to invoke with a single \"+\", is totally going in a wrong\ndirection. Besides, by making it the default and turning \"+\" into\n\"only defeat 'must fast-forward\", you will break existing setting of\npeople who have \"remote.*.push = +ref\" configured, without having a\nremote-tracking for that ref.\n\nSo it will not happen; \"lockref\" will not be on by default, even if\nit is made independent of \"must fast-forward\".\n"},{"id":"223370","messageId":"7v4nbwfooj.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"20130714212800.GA11009@google.com","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-15T04:10:52Z","receivedAt":"2013-07-15T04:10:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> \t(4) gitk @{u}@{1}..@{u}; # Is the change good?\n>\n> \t(5a) git pull --rebase; git push; # Yes, put my change on top of it\n> \t(5b) git push --force; # No, my change is better!\n>\n> So far so good. But what if yet another change is made upstream\n> between step (3) and (5)?\n>\n> If following approach (5a), that's fine. We notice the new\n> intervening change and react accordingly, again. There is a\n> possibility of starvation, but no other harm done.\n>\n> In case (5b), it may be a serious problem. I don't know about the\n> intervening change until I read the \"git push\" output, and in the\n> usual case I just won't notice. The new lockref UI is meant to\n> address this problem. So in the new world order, in case (5b) it\n> sounds like I should have instead used\n>\n> \t(5b') git push --allow-non-ff\n\nt is clear you want to allow-no-ff in this case (otherwise the push\nwill not go through), and that is what the \"--force\" option meant in\nthe old world. The compare-and-swap safety is to help this case by\nletting you say\n\n\tgit push --lockref\n\nwhich is a weaker form of \"--force\". We ignore \"fast-forward\"-ness,\nlike the current \"--force\" does, but replace it with another form of\nsafety \"we know replacing this old value with what we are pushing is\nOK---if somebody updated the ref in the meantime, then the push is\nnot OK, so please fail\".\n\n> Suppose I am writing a script that is meant to set the remote\n> repository to a known state. Other contributors are only using\n> fast-forward updates so once my change goes in they will act\n> appropriately. I just need to get my ref update in, without being\n> blocked by other ref updates.\n>\n> Then I will use\n>\n> \t(5c) git push --force\n>\n> which means not to use this new lockref trick that looks at my\n> remote-tracking branch and instead to just force the ref update.\n\nI am not sure I follow. Do other contributors update this remote\nrepository? They are \"only using fast-forward updates\", so their\nupdates may not lose anything we pushed, but with \"--force\", aren't\nyou losing their work on top of yours?\n"},{"id":"223377","messageId":"20130715044454.GA2962@elie.Belkin","threadId":"34329","inReplyTo":"7v4nbwfooj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-07-15T04:44:54Z","receivedAt":"2013-07-15T04:44:54Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder writes:\n\n>> Then I will use\n>>\n>> \t(5c) git push --force\n>>\n>> which means not to use this new lockref trick that looks at my\n>> remote-tracking branch and instead to just force the ref update.\n>\n> I am not sure I follow. Do other contributors update this remote\n> repository? They are \"only using fast-forward updates\", so their\n> updates may not lose anything we pushed, but with \"--force\", aren't\n> you losing their work on top of yours?\n\nYep, I meant that when you really *do* want to force a push\nregardless of what's on the remote end, the current --force behavior\nis more useful than --lockref.\n\nThe example I used to introduce (5c) is too vague to be useful. A\nmore compelling example (to me, at least) is the one from later in\nthat message involving a relay, which does not involve other\ncontributors at all.\n\nThat is, suppose I maintain a mirror of the branches from\ngit://repo.or.cz/git.git by pushing regularly to a hosting service\nwhere I do not have shell access. Since I can't fetch from the target\nrepository or push from the source, I instead fetch and then push from\na relay, like this:I might push like this:\n\n\tgit fetch upstream\n\tgit push --force origin refs/remotes/upstream/*:refs/heads/*\n\nOr, in the same spirit, with a detached HEAD:\n\n\tgit fetch upstream refs/heads/*:refs/heads/*\n\tgit push --force origin :\n\nThe --force is to account for \"pu\" and \"next\" rewinding.\n\nIn this scenario, assuming I have exclusive access to the repository\nand the push updates the remote-tracking branches, --lockref and\n--force work equally well. The commands might run once every 6 hours\nusing a cronjob.\n\nNow suppose my relay has some downtime. That's fine --- I can still\nmaintain the mirror by running the same commands on another machine.\nBut when the old relay comes back up, \"push --lockref\" will fail and\n\"pu\" and \"next\" in my mirror are not updated any more.\n\nThat is why I said that --force is more appropriate than --lockref\nfor this application.\n\nThanks,\nJonathan\n"},{"id":"223442","messageId":"7vfvvfdecb.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"20130715044454.GA2962@elie.Belkin","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-15T15:37:08Z","receivedAt":"2013-07-15T15:37:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Now suppose my relay has some downtime. That's fine --- I can still\n> maintain the mirror by running the same commands on another machine.\n> But when the old relay comes back up, \"push --lockref\" will fail and\n> \"pu\" and \"next\" in my mirror are not updated any more.\n>\n> That is why I said that --force is more appropriate than --lockref\n> for this application.\n\nSure.\n"},{"id":"223443","messageId":"7va9lnddug.fsf_-_@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"7vd2qkfpm8.fsf@alter.siamese.dyndns.org","subject":"Default expectation of --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-15T15:47:51Z","receivedAt":"2013-07-15T15:47:51Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\n> I think the use of \"reverse tracking\" is way overrated. It is\n> probably the only default value that we could use, if the user is\n> too lazy not to specify it, but I do not think it is particularly a\n> sensible or safe default.\n>\n> The following does not discuss \"should --lockref automatically\n> disable the 'must fast-forward' check?\". The problem highlighted is\n> the same, regardless of the answer to that question.\n>\n> After rebasing beyond what is already published, you try the\n> \"lockref\" push, e.g. (we assume you work on master and push back to\n> update master at your origin):\n>\n> $ git fetch\n> $ git rebase -i @{u}~4 ;# rebase beyond what is there\n> $ git push ;# of course this will not fast-forward\n> $ git push --lockref\n>\n> If somebody else pushed while you are working on the rebase, the\n> last step (one of the above push) will fail due to stale\n> expectation. What now?\n>\n> The user would want to keep the updated tip, so the first thing that\n> happens will always be\n>\n> $ git fetch\n> $ git log ..@{u} ;# what will we be losing?\n>\n> The right thing to do at this point is to rebase your 'master' again\n> on top of @{u}\n>\n> $ git rebase -i @{u}\n>\n> before attempting to push back again. If you do that, then you can\n> do another \"lockref\" push.\n>\n> But the thing is, a novice who does not know what he is doing will\n> likely to do this:\n>\n> $ git push --lockref\n>\n> ... rejected due to stale expectation\n> $ git fetch\n>\n> You just have updated the lockref base, so if you did, without doing\n> anything else, \n>\n> $ git push --lockref\n>\n> then you will lose the updated contents.\n\nWe _might_ be able to use the reflog on refs/remotes/origin/master\nto come up with a better default expectation.\n\nWe are pushing an updated master, and the commit at the tip of the\nbranch has a committer timestamp. refs/remotes/origin/master should\nat least have two reflog entries for it at this point. The latest\none is our latest \"git fetch\" after the previous lockref push failed\n(and we see somebody else updated the master at the origin). One\nbefore is the one we based our judgement that the rebased result can\nreplace it. They both have timestamps for reflog updates.\n\nSo we _could_ use refs/remotes/origin/master@{$timestamp} where\nthe $timestamp is the committer timestamp of the tip of 'master'\nwe are pushing to replace the 'master' branch at 'origin'.\n\nI do not particularly like this approach, though. I do not\nparticulary like the \"look at the tracking branch of what we are\nupdating\" in the first place myself, because it requires you to have\nsuch a tracking branch, but now with this, we will also require you\nto have a reflog on such a tracking branch, too, which is even\nworse. And it is making it too complex and obscure, even though I\nthink the semantics would make sense.\n"},{"id":"223480","messageId":"51E45B15.7070404@kdbg.org","threadId":"34329","inReplyTo":"7vd2qkfpm8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-07-15T20:27:01Z","receivedAt":"2013-07-15T20:27:01Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 15.07.2013 05:50, schrieb Junio C Hamano:\n> ... or also with your \"--lockref is default\"\n> \t$ git push origin +master\n> \n> \t... rejected due to stale expectation\n> $ git fetch\n> \n> You just have updated the lockref base, so if you did, without doing\n> anything else, \n> \n> \t$ git push origin +master\n> \n> then you will lose the updated contents.\n> \n> The conclusion? It does not make sense to make \"lockref\" the\n> default.\n\nPoint taken.\n\n> So it will not happen; \"lockref\" will not be on by default, even if\n> it is made independent of \"must fast-forward\".\n\nOK.\n\n-- Hannes\n"},{"id":"223481","messageId":"51E45BF7.5020905@kdbg.org","threadId":"34329","inReplyTo":"51E31131.3070005@kdbg.org","subject":"Re: [PATCH 7/7] push: document --lockref","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-07-15T20:30:47Z","receivedAt":"2013-07-15T20:30:47Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 14.07.2013 22:59, schrieb Johannes Sixt:\n> ... I wonder how Junio's last\n> example with push.default=simple can work today:\n> \n> $ git pull --rebase # not a merge\n> $ git push\n> \n> because it is not a fast-forward.\n\n*blush* I was mostly asleep and and totally off the rails when I wrote\nthis nonsense.\n\n-- Hannes\n"},{"id":"223551","messageId":"20130716221318.GA2337@serenity.lan","threadId":"34329","inReplyTo":"1373399610-8588-5-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 4/7] remote.c: add command line option parser for --lockref","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-07-16T22:13:39Z","receivedAt":"2013-07-16T22:13:39Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Tue, Jul 09, 2013 at 12:53:27PM -0700, Junio C Hamano wrote:\n> diff --git a/remote.c b/remote.c\n> index 81bc876..e9b423a 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -1938,3 +1938,62 @@ struct ref *get_stale_heads(struct refspec *refs, int ref_count, struct ref *fet\n> \tstring_list_clear(&ref_names, 0);\n> \treturn stale_refs;\n> }\n> +\n> +/*\n> + * Lockref aka CAS\n> + */\n> +void clear_cas_option(struct push_cas_option *cas)\n> +{\n> +\tint i;\n> +\n> +\tfor (i = 0; i < cas->nr; i++)\n> +\t\tfree(cas->entry->refname);\n\nShould this be\n\n\tfree(cas->entry[i]->refname);\n\n?\n\n> +\tfree(cas->entry);\n> +\tmemset(cas, 0, sizeof(*cas));\n> +}\n"},{"id":"223595","messageId":"7vmwpl6rq3.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"20130716221318.GA2337@serenity.lan","subject":"Re: [PATCH 4/7] remote.c: add command line option parser for --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-17T17:06:44Z","receivedAt":"2013-07-17T17:06:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping writes:\n\n> On Tue, Jul 09, 2013 at 12:53:27PM -0700, Junio C Hamano wrote:\n>> diff --git a/remote.c b/remote.c\n>> index 81bc876..e9b423a 100644\n>> --- a/remote.c\n>> +++ b/remote.c\n>> @@ -1938,3 +1938,62 @@ struct ref *get_stale_heads(struct refspec *refs, int ref_count, struct ref *fet\n>> \tstring_list_clear(&ref_names, 0);\n>> \treturn stale_refs;\n>> }\n>> +\n>> +/*\n>> + * Lockref aka CAS\n>> + */\n>> +void clear_cas_option(struct push_cas_option *cas)\n>> +{\n>> +\tint i;\n>> +\n>> +\tfor (i = 0; i < cas->nr; i++)\n>> +\t\tfree(cas->entry->refname);\n>\n> Should this be\n>\n> \tfree(cas->entry[i]->refname);\n>\n> ?\n\nYes, I think so.\n"},{"id":"223596","messageId":"7vip096rl5.fsf@alter.siamese.dyndns.org","threadId":"34329","inReplyTo":"20130716221318.GA2337@serenity.lan","subject":"Re: [PATCH 4/7] remote.c: add command line option parser for --lockref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-17T17:09:42Z","receivedAt":"2013-07-17T17:09:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping writes:\n\n> On Tue, Jul 09, 2013 at 12:53:27PM -0700, Junio C Hamano wrote:\n>> diff --git a/remote.c b/remote.c\n>> index 81bc876..e9b423a 100644\n>> --- a/remote.c\n>> +++ b/remote.c\n>> @@ -1938,3 +1938,62 @@ struct ref *get_stale_heads(struct refspec *refs, int ref_count, struct ref *fet\n>> \tstring_list_clear(&ref_names, 0);\n>> \treturn stale_refs;\n>> }\n>> +\n>> +/*\n>> + * Lockref aka CAS\n>> + */\n>> +void clear_cas_option(struct push_cas_option *cas)\n>> +{\n>> +\tint i;\n>> +\n>> +\tfor (i = 0; i < cas->nr; i++)\n>> +\t\tfree(cas->entry->refname);\n>\n> Should this be\n>\n> \tfree(cas->entry[i]->refname);\n>\n> ?\n\nYes, more like \"free(cas->entry[i].refname)\".\n\nThanks for spotting.\n\n>\n>> +\tfree(cas->entry);\n>> +\tmemset(cas, 0, sizeof(*cas));\n>> +}\n"}]}