Re: [PATCH v4 4/4] notes: teach git-notes about notes.<ref>.merge option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 12, 2015 at 2:57 PM, Jacob Keller <jacob.keller@xxxxxxxxx> wrote:
> On Wed, Aug 12, 2015 at 2:46 PM, Johan Herland <johan@xxxxxxxxxxx> wrote:
>> If we don't already refuse to merge into a ref outside refs/notes, then
>> I would consider that a bug to be fixed, and not some corner use case that
>> we must preserve for all future.
>>
>> After all, we do already have a test in t3308 named 'fail to merge into
>> various non-notes refs', where one of the non-notes ref being tested are:
>>
>>   test_must_fail git -c "core.notesRef=refs/heads/master" notes merge x
>>
>
> This test is checking if the ref pointed at by refs/heads/master *is*
> a note. But you could create a ref outside of refs/notes which is a
> note but which isn't inside refs/notes
>
> I did just find that we expand remote-ref using expand_notes_ref, and
> it does *not* currently let us reference refs outside of refs/notes..
> so we can merge IN to a ref not inside refs/notes (using the
> environment variable) but we can't merge FROM
> refs/tracking/origin/notes/y for example, which means currently all
> notes we merge from have to be located into refs/notes/*
>
> There are some weird issues here.
>
> Regards,
> Jake


I spoke to soon. We have an "init_notes_check" function which shows
that it does refuse to merge outside of refs/notes/* It prevents all
notes operations outside of refs/notes

Since this is the case, I would prefer to modify the DWIM to be as I
suggested, and use this DWIM for the notes.

We will need to modify the DWIM so that it doesn't change refs/* even
if this will fail later, as we use expand_notes_ref for the remote_ref
of a merge, and we probably want to allow notes refs to be located
somewhere outside of notes such as refs/tracking/<origin>/notes or
something in the future.

So we can make our config option take only unqualified values.

Thoughts?

Regards,
Jake
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]