Skip to content

stash: clean up index-mode test merge

D. Ben Knoble git 5 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v5 Latest4 patches 2 repliesIn review
  2. v45 patches 6 repliesSuperseded
  3. v35 patches 20 repliesSuperseded
Earlier versions · 2
VersionPostedPatchesRepliesStatus
v24 patches18 repliesSuperseded
v12 patches7 repliesSuperseded

Changes in v5 from the cover letter

• Rebase on synthetic merge for the test interaction with t5520 (dropping old 4/5) [59d1ce1b6e (Merge branch 'tb/t5520-reflog-expire' into dk/stash-apply-index-incore, 2026-09-29)]
• Fix handling of tri-state merge_result.clean

Patches in v5 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/4builtin/stash: remove unused headerbuiltin/stash.c+0 -10 replies
2/4stash: prepare merge options earlierbuiltin/stash.c+5 -50 replies
3/4t3903: test failed "stash apply --index"t/t3903-stash.sh+21 -00 replies
4/4builtin/stash: merge index in-corebuiltin/stash.c t/t3903-stash.sh t/t7600-merge.sh+50 -630 replies

Reviews trailers per patch

None of the 4 patches has a Reviewed-by or Acked-by.

PatchPhillip WoodJunio C Hamano
1/4
2/4
3/4Helped-by
4/4Helped-by Helped-by

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v5 thread

Cover letter [PATCH v5 0/4] 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 adds missing test coverage (which catch breakages from prior incorrect rounds of this series). The fourth fixes a test interaction with another in-flight topic. The last holds the interesting bits.

Changes in v5:
• Rebase on synthetic merge for the test interaction with t5520 (dropping old 4/5) [59d1ce1b6e (Merge branch 'tb/t5520-reflog-expire' into dk/stash-apply-index-incore, 2026-09-29)]
• Fix handling of tri-state merge_result.clean

Changes in v4:
• Drop merge verbosity changes altogether. I was going to save-and-restore, but when looking at the index-merge test case (more below) closer, I noticed that "git apply --cached" reports conflicts on stderr. That is, "git stash apply --index" would report conflicts, and silencing the merge takes that away. So instead let's leave the configured verbosity alone.
• Only copy resulting index merge tree OID when successful
• Fix interaction with t5520 (new patch 4/5)
• Squash test from 3/5 into 5/5, since it requires actually merging trees. I've elected to keep it a separate test for now (contrary to Phillip's suggestion) since it's written and working. Adapting existing tests requires quite a bit more digging into implicit context assumptions ;)

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: <[email protected]>
v2: <[email protected]>
v3: <[email protected]>
v4: <[email protected]>

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

 builtin/stash.c  | 94 +++++++++++++-----------------------------------
 t/t3903-stash.sh | 42 ++++++++++++++++++++++
 t/t7600-merge.sh |  9 +++++
 3 files changed, 76 insertions(+), 69 deletions(-)

Diff-intervalle contre v4 :

1:  6a165c4df4 = 1:  d8f4c36459 builtin/stash: remove unused header
2:  35b64ae321 = 2:  8e99033ef0 stash: prepare merge options earlier
3:  7b0b317ce0 = 3:  ee28d0a840 t3903: test failed "stash apply --index"
4:  2ac371d2dc < -:  ---------- t5520: don't expire reflogs where it matters
5:  e21b832a6e ! 4:  ca3de1d4a3 builtin/stash: merge index in-core
    @@ builtin/stash.c: static enum stash_apply_result do_apply_stash(const char *prefi
     +			merge_incore_nonrecursive(&o, merge_base, head, merge,
     +						  &result);
     +
    -+			if (!result.clean) {
    ++			if (result.clean < 0) {
    ++				merge_finalize(&o, &result);
    ++				return error(_("index merge failed"));
    ++			} else if (!result.clean) {
     +				merge_finalize(&o, &result);
      				return error(_("conflicts in index. "
      					       "Try without --index."));

base-commit: a018953688f1b10bddf91bff8747068f5f4746a4
prerequisite-patch-id: 601853fa5478b0dbfb260ba02632418e90338219

Signature · 1 line
2.56.0.rc1.315.gc6ed9934b7.dirty