Re: [PATCH v2 2/3] push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCE

From: Jeff King <peff@peff.net>
Date: Jan 24, 2013, 06:43 UTC
Message-ID: <20130124064326.GB610@sigill.intra.peff.net>
In-Reply-To: <7vip6nj22m.fsf@alter.siamese.dyndns.org>

On Wed, Jan 23, 2013 at 08:28:49AM -0800, Junio C Hamano wrote:

Show 12 quoted lines

> How about doing this?
> 
> For "needs force" cases, we say this instead:
> 
>  hint: you cannot update a ref that points at a non-commit object, or
>  hint: update a ref to point at a non-commit object, without --force.
> 
> Being explicit about "non-commit" twice will catch user's eyes and
> cause him to double check that it is not a mistyped LHS of the push
> refspec (if he is sending a non-commit) or mistyped RHS (if the ref
> is pointing at a non-commit).  If he _is_ trying to push a blob out,
> the advice makes it clear what to do next: he does want to force it.

Yeah, I think that is sensible.

> Note that you _could_ split the "needs force" case into two, namely,
> "cannot replace a non-commit" and "cannot push a non-commit".  You
> could even further split them [...etc...]

I do not think it is worth worrying too much about. This should really not happen very often, and the user should be able to investigate and figure out what is going on. I think making the error message extremely specific is just going to end up making it harder to understand.

Show 5 quoted lines

> If we did that, then we could loosen the "You should fetch first"
> case to say something like this:
> 
>  hint: you do not have the object at the tip of the remote ref;
>  hint: perhaps you want to pull from there first?

Yeah, better. I'll comment on the specific message you used in response to the patch itself.

Show 6 quoted lines

> This explicitly denies one of Chris's wish "we shouldn't suggest to
> merge something that we may not be able to", but in the "You should
> fetch first" case, we cannot fundamentally know if we can merge
> until we fetch.  I agree with you that the most common case is that
> the unknown object is a commit, and that suggesting to pull is a
> good compromise.

I thought the wish was more about "we shouldn't suggest to merge something we _know_ we will not be able to", and you are still handling that (i.e., the "needs force" case).

-Peff