midx: reject empty `--preferred-pack`'s

The soon-to-be-implemented multi-pack bitmap treats object in the first
bit position specially by assuming that all objects in the pack it was
selected from are also represented from that pack in the MIDX. In other
words, the pack from which the first object was selected must also have
all of its other objects selected from that same pack in the MIDX in
case of any duplicates.

But this assumption relies on the fact that there is at least one object
in that pack to begin with; otherwise the object in the first bit
position isn't from a preferred pack, in which case we can no longer
assume that all objects in that pack were also selected from the same
pack.

Guard this assumption by checking the number of objects in the given
preferred pack, and failing if the given pack is empty.

To make sure we can safely perform this check, open any packs which are
contained in an existing MIDX via prepare_midx_pack(). The same is done
for new packs via the add_pack_to_midx() callback, but packs picked up
from a previous MIDX will not yet have these opened.

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

diff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt
index ffd601bc17b4dc8f453e6a5b28fb4d6239176d96..c9b063d31e1df8ab8a0e0d26fdf5c963ae9a1b54 100644 (file)
--- a/Documentation/git-multi-pack-index.txt
+++ b/Documentation/git-multi-pack-index.txt
@@ -37,9 +37,9 @@ write::
 --
        --preferred-pack=<pack>::
                Optionally specify the tie-breaking pack used when
-               multiple packs contain the same object. If not given,
-               ties are broken in favor of the pack with the lowest
-               mtime.
+               multiple packs contain the same object. `<pack>` must
+               contain at least one object. If not given, ties are
+               broken in favor of the pack with the lowest mtime.
 --
 
 verify::

diff --git a/midx.c b/midx.c
index 0bcb403bae23a4b7ada87735f4ecc1f687cc4be9..26089ec9c793454925ef30363bf849eebcdb2c78 100644 (file)
--- a/midx.c
+++ b/midx.c
@@ -934,6 +934,25 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *
                        ctx.info[ctx.nr].pack_name = xstrdup(ctx.m->pack_names[i]);
                        ctx.info[ctx.nr].p = NULL;
                        ctx.info[ctx.nr].expired = 0;
+
+                       if (flags & MIDX_WRITE_REV_INDEX) {
+                               /*
+                                * If generating a reverse index, need to have
+                                * packed_git's loaded to compare their
+                                * mtimes and object count.
+                                */
+                               if (prepare_midx_pack(the_repository, ctx.m, i)) {
+                                       error(_("could not load pack"));
+                                       result = 1;
+                                       goto cleanup;
+                               }
+
+                               if (open_pack_index(ctx.m->packs[i]))
+                                       die(_("could not open index for %s"),
+                                           ctx.m->packs[i]->pack_name);
+                               ctx.info[ctx.nr].p = ctx.m->packs[i];
+                       }
+
                        ctx.nr++;
                }
        }
@@ -961,6 +980,16 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *
                }
        }
 
+       if (ctx.preferred_pack_idx > -1) {
+               struct packed_git *preferred = ctx.info[ctx.preferred_pack_idx].p;
+               if (!preferred->num_objects) {
+                       error(_("cannot select preferred pack %s with no objects"),
+                             preferred->pack_name);
+                       result = 1;
+                       goto cleanup;
+               }
+       }
+
        ctx.entries = get_sorted_entries(ctx.m, ctx.info, ctx.nr, &ctx.entries_nr,
                                         ctx.preferred_pack_idx);
 

diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index e953cdd6d1ce284a1c690ff77a02b11dbfeb28fe..d7e4988f2bd3669dcb6cad6c8c4c6c941379de41 100755 (executable)
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -305,6 +305,23 @@ test_expect_success 'midx picks objects from preferred pack' '
        )
 '
 
+test_expect_success 'preferred packs must be non-empty' '
+       test_when_finished rm -rf preferred.git &&
+       git init preferred.git &&
+       (
+               cd preferred.git &&
+
+               test_commit base &&
+               git repack -ad &&
+
+               empty="$(git pack-objects $objdir/pack/pack </dev/null)" &&
+
+               test_must_fail git multi-pack-index write \
+                       --preferred-pack=pack-$empty.pack 2>err &&
+               grep "with no objects" err
+       )
+'
+
 test_expect_success 'verify multi-pack-index success' '
        git multi-pack-index verify --object-dir=$objdir
 '