On Wed, Aug 12, 2015 at 2:57 PM, Jacob Keller <jacob.kel...@gmail.com> wrote: > On Wed, Aug 12, 2015 at 2:46 PM, Johan Herland <jo...@herland.net> 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
