pack-objects: clarify the use of object_entry::size

While this field most of the time contains the canonical object size, there is one case it does not: when we have found that the base object of the delta in question is also to be packed, we will very happily reuse the delta by copying it over instead of regenerating the new delta. "size" in this case will record the delta size, not canonical object size. Later on in write_reuse_object(), we reconstruct the delta header and "size" is used for this purpose. When this happens, the "type" field contains a delta type instead of a canonical type. Highlight this in the code since it could be tricky to see. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Nguyễn Thái Ngọc Duy committed Apr 14, 2018 at 17:35 UTC 27a7d0679f17bd536f566e76e51058de0e1fa17a
2 files changed +11 -4
builtin/pack-objects.c
+8 -3
@@ -1418,6 +1418,7 @@ static void check_object(struct object_entry *entry)
1418 off_t ofs;
1419 unsigned char *buf, c;
1420 enum object_type type;
1421 + unsigned long in_pack_size;
1422
1423 buf = use_pack(p, &w_curs, entry->in_pack_offset, &avail);
1424
@@ -1427,7 +1428,7 @@ static void check_object(struct object_entry *entry)
1428 */
1429 used = unpack_object_header_buffer(buf, avail,
1430 &type,
1430 - &entry->size);
1431 + &in_pack_size);
1432 if (used == 0)
1433 goto give_up;
1434
@@ -1444,6 +1445,7 @@ static void check_object(struct object_entry *entry)
1445 default:
1446 /* Not a delta hence we've already got all we need. */
1447 oe_set_type(entry, entry->in_pack_type);
1448 + entry->size = in_pack_size;
1449 entry->in_pack_header_size = used;
1450 if (oe_type(entry) < OBJ_COMMIT || oe_type(entry) > OBJ_BLOB)
1451 goto give_up;
@@ -1500,6 +1502,7 @@ static void check_object(struct object_entry *entry)
1502 * circular deltas.
1503 */
1504 oe_set_type(entry, entry->in_pack_type);
1505 + entry->size = in_pack_size; /* delta size */
1506 SET_DELTA(entry, base_entry);
1507 entry->delta_size = entry->size;
1508 entry->delta_sibling_idx = base_entry->delta_child_idx;
@@ -1509,13 +1512,15 @@ static void check_object(struct object_entry *entry)
1512 }
1513
1514 if (oe_type(entry)) {
1515 + off_t delta_pos;
1516 +
1517 /*
1518 * This must be a delta and we already know what the
1519 * final object type is. Let's extract the actual
1520 * object size from the delta header.
1521 */
1517 - entry->size = get_size_from_delta(p, &w_curs,
1518 - entry->in_pack_offset + entry->in_pack_header_size);
1522 + delta_pos = entry->in_pack_offset + entry->in_pack_header_size;
1523 + entry->size = get_size_from_delta(p, &w_curs, delta_pos);
1524 if (entry->size == 0)
1525 goto give_up;
1526 unuse_pack(&w_curs);
pack-objects.h
+3 -1
@@ -32,7 +32,9 @@ enum dfs_state {
32 *
33 * "size" is the uncompressed object size. Compressed size of the raw
34 * data for an object in a pack is not stored anywhere but is computed
35 - * and made available when reverse .idx is made.
35 + * and made available when reverse .idx is made. Note that when a
36 + * delta is reused, "size" is the uncompressed _delta_ size, not the
37 + * canonical one after the delta has been applied.
38 *
39 * "hash" contains a path name hash which is used for sorting the
40 * delta list and also during delta searching. Once prepare_pack()