@matheusbsbs / matheus-bagliano / commits / 861d4bc292

pack-objects: introduce GIT_TEST_PACK_PATH_WALK

There are many tests that validate whether 'git pack-objects' works as expected. Instead of duplicating these tests, add a new test environment variable, GIT_TEST_PACK_PATH_WALK, that implies --path-walk by default when specified. This was useful in testing the implementation of the --path-walk implementation, helping to find tests that are overly specific to the default object walk. These include: - t0411-clone-from-partial.sh : One test fetches from a repo that does not have the boundary objects. This causes the path-based walk to fail. Disable the variable for this test. - t5306-pack-nobase.sh : Similar to t0411, one test fetches from a repo without a boundary object. - t5310-pack-bitmaps.sh : One test compares the case when packing with bitmaps to the case when packing without them. Since we disable the test variable when writing bitmaps, this causes a difference in the object list (the --path-walk option adds an extra object). Specify --no-path-walk in both processes for the comparison. Another test checks for a specific delta base, but when computing dynamically without using bitmaps, the base object it too small to be considered in the delta calculations so no base is used. - t5316-pack-delta-depth.sh : This script cares about certain delta choices and their chain lengths. The --path-walk option changes how these chains are selected, and thus changes the results of this test. - t5322-pack-objects-sparse.sh : This demonstrates the effectiveness of the --sparse option and how it combines with --path-walk. - t5332-multi-pack-reuse.sh : This test verifies that the preferred pack is used for delta reuse when possible. The --path-walk option is not currently aware of the preferred pack at all, so finds a different delta base. - t7406-submodule-update.sh : When using the variable, the --depth option collides with the --path-walk feature, resulting in a warning message. Disable the variable so this warning does not appear. I want to call out one specific test change that is only temporary: - t5530-upload-pack-error.sh : One test cares specifically about an "unable to read" error message. Since the current implementation performs delta calculations within the path-walk API callback, a different "unable to get size" error message appears. When this is changed in a future refactoring, this test change can be reverted. Similar to GIT_TEST_NAME_HASH_VERSION, we do not add this option to the linux-TEST-vars CI build as that's already an overloaded build. Signed-off-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed May 16, 2025 at 18:11 UTC 861d4bc2925bfeed44ebb681b88b7baa6652d8a2
9 files changed +58 -7
builtin/pack-objects.c
+10 -2
@@ -226,7 +226,7 @@ static int delta_search_threads;
226 static int pack_to_stdout;
227 static int sparse;
228 static int thin;
229 -static int path_walk;
229 +static int path_walk = -1;
230 static int num_preferred_base;
231 static struct progress *progress_state;
232
@@ -4230,7 +4230,7 @@ static int add_objects_by_path(const char *path,
4230 struct object_id *oid = &oids->oid[i];
4231
4232 /* Skip objects that do not exist locally. */
4233 - if (exclude_promisor_objects &&
4233 + if ((exclude_promisor_objects || arg_missing_action != MA_ERROR) &&
4234 oid_object_info_extended(the_repository, oid, &oi,
4235 OBJECT_INFO_FOR_PREFETCH) < 0)
4236 continue;
@@ -4648,6 +4648,14 @@ int cmd_pack_objects(int argc,
4648 if (pack_to_stdout != !base_name || argc)
4649 usage_with_options(pack_usage, pack_objects_options);
4650
4651 + if (path_walk < 0) {
4652 + if (use_bitmap_index > 0 ||
4653 + !use_internal_rev_list)
4654 + path_walk = 0;
4655 + else
4656 + path_walk = git_env_bool("GIT_TEST_PACK_PATH_WALK", 0);
4657 + }
4658 +
4659 if (depth < 0)
4660 depth = 0;
4661 if (depth >= (1 << OE_DEPTH_BITS)) {
t/README
+4
@@ -415,6 +415,10 @@ GIT_TEST_PACK_SPARSE=<boolean> if disabled will default the pack-objects
415 builtin to use the non-sparse object walk. This can still be overridden by
416 the --sparse command-line argument.
417
418 +GIT_TEST_PACK_PATH_WALK=<boolean> if enabled will default the pack-objects
419 +builtin to use the path-walk API for the object walk. This can still be
420 +overridden by the --no-path-walk command-line argument.
421 +
422 GIT_TEST_PRELOAD_INDEX=<boolean> exercises the preload-index code path
423 by overriding the minimum number of cache entries required per thread.
424
t/t0411-clone-from-partial.sh
+6
@@ -59,6 +59,12 @@ test_expect_success 'pack-objects should fetch from promisor remote and execute
59
60 test_expect_success 'clone from promisor remote does not lazy-fetch by default' '
61 rm -f script-executed &&
62 +
63 + # The --path-walk feature of "git pack-objects" is not
64 + # compatible with this kind of fetch from an incomplete repo.
65 + GIT_TEST_PACK_PATH_WALK=0 &&
66 + export GIT_TEST_PACK_PATH_WALK &&
67 +
68 test_must_fail git clone evil no-lazy 2>err &&
69 test_grep "lazy fetching disabled" err &&
70 test_path_is_missing script-executed
t/t5306-pack-nobase.sh
+5
@@ -59,6 +59,11 @@ test_expect_success 'indirectly clone patch_clone' '
59 git pull ../.git &&
60 test $(git rev-parse HEAD) = $B &&
61
62 + # The --path-walk feature of "git pack-objects" is not
63 + # compatible with this kind of fetch from an incomplete repo.
64 + GIT_TEST_PACK_PATH_WALK=0 &&
65 + export GIT_TEST_PACK_PATH_WALK &&
66 +
67 git pull ../patch_clone/.git &&
68 test $(git rev-parse HEAD) = $C
69 )
t/t5310-pack-bitmaps.sh
+11 -2
@@ -158,8 +158,9 @@ test_bitmap_cases () {
158 ls .git/objects/pack/ | grep bitmap >output &&
159 test_line_count = 1 output &&
160 # verify equivalent packs are generated with/without using bitmap index
161 - packasha1=$(git pack-objects --no-use-bitmap-index --all packa </dev/null) &&
162 - packbsha1=$(git pack-objects --use-bitmap-index --all packb </dev/null) &&
161 + # Be careful to not use the path-walk option in either case.
162 + packasha1=$(git pack-objects --no-use-bitmap-index --no-path-walk --all packa </dev/null) &&
163 + packbsha1=$(git pack-objects --use-bitmap-index --no-path-walk --all packb </dev/null) &&
164 list_packed_objects packa-$packasha1.idx >packa.objects &&
165 list_packed_objects packb-$packbsha1.idx >packb.objects &&
166 test_cmp packa.objects packb.objects
@@ -388,6 +389,14 @@ test_bitmap_cases () {
389 git init --bare client.git &&
390 (
391 cd client.git &&
392 +
393 + # This test relies on reusing a delta, but if the
394 + # path-walk machinery is engaged, the base object
395 + # is considered too small to use during the
396 + # dynamic computation, so is not used.
397 + GIT_TEST_PACK_PATH_WALK=0 &&
398 + export GIT_TEST_PACK_PATH_WALK &&
399 +
400 git config transfer.unpackLimit 1 &&
401 git fetch .. delta-reuse-old:delta-reuse-old &&
402 git fetch .. delta-reuse-new:delta-reuse-new &&
t/t5316-pack-delta-depth.sh
+6 -3
@@ -89,15 +89,18 @@ max_chain() {
89 # adjusted (or scrapped if the heuristics have become too unreliable)
90 test_expect_success 'packing produces a long delta' '
91 # Use --window=0 to make sure we are seeing reused deltas,
92 - # not computing a new long chain.
93 - pack=$(git pack-objects --all --window=0 </dev/null pack) &&
92 + # not computing a new long chain. (Also avoid the --path-walk
93 + # option as it may break delta chains.)
94 + pack=$(git pack-objects --all --window=0 --no-path-walk </dev/null pack) &&
95 echo 9 >expect &&
96 max_chain pack-$pack.pack >actual &&
97 test_cmp expect actual
98 '
99
100 test_expect_success '--depth limits depth' '
100 - pack=$(git pack-objects --all --depth=5 </dev/null pack) &&
101 + # Avoid --path-walk to avoid breaking delta chains across path
102 + # boundaries.
103 + pack=$(git pack-objects --all --depth=5 --no-path-walk </dev/null pack) &&
104 echo 5 >expect &&
105 max_chain pack-$pack.pack >actual &&
106 test_cmp expect actual
t/t5332-multi-pack-reuse.sh
+7
@@ -7,6 +7,13 @@ test_description='pack-objects multi-pack reuse'
7
8 GIT_TEST_MULTI_PACK_INDEX=0
9 GIT_TEST_MULTI_PACK_INDEX_WRITE_INCREMENTAL=0
10 +
11 +# The --path-walk option does not consider the preferred pack
12 +# at all for reusing deltas, so this variable changes the
13 +# behavior of this test, if enabled.
14 +GIT_TEST_PACK_PATH_WALK=0
15 +export GIT_TEST_PACK_PATH_WALK
16 +
17 objdir=.git/objects
18 packdir=$objdir/pack
19
t/t5530-upload-pack-error.sh
+6
@@ -34,6 +34,12 @@ test_expect_success 'upload-pack fails due to error in pack-objects packing' '
34 hexsz=$(test_oid hexsz) &&
35 printf "%04xwant %s\n00000009done\n0000" \
36 $(($hexsz + 10)) $head >input &&
37 +
38 + # The current implementation of path-walk causes a different
39 + # error message. This will be changed by a future refactoring.
40 + GIT_TEST_PACK_PATH_WALK=0 &&
41 + export GIT_TEST_PACK_PATH_WALK &&
42 +
43 test_must_fail git upload-pack . <input >/dev/null 2>output.err &&
44 test_grep "unable to read" output.err &&
45 test_grep "pack-objects died" output.err
t/t7406-submodule-update.sh
+3
@@ -1095,12 +1095,15 @@ test_expect_success 'submodule update --quiet passes quietness to fetch with a s
1095 (cd super5 &&
1096 # This test var can mess with the stderr output checked in this test.
1097 GIT_TEST_NAME_HASH_VERSION=1 \
1098 + GIT_TEST_PACK_PATH_WALK=0 \
1099 git submodule update --quiet --init --depth=1 submodule3 >out 2>err &&
1100 test_must_be_empty out &&
1101 test_must_be_empty err
1102 ) &&
1103 git clone super4 super6 &&
1104 (cd super6 &&
1105 + # This test variable will create a "warning" message to stderr
1106 + GIT_TEST_PACK_PATH_WALK=0 \
1107 git submodule update --init --depth=1 submodule3 >out 2>err &&
1108 test_file_not_empty out &&
1109 test_file_not_empty err