[PATCH v3 0/5] stash: clean up index-mode test merge

Hi all,

This small patch series fixes a bug reported by Eli Barzilay in the interaction between autostashing, staged index entries, and stash.index=true.

The first patch is an incidental cleanup, and the second re-arranges one line to make the change easier. The third and fourth add missing test coverage (which catch breakages from prior incorrect rounds of this series), while the last holds the interesting bits.

Changes in v3:

• Change conflict label for current index
• Fix memory leak of merge_result
• Fix order of trees to make the correct merge (cherry-pick)
    • New test (3/5) to validate this
• Fix test in 4/5 to assert more details of expected state

Changes in v2:

• Do give branch labels for the incore merge, although they are never
  seen (and clarify commit message as a result, also keeping the
  merge-ort asserts). Phillip was right: without those, we do segfault
  on conflicts.
• Use the ui merge options to keep the same diff algorithm.
• Use merge_finalize instead of clear_merge_options, and reuse the
  options between merge calls if they are already initialized.
• Add a new 2/4 to simplify merge options initialization.
• Add a new 3/4 with a test case for conflicted index merges.

v1: <cover.1789853192.git.ben.knoble@gmail.com> v2: <cover.1790168285.git.ben.knoble@gmail.com>

[1/5] builtin/stash: remove unused header [2/5] stash: prepare merge options earlier [3/5] t3903: test stash --index merges [4/5] t3903: test failed "stash apply --index" [5/5] builtin/stash: merge index in-core

 builtin/stash.c  | 85 +++++++++++-------------------------------------
 t/t3903-stash.sh | 42 ++++++++++++++++++++++++
 t/t7600-merge.sh |  9 +++++
 3 files changed, 70 insertions(+), 66 deletions(-)

Diff-intervalle contre v2 :
1:  b6798c8a25 = 1:  6a165c4df4 builtin/stash: remove unused header
2:  1e2343c7fc = 2:  d9a9e18f3a stash: prepare merge options earlier
-:  ---------- > 3:  8b5ea5e6f4 t3903: test stash --index merges
3:  5bd4b78cac ! 4:  d39e16905d t: test failed "stash apply --index"
    @@ Metadata
     Author: D. Ben Knoble <ben.knoble@gmail.com>
     
      ## Commit message ##
    -    t: test failed "stash apply --index"
    +    t3903: test failed "stash apply --index"
     
         The next commit will refactor index handling for applied stashes, so
         let's make sure we cover conflicted index merging, too.
     
    +    Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
    +
      ## t/t3903-stash.sh ##
     @@ t/t3903-stash.sh: setup_stash() {
    - 	test_cmp expect actual
    + 	test_cmp expect file
      '
      
     +test_expect_success 'stash apply --index leaves everything untouched on failure' '
    @@ t/t3903-stash.sh: setup_stash() {
     +	echo unrelated >file &&
     +	echo unrelated >another-file &&
     +	git add another-file &&
    -+	git diff-files >expect &&
    -+
     +	echo conflict >other-file &&
     +	git add other-file &&
    ++	git diff-files -p >expect &&
    ++	git diff-index --cached HEAD >expect-index &&
    ++
     +	test_must_fail git stash apply --index 2>err &&
     +	test_grep "conflicts in index. Try without --index" err &&
    -+	git diff-files >actual &&
    -+	test_cmp expect actual
    ++	git diff-files -p >actual &&
    ++	test_cmp expect actual &&
    ++	git diff-index --cached HEAD >actual-index &&
    ++	test_cmp expect-index actual-index
     +'
     +
      test_expect_success 'stash -k' '
4:  e49936ee12 ! 5:  fde7fb7988 builtin/stash: merge index in-core
    @@ Commit message
     
         Reported-by: Eli Barzilay <eli@barzilay.org>
         Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
    +    Helped-by: Junio C Hamano <gitster@pobox.com>
     
      ## builtin/stash.c ##
     @@ builtin/stash.c: static int create_index_from_tree(const struct object_id *tree_id,
    @@ builtin/stash.c: static enum stash_apply_result do_apply_stash(const char *prefi
     -				return error(_("could not generate diff %s^!."),
     -					     oid_to_hex(&info->w_commit));
     -			}
    -+			o.branch1 = "Upstream index";
    ++			o.branch1 = "Current index";
     +			o.branch2 = "Stashed index changes";
     +			o.ancestor = "Stash base";
      
    @@ builtin/stash.c: static enum stash_apply_result do_apply_stash(const char *prefi
     +			merge = lookup_tree(o.repo, &info->i_tree);
     +			merge_base = lookup_tree(o.repo, &info->b_tree);
     +
    -+			merge_incore_nonrecursive(&o, head, merge, merge_base,
    ++			merge_incore_nonrecursive(&o, merge_base, head, merge,
     +						  &result);
     +
    ++			oidcpy(&index_tree, &result.tree->object.oid);
    ++			merge_finalize(&o, &result);
    ++
     +			if (!result.clean)
      				return error(_("conflicts in index. "
      					       "Try without --index."));
    - 
    +-
     -			discard_index(the_repository->index);
     -			repo_read_index(the_repository);
     -			if (write_index_as_tree(&index_tree, the_repository->index,
    @@ builtin/stash.c: static enum stash_apply_result do_apply_stash(const char *prefi
     -			reset_head();
     -			discard_index(the_repository->index);
     -			repo_read_index(the_repository);
    -+			oidcpy(&index_tree, &result.tree->object.oid);
    -+			merge_finalize(&o, &result);
      		}
      	}
      

base-commit: d38352cd43ab9745686d697872408bc3249a153f