Re: [PATCH 1/2] notes: don't expand qualified refs in expand_notes_ref

From: Jacob Keller <jacob.keller@gmail.com>
Date: Sep 22, 2015, 17:54 UTC
Message-ID: <CA+P7+xrwM44tMHMW+dEJx_tMtcMVEQELNq=J=rqs1uQ+feuZqQ@mail.gmail.com>
In-Reply-To: <xmqqsi665yjm.fsf@gitster.mtv.corp.google.com>

On Tue, Sep 22, 2015 at 7:17 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 9 quoted lines

> Calls expand_notes_ref() made on a command line argument that
> specifies the source (which I think is similar to what the other
> recent topic calls "read-only") should be made to calls to a more
> lenient version (and you can start with get_sha1() for that purpose
> without introducing your own DWIMs in the first step), while leaving
> calls to expand_notes_ref() for destination and the implementation
> of expand_notes_ref() itself unmolested, so that we can keep the
> safety in expands_notes_ref() that makes sure that the destination
> of a local operation is under refs/notes/*.

The other issue here is that expand_notes_ref is called on the --ref argument waaay before the current code even decides if the operation is "read" or "write". Thus we'd have to break this out and handle things very differently.

I don't believe expand_notes_ref() is actually providing any safety. That is all done by init_notes_check. Note that passing the environment variable bypasses the expand_notes_ref entirely, though I think we did use it as part of the refs configuration.

It seems like a lot more heavy lifting to change the entire flow of how we decide when to expand "--ref" for "read" operations vs "write" operations.

That is, git notes list.

It's easy to change behavior of git notes merge as it handles its argument for where to merge from separately, but it's not so easy to change git notes show...

Regards, Jake