Re: [PATCH 1/4] notes: print note blob to stdout directly

From: Jeff King <peff@peff.net>
Date: Feb 15, 2024, 05:26 UTC
Message-ID: <20240215052640.GC2821179@coredump.intra.peff.net>
In-Reply-To: <xmqq5xys6zdr.fsf@gitster.g>

On Tue, Feb 13, 2024 at 09:35:28AM -0800, Junio C Hamano wrote:

Show 6 quoted lines

> Remind me if you can if we (1) had plans to allow non-blob objects
> as notes, or (2) actively support to have non-text blob objects as
> notes.  I _think_ we do assume people may try to add non-text blob
> as notes (while they accept that they cannot merge two such notes on
> the same object), but I do not recall if we planned to allow them to
> store trees and commits.

Short answer: no for (1) and yes for (2).

In my head non-blob notes were always something we'd eventually allow. But I don't think the "git notes" tool really helps you at all (it insists on being fed message content and makes the blob itself, rather than letting you specify an oid).

I wondered what the display side might do here (like, say, segfault). But it looks like the notes code does not even consider a subtree like this to be a note. It ends up in the "non_note" list of the notes struct (and format_note() on the display side explicitly ignores non-blobs anyway).

So it looks like it did not ever work, and nobody can even be using it accidentally (of course you can put whatever garbage you like under refs/notes/*, but if neither git-notes nor the display machinery works with it, I think we can discount that entirely).

Non-text blob objects, on the other hand, should be easy to do with "git notes add -F". Interestingly, the display code (in format_note() again) converts embedded NULs into newlines. I think this is an accidental behavior due to the use of strchrnul().

Of course it's reasonable to also store notes that aren't meant to be displayed via git-log, etc, at all. The textconv-caching machinery stores its results in a separate notes ref. Those should generally be text (since the point is to come up with something diff-able). But I think it would be perfectly reasonable for another mechanism to make use of notes in the same way and store binary goo.

(And of course we can store notes on any object, not just commits, as the textconv cache demonstrates. But I think that is orthogonal to what you're asking).

-Peff