| Message ID | 81ab11e18c0b00030019f9f521216f3469fdd744.1610129796.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | pack-revindex: prepare for on-disk reverse index | expand |
On Fri, Jan 08, 2021 at 01:16:53PM -0500, Taylor Blau wrote: > - offset = reuse_packfile->revindex[pos].offset; > - next = reuse_packfile->revindex[pos + 1].offset; > + offset = pack_pos_to_offset(reuse_packfile, pos); > + next = pack_pos_to_offset(reuse_packfile, pos + 1); Makes sense. > @@ -887,11 +887,15 @@ static void write_reused_pack_one(size_t pos, struct hashfile *out, > > /* Convert to REF_DELTA if we must... */ > if (!allow_ofs_delta) { > - int base_pos = find_revindex_position(reuse_packfile, base_offset); > + uint32_t base_pos; > struct object_id base_oid; > > + if (offset_to_pack_pos(reuse_packfile, base_offset, &base_pos) < 0) > + die(_("expected object at offset %"PRIuMAX), > + (uintmax_t)base_offset); This error does mention the offset, which is good. But not the pack name (nor the object name, but we don't have it!). -Peff
On Tue, Jan 12, 2021 at 03:50:05AM -0500, Jeff King wrote: > > @@ -887,11 +887,15 @@ static void write_reused_pack_one(size_t pos, struct hashfile *out, > > > > /* Convert to REF_DELTA if we must... */ > > if (!allow_ofs_delta) { > > - int base_pos = find_revindex_position(reuse_packfile, base_offset); > > + uint32_t base_pos; > > struct object_id base_oid; > > > > + if (offset_to_pack_pos(reuse_packfile, base_offset, &base_pos) < 0) > > + die(_("expected object at offset %"PRIuMAX), > > + (uintmax_t)base_offset); > > This error does mention the offset, which is good. But not the pack name > (nor the object name, but we don't have it!). Indeed we don't have the object name, but the pack is reuse_packfile (which is statistically initialized), so we could do something like: diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 10a16ced1e..8e40b19ee8 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -893,8 +893,10 @@ static void write_reused_pack_one(size_t pos, struct hashfile *out, struct object_id base_oid; if (offset_to_pack_pos(reuse_packfile, base_offset, &base_pos) < 0) - die(_("expected object at offset %"PRIuMAX), - (uintmax_t)base_offset); + die(_("expected object at offset %"PRIuMAX" " + "in pack %s"), + (uintmax_t)base_offset, + reuse_packfile->pack_name); nth_packed_object_id(&base_oid, reuse_packfile, pack_pos_to_index(reuse_packfile, base_pos)); Which I think would be clearer. > -Peff Thanks, Taylor
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 03d25db442..ea7df9270f 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -866,8 +866,8 @@ static void write_reused_pack_one(size_t pos, struct hashfile *out, enum object_type type; unsigned long size; - offset = reuse_packfile->revindex[pos].offset; - next = reuse_packfile->revindex[pos + 1].offset; + offset = pack_pos_to_offset(reuse_packfile, pos); + next = pack_pos_to_offset(reuse_packfile, pos + 1); record_reused_object(offset, offset - hashfile_total(out)); @@ -887,11 +887,15 @@ static void write_reused_pack_one(size_t pos, struct hashfile *out, /* Convert to REF_DELTA if we must... */ if (!allow_ofs_delta) { - int base_pos = find_revindex_position(reuse_packfile, base_offset); + uint32_t base_pos; struct object_id base_oid; + if (offset_to_pack_pos(reuse_packfile, base_offset, &base_pos) < 0) + die(_("expected object at offset %"PRIuMAX), + (uintmax_t)base_offset); + nth_packed_object_id(&base_oid, reuse_packfile, - reuse_packfile->revindex[base_pos].nr); + pack_pos_to_index(reuse_packfile, base_pos)); len = encode_in_pack_object_header(header, sizeof(header), OBJ_REF_DELTA, size);
Replace direct revindex accesses with calls to 'pack_pos_to_offset()' and 'pack_pos_to_index()'. Signed-off-by: Taylor Blau <me@ttaylorr.com> --- builtin/pack-objects.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)