Re: [PATCH v6 0/4] rebase: support --trailer

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 5, 2025, 16:30 UTC
Message-ID: <xmqqecqcmohf.fsf@gitster.g>
In-Reply-To: <20251105142944.73061-1-me@linux.beauty>

Li Chen <me@linux.beauty> writes:

> From: Li Chen <chenl311@chinatelecom.cn>
>
> This series routes all trailer insertion through an in-process path, removing
> the fork/exec to builtin/interpret-trailers and tempfile juggling.

This description makes it sound as if the code before this patch series drove "interpret-trailers" via fork/exec and tempfile juggling. And that contradicts the title of the topic, "rebase: support --trailer", which implies that the topic is about the "git rebase" command, and that "git rebase" before this patch series did not support trailers, not even with fork/exec and tempfile juggling.

Which is it?

I see trailer.c:amend_file_with_trailers() does fork out to the "git interpret-trailers" command and is called by "git commit" and "git tag". Perhaps you are updating the amend_file_with_trailers() helper function to do the in-process thing, so that "git commit" and "git tag" no longer needs fork/exec and tempfile juggling?

That would be great, regardless of "rebase", and if you used that updated helper function to teach "rebase" to deal with trailers in-process, that would be wonderful.

If the main part of the series (i.e. [1/4]-[4/4]) needs rerolling, could you be a bit more careful when writing the cover letter to make it easier for even those who are seeing this series for the first time to understand what is going on?

> The first
> three commits centralize logic to reduce overhead and simplify error handling.

... in what code paths? "In command X and Y where they do Z", "All the call flows that lead to helper function F by eliminating the need to do G that is costly and replacing it with H", etc., is what I would expect to see in such a description.

> The final commit adds git rebase --trailer, currently supported
> with the merge backend only (rejecting apply-only scenarios and
> validating input early).

Sounds sensible. Li Chen <me@linux.beauty> writes: