Re: [PATCH v3 1/1] builtin/pack-objects.c: freshen objects from existing cruft packs
From: Patrick Steinhardt <hidden>
Date: 2025-03-06 10:31:45
On Tue, Mar 04, 2025 at 07:15:18PM -0500, Taylor Blau wrote:
quoted hunk ↗ jump to hunk
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 58a9b161262..79e1e6fb52b 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c@@ -1502,8 +1503,60 @@ static int have_duplicate_entry(const struct object_id *oid, return 1; } +static int want_cruft_object_mtime(struct repository *r, + const struct object_id *oid, + unsigned flags, uint32_t mtime) +{ + struct packed_git **cache; + + for (cache = kept_pack_cache(r, flags); *cache; cache++) { + struct packed_git *p = *cache; + off_t ofs; + uint32_t candidate_mtime; + + ofs = find_pack_entry_one(oid, p); + if (!ofs) + continue; + + /* + * We have a copy of the object 'oid' in a non-cruft + * pack. We can avoid packing an additional copy + * regardless of what the existing copy's mtime is since + * it is outside of a cruft pack. + */ + if (!p->is_cruft) + return 0; + + /* + * If we have a copy of the object 'oid' in a cruft + * pack, then either read the cruft pack's mtime for + * that object, or, if that can't be loaded, assume the + * pack's mtime itself. + */ + if (!load_pack_mtimes(p)) { + uint32_t pos; + if (offset_to_pack_pos(p, ofs, &pos) < 0) + continue; + candidate_mtime = nth_packed_mtime(p, pos); + } else { + candidate_mtime = p->mtime; + } + + /* + * We have a surviving copy of the object in a cruft + * pack whose mtime is greater than or equal to the one + * we are considering. We can thus avoid packing an + * additional copy of that object. + */ + if (mtime <= candidate_mtime) + return 0; + } + + return -1; +} +
Minor nit: it is a bit unusual that a negative value, which typically indicates an error, is used as a boolean value here to indicate that we don't want to have the object.
quoted hunk ↗ jump to hunk
diff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh index 959e6e26488..f427150de5b 100755 --- a/t/t7704-repack-cruft.sh +++ b/t/t7704-repack-cruft.sh@@ -304,6 +304,69 @@ test_expect_success '--max-cruft-size with freshened objects (packed)' ' ) ' +test_expect_success '--max-cruft-size with freshened objects (previously cruft)' ' + git init max-cruft-size-threshold &&
Let's also delete the repository via `test_when_finished`. Patrick