From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Maarten Bosmans <mkbosmans@gmail.com>,
git@vger.kernel.org, Teng Long <dyroneteng@gmail.com>,
Maarten Bosmans <maarten.bosmans@vortech.nl>
Subject: Re: [PATCH 1/4] notes: print note blob to stdout directly
Date: Thu, 15 Feb 2024 22:25:45 -0800 [thread overview]
Message-ID: <xmqqcysxkjrq.fsf@gitster.g> (raw)
In-Reply-To: <20240215052640.GC2821179@coredump.intra.peff.net> (Jeff King's message of "Thu, 15 Feb 2024 00:26:40 -0500")
Jeff King <peff@peff.net> writes:
> On Tue, Feb 13, 2024 at 09:35:28AM -0800, Junio C Hamano wrote:
>
>> 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).
OK, that makes my worry about dropping "git show" from the codepath
quite a lot. At least we only have to worry about blobs.
> 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.
Yup.
The question is if our current use of "git show" allows creative use
of such binary data attached as notes. The patch in question will
break such users, as it would become impossible once we start
bypassing the "git show" and emitting the binary data directly to
the standard output stream.
Because the pathname a note blob is stored at is unpredictable and
not under end-user control, it would not be practical to define
different smudge filters per note object using the attribute
mechanism, but if you limit the types of binary data you store in
your notes (e.g., refs/notes/thumbnail may be a note ref whose
contents are all jpeg thumbnail images, where your main history is
your photo archive), you might be able to convince the "git show"
invocation we use to show the note object to show that thumbnail
image by using something involving "display" (from imagemagick---of
course you could use other programs that allows you to view the
image on different platforms) in your smudge filter. Bypassing "git
show" and sending the blob contents directly to the standard output
would be a grave regression for such a user.
next prev parent reply other threads:[~2024-02-16 6:25 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-05 20:49 [PATCH 0/4] Speed up git-notes show Maarten Bosmans
2024-02-05 20:49 ` [PATCH 1/4] notes: print note blob to stdout directly Maarten Bosmans
2024-02-06 3:44 ` Junio C Hamano
2024-02-06 9:55 ` Maarten Bosmans
2024-02-06 17:52 ` Junio C Hamano
2024-02-13 8:00 ` Jeff King
2024-02-13 17:35 ` Junio C Hamano
2024-02-15 5:26 ` Jeff King
2024-02-16 6:25 ` Junio C Hamano [this message]
2024-02-17 5:16 ` Jeff King
2024-02-17 5:56 ` Junio C Hamano
2024-02-17 6:09 ` Jeff King
2024-02-15 7:46 ` Maarten Bosmans
2024-02-15 15:04 ` Jeff King
2024-02-17 12:45 ` Maarten Bosmans
2024-02-20 1:51 ` Jeff King
2024-02-15 7:41 ` Maarten Bosmans
2024-02-06 13:55 ` Kristoffer Haugsbakk
2024-02-05 20:49 ` [PATCH 2/4] notes: use exisisting function stream_blob_to_fd Maarten Bosmans
2024-02-05 22:00 ` Eric Sunshine
2024-02-05 20:49 ` [PATCH 3/4] notes: do not clean up right before calling die() Maarten Bosmans
2024-02-05 20:49 ` [PATCH 4/4] notes: use strbuf_attach to take ownership of the object contents Maarten Bosmans
2024-02-06 7:08 ` [PATCH 0/4] Speed up git-notes show Kristoffer Haugsbakk
2024-02-06 8:51 ` Maarten Bosmans
2024-02-18 19:59 ` [PATCH v2 0/5] " Maarten Bosmans
2024-02-18 19:59 ` [PATCH v2 1/5] log: Move show_blob_object() to log.c Maarten Bosmans
2024-02-20 1:22 ` Junio C Hamano
2024-02-20 1:59 ` Jeff King
2024-02-20 3:03 ` Junio C Hamano
2024-02-20 11:40 ` Maarten Bosmans
2024-02-18 19:59 ` [PATCH v2 2/5] notes: avoid launching a child process to show a note blob Maarten Bosmans
2024-02-18 19:59 ` [PATCH v2 3/5] notes: use existing function stream_blob_to_fd Maarten Bosmans
2024-02-18 19:59 ` [PATCH v2 4/5] notes: do not clean up right before calling die() Maarten Bosmans
2024-02-18 19:59 ` [PATCH v2 5/5] notes: use strbuf_attach to take ownership of the object contents Maarten Bosmans
2024-02-20 2:12 ` Jeff King
2024-02-20 7:42 ` Maarten Bosmans
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqcysxkjrq.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=dyroneteng@gmail.com \
--cc=git@vger.kernel.org \
--cc=maarten.bosmans@vortech.nl \
--cc=mkbosmans@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).