Re: [PATCH v2 03/10] hook: convert 'post-rewrite' hook in sequencer.c to hook API

From: Adrian Ratiu <adrian.ratiu@collabora.com>
Date: Oct 21, 2025, 15:44 UTC
Message-ID: <87v7k8p8do.fsf@collabora.com>
In-Reply-To: <aPc5EzFRrDuI-0XH@pks.im>

On Tue, 21 Oct 2025, Patrick Steinhardt <ps@pks.im> wrote:

Show 14 quoted lines

> On Fri, Oct 17, 2025 at 05:15:37PM +0300, Adrian Ratiu wrote: 
>> diff --git a/sequencer.c b/sequencer.c index 
>> 5476d39ba9..71ed31c774 100644 --- a/sequencer.c +++ 
>> b/sequencer.c @@ -1292,32 +1292,40 @@ int 
>> update_head_with_reflog(const struct commit *old_head, 
>>  	return ret; }  
>> +static int pipe_from_strbuf(int hook_stdin_fd, void *pp_cb, 
>> void *pp_task_cb UNUSED) +{ +	struct hook_cb_data 
>> *hook_cb = pp_cb; +	struct strbuf *to_pipe = 
>> hook_cb->options->feed_pipe_ctx; 
> 
> Not sure, but shouldn't it be possible to set 
> `opt.feed_pipe_cb_data` instead and then access the context via 
> `pp_task_cb` here instead of having to reach into the struct?

No, because they are different structures. You can look inside the existing run_hooks_opt(), it does:

struct hook_cb_data cb_data = {
    ...
    .options = options,
};

They're different nested options structures. :)

Maybe we could simplify these, however refactoring this options API is outside the scope of this series, just like refactoring the 1/0/-1 fd values used by run-command.c accross the entire source tree we discussed in the previous version (I tried, then I gave up because it's too big & risky for this series).

I'd like to fix all these APIs btw, however it's a big independent effort. Who knows, maybe some day in the future... :)