git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Chris Rorvick <chris@rorvick.com>
Subject: Re: [PATCH v2 2/3] push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCE
Date: Wed, 23 Jan 2013 01:56:40 -0500	[thread overview]
Message-ID: <20130123065640.GB10306@sigill.intra.peff.net> (raw)
In-Reply-To: <1358836230-9197-3-git-send-email-gitster@pobox.com>

On Mon, Jan 21, 2013 at 10:30:29PM -0800, Junio C Hamano wrote:

> When we push to update an existing ref, if:
> 
>  * we do not have the object at the tip of the remote; or
>  * the object at the tip of the remote is not a commit; or
>  * the object we are pushing is not a commit,
> 
> there is no point suggesting to fetch, integrate and push again.
> 
> If we do not have the current object at the tip of the remote, we
> should tell the user to fetch first and evaluate the situation
> before deciding what to do next.

Should we? I know that it is more correct to do so, because we do not
even know for sure that the remote object is a commit, and fetching
_might_ lead to us saying "hey, this is not something that can be
fast-forwarded".

But by far the common case will be that it _is_ a commit, and the right
thing is going to be to pull. Adding in the extra steps makes the
workflow longer and more complicated, and most of the time doesn't
matter. For example, imagine that Alice is working on "master", and when
she goes to push, she finds that Bob has already pushed his work. With
the current code, she sees:

  $ git push
  To ...
   ! [rejected]        HEAD -> master (non-fast-forward)
  error: failed to push some refs to '...'
  hint: Updates were rejected because the tip of your current branch is behind
  hint: its remote counterpart. Merge the remote changes (e.g. 'git pull')
  hint: before pushing again.

and she presumably pulls, and all is well with the follow-up push.

With your patch, she sees:

  $ git push
  To ...
   ! [rejected]        HEAD -> master (fetch first)
  error: failed to push some refs to '...'
  hint: Updates were rejected; you need to fetch the destination reference
  hint: to decide what to do.

  $ git fetch
  ...

  $ git push
  To ...
   ! [rejected]        HEAD -> master (non-fast-forward)
  error: failed to push some refs to '...'
  hint: Updates were rejected because the tip of your current branch is behind
  hint: its remote counterpart. Merge the remote changes (e.g. 'git pull')
  hint: before pushing again.
  hint: See the 'Note about fast-forwards' in 'git push --help' for details.

which is technically more correct (it's possible that in the second
step, she would find that Bob pushed a tree or something). But in the
common case that it is a commit, we've needlessly added two extra steps
(a fetch and another failed push), both of which involve network access
(so they are slow, and may involve Alice having to type her credentials).

Is the extra hassle in the common case worth it for the off chance that
we might give a more accurate message? Should the "fetch first" message
be some hybrid that covers both cases accurately, but still points the
user towards "git pull" (which will fail anyway if the remote ref is not
a commit)?

-Peff

  reply	other threads:[~2013-01-23  6:57 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-30  1:41 [PATCH v6 0/8] push: update remote tags only with force Chris Rorvick
2012-11-30  1:41 ` [PATCH v6 1/8] push: return reject reasons as a bitset Chris Rorvick
2012-11-30  1:41 ` [PATCH v6 2/8] push: add advice for rejected tag reference Chris Rorvick
2012-12-02 10:42   ` Junio C Hamano
2012-12-03  3:27     ` [PATCH 0/2] push: honor advice.* configuration Chris Rorvick
2012-12-03  3:27       ` [PATCH 1/2] push: rename config variable for more general use Chris Rorvick
2012-12-03  3:27       ` [PATCH 2/2] push: allow already-exists advice to be disabled Chris Rorvick
2012-11-30  1:41 ` [PATCH v6 3/8] push: flag updates Chris Rorvick
2012-11-30  1:41 ` [PATCH v6 4/8] push: flag updates that require force Chris Rorvick
2012-11-30  1:41 ` [PATCH v6 5/8] push: require force for refs under refs/tags/ Chris Rorvick
2012-11-30  1:41 ` [PATCH v6 6/8] push: require force for annotated tags Chris Rorvick
2012-11-30  1:41 ` [PATCH v6 7/8] push: clarify rejection of update to non-commit-ish Chris Rorvick
2012-11-30  1:41 ` [PATCH v6 8/8] push: cleanup push rules comment Chris Rorvick
2012-12-02 20:43   ` [PATCH] remote.c: fix grammatical error in comment Chris Rorvick
2012-12-03 18:53 ` [PATCH v6 0/8] push: update remote tags only with force Junio C Hamano
2013-01-16 13:32 ` Max Horn
2013-01-16 16:00   ` Junio C Hamano
2013-01-16 16:01   ` Jeff King
2013-01-16 17:10     ` Junio C Hamano
2013-01-16 17:43       ` Jeff King
2013-01-16 21:02         ` Junio C Hamano
2013-01-17  2:19         ` Chris Rorvick
2013-01-17  3:11           ` Jeff King
2013-01-17  3:42             ` Chris Rorvick
2013-01-16 16:36   ` Junio C Hamano
2013-01-16 16:48     ` Junio C Hamano
2013-01-17  6:20       ` Chris Rorvick
2013-01-17  6:59         ` Junio C Hamano
2013-01-17 13:09           ` Chris Rorvick
2013-01-18  1:06             ` Jeff King
2013-01-18  3:18               ` Chris Rorvick
2013-01-21 23:40                 ` Jeff King
2013-01-21 23:53                   ` Junio C Hamano
2013-01-22  4:59                   ` Chris Rorvick
2013-01-22  6:44                     ` Junio C Hamano
2013-01-22  5:53                   ` [PATCH 0/3] Finishing touches to "push" advises Junio C Hamano
2013-01-22  5:53                     ` [PATCH 1/3] push: further clean up fields of "struct ref" Junio C Hamano
2013-01-22  5:53                     ` [PATCH 2/3] push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCE Junio C Hamano
2013-01-22  6:04                       ` Junio C Hamano
2013-01-22  5:53                     ` [PATCH 3/3] push: further reduce "struct ref" and simplify the logic Junio C Hamano
2013-01-22  6:21                       ` Junio C Hamano
2013-01-22  6:30                   ` [PATCH 0/3] Finishing touches to "push" advises Junio C Hamano
2013-01-22  6:30                     ` [PATCH v2 1/3] push: further clean up fields of "struct ref" Junio C Hamano
2013-01-23  6:43                       ` Jeff King
2013-01-22  6:30                     ` [PATCH v2 2/3] push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCE Junio C Hamano
2013-01-23  6:56                       ` Jeff King [this message]
2013-01-23 16:28                         ` Junio C Hamano
2013-01-24  6:43                           ` Jeff King
2013-01-22  6:30                     ` [PATCH v2 3/3] push: further simplify the logic to assign rejection status Junio C Hamano
2013-01-22  7:26                     ` [PATCH 0/3] Finishing touches to "push" advises Junio C Hamano
2013-01-23 21:55                   ` [PATCH v4 " Junio C Hamano
2013-01-23 21:55                     ` [PATCH v4 1/3] push: further clean up fields of "struct ref" Junio C Hamano
2013-01-24 22:22                       ` Eric Sunshine
2013-01-23 21:55                     ` [PATCH v4 2/3] push: further simplify the logic to assign rejection reason Junio C Hamano
2013-01-23 21:55                     ` [PATCH v4 3/3] push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCE Junio C Hamano
2013-01-24  6:58                       ` Jeff King
2013-01-24 17:19                         ` Junio C Hamano
2013-01-25  4:31                     ` [PATCH v4 0/3] Finishing touches to "push" advises Chris Rorvick
2013-01-25  5:04                       ` Junio C Hamano
2013-01-25  5:14                         ` Chris Rorvick
2013-01-18  4:36               ` [PATCH v6 0/8] push: update remote tags only with force 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=20130123065640.GB10306@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=chris@rorvick.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).