On Thu, Jul 5, 2018 at 8:54 PM Derrick Stolee <sto...@gmail.com> wrote:
> The core.multiPackIndex config setting controls the multi-pack-
> index (MIDX) feature. If false, the setting will disable all reads
> from the multi-pack-index file.
>
> Add comparison commands in t5319-multi-pack-index.sh to check
> typical Git behavior remains the same as the config setting is turned
> on and off. This currently includes 'git rev-list' and 'git log'
> commands to trigger several object database reads.
>
> Signed-off-by: Derrick Stolee <dsto...@microsoft.com>
> ---
> diff --git a/cache.h b/cache.h
> @@ -814,6 +814,7 @@ extern char *git_replace_ref_base;
> +extern int core_multi_pack_index;
> diff --git a/config.c b/config.c
> @@ -1313,6 +1313,11 @@ static int git_default_core_config(const char *var,
> const char *value)
> + if (!strcmp(var, "core.multipackindex")) {
> + core_multi_pack_index = git_config_bool(var, value);
> + return 0;
> + }
This is a rather unusual commit. This new configuration is assigned,
but it's never actually consulted, which means that it's impossible
for it to have any impact on functionality, yet tests are being added
to check whether it _did_ have any impact on functionality. Confusing.
Patch 17 does consult 'core_multi_pack_index', so it's only at that
point that it could have any impact. This situation would be less
confusing if you swapped patches 16 and 17 (and, of course, move the
declaration of 'core_multi_pack_index' to patch 17 with a reasonable
default value).
> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
> @@ -61,12 +63,42 @@ test_expect_success 'write midx with one v1 pack' '
> +midx_git_two_modes() {
> + git -c core.multiPackIndex=false $1 >expect &&
> + git -c core.multiPackIndex=true $1 >actual &&
> + test_cmp expect actual
> +}
> +
> +compare_results_with_midx() {
> + MSG=$1
> + test_expect_success "check normal git operations: $MSG" '
> + midx_git_two_modes "rev-list --objects --all" &&
> + midx_git_two_modes "log --raw"
> + '
> +}
Here, you define midx_git_two_modes() and compare_results_with_midx()...
> test_expect_success 'write midx with one v2 pack' '
> - git pack-objects --index-version=2,0x40 pack/test <obj-list &&
> - git multi-pack-index --object-dir=. write &&
> - midx_read_expect 1 17 4 .
> + git pack-objects --index-version=2,0x40 $objdir/pack/test <obj-list &&
> + git multi-pack-index --object-dir=$objdir write &&
> + midx_read_expect 1 17 4 $objdir
> '
>
> +midx_git_two_modes() {
> + git -c core.multiPackIndex=false $1 >expect &&
> + git -c core.multiPackIndex=true $1 >actual &&
> + test_cmp expect actual
> +}
> +
> +compare_results_with_midx() {
> + MSG=$1
> + test_expect_success "check normal git operations: $MSG" '
> + midx_git_two_modes "rev-list --objects --all" &&
> + midx_git_two_modes "log --raw"
> + '
> +}
... and here you define both functions again.
> +
> +compare_results_with_midx "one v2 pack"
