Josh Steadmon <steadmon@xxxxxxxxxx> writes:
> > @@ -1719,6 +1772,57 @@ static void show_pack_info(int stat_only)
> > free(chain_histogram);
> > }
> >
> > +static void repack_local_links(void)
> > +{
> > + struct child_process cmd = CHILD_PROCESS_INIT;
> > + FILE *out;
> > + struct strbuf line = STRBUF_INIT;
> > + struct oidset_iter iter;
> > + struct object_id *oid;
> > + char *base_name;
> > +
> > + if (!oidset_size(&local_links))
> > + return;
> > +
> > + base_name = mkpathdup("%s/pack/pack", repo_get_object_directory(the_repository));
> > +
> > + strvec_push(&cmd.args, "pack-objects");
> > + strvec_push(&cmd.args, "--exclude-promisor-objects-best-effort");
> > + strvec_push(&cmd.args, base_name);
> > + cmd.git_cmd = 1;
> > + cmd.in = -1;
> > + cmd.out = -1;
> > + if (start_command(&cmd))
> > + die(_("could not start pack-objects to repack local links"));
> > +
> > + oidset_iter_init(&local_links, &iter);
> > + while ((oid = oidset_iter_next(&iter))) {
> > + if (write_in_full(cmd.in, oid_to_hex(oid), the_hash_algo->hexsz) < 0 ||
> > + write_in_full(cmd.in, "\n", 1) < 0)
> > + die(_("failed to feed local object to pack-objects"));
> > + }
> > + close(cmd.in);
> > +
> > + out = xfdopen(cmd.out, "r");
> > + while (strbuf_getline_lf(&line, out) != EOF) {
> > + unsigned char binary[GIT_MAX_RAWSZ];
> > + if (line.len != the_hash_algo->hexsz ||
> > + !hex_to_bytes(binary, line.buf, line.len))
> > + die(_("index-pack: Expecting full hex object ID lines only from pack-objects."));
>
> I'm not sure why we check the pack-objects output here, is this just to
> detect errors? Could we instead just check the exit status of
> pack-objects, and discard the output?
The output is a hex object ID that tells us what we need to name
the .promisor file. (Later in this function, "binary" is used.) So we
can't discard it.
> > diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> > index e15fbaeb21..a565ab9b40 100644
> > --- a/builtin/pack-objects.c
> > +++ b/builtin/pack-objects.c
> > @@ -4310,6 +4310,18 @@ static int option_parse_cruft_expiration(const struct option *opt UNUSED,
> > return 0;
> > }
> >
> > +static int should_include_obj(struct object *obj, void *data UNUSED)
> > +{
> > + struct object_info info = OBJECT_INFO_INIT;
> > + if (oid_object_info_extended(the_repository, &obj->oid, &info, 0))
> > + BUG("should_include_obj should only be called on existing objects");
> > + return info.whence != OI_PACKED || !info.u.packed.pack->pack_promisor;
> > +}
> > +
> > +static int should_include(struct commit *commit, void *data) {
> > + return should_include_obj((struct object *) commit, data);
> > +}
> > +
>
> Nit: these two functions could be named a bit more descriptively.
OK, done.
