diff mbox series

[2/9] midx: allow marking a pack as preferred

Message ID 4a358d57cf6e834cd1756e70bf713d4d104f321e.1612998106.git.me@ttaylorr.com (mailing list archive)
State Superseded
Headers show
Series midx: implement a multi-pack reverse index | expand

Commit Message

Taylor Blau Feb. 10, 2021, 11:02 p.m. UTC
When multiple packs in the multi-pack index contain the same object, the
MIDX machinery must make a choice about which pack it associates with
that object. Prior to this patch, the lowest-ordered[1] pack was always
selected.

Pack selection for duplicate objects is relatively unimportant today,
but it will become important for multi-pack bitmaps. This is because we
can only invoke the pack-reuse mechanism when all of the bits for reused
objects come from the reuse pack (in order to ensure that all reused
deltas can find their base objects in the same pack).

To encourage the pack selection process to prefer one pack over another
(the pack to be preferred is the one a caller would like to later use as
a reuse pack), introduce the concept of a "preferred pack". When
provided, the MIDX code will always prefer an object found in a
preferred pack over any other.

No format changes are required to store the preferred pack, since it
will be able to be inferred with a corresponding MIDX bitmap, by looking
up the pack associated with the object in the first bit position (this
ordering is described in detail in a subsequent commit).

[1]: the ordering is specified by MIDX internals; for our purposes we
can consider the "lowest ordered" pack to be "the one with the
most-recent mtime.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 Documentation/git-multi-pack-index.txt       | 11 ++-
 Documentation/technical/multi-pack-index.txt |  5 +-
 builtin/multi-pack-index.c                   | 10 +-
 builtin/repack.c                             |  2 +-
 midx.c                                       | 97 ++++++++++++++++++--
 midx.h                                       |  2 +-
 t/t5319-multi-pack-index.sh                  | 39 ++++++++
 7 files changed, 151 insertions(+), 15 deletions(-)

Comments

SZEDER Gábor Feb. 11, 2021, 7:33 p.m. UTC | #1
On Wed, Feb 10, 2021 at 06:02:42PM -0500, Taylor Blau wrote:
> diff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt
> index eb0caa0439..dd14eab781 100644
> --- a/Documentation/git-multi-pack-index.txt
> +++ b/Documentation/git-multi-pack-index.txt
> @@ -9,7 +9,8 @@ git-multi-pack-index - Write and verify multi-pack-indexes
>  SYNOPSIS
>  --------
>  [verse]
> -'git multi-pack-index' [--object-dir=<dir>] [--[no-]progress] <subcommand>
> +'git multi-pack-index' [--object-dir=<dir>] [--[no-]progress]
> +	[--preferred-pack=<pack>] <subcommand>
>  
>  DESCRIPTION
>  -----------
> @@ -27,6 +28,14 @@ OPTIONS
>  	Turn progress on/off explicitly. If neither is specified, progress is
>  	shown if standard error is connected to a terminal.
>  
> +--preferred-pack=<pack>::
> +	When using the `write` subcommand, optionally specify the
> +	tie-breaking pack used when multiple packs contain the same
> +	object. Incompatible with other subcommands, including `repack`,

I think this shouldn't be an option of the 'git multi-pack-index'
command but an option of its 'write' subcommand.
Taylor Blau Feb. 15, 2021, 3:49 p.m. UTC | #2
On Thu, Feb 11, 2021 at 08:33:14PM +0100, SZEDER Gábor wrote:
> > +--preferred-pack=<pack>::
> > +	When using the `write` subcommand, optionally specify the
> > +	tie-breaking pack used when multiple packs contain the same
> > +	object. Incompatible with other subcommands, including `repack`,
>
> I think this shouldn't be an option of the 'git multi-pack-index'
> command but an option of its 'write' subcommand.

:-/. I wrote a lengthy response on Friday, but Gmail must have eaten it.

The gist of my response was that the intermingling of sub-commands with
options from other sub-commands goes deeper than just the documentation,
since command-line arguments are only parsed once in
builtin/multi-pack-index.c.

I explored whether or not it would be worth it to parse the common
options first, and then have separate options for each of the
sub-commands (as is done in the commit-graph builtin). But, this is
tricky, since we accept common options on either side of the sub-command
(i.e., we'd expect both 'git multi-pack-index --object-dir=... write' to
behave the same as 'git multi-pack-index write --object-dir=...').

So you could let the first call to parse_options() parse all of the
arguments, but then specialized arguments (e.g., 'repack --batch-size')
would cause the parse-options API to barf because the first call to
parse_options() doesn't recognize '--batch-size'.

I think the easiest way to do it would be to pass
PARSE_OPT_STOP_AT_NON_OPTION, and then let the subsequent calls to
parse_options() pass an array of option structs that also includes the
common options so they can be parsed on either side of the sub-command.

Obviously this leads to a lot of rather unfortunate duplication. So,
I'm content to leave it all as-is, and let the multi-pack-index
builtin check the disallowed combinations itself (e.g., if you passed
'--preferred-pack' but aren't in 'write' mode, then we should complain).

I can certainly move this piece of documentation into the 'write'
section, though, which should alleviate your immediate concern.

Thanks,
Taylor
Ævar Arnfjörð Bjarmason Feb. 15, 2021, 5:01 p.m. UTC | #3
On Mon, Feb 15 2021, Taylor Blau wrote:

> On Thu, Feb 11, 2021 at 08:33:14PM +0100, SZEDER Gábor wrote:
>> > +--preferred-pack=<pack>::
>> > +	When using the `write` subcommand, optionally specify the
>> > +	tie-breaking pack used when multiple packs contain the same
>> > +	object. Incompatible with other subcommands, including `repack`,
>>
>> I think this shouldn't be an option of the 'git multi-pack-index'
>> command but an option of its 'write' subcommand.
>
> :-/. I wrote a lengthy response on Friday, but Gmail must have eaten it.
>
> The gist of my response was that the intermingling of sub-commands with
> options from other sub-commands goes deeper than just the documentation,
> since command-line arguments are only parsed once in
> builtin/multi-pack-index.c.
>
> I explored whether or not it would be worth it to parse the common
> options first, and then have separate options for each of the
> sub-commands (as is done in the commit-graph builtin). But, this is
> tricky, since we accept common options on either side of the sub-command
> (i.e., we'd expect both 'git multi-pack-index --object-dir=... write' to
> behave the same as 'git multi-pack-index write --object-dir=...').
>
> So you could let the first call to parse_options() parse all of the
> arguments, but then specialized arguments (e.g., 'repack --batch-size')
> would cause the parse-options API to barf because the first call to
> parse_options() doesn't recognize '--batch-size'.
>
> I think the easiest way to do it would be to pass
> PARSE_OPT_STOP_AT_NON_OPTION, and then let the subsequent calls to
> parse_options() pass an array of option structs that also includes the
> common options so they can be parsed on either side of the sub-command.
>
> Obviously this leads to a lot of rather unfortunate duplication. So,
> I'm content to leave it all as-is, and let the multi-pack-index
> builtin check the disallowed combinations itself (e.g., if you passed
> '--preferred-pack' but aren't in 'write' mode, then we should complain).
>
> I can certainly move this piece of documentation into the 'write'
> section, though, which should alleviate your immediate concern.

I may be missing something, but...

It sounds to me like you're imagining this is more complex than it is
because you don't know about some/all of parse_options_concat() or
PARSE_OPT_KEEP_*.

See e.g. cmd_{switch,restore} in builti/checkout.c, or the entire family
of diff-like commands where we do parse_options() followed by
setup_revisions(). We've got a lot of commands that parse options in a
piecemeal manner.

At no point do you need to re-parse the options. You just have the
common command parse as far as it gets, and leave anything else in
argv/argc for sub-commands like "write".

I think the problem is you read the builtin/commit-graph.c code, it
could really benefit from using parse_options_concat(), now things like
"object-directory" are copy/pasted in that file. See 2087182272
(checkout: split options[] array in three pieces, 2019-03-29) for a
commit which simplified that sort of code.

In this case you'd share the "opts_multi_pack_index" struct between the
various commands, it would just have unused fields for "write" that
aren't used by "verify" or whatever.

The PARSE_OPT_STOP_AT_NON_OPTION flag isn't for what you're doing with
"write" here, since as your test shows you're doing:

    git multi-pack-index <ALL_OPTS> <SUBCOMMAND>

But PARSE_OPT_STOP_AT_NON_OPTION is for cases like "git-remote" that do:

    git multi-pack-index <COMMOT_OPTS> <SUBCOMMAND> <SUBCOMMAND_OPTS>

(Or maybe you really want the latter, and the test change isn't
representative).

So then we want to stop at the first non-option, i.e. the subcommand. I
think it's good practice not to emulate how "git remote" works for new
commands, which makes things a bit simpler to implement.

You say "since we accept common options on either side of the
sub-command" but without PARSE_OPT_STOP_AT_NON_OPTION this works, since
if you can parse everything you'll have "write" left, but if you truly
have unknown options you'll have more than that in argv.

All of the above shouldn't be taken as a "your patch should change"
comment, maybe it's fine as-is.

I just replied because it sounded like you didn't spot how to easily use
parse_options() to do this sort of thing. It's actually rather easy.
diff mbox series

Patch

diff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt
index eb0caa0439..dd14eab781 100644
--- a/Documentation/git-multi-pack-index.txt
+++ b/Documentation/git-multi-pack-index.txt
@@ -9,7 +9,8 @@  git-multi-pack-index - Write and verify multi-pack-indexes
 SYNOPSIS
 --------
 [verse]
-'git multi-pack-index' [--object-dir=<dir>] [--[no-]progress] <subcommand>
+'git multi-pack-index' [--object-dir=<dir>] [--[no-]progress]
+	[--preferred-pack=<pack>] <subcommand>
 
 DESCRIPTION
 -----------
@@ -27,6 +28,14 @@  OPTIONS
 	Turn progress on/off explicitly. If neither is specified, progress is
 	shown if standard error is connected to a terminal.
 
+--preferred-pack=<pack>::
+	When using the `write` subcommand, optionally specify the
+	tie-breaking pack used when multiple packs contain the same
+	object. Incompatible with other subcommands, including `repack`,
+	which may repack the pack marked as preferred. If not given, the
+	preferred pack is inferred from an existing `multi-pack-index`,
+	if one exists, otherwise the pack with the lowest mtime.
+
 The following subcommands are available:
 
 write::
diff --git a/Documentation/technical/multi-pack-index.txt b/Documentation/technical/multi-pack-index.txt
index e8e377a59f..fb688976c4 100644
--- a/Documentation/technical/multi-pack-index.txt
+++ b/Documentation/technical/multi-pack-index.txt
@@ -43,8 +43,9 @@  Design Details
   a change in format.
 
 - The MIDX keeps only one record per object ID. If an object appears
-  in multiple packfiles, then the MIDX selects the copy in the most-
-  recently modified packfile.
+  in multiple packfiles, then the MIDX selects the copy in the
+  preferred packfile, otherwise selecting from the most-recently
+  modified packfile.
 
 - If there exist packfiles in the pack directory not registered in
   the MIDX, then those packfiles are loaded into the `packed_git`
diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
index 5bf88cd2a8..4d1ea3fe84 100644
--- a/builtin/multi-pack-index.c
+++ b/builtin/multi-pack-index.c
@@ -4,6 +4,7 @@ 
 #include "parse-options.h"
 #include "midx.h"
 #include "trace2.h"
+#include "object-store.h"
 
 static char const * const builtin_multi_pack_index_usage[] = {
 	N_("git multi-pack-index [<options>] (write|verify|expire|repack --batch-size=<size>)"),
@@ -12,6 +13,7 @@  static char const * const builtin_multi_pack_index_usage[] = {
 
 static struct opts_multi_pack_index {
 	const char *object_dir;
+	const char *preferred_pack;
 	unsigned long batch_size;
 	int progress;
 } opts;
@@ -24,6 +26,8 @@  int cmd_multi_pack_index(int argc, const char **argv,
 	static struct option builtin_multi_pack_index_options[] = {
 		OPT_FILENAME(0, "object-dir", &opts.object_dir,
 		  N_("object directory containing set of packfile and pack-index pairs")),
+		OPT_STRING(0, "preferred-pack", &opts.preferred_pack, N_("preferred-pack"),
+		  N_("pack for reuse when computing a multi-pack bitmap")),
 		OPT_BOOL(0, "progress", &opts.progress, N_("force progress reporting")),
 		OPT_MAGNITUDE(0, "batch-size", &opts.batch_size,
 		  N_("during repack, collect pack-files of smaller size into a batch that is larger than this size")),
@@ -51,6 +55,9 @@  int cmd_multi_pack_index(int argc, const char **argv,
 		return 1;
 	}
 
+	if (strcmp(argv[0], "write") && opts.preferred_pack)
+		die(_("'--preferred-pack' requires 'write'"));
+
 	trace2_cmd_mode(argv[0]);
 
 	if (!strcmp(argv[0], "repack"))
@@ -60,7 +67,8 @@  int cmd_multi_pack_index(int argc, const char **argv,
 		die(_("--batch-size option is only for 'repack' subcommand"));
 
 	if (!strcmp(argv[0], "write"))
-		return write_midx_file(opts.object_dir, flags);
+		return write_midx_file(opts.object_dir, opts.preferred_pack,
+				       flags);
 	if (!strcmp(argv[0], "verify"))
 		return verify_midx_file(the_repository, opts.object_dir, flags);
 	if (!strcmp(argv[0], "expire"))
diff --git a/builtin/repack.c b/builtin/repack.c
index 01440de2d5..9f00806805 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -523,7 +523,7 @@  int cmd_repack(int argc, const char **argv, const char *prefix)
 	remove_temporary_files();
 
 	if (git_env_bool(GIT_TEST_MULTI_PACK_INDEX, 0))
-		write_midx_file(get_object_directory(), 0);
+		write_midx_file(get_object_directory(), NULL, 0);
 
 	string_list_clear(&names, 0);
 	string_list_clear(&rollback, 0);
diff --git a/midx.c b/midx.c
index 05c40a98e0..064670c0c0 100644
--- a/midx.c
+++ b/midx.c
@@ -451,6 +451,24 @@  static int pack_info_compare(const void *_a, const void *_b)
 	return strcmp(a->pack_name, b->pack_name);
 }
 
+static int lookup_idx_or_pack_name(struct pack_info *info,
+				   uint32_t nr,
+				   const char *pack_name)
+{
+	uint32_t lo = 0, hi = nr;
+	while (lo < hi) {
+		uint32_t mi = lo + (hi - lo) / 2;
+		int cmp = cmp_idx_or_pack_name(pack_name, info[mi].pack_name);
+		if (cmp < 0)
+			hi = mi;
+		else if (cmp > 0)
+			lo = mi + 1;
+		else
+			return mi;
+	}
+	return -1;
+}
+
 struct pack_list {
 	struct pack_info *info;
 	uint32_t nr;
@@ -502,6 +520,7 @@  struct pack_midx_entry {
 	uint32_t pack_int_id;
 	time_t pack_mtime;
 	uint64_t offset;
+	unsigned preferred : 1;
 };
 
 static int midx_oid_compare(const void *_a, const void *_b)
@@ -513,6 +532,12 @@  static int midx_oid_compare(const void *_a, const void *_b)
 	if (cmp)
 		return cmp;
 
+	/* Sort objects in a preferred pack first when multiple copies exist. */
+	if (a->preferred > b->preferred)
+		return -1;
+	if (a->preferred < b->preferred)
+		return 1;
+
 	if (a->pack_mtime > b->pack_mtime)
 		return -1;
 	else if (a->pack_mtime < b->pack_mtime)
@@ -540,7 +565,8 @@  static int nth_midxed_pack_midx_entry(struct multi_pack_index *m,
 static void fill_pack_entry(uint32_t pack_int_id,
 			    struct packed_git *p,
 			    uint32_t cur_object,
-			    struct pack_midx_entry *entry)
+			    struct pack_midx_entry *entry,
+			    int preferred)
 {
 	if (nth_packed_object_id(&entry->oid, p, cur_object) < 0)
 		die(_("failed to locate object %d in packfile"), cur_object);
@@ -549,6 +575,7 @@  static void fill_pack_entry(uint32_t pack_int_id,
 	entry->pack_mtime = p->mtime;
 
 	entry->offset = nth_packed_object_offset(p, cur_object);
+	entry->preferred = !!preferred;
 }
 
 /*
@@ -565,7 +592,8 @@  static void fill_pack_entry(uint32_t pack_int_id,
 static struct pack_midx_entry *get_sorted_entries(struct multi_pack_index *m,
 						  struct pack_info *info,
 						  uint32_t nr_packs,
-						  uint32_t *nr_objects)
+						  uint32_t *nr_objects,
+						  uint32_t preferred_pack)
 {
 	uint32_t cur_fanout, cur_pack, cur_object;
 	uint32_t alloc_fanout, alloc_objects, total_objects = 0;
@@ -602,12 +630,17 @@  static struct pack_midx_entry *get_sorted_entries(struct multi_pack_index *m,
 				nth_midxed_pack_midx_entry(m,
 							   &entries_by_fanout[nr_fanout],
 							   cur_object);
+				if (nth_midxed_pack_int_id(m, cur_object) == preferred_pack)
+					entries_by_fanout[nr_fanout].preferred = 1;
+				else
+					entries_by_fanout[nr_fanout].preferred = 0;
 				nr_fanout++;
 			}
 		}
 
 		for (cur_pack = start_pack; cur_pack < nr_packs; cur_pack++) {
 			uint32_t start = 0, end;
+			int preferred = cur_pack == preferred_pack;
 
 			if (cur_fanout)
 				start = get_pack_fanout(info[cur_pack].p, cur_fanout - 1);
@@ -615,7 +648,11 @@  static struct pack_midx_entry *get_sorted_entries(struct multi_pack_index *m,
 
 			for (cur_object = start; cur_object < end; cur_object++) {
 				ALLOC_GROW(entries_by_fanout, nr_fanout + 1, alloc_fanout);
-				fill_pack_entry(cur_pack, info[cur_pack].p, cur_object, &entries_by_fanout[nr_fanout]);
+				fill_pack_entry(cur_pack,
+						info[cur_pack].p,
+						cur_object,
+						&entries_by_fanout[nr_fanout],
+						preferred);
 				nr_fanout++;
 			}
 		}
@@ -794,7 +831,9 @@  static size_t write_midx_large_offsets(struct hashfile *f, uint32_t nr_large_off
 }
 
 static int write_midx_internal(const char *object_dir, struct multi_pack_index *m,
-			       struct string_list *packs_to_drop, unsigned flags)
+			       struct string_list *packs_to_drop,
+			       const char *preferred_pack_name,
+			       unsigned flags)
 {
 	unsigned char cur_chunk, num_chunks = 0;
 	char *midx_name;
@@ -813,6 +852,7 @@  static int write_midx_internal(const char *object_dir, struct multi_pack_index *
 	int pack_name_concat_len = 0;
 	int dropped_packs = 0;
 	int result = 0;
+	int preferred_pack_idx = -1;
 
 	midx_name = get_midx_filename(object_dir);
 	if (safe_create_leading_directories(midx_name))
@@ -853,7 +893,18 @@  static int write_midx_internal(const char *object_dir, struct multi_pack_index *
 	if (packs.m && packs.nr == packs.m->num_packs && !packs_to_drop)
 		goto cleanup;
 
-	entries = get_sorted_entries(packs.m, packs.info, packs.nr, &nr_entries);
+	if (preferred_pack_name) {
+		for (i = 0; i < packs.nr; i++) {
+			if (!cmp_idx_or_pack_name(preferred_pack_name,
+						  packs.info[i].pack_name)) {
+				preferred_pack_idx = i;
+				break;
+			}
+		}
+	}
+
+	entries = get_sorted_entries(packs.m, packs.info, packs.nr, &nr_entries,
+				     preferred_pack_idx);
 
 	for (i = 0; i < nr_entries; i++) {
 		if (entries[i].offset > 0x7fffffff)
@@ -913,6 +964,31 @@  static int write_midx_internal(const char *object_dir, struct multi_pack_index *
 			pack_name_concat_len += strlen(packs.info[i].pack_name) + 1;
 	}
 
+	/*
+	 * Recompute the preferred_pack_idx (if applicable) according to the
+	 * permuted pack order.
+	 */
+	preferred_pack_idx = -1;
+	if (preferred_pack_name) {
+		preferred_pack_idx = lookup_idx_or_pack_name(packs.info,
+							     packs.nr,
+							     preferred_pack_name);
+		if (preferred_pack_idx < 0)
+			warning(_("unknown preferred pack: '%s'"),
+				preferred_pack_name);
+		else {
+			uint32_t orig = packs.info[preferred_pack_idx].orig_pack_int_id;
+			uint32_t perm = pack_perm[orig];
+
+			if (perm == PACK_EXPIRED) {
+				warning(_("preferred pack '%s' is expired"),
+					preferred_pack_name);
+				preferred_pack_idx = -1;
+			} else
+				preferred_pack_idx = perm;
+		}
+	}
+
 	if (pack_name_concat_len % MIDX_CHUNK_ALIGNMENT)
 		pack_name_concat_len += MIDX_CHUNK_ALIGNMENT -
 					(pack_name_concat_len % MIDX_CHUNK_ALIGNMENT);
@@ -1042,9 +1118,12 @@  static int write_midx_internal(const char *object_dir, struct multi_pack_index *
 	return result;
 }
 
-int write_midx_file(const char *object_dir, unsigned flags)
+int write_midx_file(const char *object_dir,
+		    const char *preferred_pack_name,
+		    unsigned flags)
 {
-	return write_midx_internal(object_dir, NULL, NULL, flags);
+	return write_midx_internal(object_dir, NULL, NULL, preferred_pack_name,
+				   flags);
 }
 
 void clear_midx_file(struct repository *r)
@@ -1279,7 +1358,7 @@  int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla
 	free(count);
 
 	if (packs_to_drop.nr)
-		result = write_midx_internal(object_dir, m, &packs_to_drop, flags);
+		result = write_midx_internal(object_dir, m, &packs_to_drop, NULL, flags);
 
 	string_list_clear(&packs_to_drop, 0);
 	return result;
@@ -1468,7 +1547,7 @@  int midx_repack(struct repository *r, const char *object_dir, size_t batch_size,
 		goto cleanup;
 	}
 
-	result = write_midx_internal(object_dir, m, NULL, flags);
+	result = write_midx_internal(object_dir, m, NULL, NULL, flags);
 	m = NULL;
 
 cleanup:
diff --git a/midx.h b/midx.h
index b18cf53bc4..e7fea61109 100644
--- a/midx.h
+++ b/midx.h
@@ -47,7 +47,7 @@  int fill_midx_entry(struct repository *r, const struct object_id *oid, struct pa
 int midx_contains_pack(struct multi_pack_index *m, const char *idx_or_pack_name);
 int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, int local);
 
-int write_midx_file(const char *object_dir, unsigned flags);
+int write_midx_file(const char *object_dir, const char *preferred_pack_name, unsigned flags);
 void clear_midx_file(struct repository *r);
 int verify_midx_file(struct repository *r, const char *object_dir, unsigned flags);
 int expire_midx_packs(struct repository *r, const char *object_dir, unsigned flags);
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index 2fc3aadbd1..9304b33484 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -31,6 +31,14 @@  midx_read_expect () {
 	test_cmp expect actual
 }
 
+midx_expect_object_offset () {
+	OID="$1"
+	OFFSET="$2"
+	OBJECT_DIR="$3"
+	test-tool read-midx --show-objects $OBJECT_DIR >actual &&
+	grep "^$OID $OFFSET" actual
+}
+
 test_expect_success 'setup' '
 	test_oid_cache <<-EOF
 	idxoff sha1:2999
@@ -234,6 +242,37 @@  test_expect_success 'warn on improper hash version' '
 	)
 '
 
+test_expect_success 'midx picks objects from preferred pack' '
+	test_when_finished rm -rf preferred.git &&
+	git init --bare preferred.git &&
+	(
+		cd preferred.git &&
+
+		a=$(echo "a" | git hash-object -w --stdin) &&
+		b=$(echo "b" | git hash-object -w --stdin) &&
+		c=$(echo "c" | git hash-object -w --stdin) &&
+
+		# Set up two packs, duplicating the object "B" at different
+		# offsets.
+		git pack-objects objects/pack/test-AB <<-EOF &&
+		$a
+		$b
+		EOF
+		bc=$(git pack-objects objects/pack/test-BC <<-EOF
+		$b
+		$c
+		EOF
+		) &&
+
+		git multi-pack-index --object-dir=objects \
+			--preferred-pack=test-BC-$bc.idx write 2>err &&
+		test_must_be_empty err &&
+
+		ofs=$(git show-index <objects/pack/test-BC-$bc.idx | grep $b |
+			cut -d" " -f1) &&
+		midx_expect_object_offset $b $ofs objects
+	)
+'
 
 test_expect_success 'verify multi-pack-index success' '
 	git multi-pack-index verify --object-dir=$objdir