[PATCH v3 4/4] tmp-objdir: merge loose object index when migrating tmpdir

[Justin Tobler <jltobler@gmail.com>]

When a repository is configured with a compatibility hash, each loose
object write also records a mapping between the object and its
compatibility hash in a "loose-object-idx" file. This mapping file is
located in the same object directory the object is written to. If batch
fsync of loose objects is also enabled, the loose objects and mapping
file are instead written to a temporary object directory and expected to
be migrated to the primary object directory when the batch transaction
commits.

While migrating the loose objects works, the "loose-object-idx" file is
unable to be migrated if a mapping file already exists in the primary
object directory. This occurs because `finalize_object_file_flags()`
expects an already existing file to have the same contents. Otherwise,
the following error is returned:

  error: files '.git/objects/tmp_objdir-bulk-fsync-XXXXXX/loose-object-idx' and '.git/objects/loose-object-idx' differ in contents
  error: unable to migrate temporary objects

Furthermore, it is not sufficient to just replace the preexisting
mapping file with the new one as the new file only contains mapping
entries for the newly written objects.

Update the temporary directory migration logic to handle
"loose-object-idx" files explicitly by merging new entries and
atomically writing the resulting mapping file.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 loose.c                       | 47 +++++++++++++++++++++++++++++++++++
 loose.h                       |  9 +++++++
 t/t1016-compatObjectFormat.sh | 27 ++++++++++++++++++++
 tmp-objdir.c                  | 11 +++++++-
 4 files changed, 93 insertions(+), 1 deletion(-)

diff --git a/loose.c b/loose.c
index f2e111c25e..0ce9c7486e 100644
--- a/loose.c
+++ b/loose.c
@@ -311,3 +311,50 @@ void loose_object_map_clear(struct loose_object_map **map)
 	free(m);
 	*map = NULL;
 }
+
+int loose_object_idx_merge(struct repository *repo, const char *src,
+			   const char *dst)
+{
+	struct lock_file lock = LOCK_INIT;
+	struct loose_object_map *map;
+	int ret;
+	int fd;
+
+	loose_object_map_init(&map);
+
+	if (loose_object_map_read(repo, src, map, NULL) < 0) {
+		ret = error(_("unable to read loose object index %s"), src);
+		goto out;
+	}
+
+	fd = repo_hold_lock_file_for_update_timeout(repo, &lock, dst, 0,
+						    LOOSE_OBJECT_IDX_LOCK_TIMEOUT_MS);
+	if (fd < 0) {
+		ret = error_errno(_("unable to lock %s"), dst);
+		goto out;
+	}
+
+	if (loose_object_map_read(repo, dst, map, NULL) < 0) {
+		ret = error(_("unable to read loose object index %s"), dst);
+		goto out;
+	}
+
+	if (loose_object_map_write(repo, map, fd) < 0) {
+		ret = error_errno(_("unable to write %s"),
+				  get_lock_file_path(&lock));
+		goto out;
+	}
+
+	if (commit_lock_file(&lock) < 0) {
+		ret = error_errno(_("unable to write %s"), dst);
+		goto out;
+	}
+
+	ret = 0;
+
+out:
+	loose_object_map_clear(&map);
+	rollback_lock_file(&lock);
+
+	return ret;
+}
diff --git a/loose.h b/loose.h
index ed663ac550..6ad82dc0bb 100644
--- a/loose.h
+++ b/loose.h
@@ -24,4 +24,13 @@ int repo_add_loose_object_map(struct odb_source_loose *loose,
 int repo_read_loose_object_map(struct repository *repo);
 int repo_write_loose_object_map(struct repository *repo);
 
+/*
+ * Merge the mapping entries of the loose object index at `src` into the one at
+ * `dst`. The resulting mapping file is written atomically.
+ *
+ * Returns 0 on success, a negative error code otherwise.
+ */
+int loose_object_idx_merge(struct repository *repo, const char *src,
+			   const char *dst);
+
 #endif
diff --git a/t/t1016-compatObjectFormat.sh b/t/t1016-compatObjectFormat.sh
index 9cafcee509..6d0f48226f 100755
--- a/t/t1016-compatObjectFormat.sh
+++ b/t/t1016-compatObjectFormat.sh
@@ -305,4 +305,31 @@ compare_oids GPG2 'commit' signedcommit4 "$signedcommit4_sha1_oid" "$signedcommi
 compare_oids GPG2 'tag' signedtag3 "$signedtag3_sha1_oid" "$signedtag3_sha256_oid"
 compare_oids GPG2 'tag' signedtag4 "$signedtag4_sha1_oid" "$signedtag4_sha256_oid"
 
+test_expect_success 'batched fsync migrates loose object index' '
+	test_when_finished "rm -rf batch-fsync" &&
+	git init --object-format=sha1 batch-fsync &&
+	(
+		cd batch-fsync &&
+		git config core.repositoryFormatVersion 1 &&
+		git config extensions.compatObjectFormat sha256 &&
+		git config core.fsync loose-object &&
+		git config core.fsyncMethod batch &&
+
+		# Write object to generate initial loose object index.
+		echo foo >foo &&
+		git add foo &&
+		test_grep "$(git rev-parse :foo)" .git/objects/loose-object-idx &&
+
+		# Write some additional objects and ensure the corresponding
+		# temporary loose object index entries stored in the transaction
+		# are merged into the primary object directory.
+		echo bar >bar &&
+		echo baz >baz &&
+		git add bar baz &&
+		test_grep "$(git rev-parse :foo)" .git/objects/loose-object-idx &&
+		test_grep "$(git rev-parse :bar)" .git/objects/loose-object-idx &&
+		test_grep "$(git rev-parse :baz)" .git/objects/loose-object-idx
+	)
+'
+
 test_done
diff --git a/tmp-objdir.c b/tmp-objdir.c
index 3debb60270..5307717354 100644
--- a/tmp-objdir.c
+++ b/tmp-objdir.c
@@ -4,6 +4,7 @@
 #include "chdir-notify.h"
 #include "dir.h"
 #include "environment.h"
+#include "loose.h"
 #include "object-file.h"
 #include "path.h"
 #include "string-list.h"
@@ -294,6 +295,11 @@ static int is_loose_object_shard(const char *name)
 	return strlen(name) == 2 && isxdigit(name[0]) && isxdigit(name[1]);
 }
 
+static int is_loose_object_idx(const char *name)
+{
+	return !strcmp(name, "loose-object-idx");
+}
+
 static int migrate_paths(struct tmp_objdir *t,
 			 struct strbuf *src, struct strbuf *dst,
 			 enum finalize_object_file_flags flags)
@@ -317,7 +323,10 @@ static int migrate_paths(struct tmp_objdir *t,
 		if (is_loose_object_shard(name))
 			flags_copy |= FOF_SKIP_COLLISION_CHECK;
 
-		ret |= migrate_one(t, src, dst, flags_copy);
+		if (is_loose_object_idx(name))
+			ret |= loose_object_idx_merge(t->repo, src->buf, dst->buf);
+		else
+			ret |= migrate_one(t, src, dst, flags_copy);
 
 		strbuf_setlen(src, src_len);
 		strbuf_setlen(dst, dst_len);
-- 
2.56.0.173.gf57c9a7388