[PATCH] merge-tree: fix segmentation fault in read-only repositories

From: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Date: Sep 21, 2022, 15:30 UTC
Message-ID: <pull.1362.git.1663774248660.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Independent of the question whether we want `git merge-tree` to report a tree name even when it failed to write the tree objects in a read-only repository, there is no question that we should avoid a segmentation fault.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
    merge-tree: fix segmentation fault in read-only repositories
    
    Turns out that the segmentation fault reported by Taylor
    [https://lore.kernel.org/git/YyopQD+LvPucnz3w@nand.local/] happened
    while testing merge-ort in a read-only repository, and that the upstream
    version of git merge-tree is as affected as GitHub's internal version.
    
    Note: I briefly considered using the OID of the_hash_algo->empty_tree
    instead of null_oid() when no tree object could be constructed. However,
    I have come to the conclusion that this could potentially cause its own
    set of problems because it would relate to a valid tree object even if
    we do not have any valid tree object to play with.
    
    Also note: The question I hinted at in the commit message, namely
    whether or not to try harder to construct a tree object even if we
    cannot write it out, maybe merits a longer discussion, one that I think
    we should have after v2.38.0 is released, so as not to distract from
    focusing on v2.38.0.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1362%2Fdscho%2Fmerge-tree-in-read-only-repos-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1362/dscho/merge-tree-in-read-only-repos-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1362

 builtin/merge-tree.c             | 4 +++-
 t/t4301-merge-tree-write-tree.sh | 8 ++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index ae5782917b9..25c7142a882 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -412,6 +412,7 @@ static int real_merge(struct merge_tree_options *o,
 	struct commit_list *merge_bases = NULL;
 	struct merge_options opt;
 	struct merge_result result = { 0 };
+	const struct object_id *tree_oid;
 
 	parent1 = get_merge_parent(branch1);
 	if (!parent1)
@@ -446,7 +447,8 @@ static int real_merge(struct merge_tree_options *o,
 	if (o->show_messages == -1)
 		o->show_messages = !result.clean;
 
-	printf("%s%c", oid_to_hex(&result.tree->object.oid), line_termination);
+	tree_oid = result.tree ? &result.tree->object.oid : null_oid();
+	printf("%s%c", oid_to_hex(tree_oid), line_termination);
 	if (!result.clean) {
 		struct string_list conflicted_files = STRING_LIST_INIT_NODUP;
 		const char *last = NULL;
diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
index 28ca5c38bb5..e56b1ba6e50 100755
--- a/t/t4301-merge-tree-write-tree.sh
+++ b/t/t4301-merge-tree-write-tree.sh
@@ -810,4 +810,12 @@ test_expect_success 'can override merge of unrelated histories' '
 	test_cmp expect actual
 '
 
+test_expect_success 'merge-ort fails gracefully in a read-only repository' '
+	git init --bare read-only &&
+	git push read-only side1 side2 &&
+	test_when_finished "chmod -R u+w read-only" &&
+	chmod -R a-w read-only &&
+	test_must_fail git -C read-only merge-tree side1 side2
+'
+
 test_done

base-commit: dda7228a83e2e9ff584bf6adbf55910565b41e14

-- 
gitgitgadget