Re: [PATCH v2] push: don't fetch commit object when checking existence

[Junio C Hamano <gitster@pobox.com>]

Tom Hughes <tom@compton.nu> writes:

>>> +	test_must_fail git -C client push 2>err &&
>> We try to overwrite it.  We expect it to fail with "not a fast
>> forward".
>
> Well that is what it would fail with at the moment, but it's not
> what would happen with a non-partial clone - a non-partial clone
> would fail with "fetch first" instead.

Oh, don't get me wrong.  I wasn't trying to split hairs between the
two error modes and their phrasing.  The "fetch-first" from
set_ref_status_for_push() is done before we even initiate the
transfer to stop the operation, with a cheap check, that will
eventually lead to "not a fast forward" error.  IOW, in my mind,
they are the same errors, just diagnosed at two different places in
the code and their messages phrased differently.

> So here we are testing that it's a "fetch first" and rather
> than "not a fast forward".

I think that is being overly specific, but that is fine.  As I said,
to the end users, these two errors mean the same thing (they would
need to fetch first and then integrate their changes before pushing
it out again), so it is plausible that we may in the future decide
that we want to use the same message.  When it happens, this test
must change, which may even be a good thing (it makes it clear what
the fallout from such a change looks like).

>>> +	git -C client rev-list --objects --missing=print "$COMMIT" >objects &&
>>> +	grep "^[?]$COMMIT" objects
>>> +'
>> OK.
>
> and also that it hasn't fetched the new commit.

Yes, and this is a good check that will stand the test of time, even
across a change to rephrase the error message.

Thanks.