[PATCH] rebase-i: keep old parents when preserving merges

[PATCH] rebase-i: keep old parents when preserving merges

[Stephan Beyer]

This patch fixes two issues for rebase -i with preserving
merges of unrelated branches.

Merge parents that are part of an unrelated branch have not been
kept as a parent in some cases.
That means, a rewritten merge commit has either lost a parent
or rebase tried to cherry-pick this merge commit which resulted
in an error.

Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
 git-rebase--interactive.sh |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a35212d..0df7640 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -174,6 +174,8 @@ pick_one_preserving_merges () {
 				new_parents="$new_parents $new_p"
 				;;
 			esac
+		else
+			new_parents="$new_parents $p"
 		fi
 	done
 	case $fast_forward in
-- 
1.5.6.3.357.ge6ecd

Re: [PATCH] rebase-i: keep old parents when preserving merges

[Stephan Beyer]

I have to admit that the subject is pretty ambiguous.

Sorry :) I should go to bed.
  Stephan

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

Re: [PATCH] rebase-i: keep old parents when preserving merges

[Johannes Schindelin]

Hi,

On Wed, 16 Jul 2008, Stephan Beyer wrote:

> This patch fixes two issues for rebase -i with preserving
> merges of unrelated branches.

Two?  What's the second?

Patch is obviously good.

Thanks,
Dscho

Re: [PATCH] rebase-i: keep old parents when preserving merges

[Stephan Beyer]

Hi,

Johannes Schindelin wrote:
> Hi,
> 
> On Wed, 16 Jul 2008, Stephan Beyer wrote:
> 
> > This patch fixes two issues for rebase -i with preserving
> > merges of unrelated branches.
> 
> Two?  What's the second?

Oh, eh, well ;)
It's only one bug, but it results in two different behaviors...
If you have a merge with more than two parents and not all are lost,
then it merges with too few parents.
If you have a merge with exactly two parents and one is lost, then
it tries to cherry-pick (with an error).
"Two" "issues" :)

Regards.
  Stephan

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

Re: [PATCH] rebase-i: keep old parents when preserving merges

[Johannes Schindelin]

Hi,

On Wed, 16 Jul 2008, Stephan Beyer wrote:

> Johannes Schindelin wrote:
> 
> > On Wed, 16 Jul 2008, Stephan Beyer wrote:
> > 
> > > This patch fixes two issues for rebase -i with preserving merges of 
> > > unrelated branches.
> > 
> > Two?  What's the second?
> 
> Oh, eh, well ;)
> It's only one bug, but it results in two different behaviors...

Ah, okay.  Worthwhile clarifying in the commit message.

Thanks for the patch,
Dscho

Re: [PATCH] rebase-i: keep old parents when preserving merges

[Junio C Hamano]

Stephan Beyer <s-beyer@gmx.net> writes:

... Based on the discussion thread, here is a rewrite of the log message.

> When "rebase -i -p" tries to preserve merges of unrelated branches, it
> lost some parents:
>
>  - When you have more than two parents, the commit in the new history
>    ends up with fewer than expected number of parents and this breakage
>    goes unnoticed;
>
>  - When you are rebasing a merge with two parents and one is lost, the
>    command tries to cherry-pick the original merge commit, and the command
>    fails.
>
> Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index a35212d..0df7640 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -174,6 +174,8 @@ pick_one_preserving_merges () {
>  				new_parents="$new_parents $new_p"
>  				;;
>  			esac
> +		else
> +			new_parents="$new_parents $p"
>  		fi
>  	done

Reading the surrounding code, it makes me wonder if you also need to futz
with variables like $preserve and $fast_forward.

Re: [PATCH] rebase-i: keep old parents when preserving merges

[Stephan Beyer]

Hi,

Junio C Hamano wrote:
> Stephan Beyer <s-beyer@gmx.net> writes:
> 
> ... Based on the discussion thread, here is a rewrite of the log message.
> 
> > When "rebase -i -p" tries to preserve merges of unrelated branches, it
> > lost some parents:
> >
> >  - When you have more than two parents, the commit in the new history
> >    ends up with fewer than expected number of parents and this breakage
> >    goes unnoticed;
> >
> >  - When you are rebasing a merge with two parents and one is lost, the
> >    command tries to cherry-pick the original merge commit, and the command
> >    fails.
> >
> > Signed-off-by: Stephan Beyer <s-beyer@gmx.net>

Ok, big thanks :)

> > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> > index a35212d..0df7640 100755
> > --- a/git-rebase--interactive.sh
> > +++ b/git-rebase--interactive.sh
> > @@ -174,6 +174,8 @@ pick_one_preserving_merges () {
> >  				new_parents="$new_parents $new_p"
> >  				;;
> >  			esac
> > +		else
> > +			new_parents="$new_parents $p"
> >  		fi
> >  	done
> 
> Reading the surrounding code, it makes me wonder if you also need to futz
> with variables like $preserve and $fast_forward.

No, I would not see a reason for that.

Regards,
  Stephan

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F