| Message ID | 20200108214349.GA17624@lxhi-065.adit-jv.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Unreliable 'git rebase --onto' | expand |
On Wed, Jan 08, 2020 at 10:43:49PM +0100, Eugeniu Rosca wrote: > Hello Git community, > > Below is a simple reproduction scenario for what looks to be a bug (?) > in 'git rebase --onto' (v2.25.0-rc1-19-g042ed3e048af). > > I would appreciate your confirmation of the misbehavior. > If the behavior is correct/expected, I would appreciate some feedback > how to avoid it in future, since it occurs with the default parameters. > > 1. git clone https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > > 2. ### Cherry pick an upstream commit, to contrast the results with > 'git rebase --onto': > $ git checkout -b v4.18-cherry-pick v4.18 > $ git cherry-pick 463fa44eec2fef50 > Auto-merging drivers/input/touchscreen/atmel_mxt_ts.c > warning: inexact rename detection was skipped due to too many files. > warning: you may want to set your merge.renamelimit variable to at least 7216 and retry the command. > [v4.18-cherry-pick bd142b45bf3a] Input: atmel_mxt_ts - disable IRQ across suspend > Author: Evan Green <evgreen@chromium.org> > Date: Wed Oct 2 14:00:21 2019 -0700 > 1 file changed, 4 insertions(+) > > 3. ### In spite of the warning, the result matches the original commit: > $ vimdiff <(git show 463fa44eec2fef50) <(git show v4.18-cherry-pick) > > 4. ### Now, backport the same commit via 'git rebase --onto' > $ git rebase --onto v4.18 463fa44eec2fef50~ 463fa44eec2fef50 > First, rewinding head to replay your work on top of it... > Applying: Input: atmel_mxt_ts - disable IRQ across suspend > > 5. ### The result is different: > $ git branch v4.18-rebase-onto > $ git diff v4.18-cherry-pick v4.18-rebase-onto > > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c > index b45958e89cc5..2345b587662b 100644 > --- a/drivers/input/touchscreen/atmel_mxt_ts.c > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c > @@ -3139,8 +3139,6 @@ static int __maybe_unused mxt_suspend(struct device *dev) > > mutex_unlock(&input_dev->mutex); > > - disable_irq(data->irq); > - > return 0; > } > > @@ -3162,6 +3160,8 @@ static int __maybe_unused mxt_resume(struct device *dev) > > mutex_unlock(&input_dev->mutex); > > + disable_irq(data->irq); > + > return 0; > } > > > In a nutshell, purely from user's perspective: > - I get a warning from 'git cherry pick', with perfect results > - I get no warning from 'git rebase --onto', with wrong results > > Does git still behave expectedly? TIA! This is a known issue with the 'am' backend of 'git rebase'. The good news is that work is already well under way to change the default backend from 'am' to 'merge', which will solve this issue. From the log message of aa523de170 (rebase: change the default backend from "am" to "merge", 2019-12-24): The am-backend drops information and thus limits what we can do: [...] * reduction in context from only having a few lines beyond those changed means that when context lines are non-unique we can apply patches incorrectly.[2] [...] [2] https://lore.kernel.org/git/CABPp-BGiu2nVMQY_t-rnFR5GQUz_ipyEE8oDocKeO+> Alas, there is unexpected bad news: with that commit the runtime of your 'git rebase --onto' command goes from <1sec to over 50secs. Cc-ing Elijah, author of that patch...
user 0m9.644s sys 0m3.620s On Wed, Jan 8, 2020 at 2:36 PM SZEDER Gábor <szeder.dev@gmail.com> wrote: > > On Wed, Jan 08, 2020 at 10:43:49PM +0100, Eugeniu Rosca wrote: > > Hello Git community, > > > > Below is a simple reproduction scenario for what looks to be a bug (?) > > in 'git rebase --onto' (v2.25.0-rc1-19-g042ed3e048af). > > > > I would appreciate your confirmation of the misbehavior. > > If the behavior is correct/expected, I would appreciate some feedback > > how to avoid it in future, since it occurs with the default parameters. > > > > 1. git clone https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > > > > 2. ### Cherry pick an upstream commit, to contrast the results with > > 'git rebase --onto': > > $ git checkout -b v4.18-cherry-pick v4.18 > > $ git cherry-pick 463fa44eec2fef50 > > Auto-merging drivers/input/touchscreen/atmel_mxt_ts.c > > warning: inexact rename detection was skipped due to too many files. > > warning: you may want to set your merge.renamelimit variable to at least 7216 and retry the command. Lots of renames... > > [v4.18-cherry-pick bd142b45bf3a] Input: atmel_mxt_ts - disable IRQ across suspend > > Author: Evan Green <evgreen@chromium.org> > > Date: Wed Oct 2 14:00:21 2019 -0700 > > 1 file changed, 4 insertions(+) > > > > 3. ### In spite of the warning, the result matches the original commit: > > $ vimdiff <(git show 463fa44eec2fef50) <(git show v4.18-cherry-pick) > > > > 4. ### Now, backport the same commit via 'git rebase --onto' > > $ git rebase --onto v4.18 463fa44eec2fef50~ 463fa44eec2fef50 > > First, rewinding head to replay your work on top of it... > > Applying: Input: atmel_mxt_ts - disable IRQ across suspend > > > > 5. ### The result is different: > > $ git branch v4.18-rebase-onto > > $ git diff v4.18-cherry-pick v4.18-rebase-onto > > > > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c > > index b45958e89cc5..2345b587662b 100644 > > --- a/drivers/input/touchscreen/atmel_mxt_ts.c > > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c > > @@ -3139,8 +3139,6 @@ static int __maybe_unused mxt_suspend(struct device *dev) > > > > mutex_unlock(&input_dev->mutex); > > > > - disable_irq(data->irq); > > - > > return 0; > > } > > > > @@ -3162,6 +3160,8 @@ static int __maybe_unused mxt_resume(struct device *dev) > > > > mutex_unlock(&input_dev->mutex); > > > > + disable_irq(data->irq); > > + > > return 0; > > } > > > > > > In a nutshell, purely from user's perspective: > > - I get a warning from 'git cherry pick', with perfect results > > - I get no warning from 'git rebase --onto', with wrong results > > > > Does git still behave expectedly? TIA! > > This is a known issue with the 'am' backend of 'git rebase'. > > The good news is that work is already well under way to change the > default backend from 'am' to 'merge', which will solve this issue. > From the log message of aa523de170 (rebase: change the default backend > from "am" to "merge", 2019-12-24): > > The am-backend drops information and thus limits what we can do: > [...] > * reduction in context from only having a few lines beyond those > changed means that when context lines are non-unique we can apply > patches incorrectly.[2] > [...] > [2] https://lore.kernel.org/git/CABPp-BGiu2nVMQY_t-rnFR5GQUz_ipyEE8oDocKeO+> > > Alas, there is unexpected bad news: with that commit the runtime of > your 'git rebase --onto' command goes from <1sec to over 50secs. > Cc-ing Elijah, author of that patch... I see slowdown, but not nearly as big as you report: $ git checkout -b v4.18-cherry-pick v4.18 $ time git cherry-pick 463fa44eec2fef50 Auto-merging drivers/input/touchscreen/atmel_mxt_ts.c warning: inexact rename detection was skipped due to too many files. warning: you may want to set your merge.renamelimit variable to at least 7216 and retry the command. [v4.18-cherry-pick 88d39cdf3e80] Input: atmel_mxt_ts - disable IRQ across suspend Author: Evan Green <evgreen@chromium.org> Date: Wed Oct 2 14:00:21 2019 -0700 1 file changed, 4 insertions(+) real 0m1.110s user 0m0.956s sys 0m0.284s $ time git rebase --onto v4.18 463fa44eec2fef50~ 463fa44eec2fef5 First, rewinding head to replay your work on top of it... Applying: Input: atmel_mxt_ts - disable IRQ across suspend real 0m1.643s user 0m1.296s sys 0m0.264s $ time git rebase -m --onto v4.18 463fa44eec2fef50~ 463fa44eec2fef50 warning: inexact rename detection was skipped due to too many files. warning: you may want to set your merge.renamelimit variable to at least 7216 and retry the command. Successfully rebased and updated detached HEAD. real 0m13.305s user 0m9.644s sys 0m3.620s Interestingly, turning off rename detection only speeds it up a little bit: $ time git rebase -m -Xno-renames --onto v4.18 463fa44eec2fef50~ 463fa44eec2fef50 Successfully rebased and updated detached HEAD. real 0m11.955s user 0m8.732s sys 0m3.424s This is an interesting testcase; I'm going to try to find some time to dig in further.
Hi Elijah, hi Szeder, On Wed, Jan 08, 2020 at 02:06:22PM -0800, Elijah Newren wrote: > This looks like a known bug in rebase, in particular in the am-backend that > rebase uses by default. If I'm correct that it's just a context region > issue, then this is the same bug that was recently discussed at > https://lore.kernel.org/git/CAN_72e2h2avv-U9BVBYqXVKiC+5kHy-pjejyMSD3X22uRXE39g@mail.gmail.com/. > The current plan is to switch the default over to the merge backend (the > same machinery that cherry-pick uses), which doesn't suffer from the same > shortcomings (you can see the current work being done in this area at > https://lore.kernel.org/git/pull.679.v3.git.git.1577217299.gitgitgadget@gmail.com/ > ). Thank you for your feedback and references, here and in [*]. Once hit by this or similar issues, I think there is high chance for people to go through the same feelings as described by Pavel in [**]: --- That's so scary that I'm going to stop using "git rebase" for now. --- Some years ago I was hit by 'git merge' producing slightly different results compared to 'git rebase --onto' and 'git cherry-pick A..B' (maybe I can come up with a reproduction scenario for that too). Since then, I usually contrast the outcomes of merging, cherry-picking and rebasing, to make sure they match, but that's painful and time-consuming. > In the mean time, you can pass the -m flag to rebase to avoid these types > of problems. In fact, if you could retry with -m you may be able to > confirm whether it's the same issue. Indeed, neither `git rebase -m` nor `git rebase -i` exhibit the problem. [*] https://lore.kernel.org/git/CABPp-BHsy75UGm4wTOP2_AYik_dZi-_BxtAn-hyi-ZrNRRWGuw@mail.gmail.com/T/#m1cbf80ef56c260a626146d61291d7fbabd108f1b [**] https://lore.kernel.org/git/CAN_72e2h2avv-U9BVBYqXVKiC+5kHy-pjejyMSD3X22uRXE39g@mail.gmail.com/ Thanks again.
Hi Szeder, On Wed, Jan 08, 2020 at 11:35:57PM +0100, SZEDER Gábor wrote: > This is a known issue with the 'am' backend of 'git rebase'. > > The good news is that work is already well under way to change the > default backend from 'am' to 'merge', which will solve this issue. > From the log message of aa523de170 (rebase: change the default backend > from "am" to "merge", 2019-12-24): > > The am-backend drops information and thus limits what we can do: > [...] > * reduction in context from only having a few lines beyond those > changed means that when context lines are non-unique we can apply > patches incorrectly.[2] > [...] > [2] https://lore.kernel.org/git/CABPp-BGiu2nVMQY_t-rnFR5GQUz_ipyEE8oDocKeO+> > > Alas, there is unexpected bad news: with that commit the runtime of > your 'git rebase --onto' command goes from <1sec to over 50secs. > Cc-ing Elijah, author of that patch... [$.02] I would personally take the route of regaining users' trust in 'git rebase' first, with fixing the performance penalty later on. I was quite impressed by the recent 2.24.0 performance improvements, which tells there might be room for improvement for `git rebase` too, once it is fixed.
On Wed, Jan 08, 2020 at 04:55:46PM -0800, Elijah Newren wrote: > > Alas, there is unexpected bad news: with that commit the runtime of > > your 'git rebase --onto' command goes from <1sec to over 50secs. > > Cc-ing Elijah, author of that patch... > > I see slowdown, but not nearly as big as you report: The linux repo is big, my notebook is small, the poor thing :) > $ time git rebase -m --onto v4.18 463fa44eec2fef50~ 463fa44eec2fef50 > warning: inexact rename detection was skipped due to too many files. > warning: you may want to set your merge.renamelimit variable to at > least 7216 and retry the command. > Successfully rebased and updated detached HEAD. > > real 0m13.305s > user 0m9.644s > sys 0m3.620s > Interestingly, turning off rename detection only speeds it up a little bit: > $ time git rebase -m -Xno-renames --onto v4.18 463fa44eec2fef50~ > 463fa44eec2fef50 > Successfully rebased and updated detached HEAD. > > real 0m11.955s > user 0m8.732s > sys 0m3.424s > > > This is an interesting testcase; I'm going to try to find some time to > dig in further. The culprits are two seemingly unnecessary back-and-forth checkouts. I didn't realize I could use 'git rebase -m', so ran some tests with it, and turns out that the slowdown started with 68aa495b59 (rebase: implement --merge via the interactive machinery, 2018-12-11), where the runtime suddenly went from <1.5s to 45+s. Running 'git rebase -i --onto <those-same-commits>' is just as slow, and it appears that it has always been (the oldest I tried was v1.8.0), and it spends a long time both before and after popping up the editor for the rebase instructions. That's highly suspicious, so: $ git log --oneline -1 94710cac0ef4 (HEAD, tag: v4.18) Linux 4.18 $ git rebase -i --onto v4.18 463fa44eec2fef50~ 463fa44eec2fef50 hint: Waiting for your editor to close the file... # Hit ctrl-z in the editor $ git log --oneline -1 463fa44eec2f (HEAD) Input: atmel_mxt_ts - disable IRQ across suspend Oh. So 'git rebase -i' apparently checks out the tip commit of the to-be-rebased revision range before invoking the editor for the rebase instructions, only to check out the --onto commit (i.e. the commit we've started from!) to apply the selected commit on top. And indeed those two checkouts account for all the wasted runtime: $ time { git checkout 463fa44eec2fef50 && git checkout v4.18 ; } Updating files: 100% (49483/49483), done. Previous HEAD position was 94710cac0ef4 Linux 4.18 HEAD is now at 463fa44eec2f Input: atmel_mxt_ts - disable IRQ across suspend Updating files: 100% (49483/49483), done. Previous HEAD position was 463fa44eec2f Input: atmel_mxt_ts - disable IRQ across suspend HEAD is now at 94710cac0ef4 Linux 4.18 real 0m48.801s user 0m13.963s sys 0m5.114s
Hi, On Thu, Jan 9, 2020 at 7:03 AM SZEDER Gábor <szeder.dev@gmail.com> wrote: > > On Wed, Jan 08, 2020 at 04:55:46PM -0800, Elijah Newren wrote: > > > Alas, there is unexpected bad news: with that commit the runtime of > > > your 'git rebase --onto' command goes from <1sec to over 50secs. > > > Cc-ing Elijah, author of that patch... > > > > I see slowdown, but not nearly as big as you report: > > The linux repo is big, my notebook is small, the poor thing :) It went to just over 64secs on my home laptop (older and with spinny disks), so yeah, a big difference from my work machine which has an SSD. > > $ time git rebase -m --onto v4.18 463fa44eec2fef50~ 463fa44eec2fef50 > > warning: inexact rename detection was skipped due to too many files. > > warning: you may want to set your merge.renamelimit variable to at > > least 7216 and retry the command. > > Successfully rebased and updated detached HEAD. > > > > real 0m13.305s > > user 0m9.644s > > sys 0m3.620s > > > Interestingly, turning off rename detection only speeds it up a little bit: > > $ time git rebase -m -Xno-renames --onto v4.18 463fa44eec2fef50~ > > 463fa44eec2fef50 > > Successfully rebased and updated detached HEAD. > > > > real 0m11.955s > > user 0m8.732s > > sys 0m3.424s > > > > > > This is an interesting testcase; I'm going to try to find some time to > > dig in further. > > The culprits are two seemingly unnecessary back-and-forth checkouts. > > I didn't realize I could use 'git rebase -m', so ran some tests with > it, and turns out that the slowdown started with 68aa495b59 (rebase: > implement --merge via the interactive machinery, 2018-12-11), where > the runtime suddenly went from <1.5s to 45+s. > > Running 'git rebase -i --onto <those-same-commits>' is just as slow, > and it appears that it has always been (the oldest I tried was > v1.8.0), and it spends a long time both before and after popping up > the editor for the rebase instructions. That's highly suspicious, so: > > $ git log --oneline -1 > 94710cac0ef4 (HEAD, tag: v4.18) Linux 4.18 > $ git rebase -i --onto v4.18 463fa44eec2fef50~ 463fa44eec2fef50 > hint: Waiting for your editor to close the file... > # Hit ctrl-z in the editor > $ git log --oneline -1 > 463fa44eec2f (HEAD) Input: atmel_mxt_ts - disable IRQ across suspend > > Oh. > > So 'git rebase -i' apparently checks out the tip commit of the > to-be-rebased revision range before invoking the editor for the rebase > instructions, only to check out the --onto commit (i.e. the commit > we've started from!) to apply the selected commit on top. > > And indeed those two checkouts account for all the wasted runtime: > > $ time { git checkout 463fa44eec2fef50 && git checkout v4.18 ; } > Updating files: 100% (49483/49483), done. > Previous HEAD position was 94710cac0ef4 Linux 4.18 > HEAD is now at 463fa44eec2f Input: atmel_mxt_ts - disable IRQ across suspend > Updating files: 100% (49483/49483), done. > Previous HEAD position was 463fa44eec2f Input: atmel_mxt_ts - disable IRQ across suspend > HEAD is now at 94710cac0ef4 Linux 4.18 > > real 0m48.801s > user 0m13.963s > sys 0m5.114s Oh, cool, sounds like you're already investigating and found the problem.
On Thu, Jan 9, 2020 at 2:53 AM Eugeniu Rosca <erosca@de.adit-jv.com> wrote: > > Hi Elijah, hi Szeder, > > On Wed, Jan 08, 2020 at 02:06:22PM -0800, Elijah Newren wrote: > > This looks like a known bug in rebase, in particular in the am-backend that > > rebase uses by default. If I'm correct that it's just a context region > > issue, then this is the same bug that was recently discussed at > > https://lore.kernel.org/git/CAN_72e2h2avv-U9BVBYqXVKiC+5kHy-pjejyMSD3X22uRXE39g@mail.gmail.com/. > > The current plan is to switch the default over to the merge backend (the > > same machinery that cherry-pick uses), which doesn't suffer from the same > > shortcomings (you can see the current work being done in this area at > > https://lore.kernel.org/git/pull.679.v3.git.git.1577217299.gitgitgadget@gmail.com/ > > ). > > Thank you for your feedback and references, here and in [*]. > > Once hit by this or similar issues, I think there is high chance for > people to go through the same feelings as described by Pavel in [**]: > > --- > That's so scary that I'm going to stop using "git rebase" for now. > --- Yep, I understand; that kind of feeling is why I wanted to jump in and try to help fix it. I want merge/rebase/cherry-pick to be reliable. > Some years ago I was hit by 'git merge' producing slightly different > results compared to 'git rebase --onto' and 'git cherry-pick A..B' > (maybe I can come up with a reproduction scenario for that too). If you can, I'd be interested to see it and take a look. I'd normally assume it was just some case where A..B included "evil" merge commits (merge commits that made additional changes not part of the actual merging) since rebasing or cherry-picking such a range would exclude the merge commits and thus drop those changes -- but you identified a real bug with the default rebase backend so I'm interested to see if you happen to have more bugs I should know about. > > Since then, I usually contrast the outcomes of merging, cherry-picking > and rebasing, to make sure they match, but that's painful and > time-consuming. > > > In the mean time, you can pass the -m flag to rebase to avoid these types > > of problems. In fact, if you could retry with -m you may be able to > > confirm whether it's the same issue. > > Indeed, neither `git rebase -m` nor `git rebase -i` exhibit the problem. That's good news. Unfortunately, you should note that git-2.25 is going to have the same bug you reported; there are still some loose ends with my series to make -m the default, and the 2.25 release is expected within a few days, so my change of default won't happen until 2.26. (That series would have needed to be completed several weeks ago for it to go into 2.25).
Hi Elijah, On Thu, Jan 09, 2020 at 10:05:52AM -0800, Elijah Newren wrote: > On Thu, Jan 9, 2020 at 2:53 AM Eugeniu Rosca <erosca@de.adit-jv.com> wrote: > > Some years ago I was hit by 'git merge' producing slightly different > > results compared to 'git rebase --onto' and 'git cherry-pick A..B' > > (maybe I can come up with a reproduction scenario for that too). > > If you can, I'd be interested to see it and take a look. I'd normally > assume it was just some case where A..B included "evil" merge commits > (merge commits that made additional changes not part of the actual > merging) since rebasing or cherry-picking such a range would exclude > the merge commits and thus drop those changes -- but you identified a > real bug with the default rebase backend so I'm interested to see if > you happen to have more bugs I should know about. Here is a _simplified_ scenario to get a totally unexpected result from 'git merge' (initially reproduced years ago, but still happening on 2.25.0.rc2): ## Preparation 0. git --version git version 2.25.0.rc2 1. git clone https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2. git remote add linux-stable https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git 3. git fetch linux-stable # Reproduction 4. git checkout f7a8e38f07a1 5. git merge --no-edit e18da11fc0f959 ## Merge v4.4.3 commit https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=e18da11fc0f959 which is a linux-stable backport of vanilla v4.5-rc1 commit https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f7a8e38f07a1 the latter being checked out at step 4. 6. git show HEAD ## Inspect the _automatic_ conflict resolution performed by git in drivers/mtd/nand/nand_base.c. Git decided to integrate e18da11fc0f959 alongside f7a8e38f07a1, while essentially they are the same commit. We end up with two times commit f7a8e38f07a1. What do you think about that? > Unfortunately, you should note that git-2.25 is going to have the same > bug you reported; there are still some loose ends with my series to > make -m the default, and the 2.25 release is expected within a few > days, so my change of default won't happen until 2.26. (That series > would have needed to be completed several weeks ago for it to go into > 2.25). Thanks for this piece of information and for the time/effort spent!
Hi Eugeniu, On Thu, Jan 9, 2020 at 4:06 PM Eugeniu Rosca <roscaeugeniu@gmail.com> wrote: > > Hi Elijah, > > On Thu, Jan 09, 2020 at 10:05:52AM -0800, Elijah Newren wrote: > > On Thu, Jan 9, 2020 at 2:53 AM Eugeniu Rosca <erosca@de.adit-jv.com> wrote: > > > Some years ago I was hit by 'git merge' producing slightly different > > > results compared to 'git rebase --onto' and 'git cherry-pick A..B' > > > (maybe I can come up with a reproduction scenario for that too). > > > > If you can, I'd be interested to see it and take a look. I'd normally > > assume it was just some case where A..B included "evil" merge commits > > (merge commits that made additional changes not part of the actual > > merging) since rebasing or cherry-picking such a range would exclude > > the merge commits and thus drop those changes -- but you identified a > > real bug with the default rebase backend so I'm interested to see if > > you happen to have more bugs I should know about. > > Here is a _simplified_ scenario to get a totally unexpected result from > 'git merge' (initially reproduced years ago, but still happening on > 2.25.0.rc2): > > ## Preparation > 0. git --version > git version 2.25.0.rc2 > 1. git clone https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > 2. git remote add linux-stable https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git > 3. git fetch linux-stable > > # Reproduction > 4. git checkout f7a8e38f07a1 > 5. git merge --no-edit e18da11fc0f959 > ## Merge v4.4.3 commit > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=e18da11fc0f959 > which is a linux-stable backport of vanilla v4.5-rc1 commit > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f7a8e38f07a1 > the latter being checked out at step 4. > > 6. git show HEAD > ## Inspect the _automatic_ conflict resolution performed by git in > drivers/mtd/nand/nand_base.c. Git decided to integrate e18da11fc0f959 > alongside f7a8e38f07a1, while essentially they are the same commit. > We end up with two times commit f7a8e38f07a1. > > What do you think about that? Ooh, interesting case; thanks for sending it along. I think this is the same as https://lore.kernel.org/git/20190816184051.GB13894@sigill.intra.peff.net/ , which struck git itself not that long back. It didn't do any actual harm, though, it was just surprising. I'm not familiar with the xdiff part of the codebase, so I don't know if this is a heuristic thing, or something more along the lines of the diff3 issues mentioned at https://www.cis.upenn.edu/~bcpierce/papers/diff3-short.pdf. I read up on this area a little bit a few months ago and I'd like to dig more at the diff3 stuff in general, but it may be a little while. If you see more issues like this, though, I'm definitely interested in saving and cataloging them for when I get back to this. Elijah
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c index b45958e89cc5..2345b587662b 100644 --- a/drivers/input/touchscreen/atmel_mxt_ts.c +++ b/drivers/input/touchscreen/atmel_mxt_ts.c @@ -3139,8 +3139,6 @@ static int __maybe_unused mxt_suspend(struct device *dev) mutex_unlock(&input_dev->mutex); - disable_irq(data->irq); - return 0; } @@ -3162,6 +3160,8 @@ static int __maybe_unused mxt_resume(struct device *dev) mutex_unlock(&input_dev->mutex); + disable_irq(data->irq); + return 0; }
Hello Git community, Below is a simple reproduction scenario for what looks to be a bug (?) in 'git rebase --onto' (v2.25.0-rc1-19-g042ed3e048af). I would appreciate your confirmation of the misbehavior. If the behavior is correct/expected, I would appreciate some feedback how to avoid it in future, since it occurs with the default parameters. 1. git clone https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2. ### Cherry pick an upstream commit, to contrast the results with 'git rebase --onto': $ git checkout -b v4.18-cherry-pick v4.18 $ git cherry-pick 463fa44eec2fef50 Auto-merging drivers/input/touchscreen/atmel_mxt_ts.c warning: inexact rename detection was skipped due to too many files. warning: you may want to set your merge.renamelimit variable to at least 7216 and retry the command. [v4.18-cherry-pick bd142b45bf3a] Input: atmel_mxt_ts - disable IRQ across suspend Author: Evan Green <evgreen@chromium.org> Date: Wed Oct 2 14:00:21 2019 -0700 1 file changed, 4 insertions(+) 3. ### In spite of the warning, the result matches the original commit: $ vimdiff <(git show 463fa44eec2fef50) <(git show v4.18-cherry-pick) 4. ### Now, backport the same commit via 'git rebase --onto' $ git rebase --onto v4.18 463fa44eec2fef50~ 463fa44eec2fef50 First, rewinding head to replay your work on top of it... Applying: Input: atmel_mxt_ts - disable IRQ across suspend 5. ### The result is different: $ git branch v4.18-rebase-onto $ git diff v4.18-cherry-pick v4.18-rebase-onto In a nutshell, purely from user's perspective: - I get a warning from 'git cherry pick', with perfect results - I get no warning from 'git rebase --onto', with wrong results Does git still behave expectedly? TIA!