midx: pass custom '--base' through incremental writes

The 'multi-pack-index' builtin parses '--base' for incremental writes,
but the normal write path does not pass that value through to
`write_midx_file()`.

As a result, something like:

    $ git multi-pack-index write --incremental --base=<base>

behaves as if no custom base had been given (unless the caller used the
'--stdin-packs' path).

Thread the parsed base through `write_midx_file()`, and update the
repack caller to pass NULL for the new argument where no custom base
selection is needed.

This exposes a pre-existing problem in incremental writes with custom
bases: the writer skips packs from the full existing MIDX chain, even
when the caller selected an older base or no base at all.

The affected t5334 cases fail while trying to write MIDX bitmaps. The
detached layer omits packs above the selected base, and thus the
resulting MIDX does not have a reachability closure, making it
impossible to generate reachability bitmaps.

Mark those tests as expected failures accordingly. The following commit
will fix the broken behavior and restore these tests.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
index 00ffb36394d08c5bf4fb77cf880f1732ac57686b..949bfa796b28a59c90d17b638df28cd5b9234079 100644 (file)
--- a/builtin/multi-pack-index.c
+++ b/builtin/multi-pack-index.c
@@ -224,7 +224,8 @@ static int cmd_multi_pack_index_write(int argc, const char **argv,
        }
 
        ret = write_midx_file(source, opts.preferred_pack,
-                             opts.refs_snapshot, opts.flags);
+                             opts.refs_snapshot, opts.incremental_base,
+                             opts.flags);
 
        free(opts.refs_snapshot);
        return ret;

diff --git a/builtin/repack.c b/builtin/repack.c
index 1524a9c13ad5b87e97adc7b5b5a710f22cadbe4c..0092a72a996caeff5ffb9b1b8cd13de0e5a39379 100644 (file)
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -629,7 +629,7 @@ int cmd_repack(int argc,
                unsigned flags = 0;
                if (git_env_bool(GIT_TEST_MULTI_PACK_INDEX_WRITE_INCREMENTAL, 0))
                        flags |= MIDX_WRITE_INCREMENTAL;
-               write_midx_file(existing.source, NULL, NULL, flags);
+               write_midx_file(existing.source, NULL, NULL, NULL, flags);
        }
 
 cleanup:

diff --git a/midx-write.c b/midx-write.c
index 561e9eedc0e6ef245acc193e721cc4f6b3407101..aa438775ebd23fbd5eec0b4277a0fdf6af8b7987 100644 (file)
--- a/midx-write.c
+++ b/midx-write.c
@@ -1850,12 +1850,14 @@ cleanup:
 int write_midx_file(struct odb_source *source,
                    const char *preferred_pack_name,
                    const char *refs_snapshot,
+                   const char *incremental_base,
                    unsigned flags)
 {
        struct write_midx_opts opts = {
                .source = source,
                .preferred_pack_name = preferred_pack_name,
                .refs_snapshot = refs_snapshot,
+               .incremental_base = incremental_base,
                .flags = flags,
        };
 

diff --git a/midx.h b/midx.h
index 63853a03a47fd11ee3df3f90175a282bbb6ccbb3..92ed29d913d9a39b1ed1d35e451e5a9f6cec49d2 100644 (file)
--- a/midx.h
+++ b/midx.h
@@ -131,7 +131,7 @@ int prepare_multi_pack_index_one(struct odb_source *source);
  */
 int write_midx_file(struct odb_source *source,
                    const char *preferred_pack_name, const char *refs_snapshot,
-                   unsigned flags);
+                   const char *incremental_base, unsigned flags);
 int write_midx_file_only(struct odb_source *source,
                         struct string_list *packs_to_include,
                         const char *preferred_pack_name,

diff --git a/t/t5334-incremental-multi-pack-index.sh b/t/t5334-incremental-multi-pack-index.sh
index 68a103d13d23c349f835643b44f26809036492e5..69e96bf8d93713a9bd2c584954f3b819bfaae2d9 100755 (executable)
--- a/t/t5334-incremental-multi-pack-index.sh
+++ b/t/t5334-incremental-multi-pack-index.sh
@@ -119,7 +119,7 @@ test_expect_success 'write MIDX layer with --base without --no-write-chain-file'
        test_grep "cannot use --base without --no-write-chain-file" err
 '
 
-test_expect_success 'write MIDX layer with --base=none and --no-write-chain-file' '
+test_expect_failure 'write MIDX layer with --base=none and --no-write-chain-file' '
        test_commit base-none &&
        git repack -d &&
 
@@ -128,19 +128,33 @@ test_expect_success 'write MIDX layer with --base=none and --no-write-chain-file
                --no-write-chain-file --base=none)" &&
 
        test_cmp "$midx_chain.bak" "$midx_chain" &&
-       test_path_is_file "$midxdir/multi-pack-index-$layer.midx"
+       test_path_is_file "$midxdir/multi-pack-index-$layer.midx" &&
+
+       echo "$layer" >"$midx_chain" &&
+       test-tool read-midx --show-objects "$objdir" "$layer" >midx.objects &&
+       test_grep "^$(git rev-parse 2.2) " midx.objects &&
+       cp "$midx_chain.bak" "$midx_chain"
 '
 
-test_expect_success 'write MIDX layer with --base=<hash> and --no-write-chain-file' '
+test_expect_failure 'write MIDX layer with --base=<hash> and --no-write-chain-file' '
        test_commit base-hash &&
        git repack -d &&
 
        cp "$midx_chain" "$midx_chain.bak" &&
+       base="$(nth_line 1 "$midx_chain")" &&
        layer="$(git multi-pack-index write --bitmap --incremental \
-               --no-write-chain-file --base="$(nth_line 1 "$midx_chain")")" &&
+               --no-write-chain-file --base="$base")" &&
 
        test_cmp "$midx_chain.bak" "$midx_chain" &&
-       test_path_is_file "$midxdir/multi-pack-index-$layer.midx"
+       test_path_is_file "$midxdir/multi-pack-index-$layer.midx" &&
+
+       {
+               echo "$base" &&
+               echo "$layer"
+       } >"$midx_chain" &&
+       test-tool read-midx --show-objects "$objdir" "$layer" >midx.objects &&
+       test_grep "^$(git rev-parse 2.2) " midx.objects &&
+       cp "$midx_chain.bak" "$midx_chain"
 '
 
 for reuse in false single multi