git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFD] Making "git push [--force/--delete]" safer?

From
Jonathan del Strother <maillist@steelskies.com>
Date
Jul 3, 2013, 10:06 UTC
Message-ID
<CAF5DW8++sc2VYmdJEjbD_ue_wtDFj21vcyFzNWU0M+rAm2X0sQ@mail.gmail.com>
In-Reply-To
<CALKQrge_REZKfds0T-owJOn2BvfLmHpk7yQeSog=yvofE_zKJQ@mail.gmail.com>
On 3 July 2013 11:00, Johan Herland <johan@herland.net> wrote:
Show 56 quoted lines
> On Wed, Jul 3, 2013 at 10:49 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Johan Herland <johan@herland.net> writes:
>>> Overnight, it occured to me that --force-if-expected could be
>>> simplified by leveraging the existing --force option; for the above
>>> two examples, respectively:
>>>
>>>   $ git push --force --expect
>>>   # validate foo @ origin == @{upstream} before pushing
>>>
>>> and
>>>
>>>   $ git push --force --expect=refs/original/foo my_remote HEAD:foo
>>>   # validate foo @ my_remote == refs/original/foo before pushing
>>
>> First, on the name.
>>
>> I do not think either "--validate" or "--expect" is particularly a
>> good one.  The former lets this feature squat on a good name that
>> covers a much broader spectrum, forbidding people from adding other
>> kinds of validation later.  "--expect" is slightly less bad in that
>> sense; saying "we expect this" does imply "otherwise it is an
>> unexpected situation and we would fail", but the name still does not
>> feel ideal.
>>
>> What is the essense of compare-and-swap?  Perhaps we can find a good
>> word by thinking that question through.
>>
>> To me, it is a way to implement a "lock" on the remote ref without
>> actually taking a lock (which would leave us open for a stale lock),
>> and this "lock"-ness is what we want in order to guarantee safety.
>>
>> So we could perhaps call it "--lockref"?
>>
>> I'll leave the name open but tentatively use this name in the
>> following, primarily to see how well it sits on the command line
>> examples.
>
> I agree that neither --expect nor --validate are very good. I also
> don't like --lockref, mostly because there is no locking involved, and
> I think most users will jump to an incorrect conclusion about what
> this option does, unless they read the documentation.
>
> Some other suggestions:
>
> a) --update-if. I think this reads quite nicely in the fully specified
> variant: --update-if=theirRefName:expectedValue, but it becomes more
> cryptic when defaults are assumed (i.e. --update-if without any
> arguments).
>
> b) --precond. This makes it clear that we're specifying a precondition
> on the push. Again, I think the fully specified version reads nicely,
> but it might seem a little cryptic when no arguments are given.
>
> c) --pre-verify, --pre-check are merely variations on (b), other
> variations include --pre-verify-ref or --pre-check-ref, making things
> more explicit at the cost of option name length.

I'm struggling to think of instances where I wouldn't want this CAS-like behaviour. Wouldn't it be better to make it the default when pushing, and allowing the current behaviour with "git push --blind-force" or something?

Previous: Johan HerlandNext: Johan Herland
Message 6 of 62 in “[RFD] Making "git push [--force/--delete]" safer?”
  1. Junio C HamanoJul 2, 2013
  2. Johan HerlandJul 2, 2013
  3. Johan HerlandJul 3, 2013
  4. Junio C HamanoJul 3, 2013
  5. Johan HerlandJul 3, 2013
  6. Jonathan del StrotherJul 3, 2013
  7. Johan HerlandJul 3, 2013
  8. Michael HaggertyJul 3, 2013
  9. Johannes SixtJul 3, 2013
  10. Junio C HamanoJul 3, 2013
  11. Johannes SixtJul 4, 2013
  12. Junio C HamanoJul 4, 2013
  13. Junio C HamanoJul 3, 2013
  14. Junio C HamanoJul 3, 2013
  15. Junio C HamanoJul 3, 2013
  16. 0/7 safer "push --force" with compare-and-swapJunio C Hamano, Jul 9, 2013
  17. 1/7 cache.h: move remote/connect API out of itJunio C Hamano, Jul 9, 2013
  18. 2/7 builtin/push.c: use OPT_BOOL, not OPT_BOOLEANJunio C Hamano, Jul 9, 2013
  19. 3/7 push: beginning of compare-and-swap "force/delete safety"Junio C Hamano, Jul 9, 2013
  20. 4/7 remote.c: add command line option parser for --lockrefJunio C Hamano, Jul 9, 2013
  21. John KeepingJul 16, 2013
  22. Junio C HamanoJul 17, 2013
  23. Junio C HamanoJul 17, 2013
  24. 5/7 push --lockref: implement logic to populate old_sha1_expect[]Junio C Hamano, Jul 9, 2013
  25. 6/7 t5533: test "push --lockref"Junio C Hamano, Jul 9, 2013
  26. 7/7 push: document --lockrefJunio C Hamano, Jul 9, 2013
  27. Aaron SchrabJul 9, 2013
  28. Junio C HamanoJul 9, 2013
  29. Johannes SixtJul 9, 2013
  30. Junio C HamanoJul 9, 2013
  31. Johannes SixtJul 9, 2013
  32. Junio C HamanoJul 9, 2013
  33. Junio C HamanoJul 9, 2013
  34. Johannes SixtJul 11, 2013
  35. Junio C HamanoJul 11, 2013
  36. Junio C HamanoJul 11, 2013
  37. Johannes SixtJul 12, 2013
  38. Junio C HamanoJul 12, 2013
  39. Johannes SixtJul 12, 2013
  40. Junio C HamanoJul 12, 2013
  41. Johannes SixtJul 13, 2013
  42. Junio C HamanoJul 13, 2013
  43. Junio C HamanoJul 13, 2013
  44. Johannes SixtJul 13, 2013
  45. John KeepingJul 14, 2013
  46. Johannes SixtJul 13, 2013
  47. Junio C HamanoJul 14, 2013
  48. Johannes SixtJul 14, 2013
  49. Jonathan NiederJul 14, 2013
  50. Jonathan NiederJul 14, 2013
  51. Johannes SixtJul 14, 2013
  52. Jonathan NiederJul 14, 2013
  53. Junio C HamanoJul 15, 2013
  54. Jonathan NiederJul 15, 2013
  55. Junio C HamanoJul 15, 2013
  56. Johannes SixtJul 15, 2013
  57. Junio C HamanoJul 15, 2013
  58. Default expectation of --lockrefJunio C Hamano, Jul 15, 2013
  59. Johannes SixtJul 15, 2013
  60. Marc BranchaudJul 9, 2013
  61. Michael HaggertyJul 9, 2013
  62. Junio C HamanoJul 9, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.