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

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

Independent of the question whether we want `git merge-tree` to report the 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.

And when we report an invalid tree name (because the tree could not be written), we need the exit status to be non-zero.

Let's make it so.

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.
    
    Changes since v1:
    
     * Using the SANITY prereq now
     * If the merge was aborted due to a write error, exit with a non-zero
       code even if the (potentially partial) merge is clean
     * The test case now also verifies that the git merge-tree command fails
       in a read-only repository even if the merge would have succeeded

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

Range-diff vs v1:

 1:  beba3fd9a95 ! 1:  1de4df3f471 merge-tree: fix segmentation fault in read-only repositories
     @@ Metadata
       ## Commit message ##
          merge-tree: fix segmentation fault in read-only repositories
      
     -    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.
     +    Independent of the question whether we want `git merge-tree` to report
     +    the 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.
     +
     +    And when we report an invalid tree name (because the tree could not be
     +    written), we need the exit status to be non-zero.
     +
     +    Let's make it so.
      
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      
     @@ builtin/merge-tree.c: static int real_merge(struct merge_tree_options *o,
       	if (!result.clean) {
       		struct string_list conflicted_files = STRING_LIST_INIT_NODUP;
       		const char *last = NULL;
     +@@ builtin/merge-tree.c: static int real_merge(struct merge_tree_options *o,
     + 					      &result);
     + 	}
     + 	merge_finalize(&opt, &result);
     +-	return !result.clean; /* result.clean < 0 handled above */
     ++	return !result.tree || !result.clean; /* result.clean < 0 handled above */
     + }
     + 
     + int cmd_merge_tree(int argc, const char **argv, const char *prefix)
      
       ## t/t4301-merge-tree-write-tree.sh ##
      @@ t/t4301-merge-tree-write-tree.sh: 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' '
     ++test_expect_success SANITY 'merge-ort fails gracefully in a read-only repository' '
      +	git init --bare read-only &&
     -+	git push read-only side1 side2 &&
     ++	git push read-only side1 side2 side3 &&
      +	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 side3 &&
      +	test_must_fail git -C read-only merge-tree side1 side2
      +'
      +

 builtin/merge-tree.c             | 6 ++++--
 t/t4301-merge-tree-write-tree.sh | 9 +++++++++
 2 files changed, 13 insertions(+), 2 deletions(-)