All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Sixt <j6t@kdbg.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 7/7] push: document --lockref
Date: Sat, 13 Jul 2013 22:17:31 +0200	[thread overview]
Message-ID: <51E1B5DB.9080904@kdbg.org> (raw)
In-Reply-To: <7vwqougwec.fsf@alter.siamese.dyndns.org>

Am 13.07.2013 20:14, schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
>>> Your table above makes this fail:
>>>
>>>         git push --lockref topic
>>>
>>> and the user has to force it,
>>
>> Of course.
>>
>>> like this?
>>>
>>> 	git push --lockref --force topic ;# or alternatively
>>>         git push --lockref +topic
>>>
>>> Why is it even necessary?
> 
>> Because it is no-ff. How do you achieve the push today (without
>> --lockref)? You use one of these two options. It does not change with
>> --lockref.
> 
> But by going that route, you are making --lockref _less_ useful, no?
> 
> "git push topic" in no-ff/match case fails as it should.  The whole
> purpose of "--lockref" is to make this case easier and safer than
> the today's system, where the anything-goes "--force" is the only
> way to make this push.  We want to give a user who
> 
>  - rebased the topic, and
> 
>  - knows where the topic at the remote should be
> 
> a way to say "I know I am pushing a no-ff, and I want to make sure
> the current value is this" in order to avoid losing somebody else's
> work queued on top of the topic at the remote while he was rebasing.
> 
> You _CAN_ introduce a new --allow-no-ff at the same time and fail a
> no-ff/match push:
> 
> 	git push --lockref topic
> 
> and then allow it back with:
> 
> 	git push --lockref --allow-no-ff topic
> 	git push --lockref +topic ;# +topic is now --allow-no-ff topic
> 
> but why _SHOULD_ we?  As soon as the user _says_ --lockref, the user
> is telling us he is pushing a no-ff.  If that is not the case, the
> user can push without --lockref in the first place.
> 
> The only potential thing you are gaining with such a change is that
> you are allowing people to say "this will fast-forward _and_ the I
> know the current value; if either of these two assumptions is
> violated, please fail this push".
> 
> If "--lockref" automatically implies "--allow-no-ff" (the design in
> the reposted patch), you cannot express that combination.  But once
> you use "--lockref" in such a situation , for the push to succeed,
> you know that the push replaces not just _any_ ancestor of what you
> are pushing, but replaces the exact current value.  So I do not think
> your implicit introduction of --allow-no-ff via redefining the
> semantics of the plus prefix is not adding much value (if any),
> while making the common case less easy to use.
> 
>> No; --lockref only adds the check that the destination is at the
>> expected revision, but does *NOT* override the no-ff check.
> 
> You _could_ do it in that way, but that is less useful.

All you have been saying is that you find your

   git push --lockref there topic

is more useful than my

   git push --lockref there +topic

You are trading crystal clear semantics to save users ONE character to
type. IMO, it's a bad deal.

The crystal clear semantics would be:

 - to override no-ff safety, use +refspec;

 - to override "mismatch" safety, do not use --lockref/use --no-lockref;

 - do not use --force unless you know the consequences.

I actually think that by implying allow-no-ff in --lockref, you are
hurting users who have configured a push refspec without a + prefix:
They suddenly do not get the push denied when it is not a fast-forward
anymore. For example, when you have

    [remote "ko"]
        push = master
        push = +pu

and you accidentally rewound master before the point that is already
published, then

   git push --lockref ko

will happily push the rewound master.

-- Hannes

  parent reply	other threads:[~2013-07-13 20:17 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-02 20:57 [RFD] Making "git push [--force/--delete]" safer? Junio C Hamano
2013-07-02 22:55 ` Johan Herland
2013-07-03  6:34   ` Johan Herland
2013-07-03  8:49     ` Junio C Hamano
2013-07-03 10:00       ` Johan Herland
2013-07-03 10:06         ` Jonathan del Strother
2013-07-03 10:11           ` Johan Herland
2013-07-03 10:50             ` Michael Haggerty
2013-07-03 12:06               ` Johannes Sixt
2013-07-03 19:53                 ` Junio C Hamano
2013-07-04  5:37                   ` Johannes Sixt
2013-07-04  5:46                     ` Junio C Hamano
2013-07-03 19:50           ` Junio C Hamano
2013-07-03 20:18             ` Junio C Hamano
2013-07-03 19:48         ` Junio C Hamano
2013-07-09 19:53 ` [PATCH 0/7] safer "push --force" with compare-and-swap Junio C Hamano
2013-07-09 19:53   ` [PATCH 1/7] cache.h: move remote/connect API out of it Junio C Hamano
2013-07-09 19:53   ` [PATCH 2/7] builtin/push.c: use OPT_BOOL, not OPT_BOOLEAN Junio C Hamano
2013-07-09 19:53   ` [PATCH 3/7] push: beginning of compare-and-swap "force/delete safety" Junio C Hamano
2013-07-09 19:53   ` [PATCH 4/7] remote.c: add command line option parser for --lockref Junio C Hamano
2013-07-16 22:13     ` John Keeping
2013-07-17 17:06       ` Junio C Hamano
2013-07-17 17:09       ` Junio C Hamano
2013-07-09 19:53   ` [PATCH 5/7] push --lockref: implement logic to populate old_sha1_expect[] Junio C Hamano
2013-07-09 19:53   ` [PATCH 6/7] t5533: test "push --lockref" Junio C Hamano
2013-07-09 19:53   ` [PATCH 7/7] push: document --lockref Junio C Hamano
2013-07-09 20:17     ` Aaron Schrab
2013-07-09 20:39       ` Junio C Hamano
2013-07-09 20:24     ` Johannes Sixt
2013-07-09 20:37       ` Junio C Hamano
2013-07-09 20:55         ` Johannes Sixt
2013-07-09 22:09           ` Junio C Hamano
2013-07-09 23:08             ` Junio C Hamano
2013-07-11 21:10               ` Johannes Sixt
2013-07-11 21:57                 ` Junio C Hamano
2013-07-11 22:14                 ` Junio C Hamano
2013-07-12 17:21                   ` Johannes Sixt
2013-07-12 17:40                     ` Junio C Hamano
2013-07-12 20:00                       ` Johannes Sixt
2013-07-12 21:19                         ` Junio C Hamano
2013-07-13  6:52                           ` Johannes Sixt
2013-07-13 18:14                             ` Junio C Hamano
2013-07-13 20:08                               ` Junio C Hamano
2013-07-13 21:11                                 ` Johannes Sixt
2013-07-14 14:28                                 ` John Keeping
2013-07-13 20:17                               ` Johannes Sixt [this message]
2013-07-14 19:17                                 ` Junio C Hamano
2013-07-14 20:21                                   ` Johannes Sixt
2013-07-14 20:34                                     ` Jonathan Nieder
2013-07-14 20:49                                       ` Jonathan Nieder
2013-07-14 20:59                                       ` Johannes Sixt
2013-07-14 21:28                                         ` Jonathan Nieder
2013-07-15  4:10                                           ` Junio C Hamano
2013-07-15  4:44                                             ` Jonathan Nieder
2013-07-15 15:37                                               ` Junio C Hamano
2013-07-15 20:30                                         ` Johannes Sixt
2013-07-15  3:50                                     ` Junio C Hamano
2013-07-15 15:47                                       ` Default expectation of --lockref Junio C Hamano
2013-07-15 20:27                                       ` [PATCH 7/7] push: document --lockref Johannes Sixt
2013-07-09 21:37         ` Marc Branchaud
2013-07-09 20:27     ` Michael Haggerty
2013-07-09 20:42       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51E1B5DB.9080904@kdbg.org \
    --to=j6t@kdbg.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.