@matheusbsbs / matheus-bagliano / commits / d96075428a
Fork 0

multi-pack-index: use hash version byte

Similar to the commit-graph format, the multi-pack-index format has a byte in the header intended to track the hash version used to write the file. This allows one to interpret the hash length without having the context of the repository config specifying the hash length. This was not modified as part of the SHA-256 work because the hash length was automatically up-shifted due to that config. Since we have this byte available, we can make the file formats more obviously incompatible instead of relying on other context from the repository. Add a new oid_version() method in midx.c similar to the one in commit-graph.c. This is specifically made separate from that implementation to avoid artificially linking the formats. The test impact requires a few more things than the corresponding change in the commit-graph format. Specifically, 'test-tool read-midx' was not writing anything about this header value to output. Since the value available in 'struct multi_pack_index' is hash_len instead of a version value, we output "20" or "32" instead of "1" or "2". Since we want a user to not have their Git commands fail if their multi-pack-index has the incorrect hash version compared to the repository's hash version, we relax the die() to an error() in load_multi_pack_index(). This has some effect on 'git multi-pack-index verify' as we need to check that a failed parse of a file that exists is actually a verify error. For that test that checks the hash version matches, we change the corrupted byte from "2" to "3" to ensure the test fails for both hash algorithms. Helped-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Reviewed-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Aug 17, 2020 at 14:04 UTC d96075428a9a1de720f6019242dadb5dbaa0ac16
4 files changed +80 -13
Documentation/technical/pack-format.txt
+6 -1
@@ -273,7 +273,12 @@ HEADER:
273 Git only writes or recognizes version 1.
274
275 1-byte Object Id Version
276 - Git only writes or recognizes version 1 (SHA1).
276 + We infer the length of object IDs (OIDs) from this value:
277 + 1 => SHA-1
278 + 2 => SHA-256
279 + If the hash type does not match the repository's hash algorithm,
280 + the multi-pack-index file should be ignored with a warning
281 + presented to the user.
282
283 1-byte number of "chunks"
284
midx.c
+29 -6
@@ -17,7 +17,6 @@
17 #define MIDX_BYTE_HASH_VERSION 5
18 #define MIDX_BYTE_NUM_CHUNKS 6
19 #define MIDX_BYTE_NUM_PACKS 8
20 -#define MIDX_HASH_VERSION 1
20 #define MIDX_HEADER_SIZE 12
21 #define MIDX_MIN_SIZE (MIDX_HEADER_SIZE + the_hash_algo->rawsz)
22
@@ -36,6 +35,18 @@
35
36 #define PACK_EXPIRED UINT_MAX
37
38 +static uint8_t oid_version(void)
39 +{
40 + switch (hash_algo_by_ptr(the_hash_algo)) {
41 + case GIT_HASH_SHA1:
42 + return 1;
43 + case GIT_HASH_SHA256:
44 + return 2;
45 + default:
46 + die(_("invalid hash version"));
47 + }
48 +}
49 +
50 static char *get_midx_filename(const char *object_dir)
51 {
52 return xstrfmt("%s/pack/multi-pack-index", object_dir);
@@ -90,8 +101,11 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local
101 m->version);
102
103 hash_version = m->data[MIDX_BYTE_HASH_VERSION];
93 - if (hash_version != MIDX_HASH_VERSION)
94 - die(_("hash version %u does not match"), hash_version);
104 + if (hash_version != oid_version()) {
105 + error(_("multi-pack-index hash version %u does not match version %u"),
106 + hash_version, oid_version());
107 + goto cleanup_fail;
108 + }
109 m->hash_len = the_hash_algo->rawsz;
110
111 m->num_chunks = m->data[MIDX_BYTE_NUM_CHUNKS];
@@ -418,7 +432,7 @@ static size_t write_midx_header(struct hashfile *f,
432
433 hashwrite_be32(f, MIDX_SIGNATURE);
434 byte_values[0] = MIDX_VERSION;
421 - byte_values[1] = MIDX_HASH_VERSION;
435 + byte_values[1] = oid_version();
436 byte_values[2] = num_chunks;
437 byte_values[3] = 0; /* unused */
438 hashwrite(f, byte_values, sizeof(byte_values));
@@ -1105,8 +1119,17 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag
1119 struct multi_pack_index *m = load_multi_pack_index(object_dir, 1);
1120 verify_midx_error = 0;
1121
1108 - if (!m)
1109 - return 0;
1122 + if (!m) {
1123 + int result = 0;
1124 + struct stat sb;
1125 + char *filename = get_midx_filename(object_dir);
1126 + if (!stat(filename, &sb)) {
1127 + error(_("multi-pack-index file exists, but failed to parse"));
1128 + result = 1;
1129 + }
1130 + free(filename);
1131 + return result;
1132 + }
1133
1134 if (flags & MIDX_PROGRESS)
1135 progress = start_progress(_("Looking for referenced packfiles"),
t/helper/test-read-midx.c
+6 -2
@@ -7,14 +7,18 @@
7 static int read_midx_file(const char *object_dir)
8 {
9 uint32_t i;
10 - struct multi_pack_index *m = load_multi_pack_index(object_dir, 1);
10 + struct multi_pack_index *m;
11 +
12 + setup_git_directory();
13 + m = load_multi_pack_index(object_dir, 1);
14
15 if (!m)
16 return 1;
17
15 - printf("header: %08x %d %d %d\n",
18 + printf("header: %08x %d %d %d %d\n",
19 m->signature,
20 m->version,
21 + m->hash_len,
22 m->num_chunks,
23 m->num_packs);
24
t/t5319-multi-pack-index.sh
+39 -4
@@ -5,6 +5,8 @@ test_description='multi-pack-indexes'
5
6 objdir=.git/objects
7
8 +HASH_LEN=$(test_oid rawsz)
9 +
10 midx_read_expect () {
11 NUM_PACKS=$1
12 NUM_OBJECTS=$2
@@ -13,7 +15,7 @@ midx_read_expect () {
15 EXTRA_CHUNKS="$5"
16 {
17 cat <<-EOF &&
16 - header: 4d494458 1 $NUM_CHUNKS $NUM_PACKS
18 + header: 4d494458 1 $HASH_LEN $NUM_CHUNKS $NUM_PACKS
19 chunks: pack-names oid-fanout oid-lookup object-offsets$EXTRA_CHUNKS
20 num_objects: $NUM_OBJECTS
21 packs:
@@ -46,7 +48,7 @@ test_expect_success "don't write midx with no packs" '
48 test_path_is_missing pack/multi-pack-index
49 '
50
49 -test_expect_success "Warn if a midx contains no oid" '
51 +test_expect_success SHA1 'warn if a midx contains no oid' '
52 cp "$TEST_DIRECTORY"/t5319/no-objects.midx $objdir/pack/multi-pack-index &&
53 test_must_fail git multi-pack-index verify &&
54 rm $objdir/pack/multi-pack-index
@@ -198,6 +200,40 @@ test_expect_success 'write midx with twelve packs' '
200
201 compare_results_with_midx "twelve packs"
202
203 +test_expect_success 'warn on improper hash version' '
204 + git init --object-format=sha1 sha1 &&
205 + (
206 + cd sha1 &&
207 + git config core.multiPackIndex true &&
208 + test_commit 1 &&
209 + git repack -a &&
210 + git multi-pack-index write &&
211 + mv .git/objects/pack/multi-pack-index ../mpi-sha1
212 + ) &&
213 + git init --object-format=sha256 sha256 &&
214 + (
215 + cd sha256 &&
216 + git config core.multiPackIndex true &&
217 + test_commit 1 &&
218 + git repack -a &&
219 + git multi-pack-index write &&
220 + mv .git/objects/pack/multi-pack-index ../mpi-sha256
221 + ) &&
222 + (
223 + cd sha1 &&
224 + mv ../mpi-sha256 .git/objects/pack/multi-pack-index &&
225 + git log -1 2>err &&
226 + test_i18ngrep "multi-pack-index hash version 2 does not match version 1" err
227 + ) &&
228 + (
229 + cd sha256 &&
230 + mv ../mpi-sha1 .git/objects/pack/multi-pack-index &&
231 + git log -1 2>err &&
232 + test_i18ngrep "multi-pack-index hash version 1 does not match version 2" err
233 + )
234 +'
235 +
236 +
237 test_expect_success 'verify multi-pack-index success' '
238 git multi-pack-index verify --object-dir=$objdir
239 '
@@ -243,7 +279,6 @@ test_expect_success 'verify bad signature' '
279 "multi-pack-index signature"
280 '
281
246 -HASH_LEN=$(test_oid rawsz)
282 NUM_OBJECTS=74
283 MIDX_BYTE_VERSION=4
284 MIDX_BYTE_OID_VERSION=5
@@ -272,7 +307,7 @@ test_expect_success 'verify bad version' '
307 '
308
309 test_expect_success 'verify bad OID version' '
275 - corrupt_midx_and_verify $MIDX_BYTE_OID_VERSION "\02" $objdir \
310 + corrupt_midx_and_verify $MIDX_BYTE_OID_VERSION "\03" $objdir \
311 "hash version"
312 '
313