On Thu, Jun 15, 2023 at 04:46:43PM -0700, Junio C Hamano wrote:
> Christian Couder <christian.couder@xxxxxxxxx> 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 <chriscool@xxxxxxxxxxxxx>
> > ---
> > 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
