From 73113c592222eac40dfdf76d9b2bbc2137d73783 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 19 Feb 2020 17:04:06 +0000 Subject: [PATCH 1/2] t3433: new rebase testcase documenting a stat-dirty-like failure A user discovered a case where they had a stack of 20 simple commits to rebase, and the rebase would succeed in picking the first commit and then error out with a pair of "Could not execute the todo command" and "Your local changes to the following files would be overwritten by merge" messages. Their steps actually made use of the -i flag, but I switched it over to -m to make it simpler to trigger the bug. With that flag, it bisects back to commit 68aa495b590d (rebase: implement --merge via the interactive machinery, 2018-12-11), but that's misleading. If you change the -m flag to --keep-empty, then the problem persists and will bisect back to 356ee4659bb5 (sequencer: try to commit without forking 'git commit', 2017-11-24) After playing with the testcase for a bit, I discovered that added --exec "sleep 1" to the command line makes the rebase succeed, making me suspect there is some kind of discard and reloading of caches that lead us to believe that something is stat dirty, but I didn't succeed in digging any further than that. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t3433-rebase-across-mode-change.sh | 48 ++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100755 t/t3433-rebase-across-mode-change.sh diff --git a/t/t3433-rebase-across-mode-change.sh b/t/t3433-rebase-across-mode-change.sh new file mode 100755 index 0000000000..f11fc35c3e --- /dev/null +++ b/t/t3433-rebase-across-mode-change.sh @@ -0,0 +1,48 @@ +#!/bin/sh + +test_description='git rebase across mode change' + +. ./test-lib.sh + +test_expect_success 'setup' ' + mkdir DS && + >DS/whatever && + git add DS && + git commit -m base && + + git branch side1 && + git branch side2 && + + git checkout side1 && + git rm -rf DS && + test_ln_s_add unrelated DS && + git commit -m side1 && + + git checkout side2 && + >unrelated && + git add unrelated && + git commit -m commit1 && + + echo >>unrelated && + git commit -am commit2 +' + +test_expect_success 'rebase changes with the apply backend' ' + test_when_finished "git rebase --abort || true" && + git checkout -b apply-backend side2 && + git rebase side1 +' + +test_expect_failure 'rebase changes with the merge backend' ' + test_when_finished "git rebase --abort || true" && + git checkout -b merge-backend side2 && + git rebase -m side1 +' + +test_expect_success 'rebase changes with the merge backend with a delay' ' + test_when_finished "git rebase --abort || true" && + git checkout -b merge-delay-backend side2 && + git rebase -m --exec "sleep 1" side1 +' + +test_done From fb1c18fc466c6a9c6ea1b072c9db93c0046f4cbd Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 19 Feb 2020 17:04:07 +0000 Subject: [PATCH 2/2] merge-recursive: fix the refresh logic in update_file_flags If we need to delete a higher stage entry in the index to place the file at stage 0, then we'll lose that file's stat information. In such situations we may still be able to detect that the file on disk is the version we want (as noted by our comment in the code: /* do not overwrite file if already present */ ), but we do still need to update the mtime since we are creating a new cache_entry for that file. Update the logic used to determine whether we refresh a file's mtime. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 7 +++++-- t/t3433-rebase-across-mode-change.sh | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index 10dca5644b..9657adc4df 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -998,10 +998,13 @@ static int update_file_flags(struct merge_options *opt, free(buf); } update_index: - if (!ret && update_cache) - if (add_cacheinfo(opt, contents, path, 0, update_wd, + if (!ret && update_cache) { + int refresh = (!opt->priv->call_depth && + contents->mode != S_IFGITLINK); + if (add_cacheinfo(opt, contents, path, 0, refresh, ADD_CACHE_OK_TO_ADD)) return -1; + } return ret; } diff --git a/t/t3433-rebase-across-mode-change.sh b/t/t3433-rebase-across-mode-change.sh index f11fc35c3e..05df964670 100755 --- a/t/t3433-rebase-across-mode-change.sh +++ b/t/t3433-rebase-across-mode-change.sh @@ -33,7 +33,7 @@ test_expect_success 'rebase changes with the apply backend' ' git rebase side1 ' -test_expect_failure 'rebase changes with the merge backend' ' +test_expect_success 'rebase changes with the merge backend' ' test_when_finished "git rebase --abort || true" && git checkout -b merge-backend side2 && git rebase -m side1