pack-objects: support '--refs-snapshot' with 'follow-reachable'

The '--stdin-packs=follow-reachable' mode walks from reference tips to
determine which objects in included packs are reachable. Without a
snapshot, pack-objects discovers refs by iterating live references,
which may change between the time the repack writes the geometric pack
and the time it writes the MIDX bitmap.

If a reference is updated during that window, the set of reachable
objects seen by pack-objects may differ from the set seen by the MIDX
bitmap writer. This can cause reachable objects to end up in the cruft
pack (because pack-objects did not see the reference that makes them
reachable) rather than the geometric pack. While this does not cause
data loss, it has two undesirable consequences:

 - Reachable objects in the cruft pack cannot receive bitmap coverage
   (since the cruft pack may be excluded from the MIDX when
   'repack.midxMustContainCruft' is false).

 - Serving fetches that need those objects requires loading the cruft
   pack, which may contain many unrelated unreachable objects.

To avoid this, teach pack-objects to accept '--refs-snapshot=<path>'
when used with '--stdin-packs=follow-reachable'. The snapshot file uses
the same format as the MIDX bitmap writer: one hex OID per line, with
an optional '+' prefix for preferred bitmap commits.

'pack-objects' happily ignores the '+' prefix for indicating preferred
bitmap commits as a convenience, so that the ref-snapshot can be shared
between the MIDX generation machinery and 'pack-objects'.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-pack-objects.adoc b/Documentation/git-pack-objects.adoc
index d7b2e39e76c5e38436eeb7d66a4ff85abda69809..4ebe407cfafeac4f46c042c5e48b61b83fa098d1 100644 (file)
--- a/Documentation/git-pack-objects.adoc
+++ b/Documentation/git-pack-objects.adoc
@@ -133,6 +133,14 @@ commits and annotated tag objects.
 Incompatible with `--revs`, or options that imply `--revs` (such as
 `--all`), with the exception of `--unpacked`, which is compatible.
 
+--refs-snapshot=<path>::
+       When used with `--stdin-packs=follow-reachable`, read reference
+       tips from `<path>` instead of iterating live references. The file
+       format is one hex object ID per line, with an optional `+` prefix
+       (for preferred bitmap commits). This ensures a consistent view of
+       references when the same snapshot is shared with other tools (e.g.,
+       the MIDX bitmap writer).
+
 --cruft::
        Packs unreachable objects into a separate "cruft" pack, denoted
        by the existence of a `.mtimes` file. Typically used by `git

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 5d96757b645323a460817bed28d9675271c19462..082ff760abc6ca9f4f10b61e28d103f9c3d563b9 100644 (file)
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -219,6 +219,7 @@ static int incremental;
 static int ignore_packed_keep_on_disk;
 static int ignore_packed_keep_in_core;
 static int ignore_packed_keep_in_core_open;
+static const char *stdin_packs_refs_snapshot;
 static int ignore_packed_keep_in_core_has_cruft;
 static int allow_ofs_delta;
 static struct pack_idx_option pack_idx_opts;
@@ -4009,6 +4010,38 @@ static int add_ref_to_pending(const struct reference *ref, void *cb_data)
        return 0;
 }
 
+static void read_refs_snapshot(const char *refs_snapshot,
+                             struct rev_info *revs)
+{
+       struct strbuf buf = STRBUF_INIT;
+       struct object_id oid;
+       FILE *f = xfopen(refs_snapshot, "r");
+
+       while (strbuf_getline(&buf, f) != EOF) {
+               struct object *object;
+               const char *hex = buf.buf;
+               const char *end = NULL;
+
+               if (*hex == '+')
+                       hex++;
+
+               if (parse_oid_hex_algop(hex, &oid, &end,
+                                       the_repository->hash_algo) < 0)
+                       die(_("could not parse line: %s"), buf.buf);
+               if (*end)
+                       die(_("malformed line: %s"), buf.buf);
+
+               object = parse_object(the_repository, &oid);
+               if (!object)
+                       continue;
+
+               add_pending_object(revs, object, "");
+       }
+
+       fclose(f);
+       strbuf_release(&buf);
+}
+
 static void stdin_packs_add_reachable_pack_entries(struct string_list *keys,
                                                   struct rev_info *revs,
                                                   int rev_list_unpacked)
@@ -4065,8 +4098,11 @@ static void stdin_packs_add_reachable_pack_entries(struct string_list *keys,
        pre_walk.keep_pack_cache_flags |= KEPT_PACK_IN_CORE;
        pre_walk.ignore_missing_links = 1;
 
-       refs_for_each_ref(get_main_ref_store(the_repository),
-                         add_ref_to_pending, &pre_walk);
+       if (stdin_packs_refs_snapshot)
+               read_refs_snapshot(stdin_packs_refs_snapshot, &pre_walk);
+       else
+               refs_for_each_ref(get_main_ref_store(the_repository),
+                                 add_ref_to_pending, &pre_walk);
 
        if (prepare_revision_walk(&pre_walk))
                die(_("revision walk setup failed"));
@@ -5267,6 +5303,8 @@ int cmd_pack_objects(int argc,
                OPT_CALLBACK_F(0, "stdin-packs", &stdin_packs, N_("mode"),
                             N_("read packs from stdin"),
                             PARSE_OPT_OPTARG, parse_stdin_packs_mode),
+               OPT_FILENAME(0, "refs-snapshot", &stdin_packs_refs_snapshot,
+                            N_("refs snapshot for follow-reachable traversal")),
                OPT_BOOL(0, "stdout", &pack_to_stdout,
                         N_("output pack to stdout")),
                OPT_BOOL(0, "include-tag", &include_tag,
@@ -5484,6 +5522,10 @@ int cmd_pack_objects(int argc,
        if (stdin_packs && use_internal_rev_list)
                die(_("cannot use internal rev list with --stdin-packs"));
 
+       if (stdin_packs_refs_snapshot &&
+           stdin_packs != STDIN_PACKS_MODE_FOLLOW_REACHABLE)
+               die(_("--refs-snapshot can only be used with --stdin-packs=follow-reachable"));
+
        if (cruft) {
                if (use_internal_rev_list)
                        die(_("cannot use internal rev list with --cruft"));