On Mon, Oct 24, 2022 at 02:30:49PM -0700, Junio C Hamano wrote:
> > - prepare_pack_objects(&cmd, args, packtmp);
> > + prepare_pack_objects(&cmd, args, destination);
> >
> > strvec_push(&cmd.args, "--cruft");
> > if (cruft_expiration)
> > @@ -714,7 +717,12 @@ static int write_cruft_pack(const struct pack_objects_args *args,
> > if (line.len != the_hash_algo->hexsz)
> > die(_("repack: Expecting full hex object ID lines only "
> > "from pack-objects."));
> > - string_list_append(names, line.buf);
> > + /*
> > + * avoid putting packs written outside of the repository in the
> > + * list of names
> > + */
> > + if (local)
> > + string_list_append(names, line.buf);
> > }
>
> Even if we do not want to contaminate the "names" list with packs
> that are not in the repository, wouldn't our caller still want to be
> able to tell what packs they are?
>
> What I am wondering is if it makes more sense to have the caller
> pass &names (which can be NULL to just discard the output from the
> pack-objects command) so that this function can concentrate on what
> it does (i.e. formulate the command to write cruft packs and then
> report the packs that are created), without having to worry about
> the management of the &names thing, which can be done by the caller
> of this function? We are already passing &names, so it may be the
> matter of caller deciding to pass &names or NULL based on the value
> of destination it passes to the function?
It would be nice if it were possible to avoid passing `names` entirely
here, but we still need it to determine the set of packs we already
wrote a few lines above when we write the input to `pack-objects --cruft`.
Thanks,
Taylor
