[PATCH v5 0/4] stash: clean up index-mode test merge
- From
D. Ben Knoble <ben.knoble@gmail.com>
- Date
- Sep 30, 2026, 21:24 UTC
- Message-ID
- <cover.1790803471.git.ben.knoble@gmail.com>
- In-Reply-To
- <cover.1789853192.git.ben.knoble@gmail.com>
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 stateChanges 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> v3: <cover.1790425008.git.ben.knoble@gmail.com> v4: <cover.1790684309.git.ben.knoble@gmail.com>
[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
-- 2.56.0.rc1.315.gc6ed9934b7.dirty