Subject: Re: [PATCH 4/9] repack: refactor piping an oid to a command

From: Taylor Blau <[email-masked]>
Date: Wed, 21 Jun 2023 06:55:42 -0400

On Thu, Jun 15, 2023 at 04:46:43PM -0700, Junio C Hamano wrote:
> Christian Couder <[email-masked]> writes:
>
> > Create a new write_oid_hex_cmd() function to send an oid to the standard
> > input of a running command. This new function will be used in a
> > following commit.
> >
> > Signed-off-by: Christian Couder <[email-masked]>
> > ---
> >  builtin/repack.c | 20 +++++++++++++-------
> >  1 file changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/builtin/repack.c b/builtin/repack.c
> > index 0541c3ce15..e591c295cf 100644
> > --- a/builtin/repack.c
> > +++ b/builtin/repack.c
> > @@ -182,6 +182,17 @@ static void prepare_pack_objects(struct child_process *cmd,
> >  	cmd->out = -1;
> >  }
> >
> > +static void write_oid_hex_cmd(const char *oid_hex,
> > +			      struct child_process *cmd,
> > +			      const char *err_msg)
> > +{
> > +	if (cmd->in == -1 && start_command(cmd))
> > +		die("%s", err_msg);
>
> I am not sure why we would want to conflate the "if we haven't
> started the command, auto-start it upon our first attempt to write"
> in these low-level "I am designed to do one thing, which is to feed
> the object name to the process, and do it well" function.

I agree, the implementation of `write_oid_hex_cmd()` seems too magical
to me.

Perhaps there was some awkwardness with using the pre-image w.r.t some
later change? Let's see...

Thanks,
Taylor