From 5a3c19d05974f8af1c938bdb68e806d7ea0a73ef Mon Sep 17 00:00:00 2001
From: Derrick Stolee <dstolee@microsoft.com>
Date: Thu, 3 Jan 2019 10:36:24 -0500
Subject: [PATCH] fixup! multi-pack-index: implement 'expire' verb

There is a bug in the 'git multi-pack-index expire' subcommand. When
expiring packs while also adding a pack not previously covered by
the multi-pack-index, the logic around the pack permutations did not
work correctly. It would improperly assign objects to the wrong
pack-int-ids.

Rework the logic around expired packs in the following ways:

1. We track the original order of the packs to delete, but do not
   remove them from the list right away.

2. We append the new packs to the total list of packs.

3. After sorting the full list of packs, we remove the expired
   packs from the list so we do not write them in the new
   multi-pack-index.

4444. Finally, we "puncture" the pack permutation to remove the
   entries that will be deleted. This requires reducing the int-ids
   for the packs that have final int-id larger than the expected
   int-id for the expired pack.
---
 midx.c                      | 41 +++++++++++++++++++++++++++++++++----
 t/t5319-multi-pack-index.sh | 28 +++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/midx.c b/midx.c
index f6bc111438cac0..824b4fca4252bd 100644
--- a/midx.c
+++ b/midx.c
@@ -768,6 +768,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *
 	struct pack_midx_entry *entries = NULL;
 	int large_offsets_needed = 0;
 	int result = 0;
+	uint32_t *expire_int_ids = NULL;
 
 	midx_name = get_midx_filename(object_dir);
 	if (safe_create_leading_directories(midx_name)) {
@@ -792,6 +793,9 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *
 	ALLOC_ARRAY(packs.names, packs.alloc_names);
 	ALLOC_ARRAY(packs.perm, packs.alloc_perm);
 
+	if (packs_to_drop && packs_to_drop->nr)
+		expire_int_ids = xcalloc(packs_to_drop->nr, sizeof(uint32_t));
+
 	if (packs.m) {
 		int drop_index = 0, missing_drops = 0;
 		for (i = 0; i < packs.m->num_packs; i++) {
@@ -800,8 +804,8 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *
 						 packs_to_drop->items[drop_index].string);
 
 				if (!cmp) {
+					expire_int_ids[drop_index] = i;
 					drop_index++;
-					continue;
 				} else if (cmp > 0) {
 					error(_("did not see pack-file %s to drop"),
 					      packs_to_drop->items[drop_index].string);
@@ -832,15 +836,43 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *
 
 	for_each_file_in_pack_dir(object_dir, add_pack_to_midx, &packs);
 
-	if (packs.m && packs.nr == packs.m->num_packs)
+	if (packs.m && packs.nr == packs.m->num_packs && !packs_to_drop)
 		goto cleanup;
 
+	sort_packs_by_name(packs.names, packs.nr, packs.perm);
+
+	if (packs_to_drop && packs_to_drop->nr) {
+		int drop_index = 0;
+		int j;
+
+		/* truncate the list of packs */
+		for (i = 0; i < packs.nr; i++) {
+			if (drop_index < packs_to_drop->nr &&
+			    !strcmp(packs.names[i], packs_to_drop->items[drop_index].string)) {
+				packs.pack_name_concat_len -= strlen(packs_to_drop->items[drop_index].string) + 1;
+				drop_index++;
+			} else {
+				packs.list[i - drop_index] = packs.list[i];
+				packs.names[i - drop_index] = packs.names[i];
+			}
+		}
+
+		for (j = packs_to_drop->nr - 1; j >= 0; j--) {
+			int expired_int_id = packs.perm[expire_int_ids[j]];
+
+			for (i = 0; i < packs.nr; i++) {
+				if (packs.perm[i] > expired_int_id)
+					packs.perm[i] = packs.perm[i] - 1;
+			}
+		}
+
+		packs.nr = packs.nr - packs_to_drop->nr;
+	}
+
 	if (packs.pack_name_concat_len % MIDX_CHUNK_ALIGNMENT)
 		packs.pack_name_concat_len += MIDX_CHUNK_ALIGNMENT -
 					      (packs.pack_name_concat_len % MIDX_CHUNK_ALIGNMENT);
 
-	sort_packs_by_name(packs.names, packs.nr, packs.perm);
-
 	entries = get_sorted_entries(packs.m, packs.list, packs.perm, packs.nr, &nr_entries);
 
 	for (i = 0; i < nr_entries; i++) {
@@ -962,6 +994,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *
 	free(packs.perm);
 	free(entries);
 	free(midx_name);
+	free(expire_int_ids);
 	return result;
 }
 
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index 3f5e9ea6534206..13af72e994df00 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -446,4 +446,32 @@ test_expect_success 'expire removes repacked packs' '
 	)
 '
 
+test_expect_success 'expire works when adding new packs' '
+	(
+		cd dup &&
+		git pack-objects --revs .git/objects/pack/pack-combined <<-EOF &&
+		refs/heads/A
+		^refs/heads/B
+		EOF
+		git pack-objects --revs .git/objects/pack/pack-combined <<-EOF &&
+		refs/heads/B
+		^refs/heads/C
+		EOF
+		git pack-objects --revs .git/objects/pack/pack-combined <<-EOF &&
+		refs/heads/C
+		^refs/heads/D
+		EOF
+		git multi-pack-index write &&
+		git pack-objects --revs .git/objects/pack/pack-a <<-EOF &&
+		refs/heads/D
+		^refs/heads/E
+		EOF
+		git multi-pack-index write &&
+		git pack-objects --revs .git/objects/pack/pack-z <<-EOF &&
+		refs/heads/E
+		EOF
+		git multi-pack-index expire &&	
+		git multi-pack-index verify
+	)
+'
 test_done