Re: [PATCH] t3424: new rebase testcase documenting a stat-dirty-like failure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Elijah

On 18/02/2020 15:59, Elijah Newren wrote:
On Tue, Feb 18, 2020 at 7:05 AM Phillip Wood <phillip.wood123@xxxxxxxxx> wrote:

Hi Elijah

On 17/02/2020 19:12, Elijah Newren wrote:
I forgot to add some cc's in GitGitGadget before submitting; adding now...

On Mon, Feb 17, 2020 at 9:15 AM Elijah Newren via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:

From: Elijah Newren <newren@xxxxxxxxx>

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.

I think `--exec true` would be better as it makes it clear that it's not a timing issue. I've changed do_recursive_merge() to print the mtime and mode of "DS" before and after the merge which gives

HEAD is now at abd8fe3 side1
Rebasing (1/2) # picking commit1
DS mtime, mode before merge 1582109854, 120000
DS mtime, mode after merge 0, 120000
Rebasing (2/2) # picking commit2
DS mtime, mode before merge 0, 120000
error: Your local changes to the following files would be overwritten by merge:
	DS

So it looks like the problem is that when we pick commit1 we don't update the index entry for DS properly in merge_trees()

Best Wishes

Phillip

Intriguing - it's strange that it errors out picking commit2, not
commit1 I'll try and have a look at it. There seem to be some leftover
bits at the start of the test that want removing see below

I know, right?  It's kinda weird.

+test_expect_success 'setup' '
+       rm -rf ../stupid &&
+       git init ../stupid &&
+       cd ../stupid &&

I think these 3 lines must be left over from you trying this out before
making it a test

Whoops, yes you are exactly right.  I'll remove them.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux