builtin/pack-objects.c: ensure included `--stdin-packs` exist

A subsequent patch will teach `want_object_in_pack()` to set its `*found_pack` and `*found_offset` poitners to NULL when the provided pack does not pass the `is_pack_valid()` check. The `--stdin-packs` mode of `pack-objects` is not quite prepared to handle this. To prepare it for this change, do the following two things: - Ensure provided packs pass the `is_pack_valid()` check when collecting the caller-provided packs into the "included" and "excluded" lists. - Gracefully handle any _invalid_ packs being passed to `want_object_in_pack()`. Calling `is_pack_valid()` early on makes it substantially less likely that we will have to deal with a pack going away, since we'll have an open file descriptor on its contents much earlier. But even packs with open descriptors can become invalid in the future if we (a) hit our open descriptor limit, forcing us to close some open packs, and (b) one of those just-closed packs has gone away in the meantime. `add_object_entry_from_pack()` depends on having a non-NULL `*found_pack`, since it passes that pointer to `packed_object_info()`, meaning that we would SEGV if the pointer became NULL (like we propose to do in `want_object_in_pack()` in the following patch). But avoiding calling `packed_object_info()` entirely is OK, too, since its only purpose is to identify which objects in the included packs are commits, so that they can form the tips of the advisory traversal used to discover the object namehashes. Failing to do this means that at worst we will produce lower-quality deltas, but it does not prevent us from generating the pack as long as we can find a copy of each object from the disappearing pack in some other part of the repository. Co-authored-by: Victoria Dye <vdye@github.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed May 24, 2022 at 14:54 UTC 5045759de85d40520036c0216dbc51015ef833e7
1 file changed +20 -15
builtin/pack-objects.c
+20 -15
@@ -3193,10 +3193,8 @@ static int add_object_entry_from_pack(const struct object_id *oid,
3193 uint32_t pos,
3194 void *_data)
3195 {
3196 - struct rev_info *revs = _data;
3197 - struct object_info oi = OBJECT_INFO_INIT;
3196 off_t ofs;
3199 - enum object_type type;
3197 + enum object_type type = OBJ_NONE;
3198
3199 display_progress(progress_state, ++nr_seen);
3200
@@ -3207,19 +3205,24 @@ static int add_object_entry_from_pack(const struct object_id *oid,
3205 if (!want_object_in_pack(oid, 0, &p, &ofs))
3206 return 0;
3207
3210 - oi.typep = &type;
3211 - if (packed_object_info(the_repository, p, ofs, &oi) < 0)
3212 - die(_("could not get type of object %s in pack %s"),
3213 - oid_to_hex(oid), p->pack_name);
3214 - else if (type == OBJ_COMMIT) {
3215 - /*
3216 - * commits in included packs are used as starting points for the
3217 - * subsequent revision walk
3218 - */
3219 - add_pending_oid(revs, NULL, oid, 0);
3220 - }
3208 + if (p) {
3209 + struct rev_info *revs = _data;
3210 + struct object_info oi = OBJECT_INFO_INIT;
3211
3222 - stdin_packs_found_nr++;
3212 + oi.typep = &type;
3213 + if (packed_object_info(the_repository, p, ofs, &oi) < 0) {
3214 + die(_("could not get type of object %s in pack %s"),
3215 + oid_to_hex(oid), p->pack_name);
3216 + } else if (type == OBJ_COMMIT) {
3217 + /*
3218 + * commits in included packs are used as starting points for the
3219 + * subsequent revision walk
3220 + */
3221 + add_pending_oid(revs, NULL, oid, 0);
3222 + }
3223 +
3224 + stdin_packs_found_nr++;
3225 + }
3226
3227 create_object_entry(oid, type, 0, 0, 0, p, ofs);
3228
@@ -3338,6 +3341,8 @@ static void read_packs_list_from_stdin(void)
3341 struct packed_git *p = item->util;
3342 if (!p)
3343 die(_("could not find pack '%s'"), item->string);
3344 + if (!is_pack_valid(p))
3345 + die(_("packfile %s cannot be accessed"), p->pack_name);
3346 }
3347
3348 /*