Re: 2.2.0-rc behavior changes (1/2)

[Bryan Turner <bturner@atlassian.com>]

On Mon, Nov 10, 2014 at 8:22 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Nov 10, 2014 at 07:47:32PM +1100, Bryan Turner wrote:
>
>> First change: git update-ref -d /refs/heads/nonexistent
>> <some-valid-sha1> now produces an error about ref locking that it
>> didn't produce before
>>
>> Git 2.1.x and prior produced this output:
>> error: unable to resolve reference refs/heads/nonexistent: No such
>> file or directory
>>
>> Now, in the 2.2.0 RCs, it says:
>> error: unable to resolve reference refs/heads/nonexistent: No such
>> file or directory
>> error: Cannot lock the ref 'refs/heads/nonexistent'.
>>
>> This one feels more like a bug, but again may not be. I say it feels
>> like a bug because of the order of the messages: If git has decided
>> the ref doesn't exist, why is it still trying to lock it?
>
> I don't think this is a bug. The order you see is because the code goes
> something like this:
>
>   1. the parent function calls a sub-function to lock
>
>   2. the sub-function generates the error "no such file or directory"
>      and returns failure to the caller
>
>   3. the caller reports that acquiring the lock failed
>
> The only thing that has changed between the two is step (3), but it is
> not an extra lock action after the error. It is just a more verbose
> report of the same error.
>
> That being said, the sub-function (lock_ref_sha1_basic) gives a much
> more useful message. So it would be a nice enhancement to make sure that
> it prints something useful in every return case, and then drop the
> message from the caller.
>
> As an aside, I'm also slightly confused by your output. Are you feeding
> "/refs/heads/nonexistent" (with a leading slash), or
> "refs/heads/nonexistent" (no leading slash)? If the latter, then that
> should silently succeed (and seems to in my tests). If the former, then
> the real problem is not ENOENT, but rather EINVAL; that name is not a
> valid refname.
>
> Older versions of git would produce:
>
>   error: unable to resolve reference /refs/heads/nonexistent: No such file or directory
>
> which is like the error you showed, but note that the refname is
> reported with the leading slash. In v2.2.0-rc1, this is:
>
>   error: unable to resolve reference /refs/heads/nonexistent: Invalid argument
>   error: Cannot lock the ref '/refs/heads/nonexistent'.
>
> which is more accurate. I could explain the differences in our output
> from some simple transcription errors when writing your email, but I
> wanted to make sure I am not missing something.

Sorry, no, you're not missing anything. That is indeed a transcription
error from my e-mail. The test in question is using
"refs/heads/nonexistent".

Thanks for the quick response, Jeff. With the sub-function the
ordering of the messages makes perfect sense.

>
> -Peff