git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Jacob Keller <jacob.keller@gmail.com>
Date
Aug 12, 2015, 22:03 UTC
Message-ID
<CA+P7+xp0zhqM4CHmFpKuPyNMTx3DBxiLSOsiwz=byrfTEDj3sA@mail.gmail.com>
In-Reply-To
<CA+P7+xrQnrQdE3OOhdc2-2__V3Huzc+HfGEXKBZULy2JkQR37Q@mail.gmail.com>
On Wed, Aug 12, 2015 at 2:57 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
Show 26 quoted lines
> On Wed, Aug 12, 2015 at 2:46 PM, Johan Herland <johan@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

Previous: Jacob KellerNext: Junio C Hamano
Message 19 of 21 in “add notes strategy configuration options”
  1. 0/4 add notes strategy configuration optionsJacob Keller, Aug 11, 2015
  2. 1/4 notes: document cat_sort_uniq rewriteModeJacob Keller, Aug 11, 2015
  3. Johan HerlandAug 12, 2015
  4. 2/4 notes: add tests for --commit/--abort/--strategy exclusivityJacob Keller, Aug 11, 2015
  5. Johan HerlandAug 12, 2015
  6. 3/4 notes: add notes.merge option to select default strategyJacob Keller, Aug 11, 2015
  7. Johan HerlandAug 12, 2015
  8. 4/4 notes: teach git-notes about notes.<ref>.merge optionJacob Keller, Aug 11, 2015
  9. Eric SunshineAug 12, 2015
  10. Johan HerlandAug 12, 2015
  11. Junio C HamanoAug 12, 2015
  12. Jacob KellerAug 12, 2015
  13. Junio C HamanoAug 12, 2015
  14. Jacob KellerAug 12, 2015
  15. Jacob KellerAug 12, 2015
  16. Johan HerlandAug 12, 2015
  17. Johan HerlandAug 12, 2015
  18. Jacob KellerAug 12, 2015
  19. Jacob KellerAug 12, 2015
  20. Junio C HamanoAug 12, 2015
  21. Jacob KellerAug 12, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.