Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Eugeniu Rosca <erosca@de.adit-jv.com>, Elijah Newren <newren@gmail.com>
Cc: gitster@pobox.com, peff@peff.net, avarab@gmail.com,
	git@vger.kernel.org, Eugeniu Rosca <roscaeugeniu@gmail.com>
Subject: Re: Unreliable 'git rebase --onto'
Date: Wed, 8 Jan 2020 23:35:57 +0100
Message-ID: <20200108223557.GE32750@szeder.dev> (raw)
In-Reply-To: <20200108214349.GA17624@lxhi-065.adit-jv.com>

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...


  reply index

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-08 21:43 Eugeniu Rosca
2020-01-08 22:35 ` SZEDER Gábor [this message]
2020-01-09  0:55   ` Elijah Newren
2020-01-09 15:03     ` SZEDER Gábor
2020-01-09 17:53       ` Elijah Newren
2020-01-21 19:18       ` [PATCH v1] rebase -i: stop checking out the tip of the branch to rebase Alban Gruin
2020-01-21 20:07         ` Elijah Newren
2020-01-22 20:24         ` Junio C Hamano
2020-01-22 20:47         ` Junio C Hamano
2020-01-24 14:45           ` Alban Gruin
2020-01-24 14:45         ` [PATCH v2] " Alban Gruin
2020-01-24 14:55           ` Alban Gruin
2020-01-24 18:12             ` Junio C Hamano
2020-01-24 15:05           ` [PATCH v3] " Alban Gruin
2020-01-24 18:30             ` Junio C Hamano
2020-02-05 14:31             ` Johannes Schindelin
2020-01-24 17:11           ` [PATCH v2] " Andrei Rybak
2020-01-09 11:13   ` Unreliable 'git rebase --onto' Eugeniu Rosca
     [not found] ` <CABPp-BHsyMOz+hi7EYoAnAWfzms7FRfwqCoarnu8H+vyDoN6SQ@mail.gmail.com>
2020-01-09 10:53   ` Eugeniu Rosca
2020-01-09 18:05     ` Elijah Newren
2020-01-10  0:06       ` Eugeniu Rosca
2020-01-10  2:35         ` Elijah Newren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200108223557.GE32750@szeder.dev \
    --to=szeder.dev@gmail.com \
    --cc=avarab@gmail.com \
    --cc=erosca@de.adit-jv.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    --cc=roscaeugeniu@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Mailing List Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/git/0 git/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 git git/ https://lore.kernel.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.git


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git