Re: [PATCH v2 3/3] gc: perform incremental repack when implictly enabled
From: Eric Sunshine <hidden>
Date: 2021-09-29 06:39:43
On 9/17/21 6:54 PM, Glen Choo wrote:
quoted hunk ↗ jump to hunk
builtin/gc.c has two ways of checking if multi-pack-index is enabled: - git_config_get_bool() in incremental_repack_auto_condition() - the_repository->settings.core_multi_pack_index in maintenance_task_incremental_repack() The two implementations have existed since the incremental-repack task was introduced in e841a79a13 (maintenance: add incremental-repack auto condition, 2020-09-25). These two values can diverge because prepare_repo_settings() enables the feature in the_repository->settings by default. In the case where core.multiPackIndex is not set in the config, the auto condition would fail, causing the incremental-repack task to not be run. Because we always want to consider the default values, we should just always just use the_repository->settings. Standardize on using the_repository->settings.core_multi_pack_index to check if multi-pack-index is enabled. Signed-off-by: Glen Choo <redacted> ---diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh@@ -322,31 +322,69 @@ test_expect_success EXPENSIVE 'incremental-repack 2g limit' ' test_expect_success 'maintenance.incremental-repack.auto' ' + ( + git init incremental-repack-true && + cd incremental-repack-true && + git config core.multiPackIndex true && + test_commit A && + git repack -adk && + git multi-pack-index write && + GIT_TRACE2_EVENT="$(pwd)/midx-init.txt" git \ + -c maintenance.incremental-repack.auto=1 \ + maintenance run --auto --task=incremental-repack 2>/dev/null && + test_subcommand ! git multi-pack-index write --no-progress <midx-init.txt && + test_commit B && + git pack-objects --revs .git/objects/pack/pack <<-\EOF && + HEAD + ^HEAD~1 + EOF + GIT_TRACE2_EVENT=$(pwd)/trace-A git \ + -c maintenance.incremental-repack.auto=2 \ + maintenance run --auto --task=incremental-repack 2>/dev/null && + test_subcommand ! git multi-pack-index write --no-progress <trace-A && + test_commit C && + git pack-objects --revs .git/objects/pack/pack <<-\EOF && + HEAD + ^HEAD~1 + EOF + GIT_TRACE2_EVENT=$(pwd)/trace-B git \ + -c maintenance.incremental-repack.auto=2 \ + maintenance run --auto --task=incremental-repack 2>/dev/null && + test_subcommand git multi-pack-index write --no-progress <trace-B + ) +' + +test_expect_success 'maintenance.incremental-repack.auto (when config is unset)' ' + ( + git init incremental-repack-unset && + cd incremental-repack-unset && + test_unconfig core.multiPackIndex && + test_commit A && + git repack -adk && + git multi-pack-index write && + GIT_TRACE2_EVENT="$(pwd)/midx-init.txt" git \ + -c maintenance.incremental-repack.auto=1 \ + maintenance run --auto --task=incremental-repack 2>/dev/null && + test_subcommand ! git multi-pack-index write --no-progress <midx-init.txt && + test_commit B && + git pack-objects --revs .git/objects/pack/pack <<-\EOF && + HEAD + ^HEAD~1 + EOF + GIT_TRACE2_EVENT=$(pwd)/trace-A git \ + -c maintenance.incremental-repack.auto=2 \ + maintenance run --auto --task=incremental-repack 2>/dev/null && + test_subcommand ! git multi-pack-index write --no-progress <trace-A && + test_commit C && + git pack-objects --revs .git/objects/pack/pack <<-\EOF && + HEAD + ^HEAD~1 + EOF + GIT_TRACE2_EVENT=$(pwd)/trace-B git \ + -c maintenance.incremental-repack.auto=2 \ + maintenance run --auto --task=incremental-repack 2>/dev/null && + test_subcommand git multi-pack-index write --no-progress <trace-B + ) '
The amount of code common between the two tests is significant. In
simple cases, such duplication may not be worth worrying about, but it
feels like we can do better here. There are a number of ways to re-use
the code between tests. One such way would be to do something like this:
run_incremental_repack () {
test_commit A &&
...
test_subcommand ...
}
test_expect_success 'maintenance.incremental-repack.auto' '
rm -rf incremental-repack &&
git init incremental-repack &&
(
cd incremental-repack &&
git config core.multiPackIndex true &&
run_incremental_repack
)
'
test_expect_success 'maintenance.incremental-repack.auto (config
unset)' '
rm -rf incremental-repack &&
git init incremental-repack &&
(
cd incremental-repack &&
test_unconfig core.multiPackIndex &&
run_incremental_repack
)
'