[RFC PATCH] add t3420-rebase-topology

[RFC PATCH] add t3420-rebase-topology

[Martin von Zweigbergk]

Add more test cases to check that the topology after a rebase is as
expected. Conflicts are not considered, but patch-equivalence is.
---

Tests pass and fail as indicated by the suffix
(_success/_failure). Your input especially appreciated on whether you
agree with the intent of the test cases. For example, do you agree
that 'rebase --onto does not re-apply patches in onto' is desirable?
And if you do, then do you also agree that 'rebase --root --onto
ignores patch in onto' is desirable? How about 'rebase --root is not a
no-op'? One might think that --force would be necessary, but on the
other hand, if that was the case, the only point (AFAICT) of "git
rebase --root <branch>" without --force would be to linearize history,
so I instead made the test case confirm that --root without --onto
effectively behaves as if --force was also passed.

Feedback on the structure/setup and style is of course also
appreciated.

 t/t3420-rebase-topology.sh | 348 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 348 insertions(+)
 create mode 100755 t/t3420-rebase-topology.sh

diff --git a/t/t3420-rebase-topology.sh b/t/t3420-rebase-topology.sh
new file mode 100755
index 0000000..024a2b4
--- /dev/null
+++ b/t/t3420-rebase-topology.sh
@@ -0,0 +1,348 @@
+#!/bin/sh
+
+test_description='effect of rebase on topology'
+. ./test-lib.sh
+
+
+#       q---C---r
+#      /
+# a---b---c---d!--e---p
+#      \
+#       f---g!--h
+#        \
+#         j-------E---k
+#          \       \
+#           n---H---w
+#
+# x---y---B
+#
+#
+# ! = empty
+# uppercase = cherry-picked
+# p = reverted e
+#
+# TODO:
+# prune graph to what's needed
+
+empty () {
+	git commit --allow-empty -m $1 &&
+	git tag $1
+}
+
+cherry_pick () {
+	git cherry-pick -n $1 &&
+	git commit -m $2 &&
+	git tag $2
+}
+
+revert () {
+	git revert -n $1 &&
+	git commit -m $2 &&
+	git tag $2
+}
+
+
+test_expect_success 'setup' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	empty d &&
+	test_commit e &&
+	revert e p &&
+	git checkout b &&
+	test_commit f &&
+	empty g &&
+	test_commit h &&
+	git checkout f &&
+	test_commit j &&
+	cherry_pick e E &&
+	test_commit k &&
+	git checkout j &&
+	test_commit n &&
+	cherry_pick h H &&
+	git merge -m w E &&
+	git tag w &&
+	git checkout b &&
+	test_commit q &&
+	cherry_pick c C &&
+	test_commit r &&
+	git checkout --orphan disjoint &&
+	git rm -rf . &&
+	test_commit x &&
+	test_commit y &&
+	cherry_pick b B
+'
+
+reset () {
+	git rebase --abort
+	git reset --hard
+}
+
+test_range () {
+	test "$(git log --reverse --topo-order --format=%s "$1" | xargs)" = "$2"
+}
+
+test_revisions () {
+	expected="$1"
+	shift
+	test "$(git log --format=%s --no-walk=unsorted "$@" | xargs)" = "$expected"
+}
+
+same_revision () {
+	test "$(git rev-parse $1)" = "$(git rev-parse $2)"
+}
+
+# the following 5 (?) tests copy t3400 tests, but check the history rather than status code and/or stdout
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' c j &&
+	same_revision HEAD~2 c &&
+	test_range c.. "f j"
+'
+}
+test_expect_success 'simple rebase' "$(run)"
+test_expect_success 'simple rebase -m' "$(run -m)"
+test_expect_success 'simple rebase -i' "$(run -i)"
+test_expect_success 'simple rebase -p' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' b j &&
+	same_revision HEAD j
+'
+}
+test_expect_success 'rebase is no-op if upstream is an ancestor' "$(run)"
+test_expect_success 'rebase -m is no-op if upstream is an ancestor' "$(run -m)"
+test_expect_success 'rebase -i is no-op if upstream is an ancestor' "$(run -i)"
+test_expect_success 'rebase -p is no-op if upstream is an ancestor' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' --force b j &&
+	! same_revision HEAD j &&
+	test_range b.. "f j"
+'
+}
+test_expect_success 'rebase --force' "$(run)"
+test_expect_success 'rebase -m --force' "$(run -m)"
+test_expect_success 'rebase -i --force' "$(run -i)"
+test_expect_failure 'rebase -p --force' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' j b &&
+	same_revision HEAD j
+'
+}
+test_expect_success 'rebase fast-forwards if an ancestor of upstream' "$(run)"
+test_expect_success 'rebase -m fast-forwards if an ancestor of upstream' "$(run -m)"
+test_expect_success 'rebase -i fast-forwards if an ancestor of upstream' "$(run -i)"
+test_expect_success 'rebase -p fast-forwards if an ancestor of upstream' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' p k &&
+	test_range p.. "f j k"
+'
+}
+test_expect_success 'rebase ignores patch in upstream' "$(run)"
+test_expect_failure 'rebase -m ignores patch in upstream' "$(run -m)"
+test_expect_success 'rebase -i ignores patch in upstream' "$(run -i)"
+test_expect_success 'rebase -p ignores patch in upstream' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' c h &&
+	test_range c.. "f h"
+'
+}
+test_expect_success 'rebase ignores empty commit' "$(run)"
+test_expect_success 'rebase -m ignores empty commit' "$(run -m)"
+test_expect_success 'rebase -i ignores empty commit' "$(run -i)"
+test_expect_success 'rebase -p ignores empty commit' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' --keep-empty c h &&
+	test_range c.. "f g h"
+'
+}
+test_expect_success 'rebase --keep-empty' "$(run)"
+test_expect_failure 'rebase -m --keep-empty' "$(run -m)"
+test_expect_success 'rebase -i --keep-empty' "$(run -i)"
+test_expect_failure 'rebase -p --keep-empty' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' --keep-empty p h &&
+	test_range p.. "f g h"
+'
+}
+test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' "$(run)"
+test_expect_failure 'rebase -m --keep-empty keeps empty even if already in upstream' "$(run -m)"
+test_expect_failure 'rebase -i --keep-empty keeps empty even if already in upstream' "$(run -i)"
+test_expect_failure 'rebase -p --keep-empty keeps empty even if already in upstream' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' E w &&
+	test_range E.. "n H"
+'
+}
+test_expect_success 'rebase after merge' "$(run)"
+test_expect_success 'rebase -m after merge' "$(run -m)"
+test_expect_success 'rebase -i after merge' "$(run -i)"
+
+test_expect_success 'rebase -p is no-op in history with merges' '
+	reset &&
+	git rebase -p j w &&
+	same_revision HEAD w
+'
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' j w &&
+	test_range j.. "E n H" || test_range j.. "n H E"
+'
+}
+test_expect_success 'rebase of history with merges is linearized' "$(run)"
+test_expect_success 'rebase -m of history with merges is linearized' "$(run -m)"
+test_expect_success 'rebase -i of history with merges is linearized' "$(run -i)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' --onto p h k &&
+	test_range p.. "j k"
+'
+}
+test_expect_failure 'rebase --onto does not re-apply patches in onto' "$(run)"
+test_expect_failure 'rebase -m --onto does not re-apply patches in onto' "$(run -m)"
+test_expect_failure 'rebase -i --onto does not re-apply patches in onto' "$(run -i)"
+test_expect_failure 'rebase -p --onto does not re-apply patches in onto' "$(run -p)"
+
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' --onto f d r &&
+	test_range f.. "q C r"
+'
+}
+test_expect_failure 'rebase --onto does not lose patches in upstream' "$(run)"
+test_expect_success 'rebase -m --onto does not lose patches in upstream' "$(run -m)"
+test_expect_failure 'rebase -i --onto does not lose patches in upstream' "$(run -i)"
+test_expect_failure 'rebase -p --onto does not lose patches in upstream' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' --root c &&
+	! same_revision HEAD c &&
+	test_range c "a b c"
+'
+}
+test_expect_success 'rebase --root is not a no-op' "$(run)"
+test_expect_success 'rebase -m --root is not a no-op' "$(run -m)"
+test_expect_success 'rebase -i --root is not a no-op' "$(run -i)"
+test_expect_success 'rebase -p --root is not a no-op' "$(run -p)"
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' --root --onto e y &&
+	test_range e.. "x y"
+'
+}
+test_expect_success 'rebase --root --onto' "$(run)"
+test_expect_failure 'rebase -m --root --onto' "$(run -m)"
+test_expect_success 'rebase -i --root --onto' "$(run -i)"
+test_expect_success 'rebase -p --root --onto' "$(run -p)"
+
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' --root --onto e B &&
+	test_range e.. "x y"
+'
+}
+test_expect_success 'rebase --root --onto ignores patch in onto' "$(run)"
+test_expect_failure 'rebase -m --root --onto ignores patch in onto' "$(run -m)"
+test_expect_success 'rebase -i --root --onto ignores patch in onto' "$(run -i)"
+test_expect_success 'rebase -p --root --onto ignores patch in onto' "$(run -p)"
+
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' e y &&
+	test_range e.. "x y"
+'
+}
+test_expect_success 'rebase without --root works on disjoint history' "$(run)"
+test_expect_failure 'rebase -m without --root works on disjoint history' "$(run -m)"
+test_expect_success 'rebase -i without --root works on disjoint history' "$(run -i)"
+test_expect_failure 'rebase -p without --root works on disjoint history' "$(run -p)"
+
+
+run () {
+echo '
+	reset &&
+	git rebase '"$@"' --root --onto p k &&
+	test_range p.. "f j k"
+'
+}
+test_expect_success 'rebase --root --onto with merge-base ignores --root' "$(run)"
+test_expect_failure 'rebase -m --root --onto with merge-base ignores --root' "$(run -m)"
+test_expect_success 'rebase -i --root --onto with merge-base ignores --root' "$(run -i)"
+test_expect_success 'rebase -p --root --onto with merge-base ignores --root' "$(run -p)"
+
+test_expect_success 'rebase -p re-creates merge from upstream' '
+	reset &&
+	git rebase -p k w &&
+	same_revision HEAD^ H &&
+	same_revision HEAD^2 k
+'
+
+test_expect_success 'rebase -p re-creates internal merge' '
+	reset &&
+	git rebase -p c w &&
+	test_revisions "f j n E H w" HEAD~4 HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
+'
+
+test_expect_success 'rebase -p rebuilds history around dropped commit matching upstream' '
+	reset &&
+	git rebase -p h w &&
+	test_revisions "j E n w" HEAD~2 HEAD^2 HEAD^ HEAD
+'
+
+test_expect_success 'rebase -p drops merge commit when one entire side is dropped' '
+	reset &&
+	git rebase -p p w &&
+	test_range p.. "f j n H"
+'
+
+test_expect_failure 'rebase -p --onto drops commit in <onto>' '
+	reset &&
+	git rebase -p --onto p f w &&
+	test_range p.. "j n H"
+'
+
+test_expect_success 'rebase -p with two paths to $from' '
+	reset &&
+	git rebase -p --onto c j w &&
+	test_revisions "c n E H w" HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
+'
+
+test_done
-- 
1.7.11.1.104.ge7b44f1

Re: [RFC PATCH] add t3420-rebase-topology

[Junio C Hamano]

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> do you agree
> that 'rebase --onto does not re-apply patches in onto' is desirable?

This depends on how you look at --onto.  Recall the most typical and
the original use case of rebase:

                               A'--C' your rebased work
                              /
  ---o---o---o---F---B'--o---T master
     ^            \
     v1.7.12       A---B---C your work

You could view "git rebase master" as a short-hand for

        $ git rebase --onto $(git merge-base master HEAD) master

i.e.

        $ git rebase --onto F master

If you view "onto" that way, you would certainly do not want to see
B replayed on top of T, which already contains another replay of B
in its history.

The intended use case for "--onto", however, is primarily to replay
a history to older base, i.e.


       A'--B'--C' your rebased work
      /
  ---o---o---o---F---B'--o---T master
     ^            \
     v1.7.12       A---B---C your work


        $ git rebase --onto v1.7.12 master

which is a moral equivalent of

        $ git checkout v1.7.12
        $ git cherry-pick A B C ;# or git cherry-pick master..HEAD

You could argue that you can compute the patch equivalence between
the commits in "onto..master" and commits in "master..HEAD" and
filter out the equivalent commits to reach the expected results in
either cases, and at the logical level, it certainly will produce
the result I showed in the above illustrations.

But there are two flaws in such an argument.

The "replay to an updated base" case (i.e. without "--onto") expects
to see duplicated commits in the updated history.  That is the only
reason you would want to rebase---otherwise you can keep the old
fork point and keep polishing your work and merge when it is truly
ready.  More importantly, the old master (the fork point F) and
updated master (the current tip T) are expected to have some
ancestry relationship between them.  For these reasons, running the
equivalent of "git cherry" to filter commits with patch equivalence
makes perfect sense.

On the other hand, when the user replays to an older base, she has
some idea what constitutes "a series" that she is replaying (i.e.
"$(git merge-base master HEAD)..HEAD").  It smells to go against the
user's world model if the command silently filtered commits by patch
equivalence.  The user wanted to replay A, B and C in that case, and
it is better to stop the process and make her inspect the history
when the older base happened to have a patch-equivalent commit that
would cause a conflict. That would give her a chance to assess the
situation, instead of silently dropping some of these three commits
that she initially thought constitutes the series and ending up with
a series with only two commits.

Besides, the whole point of a separate "onto" is to allow the user
to specify a commit that does not have a straightforward ancestry
relationship with the bottom of the series (i.e. either "master" or
"F"), and computation of patch equivalence is expected to be much
higher.  Given that it is unlikely to find any match, it feels
doubly wrong to always run "git cherry" equivalent in that case.

So I dunno, as I can argue both ways.

My gut feeling is that "git rebase master" and "git rebase --onto F
master" should not be identical in this regard and an explicit
"onto" should omit the patch equivalence filtering, but I can be
persuaded otherwise with good arguments.

> How about 'rebase --root is not a no-op'?

  ---o---o---o---F---B'--o---T master
     ^            \
     v1.7.12       A---B---C your work

If "git rebase F" when you are at C in the above illustration
(reproduced only the relevant parts) is a no-op (and I think it
should be), "git rebase --root" in the illustration below ought to
be as well, no?

                 F---B'--o---T master
                  \
                   A---B---C your work

Re: [RFC PATCH] add t3420-rebase-topology

[Martin von Zweigbergk]

On Tue, Sep 18, 2012 at 12:51 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> do you agree
>> that 'rebase --onto does not re-apply patches in onto' is desirable?
>
> This depends on how you look at --onto.  Recall the most typical and
> the original use case of rebase:
>
>                                A'--C' your rebased work
>                               /
>   ---o---o---o---F---B'--o---T master
>      ^            \
>      v1.7.12       A---B---C your work
>
> You could view "git rebase master" as a short-hand for
>
>         $ git rebase --onto $(git merge-base master HEAD) master

Exactly. I frequently consider it a short-hand for that. It might
be worth pointing out that 'git pull --rebase', which might be
one of the most frequent uses of rebase, internally often does

  git rebase --onto upstream upstream@{...} branch

where upstream@{...} is the most recent upstream that is an
ancestor of "branch". For example, if your work is based on
origin/pu and you send the bottom-most patch ("B" in the figure
below) to the maintainer and and it gets applied to
pu. Running "git pull --rebase" would then lead to
"git rebase --onto T A". You would want this to drop B.

                               C' your rebased work
                              /
  ---o---o---o---F---B'--o---T origin/pu
                  \
                   A origin/pu@{1}
                    \
                     B---C your work


> The intended use case for "--onto", however, is primarily to replay
> a history to older base, i.e. [...] a moral equivalent of
>
>         $ git checkout v1.7.12
>         $ git cherry-pick A B C ;# or git cherry-pick master..HEAD

Yes, this is the alternative way of looking it at and exactly why
I, too, was not sure how it should behave.

> You could argue that you can compute the patch equivalence between
> the commits in "onto..master" and commits in "master..HEAD" and
> filter out the equivalent commits

I'm not sure if you meant "master..onto" rather
than "onto..master". Rebase (well, all flavors of rebase
but "-m") currently drops patches from "master..HEAD" that are
also in "HEAD..master". This is what the "rebase --onto does not
lose patches in upstream" test is about. It is also one of the
main problems that I try to fix in my long-stalled rebase-range
series. I think we should drop patches in "master..HEAD" that are
also in "HEAD..onto" (which is almost the same
as "master..onto").

> The "replay to an updated base" case (i.e. without "--onto")

Or _with_ --onto as in the above example from "git pull --rebase".

> On the other hand, when the user replays to an older base, she has
> some idea what constitutes "a series" that she is replaying (i.e.
> "$(git merge-base master HEAD)..HEAD").  It smells to go against the
> user's world model if the command silently filtered commits by patch
> equivalence.

If it's truly about rebasing onto an older base, there can't
possibly be any patches in "HEAD..onto", so assuming you agree
with my reasoning above that those are the patches we should
drop, rebasing onto older history would be safe.

> Besides, the whole point of a separate "onto" is to allow the user
> to specify a commit that does not have a straightforward ancestry
> relationship with the bottom of the series (i.e. either "master" or
> "F"), and computation of patch equivalence is expected to be much
> higher.  Given that it is unlikely to find any match, it feels
> doubly wrong to always run "git cherry" equivalent in that case.

Yes, this was my only concern (apart from it possibly being
conceptually wrong to do, depending on what the user meant by
issuing the command).

>> How about 'rebase --root is not a no-op'?
>
>   ---o---o---o---F---B'--o---T master
>      ^            \
>      v1.7.12       A---B---C your work
>
> If "git rebase F" when you are at C in the above illustration
> (reproduced only the relevant parts) is a no-op (and I think it
> should be), "git rebase --root" in the illustration below ought to
> be as well, no?
>
>                  F---B'--o---T master
>                   \
>                    A---B---C your work

Yeah, that's what I thought as well at first. I think my test
case even started out as "rebase --root _is_ a no-op".

When thinking about how to handle roots in general, I often
imagine a single virtual root commit (parent of all "initial"
commits), and that reasoning also implies that "git rebase
--root" should be a no-op. Then I saw that the test case
failed (or perhaps I remembered how it is implemented with the
clever fake root/initial commit) and started thinking about why
anyone would use "git rebase --root" if it was a no-op. I could
only think of using it to linearize history, but that doesn't
seem like a very likely use case. So it seems like weighing
purity/correctness against usefulness to me. I'm not sure which
way to go.

Thanks for quick and detailed feedback on an RFC patch.

Martin

Re: [RFC PATCH] add t3420-rebase-topology

[Johannes Sixt]

Am 9/18/2012 8:31, schrieb Martin von Zweigbergk:
> Add more test cases to check that the topology after a rebase is as
> expected. Conflicts are not considered, but patch-equivalence is.
> ---
> 
> Tests pass and fail as indicated by the suffix
> (_success/_failure). Your input especially appreciated on whether you
> agree with the intent of the test cases. For example, do you agree
> that 'rebase --onto does not re-apply patches in onto' is desirable?
> And if you do, then do you also agree that 'rebase --root --onto
> ignores patch in onto' is desirable? How about 'rebase --root is not a
> no-op'? One might think that --force would be necessary, but on the
> other hand, if that was the case, the only point (AFAICT) of "git
> rebase --root <branch>" without --force would be to linearize history,
> so I instead made the test case confirm that --root without --onto
> effectively behaves as if --force was also passed.
> 
> Feedback on the structure/setup and style is of course also
> appreciated.
> 
>  t/t3420-rebase-topology.sh | 348 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 348 insertions(+)
>  create mode 100755 t/t3420-rebase-topology.sh
> 
> diff --git a/t/t3420-rebase-topology.sh b/t/t3420-rebase-topology.sh
> new file mode 100755
> index 0000000..024a2b4
> --- /dev/null
> +++ b/t/t3420-rebase-topology.sh
> @@ -0,0 +1,348 @@
> +#!/bin/sh
> +
> +test_description='effect of rebase on topology'
> +. ./test-lib.sh
> +
> +
> +#       q---C---r
> +#      /
> +# a---b---c---d!--e---p
> +#      \
> +#       f---g!--h
> +#        \
> +#         j-------E---k
> +#          \       \
> +#           n---H---w
> +#
> +# x---y---B
> +#
> +#
> +# ! = empty
> +# uppercase = cherry-picked
> +# p = reverted e
> +#
> +# TODO:
> +# prune graph to what's needed
> +
> +empty () {

Is it "is_empty" or "make_empty"/"empty_commit"?

> +	git commit --allow-empty -m $1 &&
> +	git tag $1
> +}

Obviously the latter.

> +
> +cherry_pick () {
> +	git cherry-pick -n $1 &&
> +	git commit -m $2 &&
> +	git tag $2
> +}
> +
> +revert () {
> +	git revert -n $1 &&
> +	git commit -m $2 &&
> +	git tag $2
> +}
> +
> +
> +test_expect_success 'setup' '
> +	test_commit a &&
> +	test_commit b &&
> +	test_commit c &&
> +	empty d &&
> +	test_commit e &&
> +	revert e p &&
> +	git checkout b &&
> +	test_commit f &&
> +	empty g &&
> +	test_commit h &&
> +	git checkout f &&
> +	test_commit j &&
> +	cherry_pick e E &&
> +	test_commit k &&
> +	git checkout j &&
> +	test_commit n &&
> +	cherry_pick h H &&
> +	git merge -m w E &&
> +	git tag w &&
> +	git checkout b &&
> +	test_commit q &&
> +	cherry_pick c C &&
> +	test_commit r &&
> +	git checkout --orphan disjoint &&
> +	git rm -rf . &&
> +	test_commit x &&
> +	test_commit y &&
> +	cherry_pick b B
> +'
> +
> +reset () {
> +	git rebase --abort
> +	git reset --hard
> +}

The 'rebase --abort' can fail, but there is no && chain. Good. Using this
function instead of the individual commands in the tests does not break
the && chain there. Good.

These following three functions should be and can be consistent with the
order of their arguments: expected actual.

> +test_range () {
> +	test "$(git log --reverse --topo-order --format=%s "$1" | xargs)" = "$2"

	expected=$1
	set -- $(git log --reverse --topo-order --format=%s "$2")
	test "expected" = "$*"

> +}
> +
> +test_revisions () {
> +	expected="$1"
> +	shift
> +	test "$(git log --format=%s --no-walk=unsorted "$@" | xargs)" = "$expected"

	set -- $(git log --format=%s --no-walk=unsorted "$@")
	test "expected" = "$*"

> +}
> +
> +same_revision () {
> +	test "$(git rev-parse $1)" = "$(git rev-parse $2)"
> +}

'test_same_revision'?

> +
> +# the following 5 (?) tests copy t3400 tests, but check the history rather than status code and/or stdout
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' c j &&
> +	same_revision HEAD~2 c &&
> +	test_range c.. "f j"
> +'
> +}
> +test_expect_success 'simple rebase' "$(run)"
> +test_expect_success 'simple rebase -m' "$(run -m)"
> +test_expect_success 'simple rebase -i' "$(run -i)"
> +test_expect_success 'simple rebase -p' "$(run -p)"

Since here and in the following tests the test cases and test descriptions
vary in the same way, wouldn't it make sense to factor the description out
as well?

test_run_rebase () {
	test_expect_success "simple rebase $*" "
		reset &&
		git rebase $* c j &&
		same_revision HEAD~2 c &&
		test_range c.. 'f j'
	"
}

test_run_rebase ''
test_run_rebase -m
test_run_rebase -i
test_run_rebase -p

(Watch your quoting, though.)

Oh, I see: some tests expect failure. How about:

test_run_rebase () {
	result=$1
	shift
	test_expect_$result "simple rebase $*" "..."
}
test_run_rebase success ''
etc.

> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' b j &&
> +	same_revision HEAD j
> +'
> +}
> +test_expect_success 'rebase is no-op if upstream is an ancestor' "$(run)"
> +test_expect_success 'rebase -m is no-op if upstream is an ancestor' "$(run -m)"
> +test_expect_success 'rebase -i is no-op if upstream is an ancestor' "$(run -i)"
> +test_expect_success 'rebase -p is no-op if upstream is an ancestor' "$(run -p)"

OK.

> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' --force b j &&
> +	! same_revision HEAD j &&
> +	test_range b.. "f j"
> +'
> +}
> +test_expect_success 'rebase --force' "$(run)"
> +test_expect_success 'rebase -m --force' "$(run -m)"
> +test_expect_success 'rebase -i --force' "$(run -i)"
> +test_expect_failure 'rebase -p --force' "$(run -p)"

Do you mean "rebase --force rewrites even if upstream is an ancestor"?
That would make sense.

> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' j b &&
> +	same_revision HEAD j
> +'
> +}
> +test_expect_success 'rebase fast-forwards if an ancestor of upstream' "$(run)"
> +test_expect_success 'rebase -m fast-forwards if an ancestor of upstream' "$(run -m)"
> +test_expect_success 'rebase -i fast-forwards if an ancestor of upstream' "$(run -i)"
> +test_expect_success 'rebase -p fast-forwards if an ancestor of upstream' "$(run -p)"

OK.

> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' p k &&
> +	test_range p.. "f j k"
> +'
> +}
> +test_expect_success 'rebase ignores patch in upstream' "$(run)"
> +test_expect_failure 'rebase -m ignores patch in upstream' "$(run -m)"
> +test_expect_success 'rebase -i ignores patch in upstream' "$(run -i)"
> +test_expect_success 'rebase -p ignores patch in upstream' "$(run -p)"

"drops" instead of "ignores"? OK.

> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' c h &&
> +	test_range c.. "f h"
> +'
> +}
> +test_expect_success 'rebase ignores empty commit' "$(run)"
> +test_expect_success 'rebase -m ignores empty commit' "$(run -m)"
> +test_expect_success 'rebase -i ignores empty commit' "$(run -i)"
> +test_expect_success 'rebase -p ignores empty commit' "$(run -p)"

"drops" instead of "ignores"? OK.

> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' --keep-empty c h &&
> +	test_range c.. "f g h"
> +'
> +}
> +test_expect_success 'rebase --keep-empty' "$(run)"
> +test_expect_failure 'rebase -m --keep-empty' "$(run -m)"
> +test_expect_success 'rebase -i --keep-empty' "$(run -i)"
> +test_expect_failure 'rebase -p --keep-empty' "$(run -p)"

OK.

> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' --keep-empty p h &&
> +	test_range p.. "f g h"
> +'
> +}
> +test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' "$(run)"
> +test_expect_failure 'rebase -m --keep-empty keeps empty even if already in upstream' "$(run -m)"
> +test_expect_failure 'rebase -i --keep-empty keeps empty even if already in upstream' "$(run -i)"
> +test_expect_failure 'rebase -p --keep-empty keeps empty even if already in upstream' "$(run -p)"

"is in upstream" is decided by the patch text. If an empty commit is
already in upstream, this adds another one with the same or a different
commit message and authorship information. Dubious, but since it is
opt-in, it should be OK.

> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' E w &&
> +	test_range E.. "n H"
> +'
> +}
> +test_expect_success 'rebase after merge' "$(run)"
> +test_expect_success 'rebase -m after merge' "$(run -m)"
> +test_expect_success 'rebase -i after merge' "$(run -i)"

OK.

> +
> +test_expect_success 'rebase -p is no-op in history with merges' '
> +	reset &&
> +	git rebase -p j w &&
> +	same_revision HEAD w
> +'
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' j w &&
> +	test_range j.. "E n H" || test_range j.. "n H E"
> +'

Chaining tests with || is dangerous: you do not know whether the first
failed because the condition is not satisfied or because of some other
failure.

Why is this needed in the first place? Shouldn't the history be
deterministic, provided that the commit timestamps are all distinct?

> +}
> +test_expect_success 'rebase of history with merges is linearized' "$(run)"
> +test_expect_success 'rebase -m of history with merges is linearized' "$(run -m)"
> +test_expect_success 'rebase -i of history with merges is linearized' "$(run -i)"

OK.

> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' --onto p h k &&
> +	test_range p.. "j k"
> +'
> +}
> +test_expect_failure 'rebase --onto does not re-apply patches in onto' "$(run)"
> +test_expect_failure 'rebase -m --onto does not re-apply patches in onto' "$(run -m)"
> +test_expect_failure 'rebase -i --onto does not re-apply patches in onto' "$(run -i)"
> +test_expect_failure 'rebase -p --onto does not re-apply patches in onto' "$(run -p)"

Makes sense.

> +
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' --onto f d r &&
> +	test_range f.. "q C r"
> +'
> +}
> +test_expect_failure 'rebase --onto does not lose patches in upstream' "$(run)"
> +test_expect_success 'rebase -m --onto does not lose patches in upstream' "$(run -m)"
> +test_expect_failure 'rebase -i --onto does not lose patches in upstream' "$(run -i)"
> +test_expect_failure 'rebase -p --onto does not lose patches in upstream' "$(run -p)"

Makes sense.

> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' --root c &&
> +	! same_revision HEAD c &&
> +	test_range c "a b c"
> +'
> +}
> +test_expect_success 'rebase --root is not a no-op' "$(run)"
> +test_expect_success 'rebase -m --root is not a no-op' "$(run -m)"
> +test_expect_success 'rebase -i --root is not a no-op' "$(run -i)"
> +test_expect_success 'rebase -p --root is not a no-op' "$(run -p)"

Why? Is it more like "--root implies --force"?

> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' --root --onto e y &&
> +	test_range e.. "x y"
> +'
> +}
> +test_expect_success 'rebase --root --onto' "$(run)"
> +test_expect_failure 'rebase -m --root --onto' "$(run -m)"
> +test_expect_success 'rebase -i --root --onto' "$(run -i)"
> +test_expect_success 'rebase -p --root --onto' "$(run -p)"

Where does this rebase start? Ah, --root stands in for the "upstream"
argument, hence, y is the tip to rebase. Right? Then it makes sense.

> +
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' --root --onto e B &&
> +	test_range e.. "x y"
> +'
> +}
> +test_expect_success 'rebase --root --onto ignores patch in onto' "$(run)"
> +test_expect_failure 'rebase -m --root --onto ignores patch in onto' "$(run -m)"
> +test_expect_success 'rebase -i --root --onto ignores patch in onto' "$(run -i)"
> +test_expect_success 'rebase -p --root --onto ignores patch in onto' "$(run -p)"

Makes sense.

> +
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' e y &&
> +	test_range e.. "x y"
> +'
> +}
> +test_expect_success 'rebase without --root works on disjoint history' "$(run)"
> +test_expect_failure 'rebase -m without --root works on disjoint history' "$(run -m)"
> +test_expect_success 'rebase -i without --root works on disjoint history' "$(run -i)"
> +test_expect_failure 'rebase -p without --root works on disjoint history' "$(run -p)"

OK.

> +
> +
> +run () {
> +echo '
> +	reset &&
> +	git rebase '"$@"' --root --onto p k &&
> +	test_range p.. "f j k"
> +'
> +}
> +test_expect_success 'rebase --root --onto with merge-base ignores --root' "$(run)"
> +test_expect_failure 'rebase -m --root --onto with merge-base ignores --root' "$(run -m)"
> +test_expect_success 'rebase -i --root --onto with merge-base ignores --root' "$(run -i)"
> +test_expect_success 'rebase -p --root --onto with merge-base ignores --root' "$(run -p)"

Makes sense.

> +
> +test_expect_success 'rebase -p re-creates merge from upstream' '
> +	reset &&
> +	git rebase -p k w &&
> +	same_revision HEAD^ H &&
> +	same_revision HEAD^2 k
> +'

IMO, this tests the wrong thing. You have this history:

 ---j-------E---k
     \       \
      n---H---w

where E is the second parent of w. What does it mean to rebase w onto k?
IMO, it is a meaningless operation, and the outcome is irrelevant.

It would make sense to test that this history results after the upstream
at H moved forward:

 ---j-------E---k
     \       \
      n---H   \
           \   \
            z---w'

That is, w began a topic by mergeing the sidebranch E; then upstream
advanced to z, and now you rebase the topic to the new upstream.

> +
> +test_expect_success 'rebase -p re-creates internal merge' '
> +	reset &&
> +	git rebase -p c w &&
> +	test_revisions "f j n E H w" HEAD~4 HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD

You must also test for c; otherwise the test would succeed if rebase did
nothing at all.

This comment applies to all other tests as well, even the "regular" rebase
tests above. (But I noticed only when I read this test.)

> +'
> +
> +test_expect_success 'rebase -p rebuilds history around dropped commit matching upstream' '
> +	reset &&
> +	git rebase -p h w &&
> +	test_revisions "j E n w" HEAD~2 HEAD^2 HEAD^ HEAD
> +'
> +
> +test_expect_success 'rebase -p drops merge commit when one entire side is dropped' '
> +	reset &&
> +	git rebase -p p w &&
> +	test_range p.. "f j n H"
> +'
> +
> +test_expect_failure 'rebase -p --onto drops commit in <onto>' '
> +	reset &&
> +	git rebase -p --onto p f w &&
> +	test_range p.. "j n H"
> +'
> +
> +test_expect_success 'rebase -p with two paths to $from' '
> +	reset &&
> +	git rebase -p --onto c j w &&
> +	test_revisions "c n E H w" HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
> +'

I didn't look at these remaining -p tests because I ran out of time.


After this plethora of tests, can we get rid of some or many from other
test scripts? (t34* tests are the ones that take the longest on Windows to
run.)

A nice summary of the rebase behavior. I like it.

-- Hannes

Re: [RFC PATCH] add t3420-rebase-topology

[Martin von Zweigbergk]

[+Chris Webb regarding "git rebase --root"]

First of all, thanks for a meticulous review!

On Tue, Sep 18, 2012 at 12:53 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Am 9/18/2012 8:31, schrieb Martin von Zweigbergk:
>
> Since here and in the following tests the test cases and test descriptions
> vary in the same way, wouldn't it make sense to factor the description out
> as well?

Definitely. I just couldn't think of a good way of doing it, so thanks
for great and concrete suggestions!

> (Watch your quoting, though.)

Switched to putting the test body in double quotes as you suggested in
your examples and used single quotes for strings within the test body.

>> +run () {
>> +echo '
>> +     reset &&
>> +     git rebase '"$@"' --keep-empty p h &&
>> +     test_range p.. "f g h"
>> +'
>> +}
>> +test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' "$(run)"
>> +test_expect_failure 'rebase -m --keep-empty keeps empty even if already in upstream' "$(run -m)"
>> +test_expect_failure 'rebase -i --keep-empty keeps empty even if already in upstream' "$(run -i)"
>> +test_expect_failure 'rebase -p --keep-empty keeps empty even if already in upstream' "$(run -p)"
>
> "is in upstream" is decided by the patch text. If an empty commit is
> already in upstream, this adds another one with the same or a different
> commit message and authorship information. Dubious, but since it is
> opt-in, it should be OK.

Yes, it is a little dubious. See
http://thread.gmane.org/gmane.comp.version-control.git/203097/focus=203159
and Junio's answer, which I think makes sense.

>> +run () {
>> +echo '
>> +     reset &&
>> +     git rebase '"$@"' j w &&
>> +     test_range j.. "E n H" || test_range j.. "n H E"
>> +'
>
> Chaining tests with || is dangerous: you do not know whether the first
> failed because the condition is not satisfied or because of some other
> failure.

Good point. Thanks.

> Why is this needed in the first place? Shouldn't the history be
> deterministic, provided that the commit timestamps are all distinct?

It may be deterministic, but it's not specified, I think, so I didn't
want to depend on the order. Thinking more about it, though, I think
it's good to protect the current behavior from patches that change the
order of the parents. Although it may not be incorrect to change the
order, it would at least protect against accidental changes.

It turns out that "rebase -i" goes through the commits in
--topo-order, while the others use default order, I think. Which
flavor should pass the test case and which should fail (and be fixed)?
I would personally prefer to say that "rebase -i" is correct in using
--topo-order and that the others should be fixed. Again, it's not
specified, but I would hate to have them behave differently.

>> +run () {
>> +echo '
>> +     reset &&
>> +     git rebase '"$@"' --root c &&
>> +     ! same_revision HEAD c &&
>> +     test_range c "a b c"
>> +'
>> +}
>> +test_expect_success 'rebase --root is not a no-op' "$(run)"
>> +test_expect_success 'rebase -m --root is not a no-op' "$(run -m)"
>> +test_expect_success 'rebase -i --root is not a no-op' "$(run -i)"
>> +test_expect_success 'rebase -p --root is not a no-op' "$(run -p)"
>
> Why? Is it more like "--root implies --force"?

It doesn't currently exactly imply --force, but the effect is the
same. Also see my reply to Junio's email in this thread.

Maybe Chris has some thoughts on this?

>> +run () {
>> +echo '
>> +     reset &&
>> +     git rebase '"$@"' --root --onto e y &&
>> +     test_range e.. "x y"
>> +'
>> +}
>> +test_expect_success 'rebase --root --onto' "$(run)"
>> +test_expect_failure 'rebase -m --root --onto' "$(run -m)"
>> +test_expect_success 'rebase -i --root --onto' "$(run -i)"
>> +test_expect_success 'rebase -p --root --onto' "$(run -p)"
>
> Where does this rebase start? Ah, --root stands in for the "upstream"
> argument, hence, y is the tip to rebase. Right? Then it makes sense.

Thanks for pointing that out. I changed the order to "git rebase
--onto e --root y". I hope that makes it clearer.

>> +test_expect_success 'rebase -p re-creates merge from upstream' '
>> +     reset &&
>> +     git rebase -p k w &&
>> +     same_revision HEAD^ H &&
>> +     same_revision HEAD^2 k
>> +'
>
> IMO, this tests the wrong thing. You have this history:
>
>  ---j-------E---k
>      \       \
>       n---H---w
>
> where E is the second parent of w. What does it mean to rebase w onto k?
> IMO, it is a meaningless operation, and the outcome is irrelevant.
>
> It would make sense to test that this history results after the upstream
> at H moved forward:
>
>  ---j-------E---k
>      \       \
>       n---H   \
>            \   \
>             z---w'
>
> That is, w began a topic by mergeing the sidebranch E; then upstream
> advanced to z, and now you rebase the topic to the new upstream.

Fair enough. Changed accordingly.

>> +test_expect_success 'rebase -p re-creates internal merge' '
>> +     reset &&
>> +     git rebase -p c w &&
>> +     test_revisions "f j n E H w" HEAD~4 HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
>
> You must also test for c; otherwise the test would succeed if rebase did
> nothing at all.
>
> This comment applies to all other tests as well, even the "regular" rebase
> tests above. (But I noticed only when I read this test.)

I did this only in one or two places thinking that that would be
enough to make sure that rebase is not normally a no-op. But I think
you are right that we should check it most of the time. It turns out
that doing this caught a case where the rebase did do something and
the right patches were in "c.." (or whatever it was; I forgot which
test case), but the new base was not "c".

> After this plethora of tests, can we get rid of some or many from other
> test scripts? (t34* tests are the ones that take the longest on Windows to
> run.)

I was afraid that this file would be the slowest of all and it might
very well be :-(. But, yes, it does replace a few test cases. I will
send out an updated version of the patch later. That version should
delete a few existing test cases as well.

I am having trouble finding enough time to get the patch into shape,
but I didn't want to put off this reply for any longer.

Martin

Re: [RFC PATCH] add t3420-rebase-topology

[Chris Webb]

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> On Tue, Sep 18, 2012 at 12:53 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>
> > Why? Is it more like "--root implies --force"?
> 
> It doesn't currently exactly imply --force, but the effect is the
> same. Also see my reply to Junio's email in this thread.
> 
> Maybe Chris has some thoughts on this?

Hi Martin and Johannes. Sorry for the slow follow-up here.

You're right that rebase --root without --onto always creates a brand new
root as a result of the implementation using a sentinel commit. Clearly this
is what's wanted with --interactive, but rebase --root with neither --onto
nor --interactive is a slightly odd combination for which I struggle to
imagine a natural use. Perhaps you're right that for consistency it should
be a no-op unless --force-rebase is given?

If we did this, this combination would be a no-op unconditionally as by
definition we're always descended from the root of our current commit.
However, given the not-very-useful behaviour, I suspect that rebase --root
is much more likely to be a mistyped version of rebase -i --root than rebase
--root --force-rebase. (Unless I'm missing a reasonable use for this?
History linearisation perhaps?)

Best wishes,

Chris.

Re: [RFC PATCH] add t3420-rebase-topology

[Martin von Zweigbergk]

On Thu, Sep 27, 2012 at 5:20 AM, Chris Webb <chris@arachsys.com> wrote:
> You're right that rebase --root without --onto always creates a brand new
> root as a result of the implementation using a sentinel commit. Clearly this
> is what's wanted with --interactive

That's not as clear as one might first think. "git rebase -i" actually
honors --force; if one marks a commit for "edit", but then --continue
without making any changes, the next commit(s) will be fast-forwarded.
See the first few lines of pick_one (or pick_one_preserving_merges).

> but rebase --root with neither --onto
> nor --interactive is a slightly odd combination for which I struggle to
> imagine a natural use. Perhaps you're right that for consistency it should
> be a no-op unless --force-rebase is given?
>
> If we did this, this combination would be a no-op unconditionally as by
> definition we're always descended from the root of our current commit.

For consistency, it seems like "git rebase -p --root" should always be
a no-op, while "git rebase [-i/-m] --root" should be no-op if the
history has no merges. Also, since "git rebase -i" tries to
fast-forward through existing commits, it seems like "git rebase -i
--root" should ideally not create a sentinel commit, but instead stop
at the first commit marked for editing.

If, OTOH, --force-rebase is given, we should rewrite history from the
first commit, which in the case of --root would mean creating a
sentinel commit.

So, in short, I have a feeling that the sentinel commit should be
created if and only if both --root and --force-rebase (but not --onto)
are given.

> However, given the not-very-useful behaviour, I suspect that rebase --root
> is much more likely to be a mistyped version of rebase -i --root than rebase
> --root --force-rebase. (Unless I'm missing a reasonable use for this?
> History linearisation perhaps?)

Yes, the "not-very-useful"-ness of this is the clear argument against
making them no-ops. But I have to say I was slightly surprised when I
tried "git rebase --root" for the first time and it created completely
new history for me. As you say, "git rebase --root" is probably often
a mistyped "git rebase -i --root", and if that is the case, it seems
nicer (in addition to being more consistent) if we don't do anything
rather than rewriting the history. The history rewriting might even go
unnoticed and come as an unpleasant surprise later.

When working on a new project, "git rebase -i --root" might even be a
convenient replacement for "git rebase -i <initial commit>" even when
one does not want to rewrite the initial commit itself, and in such a
case, the user would clearly not want a sentinel commit either.

So I'm getting more and more convinced that the sentinel commit should
only be created if --force-rebase was given. Let me know if I'm
missing something.

Martin

Re: [RFC PATCH] add t3420-rebase-topology

[Chris Webb]

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> For consistency, it seems like "git rebase -p --root" should always be
> a no-op, while "git rebase [-i/-m] --root" should be no-op if the
> history has no merges. Also, since "git rebase -i" tries to
> fast-forward through existing commits, it seems like "git rebase -i
> --root" should ideally not create a sentinel commit, but instead stop
> at the first commit marked for editing.
> 
> If, OTOH, --force-rebase is given, we should rewrite history from the
> first commit, which in the case of --root would mean creating a
> sentinel commit.
> 
> So, in short, I have a feeling that the sentinel commit should be
> created if and only if both --root and --force-rebase (but not --onto)
> are given.
[...]
> So I'm getting more and more convinced that the sentinel commit should
> only be created if --force-rebase was given. Let me know if I'm
> missing something.

No, that sounds fairly convincing to me. Personally, the only behaviour I
want to be able to get at without --force-rebase is for rebase -i --root to
allow the root commit to be dropped, edited or reworded, and commits to
reordered to make a different one the root, in the same way as normal
interactive rebase does for later commits.

The least intrusive implementation for rebase --root was the unconditional
sentinel, but as you say, you don't need it (and it's a bit surprising) if
the root commit on the instruction sheet is the same as the original root:
in the edit/reword case, you could just checkout the existing root and then
drop out in the normal way. You only really need the sentinel to deal with
the possibility of a conflict if the root is replaced with a different
commit.

I think I agree with you it would be better only to create the sentinel on
demand when it's required or if --force is given.

Cheers,

Chris.

[PATCH v2 0/7] Rebase topology test

[Martin von Zweigbergk]

After way too long, here is finally a new version of the tests I sent
at: http://thread.gmane.org/gmane.comp.version-control.git/205796.

I have split the test up into two files. They stil take quite some
time to run.

Martin von Zweigbergk (7):
  add simple tests of consistency across rebase types
  add tests for rebasing with patch-equivalence present
  add tests for rebasing of empty commits
  add tests for rebasing root
  add tests for rebasing merged history
  t3406: modernize style
  tests: move test for rebase messages from t3400 to t3406

 t/lib-rebase.sh                   |  32 ++++
 t/t3400-rebase.sh                 |  53 +-----
 t/t3401-rebase-partial.sh         |  69 --------
 t/t3404-rebase-interactive.sh     |  10 +-
 t/t3406-rebase-message.sh         |  50 +++---
 t/t3409-rebase-preserve-merges.sh |  53 ------
 t/t3420-rebase-topology-linear.sh | 350 ++++++++++++++++++++++++++++++++++++++
 t/t3425-rebase-topology-merges.sh | 250 +++++++++++++++++++++++++++
 8 files changed, 664 insertions(+), 203 deletions(-)
 delete mode 100755 t/t3401-rebase-partial.sh
 create mode 100755 t/t3420-rebase-topology-linear.sh
 create mode 100755 t/t3425-rebase-topology-merges.sh

-- 
1.8.2.674.gd17d3d2

[PATCH v2 1/7] add simple tests of consistency across rebase types

[Martin von Zweigbergk]

Helped-by: Johannes Sixt <j6t@kdbg.org>
---
 t/lib-rebase.sh                   | 15 ++++++++
 t/t3420-rebase-topology-linear.sh | 78 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)
 create mode 100755 t/t3420-rebase-topology-linear.sh

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6ccf797..62b3887 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -65,3 +65,18 @@ EOF
 	test_set_editor "$(pwd)/fake-editor.sh"
 	chmod a+x fake-editor.sh
 }
+
+# checks that the revisions in "$2" represent a linear range with the
+# subjects in "$1"
+test_linear_range () {
+	! { git log --format=%p "$2" | sane_grep " " ;} &&
+	expected=$1
+	set -- $(git log --reverse --format=%s "$2")
+	test "$expected" = "$*"
+}
+
+reset_rebase () {
+	git rebase --abort # may fail; ignore exit code
+	git reset --hard &&
+	git clean -f
+}
diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh
new file mode 100755
index 0000000..c4b32db
--- /dev/null
+++ b/t/t3420-rebase-topology-linear.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='basic rebase topology tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+# a---b---c
+#      \
+#       d---e
+test_expect_success 'setup' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git checkout b &&
+	test_commit d &&
+	test_commit e
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "simple rebase $*" "
+		reset_rebase &&
+		git rebase $* c e &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'd e' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* is no-op if upstream is an ancestor" "
+		reset_rebase &&
+		git rebase $* b e &&
+		test_cmp_rev e HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* -f rewrites even if upstream is an ancestor" "
+		reset_rebase &&
+		git rebase $* -f b e &&
+		! test_cmp_rev e HEAD &&
+		test_cmp_rev b HEAD~2 &&
+		test_linear_range 'd e' b..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* fast-forwards if an ancestor of upstream" "
+		reset_rebase &&
+		git rebase $* e b &&
+		test_cmp_rev e HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_done
-- 
1.8.2.674.gd17d3d2

Re: [PATCH v2 1/7] add simple tests of consistency across rebase types

[Martin von Zweigbergk]

On Tue, May 28, 2013 at 11:39 PM, Martin von Zweigbergk
<martinvonz@gmail.com> wrote:
>  create mode 100755 t/t3420-rebase-topology-linear.sh

Just FYI, there's another test case with the same number
(t3420-rebase-autostash) in pu. I don't know how you normally handle
such cases.

Re: [PATCH v2 1/7] add simple tests of consistency across rebase types

[Junio C Hamano]

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> On Tue, May 28, 2013 at 11:39 PM, Martin von Zweigbergk
> <martinvonz@gmail.com> wrote:
>>  create mode 100755 t/t3420-rebase-topology-linear.sh
>
> Just FYI, there's another test case with the same number
> (t3420-rebase-autostash) in pu. I don't know how you normally handle
> such cases.

Thanks for a heads-up.  Usually, the series that appears later on
the list yields and finds a unique number.

Re: [PATCH v2 1/7] add simple tests of consistency across rebase types

[Martin von Zweigbergk]

On Mon, Jun 3, 2013 at 11:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> On Tue, May 28, 2013 at 11:39 PM, Martin von Zweigbergk
>> <martinvonz@gmail.com> wrote:
>>>  create mode 100755 t/t3420-rebase-topology-linear.sh
>>
>> Just FYI, there's another test case with the same number
>> (t3420-rebase-autostash) in pu. I don't know how you normally handle
>> such cases.
>
> Thanks for a heads-up.  Usually, the series that appears later on
> the list yields and finds a unique number.

In this case, that's my series. Want a resend or do you want to do it
yourself? I'm fine either way, whatever is easiest for you.

[PATCH v2 2/7] add tests for rebasing with patch-equivalence present

[Martin von Zweigbergk]

---
 t/lib-rebase.sh                   | 17 ++++++++
 t/t3420-rebase-topology-linear.sh | 85 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+)

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 62b3887..16eeb1c 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -80,3 +80,20 @@ reset_rebase () {
 	git reset --hard &&
 	git clean -f
 }
+
+cherry_pick () {
+	git cherry-pick -n "$2" &&
+	git commit -m "$1" &&
+	git tag "$1"
+}
+
+revert () {
+	git revert -n "$2" &&
+	git commit -m "$1" &&
+	git tag "$1"
+}
+
+make_empty () {
+	git commit --allow-empty -m "$1" &&
+	git tag "$1"
+}
diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh
index c4b32db..1152491 100755
--- a/t/t3420-rebase-topology-linear.sh
+++ b/t/t3420-rebase-topology-linear.sh
@@ -75,4 +75,89 @@ test_run_rebase success -m
 test_run_rebase success -i
 test_run_rebase success -p
 
+#       f
+#      /
+# a---b---c---g---h
+#      \
+#       d---G---i
+#
+# uppercase = cherry-picked
+# h = reverted g
+#
+# Reverted patches are there for tests to be able to check if a commit
+# that introduced the same change as another commit is
+# dropped. Without reverted commits, we could get false positives
+# because applying the patch succeeds, but simply results in no
+# changes.
+test_expect_success 'setup of linear history for range selection tests' '
+	git checkout c &&
+	test_commit g &&
+	revert h g &&
+	git checkout d &&
+	cherry_pick G g &&
+	test_commit i &&
+	git checkout b &&
+	test_commit f
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* drops patches in upstream" "
+		reset_rebase &&
+		git rebase $* h i &&
+		test_cmp_rev h HEAD~2 &&
+		test_linear_range 'd i' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* can drop last patch if in upstream" "
+		reset_rebase &&
+		git rebase $* h G &&
+		test_cmp_rev h HEAD^ &&
+		test_linear_range 'd' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto does not lose patches in upstream" "
+		reset_rebase &&
+		git rebase $* --onto f h i &&
+		test_cmp_rev f HEAD~3 &&
+		test_linear_range 'd G i' f..
+	"
+}
+test_run_rebase failure ''
+test_run_rebase success -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto drops patches in onto" "
+		reset_rebase &&
+		git rebase $* --onto h f i &&
+		test_cmp_rev h HEAD~2 &&
+		test_linear_range 'd i' h..
+	"
+}
+test_run_rebase failure ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
 test_done
-- 
1.8.2.674.gd17d3d2

Re: [PATCH v2 2/7] add tests for rebasing with patch-equivalence present

[Johannes Sixt]

Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
> +#       f
> +#      /
> +# a---b---c---g---h
> +#      \
> +#       d---G---i
...
> +test_run_rebase () {
> +	result=$1
> +	shift
> +	test_expect_$result "rebase $* --onto drops patches in onto" "
> +		reset_rebase &&
> +		git rebase $* --onto h f i &&
> +		test_cmp_rev h HEAD~2 &&
> +		test_linear_range 'd i' h..

Isn't this expectation wrong? The upstream of the rebased branch is f, and
it does not contain G. Hence, G should be replayed. Since h is the
reversal of g, the state at h is the same as at c, and applying G should
succeed (it is the same change as g). Therefore, I think the correct
expectation is:

		test_linear_range 'd G i' h..

> +	"
> +}
> +test_run_rebase failure ''
> +test_run_rebase failure -m
> +test_run_rebase failure -i
> +test_run_rebase failure -p

-- Hannes

Re: [PATCH v2 2/7] add tests for rebasing with patch-equivalence present

[Martin von Zweigbergk]

On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
>> +#       f
>> +#      /
>> +# a---b---c---g---h
>> +#      \
>> +#       d---G---i
> ...
>> +test_run_rebase () {
>> +     result=$1
>> +     shift
>> +     test_expect_$result "rebase $* --onto drops patches in onto" "
>> +             reset_rebase &&
>> +             git rebase $* --onto h f i &&
>> +             test_cmp_rev h HEAD~2 &&
>> +             test_linear_range 'd i' h..
>
> Isn't this expectation wrong? The upstream of the rebased branch is f, and
> it does not contain G. Hence, G should be replayed. Since h is the
> reversal of g, the state at h is the same as at c, and applying G should
> succeed (it is the same change as g). Therefore, I think the correct
> expectation is:
>
>                 test_linear_range 'd G i' h..

Good question! It is really not obvious what the right answer is. Some
arguments in favor of dropping 'G':

1. Let's say origin/master points to 'b' when you start the 'd G i'
branch. You then send the 'G' patch to Junio who applies it as 'g'
(cherry-picking direction is reversed compared to figure, but same
effect). You then "git pull --rebase" when master on origin points to
'h'. Because of the cleverness in 'git pull --rebase', it issues 'git
rebase --onto h b i'. In this case it's clearly useful to have the
patch dropped.

2. In the test a little before the above one, we instead do 'git
rebase --onto f h i' and make sure that the 'G' is _not_ lost. In that
case it doesn't matter what's in $branch..$upstream. Do we agree that
$branch..$upstream should never matter (instead, $upstream is only
used to find merge base with $branch)? Do we also agree that 'git
rebase a b' should be identical to 'git rebase --onto a a b'? Because
'git rebase h i' should clearly drop 'G', then so should 'git rebase
--onto h h i'. Then, if we agreed that $branch..$upstream doesn't
matter, 'git rebase --onto h f i' should behave the same, no?


The set of commits to rebase that I was thinking of using was
"$upstream..$branch, unless equivalent with patch in $branch..$onto".
But I'm not very confident about my conclusions above :-)


Martin

Re: [PATCH v2 2/7] add tests for rebasing with patch-equivalence present

[Felipe Contreras]

On Thu, May 30, 2013 at 12:30 AM, Martin von Zweigbergk
<martinvonz@gmail.com> wrote:
> On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
>>> +#       f
>>> +#      /
>>> +# a---b---c---g---h
>>> +#      \
>>> +#       d---G---i
>> ...
>>> +test_run_rebase () {
>>> +     result=$1
>>> +     shift
>>> +     test_expect_$result "rebase $* --onto drops patches in onto" "
>>> +             reset_rebase &&
>>> +             git rebase $* --onto h f i &&
>>> +             test_cmp_rev h HEAD~2 &&
>>> +             test_linear_range 'd i' h..
>>
>> Isn't this expectation wrong? The upstream of the rebased branch is f, and
>> it does not contain G. Hence, G should be replayed. Since h is the
>> reversal of g, the state at h is the same as at c, and applying G should
>> succeed (it is the same change as g). Therefore, I think the correct
>> expectation is:
>>
>>                 test_linear_range 'd G i' h..
>
> Good question! It is really not obvious what the right answer is. Some
> arguments in favor of dropping 'G':

I think the answer is obvious; G should not be dropped. Maybe it made
sense to drop g in upstream, but d fixes an issue, and it makes sense
to apply G on upstream.

Git should not make any assumptions, if the user wants G to be
dropped, he can easily do that by himself.

I don't think it's easy to implement that code-wise, but I think the
ideal behavior is clear.

-- 
Felipe Contreras

Re: [PATCH v2 2/7] add tests for rebasing with patch-equivalence present

[Martin von Zweigbergk]

On Wed, May 29, 2013 at 10:41 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Thu, May 30, 2013 at 12:30 AM, Martin von Zweigbergk
> <martinvonz@gmail.com> wrote:
>> On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
>>>> +#       f
>>>> +#      /
>>>> +# a---b---c---g---h
>>>> +#      \
>>>> +#       d---G---i
>>> ...
>>>> +test_run_rebase () {
>>>> +     result=$1
>>>> +     shift
>>>> +     test_expect_$result "rebase $* --onto drops patches in onto" "
>>>> +             reset_rebase &&
>>>> +             git rebase $* --onto h f i &&
>>>> +             test_cmp_rev h HEAD~2 &&
>>>> +             test_linear_range 'd i' h..
>>>
>>> Isn't this expectation wrong? The upstream of the rebased branch is f, and
>>> it does not contain G. Hence, G should be replayed. Since h is the
>>> reversal of g, the state at h is the same as at c, and applying G should
>>> succeed (it is the same change as g). Therefore, I think the correct
>>> expectation is:
>>>
>>>                 test_linear_range 'd G i' h..
>>
>> Good question! It is really not obvious what the right answer is. Some
>> arguments in favor of dropping 'G':
>
> I think the answer is obvious; G should not be dropped. Maybe it made
> sense to drop g in upstream, but d fixes an issue, and it makes sense
> to apply G on upstream.

Well, maybe I was wrong in thinking that dropping 'G' in 'git rebase
--onto f h i' is bad. It seems to complicate things a lot, so maybe we
should just decide that it's fine to do that (to drop 'G' in that
case). Since that's mostly how it has worked forever and no one seems
to have reported a problem with it, I'm probably just being paranoid.
Thoughts?

Re: [PATCH v2 2/7] add tests for rebasing with patch-equivalence present

[Felipe Contreras]

On Thu, May 30, 2013 at 1:14 AM, Martin von Zweigbergk
<martinvonz@gmail.com> wrote:
> On Wed, May 29, 2013 at 10:41 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Thu, May 30, 2013 at 12:30 AM, Martin von Zweigbergk
>> <martinvonz@gmail.com> wrote:
>>> On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>>> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
>>>>> +#       f
>>>>> +#      /
>>>>> +# a---b---c---g---h
>>>>> +#      \
>>>>> +#       d---G---i
>>>> ...
>>>>> +test_run_rebase () {
>>>>> +     result=$1
>>>>> +     shift
>>>>> +     test_expect_$result "rebase $* --onto drops patches in onto" "
>>>>> +             reset_rebase &&
>>>>> +             git rebase $* --onto h f i &&
>>>>> +             test_cmp_rev h HEAD~2 &&
>>>>> +             test_linear_range 'd i' h..
>>>>
>>>> Isn't this expectation wrong? The upstream of the rebased branch is f, and
>>>> it does not contain G. Hence, G should be replayed. Since h is the
>>>> reversal of g, the state at h is the same as at c, and applying G should
>>>> succeed (it is the same change as g). Therefore, I think the correct
>>>> expectation is:
>>>>
>>>>                 test_linear_range 'd G i' h..
>>>
>>> Good question! It is really not obvious what the right answer is. Some
>>> arguments in favor of dropping 'G':
>>
>> I think the answer is obvious; G should not be dropped. Maybe it made
>> sense to drop g in upstream, but d fixes an issue, and it makes sense
>> to apply G on upstream.
>
> Well, maybe I was wrong in thinking that dropping 'G' in 'git rebase
> --onto f h i' is bad. It seems to complicate things a lot, so maybe we
> should just decide that it's fine to do that (to drop 'G' in that
> case). Since that's mostly how it has worked forever and no one seems
> to have reported a problem with it, I'm probably just being paranoid.
> Thoughts?

Huh? I said the opposite; G should *not* be dropped.

-- 
Felipe Contreras

Re: [PATCH v2 2/7] add tests for rebasing with patch-equivalence present

[Martin von Zweigbergk]

On Wed, May 29, 2013 at 11:40 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Thu, May 30, 2013 at 1:14 AM, Martin von Zweigbergk
> <martinvonz@gmail.com> wrote:
>> On Wed, May 29, 2013 at 10:41 PM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> On Thu, May 30, 2013 at 12:30 AM, Martin von Zweigbergk
>>> <martinvonz@gmail.com> wrote:
>>>> On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>>>> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
>>>>>> +#       f
>>>>>> +#      /
>>>>>> +# a---b---c---g---h
>>>>>> +#      \
>>>>>> +#       d---G---i
>>>>> ...
>>>>>> +test_run_rebase () {
>>>>>> +     result=$1
>>>>>> +     shift
>>>>>> +     test_expect_$result "rebase $* --onto drops patches in onto" "
>>>>>> +             reset_rebase &&
>>>>>> +             git rebase $* --onto h f i &&
>>>>>> +             test_cmp_rev h HEAD~2 &&
>>>>>> +             test_linear_range 'd i' h..
>>>>>
>>>>> Isn't this expectation wrong? The upstream of the rebased branch is f, and
>>>>> it does not contain G. Hence, G should be replayed. Since h is the
>>>>> reversal of g, the state at h is the same as at c, and applying G should
>>>>> succeed (it is the same change as g). Therefore, I think the correct
>>>>> expectation is:
>>>>>
>>>>>                 test_linear_range 'd G i' h..
>>>>
>>>> Good question! It is really not obvious what the right answer is. Some
>>>> arguments in favor of dropping 'G':
>>>
>>> I think the answer is obvious; G should not be dropped. Maybe it made
>>> sense to drop g in upstream, but d fixes an issue, and it makes sense
>>> to apply G on upstream.
>>
>> Well, maybe I was wrong in thinking that dropping 'G' in 'git rebase
>> --onto f h i' is bad. It seems to complicate things a lot, so maybe we
>> should just decide that it's fine to do that (to drop 'G' in that
>> case). Since that's mostly how it has worked forever and no one seems
>> to have reported a problem with it, I'm probably just being paranoid.
>> Thoughts?
>
> Huh? I said the opposite; G should *not* be dropped.

I suspect you missed that I said 'git rebase --onto f h i', not 'git
rebase --onto h f i'. Sorry, I should have pointed that out.

Re: [PATCH v2 2/7] add tests for rebasing with patch-equivalence present

[Johannes Sixt]

Am 30.05.2013 07:30, schrieb Martin von Zweigbergk:
> On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
>>> +#       f
>>> +#      /
>>> +# a---b---c---g---h
>>> +#      \
>>> +#       d---G---i
>> ...
>>> +test_run_rebase () {
>>> +     result=$1
>>> +     shift
>>> +     test_expect_$result "rebase $* --onto drops patches in onto" "
>>> +             reset_rebase &&
>>> +             git rebase $* --onto h f i &&
>>> +             test_cmp_rev h HEAD~2 &&
>>> +             test_linear_range 'd i' h..
>>
>> Isn't this expectation wrong? The upstream of the rebased branch is f, and
>> it does not contain G. Hence, G should be replayed. Since h is the
>> reversal of g, the state at h is the same as at c, and applying G should
>> succeed (it is the same change as g). Therefore, I think the correct
>> expectation is:
>>
>>                 test_linear_range 'd G i' h..
> 
> Good question! It is really not obvious what the right answer is. Some
> arguments in favor of dropping 'G':
> 
> 1. Let's say origin/master points to 'b' when you start the 'd G i'
> branch. You then send the 'G' patch to Junio who applies it as 'g'
> (cherry-picking direction is reversed compared to figure, but same
> effect). You then "git pull --rebase" when master on origin points to
> 'h'. Because of the cleverness in 'git pull --rebase', it issues 'git
> rebase --onto h b i'.

The reason for this git pull cleverness is to be prepared for rewritten
history:

   b'--c'--g'--h'
  /
 a---b
      \
       d---G---i

to avoid that b is rebased.

> In this case it's clearly useful to have the
> patch dropped.
> 
> 2. In the test a little before the above one, we instead do 'git
> rebase --onto f h i' and make sure that the 'G' is _not_ lost. In that
> case it doesn't matter what's in $branch..$upstream. Do we agree that
> $branch..$upstream should never matter (instead, $upstream is only
> used to find merge base with $branch)?

No, we do not agree. $branch..$upstream should be the set of patches
that should be omitted. $branch..$onto should not matter. $onto is only
used to specify the destination of the rebased commits.

> Do we also agree that 'git
> rebase a b' should be identical to 'git rebase --onto a a b'?

Absolutely!

> Because
> 'git rebase h i' should clearly drop 'G', then so should 'git rebase
> --onto h h i'.

Yes!

> Then, if we agreed that $branch..$upstream doesn't
> matter, 'git rebase --onto h f i' should behave the same, no?

Correct in the mathematically logical sense. ;) But we do not agree that
$branch..$upstream doesn't matter.

> The set of commits to rebase that I was thinking of using was
> "$upstream..$branch, unless equivalent with patch in $branch..$onto".
> But I'm not very confident about my conclusions above :-)

At least the man page says that ..$upstream counts and $onto tells just
the new base.

The way how git pull calls rebase should be revisited, I think.

-- Hannes

Re: [PATCH v2 2/7] add tests for rebasing with patch-equivalence present

[Martin von Zweigbergk]

On Thu, May 30, 2013 at 5:54 AM, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 30.05.2013 07:30, schrieb Martin von Zweigbergk:
>> On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
>>>> +#       f
>>>> +#      /
>>>> +# a---b---c---g---h
>>>> +#      \
>>>> +#       d---G---i
>>> ...
>>>> +test_run_rebase () {
>>>> +     result=$1
>>>> +     shift
>>>> +     test_expect_$result "rebase $* --onto drops patches in onto" "
>>>> +             reset_rebase &&
>>>> +             git rebase $* --onto h f i &&
>>>> +             test_cmp_rev h HEAD~2 &&
>>>> +             test_linear_range 'd i' h..
>>>
>>> Isn't this expectation wrong? The upstream of the rebased branch is f, and
>>> it does not contain G. Hence, G should be replayed. Since h is the
>>> reversal of g, the state at h is the same as at c, and applying G should
>>> succeed (it is the same change as g). Therefore, I think the correct
>>> expectation is:
>>>
>>>                 test_linear_range 'd G i' h..
>>
>> Good question! It is really not obvious what the right answer is. Some
>> arguments in favor of dropping 'G':
>>
>> 1. Let's say origin/master points to 'b' when you start the 'd G i'
>> branch. You then send the 'G' patch to Junio who applies it as 'g'
>> (cherry-picking direction is reversed compared to figure, but same
>> effect). You then "git pull --rebase" when master on origin points to
>> 'h'. Because of the cleverness in 'git pull --rebase', it issues 'git
>> rebase --onto h b i'.
>
> The reason for this git pull cleverness is to be prepared for rewritten
> history:
>
>    b'--c'--g'--h'
>   /
>  a---b
>       \
>        d---G---i
>
> to avoid that b is rebased.

Right. It doesn't currently drop 'G' and maybe it shouldn't, so let's
leave it as is for now, I would say.

>> 2. In the test a little before the above one, we instead do 'git
>> rebase --onto f h i' and make sure that the 'G' is _not_ lost. In that
>> case it doesn't matter what's in $branch..$upstream. Do we agree that
>> $branch..$upstream should never matter (instead, $upstream is only
>> used to find merge base with $branch)?
>
> No, we do not agree. $branch..$upstream should be the set of patches
> that should be omitted. $branch..$onto should not matter. $onto is only
> used to specify the destination of the rebased commits.

Ok. As I said to Felipe, I'm not sure why I was so convinced that it's
bad to lose the patch in 'git rebase --onto f h i'. It can result in
lost work, but it seems rare enough that no one has reported it,
AFAIK.

I'll change those tests in a re-roll, and perhaps I'll drop a few of
them. Let me know if you (anyone) disagree.


Martin

PS. Thanks for a meticulous review!

[PATCH v2 3/7] add tests for rebasing of empty commits

[Martin von Zweigbergk]

---
 t/t3401-rebase-partial.sh         | 24 ----------------
 t/t3420-rebase-topology-linear.sh | 58 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 24 deletions(-)

diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
index 58f4823..7ba1797 100755
--- a/t/t3401-rebase-partial.sh
+++ b/t/t3401-rebase-partial.sh
@@ -42,28 +42,4 @@ test_expect_success 'rebase --merge topic branch that was partially merged upstr
 	test_path_is_missing .git/rebase-merge
 '
 
-test_expect_success 'rebase ignores empty commit' '
-	git reset --hard A &&
-	git commit --allow-empty -m empty &&
-	test_commit D &&
-	git rebase C &&
-	test "$(git log --format=%s C..)" = "D"
-'
-
-test_expect_success 'rebase --keep-empty' '
-	git reset --hard D &&
-	git rebase --keep-empty C &&
-	test "$(git log --format=%s C..)" = "D
-empty"
-'
-
-test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' '
-	git reset --hard A &&
-	git commit --allow-empty -m also-empty &&
-	git rebase --keep-empty D &&
-	test "$(git log --format=%s A..)" = "also-empty
-D
-empty"
-'
-
 test_done
diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh
index 1152491..40fe264 100755
--- a/t/t3420-rebase-topology-linear.sh
+++ b/t/t3420-rebase-topology-linear.sh
@@ -160,4 +160,62 @@ test_run_rebase failure -m
 test_run_rebase failure -i
 test_run_rebase failure -p
 
+# a---b---c---j!
+#      \
+#       d---k!--l
+#
+# ! = empty
+test_expect_success 'setup of linear history for empty commit tests' '
+	git checkout c &&
+	make_empty j &&
+	git checkout d &&
+	make_empty k &&
+	test_commit l
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* drops empty commit" "
+		reset_rebase &&
+		git rebase $* c l &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'd l' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --keep-empty" "
+		reset_rebase &&
+		git rebase $* --keep-empty c l &&
+		test_cmp_rev c HEAD~3 &&
+		test_linear_range 'd k l' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --keep-empty keeps empty even if already in upstream" "
+		reset_rebase &&
+		git rebase $* --keep-empty j l &&
+		test_cmp_rev j HEAD~3 &&
+		test_linear_range 'd k l' j..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
 test_done
-- 
1.8.2.674.gd17d3d2

[PATCH v2 4/7] add tests for rebasing root

[Martin von Zweigbergk]

---
 t/t3420-rebase-topology-linear.sh | 129 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 129 insertions(+)

diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh
index 40fe264..2429aa8 100755
--- a/t/t3420-rebase-topology-linear.sh
+++ b/t/t3420-rebase-topology-linear.sh
@@ -218,4 +218,133 @@ test_run_rebase failure -m
 test_run_rebase failure -i
 test_run_rebase failure -p
 
+#       m
+#      /
+# a---b---c---g
+#
+# x---y---B
+#
+# uppercase = cherry-picked
+# m = reverted b
+#
+# Reverted patches are there for tests to be able to check if a commit
+# that introduced the same change as another commit is
+# dropped. Without reverted commits, we could get false positives
+# because applying the patch succeeds, but simply results in no
+# changes.
+test_expect_success 'setup of linear history for test involving root' '
+	git checkout b &&
+	revert m b &&
+	git checkout --orphan disjoint &&
+	git rm -rf . &&
+	test_commit x &&
+	test_commit y &&
+	cherry_pick B b
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root" "
+		reset_rebase &&
+		git rebase $* --onto c --root y &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'x y' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* without --onto --root with disjoint history" "
+		reset_rebase &&
+		git rebase $* c y &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'x y' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root drops patch in onto" "
+		reset_rebase &&
+		git rebase $* --onto m --root B &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'x y' m..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root with merge-base does not go to root" "
+		reset_rebase &&
+		git rebase $* --onto m --root g &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'c g' m..
+	"
+}
+
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* without --onto --root with disjoint history drops patch in onto" "
+		reset_rebase &&
+		git rebase $* m B &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'x y' m..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --root on linear history is a no-op" "
+		reset_rebase &&
+		git rebase $* --root c &&
+		test_cmp_rev c HEAD
+	"
+}
+test_run_rebase failure ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* -f --root on linear history causes re-write" "
+		reset_rebase &&
+		git rebase $* -f --root c &&
+		! test_cmp_rev a HEAD~2 &&
+		test_linear_range 'a b c' HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
 test_done
-- 
1.8.2.674.gd17d3d2

Re: [PATCH v2 4/7] add tests for rebasing root

[Johannes Sixt]

Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
> +test_run_rebase () {
> +	result=$1
> +	shift
> +	test_expect_$result "rebase $* --onto --root with merge-base does not go to root" "
> +		reset_rebase &&
> +		git rebase $* --onto m --root g &&
> +		test_cmp_rev m HEAD~2 &&
> +		test_linear_range 'c g' m..

Here you check the outcome. There is no explicit check whether the rebase
attempted to replay a and b. But that check is implicit: If a or b were
attempted to replay, the rebase would have been interrupted with "no new
changes". Right?

> +	"
> +}
> +
> +test_run_rebase success ''
> +test_run_rebase success -m
> +test_run_rebase success -i
> +test_run_rebase failure -p

Just curious: Does the last one fail because the result is not correct or
because it does go to the root?

-- Hannes

Re: [PATCH v2 4/7] add tests for rebasing root

[Martin von Zweigbergk]

On Wed, May 29, 2013 at 12:31 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
>> +test_run_rebase () {
>> +     result=$1
>> +     shift
>> +     test_expect_$result "rebase $* --onto --root with merge-base does not go to root" "
>> +             reset_rebase &&
>> +             git rebase $* --onto m --root g &&
>> +             test_cmp_rev m HEAD~2 &&
>> +             test_linear_range 'c g' m..
>
> Here you check the outcome. There is no explicit check whether the rebase
> attempted to replay a and b. But that check is implicit: If a or b were
> attempted to replay, the rebase would have been interrupted with "no new
> changes". Right?

Because 'm' is a reverted 'b', I think if it had gone to the root, we
would have seen 'b m c g' (I _think_ 'a' would be silently skipped at
least in am mode).

>> +test_run_rebase failure -p
>
> Just curious: Does the last one fail because the result is not correct or
> because it does go to the root?

Because the result is not correct; it first checks out 'm', but
something goes wrong (maybe because 'm' gets written to
/rewritten/root?) and it somehow fast-forwards to 'c' (from 'b'?).

[PATCH v2 5/7] add tests for rebasing merged history

[Martin von Zweigbergk]

---
 t/t3400-rebase.sh                 |  31 +----
 t/t3401-rebase-partial.sh         |  45 -------
 t/t3404-rebase-interactive.sh     |  10 +-
 t/t3409-rebase-preserve-merges.sh |  53 --------
 t/t3425-rebase-topology-merges.sh | 250 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 252 insertions(+), 137 deletions(-)
 delete mode 100755 t/t3401-rebase-partial.sh
 create mode 100755 t/t3425-rebase-topology-merges.sh

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b58fa1a..b436ef4 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -40,13 +40,6 @@ test_expect_success 'prepare repository with topic branches' '
 	echo Side >>C &&
 	git add C &&
 	git commit -m "Add C" &&
-	git checkout -b nonlinear my-topic-branch &&
-	echo Edit >>B &&
-	git add B &&
-	git commit -m "Modify B" &&
-	git merge side &&
-	git checkout -b upstream-merged-nonlinear &&
-	git merge master &&
 	git checkout -f my-topic-branch &&
 	git tag topic
 '
@@ -106,31 +99,9 @@ test_expect_success 'rebase from ambiguous branch name' '
 	git rebase master
 '
 
-test_expect_success 'rebase after merge master' '
-	git checkout --detach refs/tags/topic &&
-	git branch -D topic &&
-	git reset --hard topic &&
-	git merge master &&
-	git rebase master &&
-	! (git show | grep "^Merge:")
-'
-
-test_expect_success 'rebase of history with merges is linearized' '
-	git checkout nonlinear &&
-	test 4 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
-test_expect_success 'rebase of history with merges after upstream merge is linearized' '
-	git checkout upstream-merged-nonlinear &&
-	test 5 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
 test_expect_success 'rebase a single mode change' '
 	git checkout master &&
+	git branch -D topic &&
 	echo 1 >X &&
 	git add X &&
 	test_tick &&
diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
deleted file mode 100755
index 7ba1797..0000000
--- a/t/t3401-rebase-partial.sh
+++ /dev/null
@@ -1,45 +0,0 @@
-#!/bin/sh
-#
-# Copyright (c) 2006 Yann Dirson, based on t3400 by Amos Waterland
-#
-
-test_description='git rebase should detect patches integrated upstream
-
-This test cherry-picks one local change of two into master branch, and
-checks that git rebase succeeds with only the second patch in the
-local branch.
-'
-. ./test-lib.sh
-
-test_expect_success 'prepare repository with topic branch' '
-	test_commit A &&
-	git checkout -b my-topic-branch &&
-	test_commit B &&
-	test_commit C &&
-	git checkout -f master &&
-	test_commit A2 A.t
-'
-
-test_expect_success 'pick top patch from topic branch into master' '
-	git cherry-pick C &&
-	git checkout -f my-topic-branch
-'
-
-test_debug '
-	git cherry master &&
-	git format-patch -k --stdout --full-index master >/dev/null &&
-	gitk --all & sleep 1
-'
-
-test_expect_success 'rebase topic branch against new master and check git am did not get halted' '
-	git rebase master &&
-	test_path_is_missing .git/rebase-apply
-'
-
-test_expect_success 'rebase --merge topic branch that was partially merged upstream' '
-	git reset --hard C &&
-	git rebase --merge master &&
-	test_path_is_missing .git/rebase-merge
-'
-
-test_done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index a58406d..ffcaf02 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -477,19 +477,11 @@ test_expect_success 'interrupted squash works as expected (case 2)' '
 	test $one = $(git rev-parse HEAD~2)
 '
 
-test_expect_success 'ignore patch if in upstream' '
-	HEAD=$(git rev-parse HEAD) &&
-	git checkout -b has-cherry-picked HEAD^ &&
+test_expect_success '--continue tries to commit, even for "edit"' '
 	echo unrelated > file7 &&
 	git add file7 &&
 	test_tick &&
 	git commit -m "unrelated change" &&
-	git cherry-pick $HEAD &&
-	EXPECT_COUNT=1 git rebase -i $HEAD &&
-	test $HEAD = $(git rev-parse HEAD^)
-'
-
-test_expect_success '--continue tries to commit, even for "edit"' '
 	parent=$(git rev-parse HEAD^) &&
 	test_tick &&
 	FAKE_LINES="edit 1" git rebase -i HEAD^ &&
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index 6de4e22..2e0c364 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -11,14 +11,6 @@ Run "git rebase -p" and check that merges are properly carried along
 GIT_AUTHOR_EMAIL=bogus_email_address
 export GIT_AUTHOR_EMAIL
 
-# Clone 1 (trivial merge):
-#
-# A1--A2  <-- origin/master
-#  \   \
-#   B1--M  <-- topic
-#    \
-#     B2  <-- origin/topic
-#
 # Clone 2 (conflicting merge):
 #
 # A1--A2--B3   <-- origin/master
@@ -36,16 +28,6 @@ export GIT_AUTHOR_EMAIL
 #     \--A3    <-- topic2
 #      \
 #       B2     <-- origin/topic
-#
-# Clone 4 (merge using second parent as base):
-#
-# A1--A2--B3   <-- origin/master
-#  \
-#   B1--A3--M  <-- topic
-#    \     /
-#     \--A4    <-- topic2
-#      \
-#       B2     <-- origin/topic
 
 test_expect_success 'setup for merge-preserving rebase' \
 	'echo First > A &&
@@ -58,20 +40,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git checkout -f master &&
 	echo Third >> A &&
 	git commit -a -m "Modify A2" &&
-
-	git clone ./. clone1 &&
-	(cd clone1 &&
-	git checkout -b topic origin/topic &&
-	git merge origin/master
-	) &&
-
-	git clone ./. clone4 &&
-	(
-		cd clone4 &&
-		git checkout -b topic origin/topic &&
-		git merge origin/master
-	) &&
-
 	echo Fifth > B &&
 	git add B &&
 	git commit -m "Add different B" &&
@@ -101,16 +69,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git commit -a -m "Modify B2"
 '
 
-test_expect_success 'rebase -p fakes interactive rebase' '
-	(
-	cd clone1 &&
-	git fetch &&
-	git rebase -p origin/topic &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_expect_success '--continue works after a conflict' '
 	(
 	cd clone2 &&
@@ -138,15 +96,4 @@ test_expect_success 'rebase -p preserves no-ff merges' '
 	)
 '
 
-test_expect_success 'rebase -p works when base inside second parent' '
-	(
-	cd clone4 &&
-	git fetch &&
-	git rebase -p HEAD^2 &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify B" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_done
diff --git a/t/t3425-rebase-topology-merges.sh b/t/t3425-rebase-topology-merges.sh
new file mode 100755
index 0000000..0f56407
--- /dev/null
+++ b/t/t3425-rebase-topology-merges.sh
@@ -0,0 +1,250 @@
+#!/bin/sh
+
+test_description='rebase topology tests with merges'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+test_revision_subjects () {
+	expected="$1"
+	shift
+	set -- $(git log --format=%s --no-walk=unsorted "$@")
+	test "$expected" = "$*"
+}
+
+# a---b-----------c
+#      \           \
+#       d-------e   \
+#        \       \   \
+#         n---o---w---v
+#              \
+#               z
+#
+# uppercase = cherry-picked
+test_expect_success 'setup of non-linear-history' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git checkout b &&
+	test_commit d &&
+	test_commit e
+
+	git checkout c &&
+	test_commit g &&
+	revert h g &&
+	git checkout d &&
+	cherry_pick G g &&
+	test_commit i &&
+	git checkout b &&
+	test_commit f
+
+	git checkout d &&
+	test_commit n &&
+	test_commit o &&
+	test_merge w e &&
+	test_merge v c &&
+	git checkout o &&
+	test_commit z
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* after merge from upstream" "
+		reset_rebase &&
+		git rebase $* e w &&
+		test_cmp_rev e HEAD~2 &&
+		test_linear_range 'n o' e..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized in place" "
+		reset_rebase &&
+		git rebase $* d w &&
+		test_cmp_rev d HEAD~3 &&
+		test_linear_range "\'"$expected"\'" d..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'e n o' ''
+test_run_rebase success 'e n o' -m
+test_run_rebase success 'n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized upstream" "
+		reset_rebase &&
+		git rebase $* c w &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history with merges after upstream merge is linearized" "
+		reset_rebase &&
+		git rebase $* c v &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_expect_success "rebase -p is no-op in non-linear history" "
+	reset_rebase &&
+	git rebase -p d w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p is no-op when base inside second parent" "
+	reset_rebase &&
+	git rebase -p e w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_failure "rebase -p --root on non-linear history is a no-op" "
+	reset_rebase &&
+	git rebase -p --root w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p re-creates merge from side branch" "
+	reset_rebase &&
+	git rebase -p z w &&
+	test_cmp_rev z HEAD^ &&
+	test_cmp_rev w^2 HEAD^2
+"
+
+test_expect_success "rebase -p re-creates internal merge" "
+	reset_rebase &&
+	git rebase -p c w &&
+	test_revision_subjects 'c d n e o w' HEAD~4 HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p can re-create two branches on onto" "
+	reset_rebase &&
+	git rebase -p --onto c d w &&
+	test_revision_subjects 'c n e o w' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+#       f
+#      /
+# a---b---c---g---h
+#      \
+#       d---G---i
+#        \       \
+#         e-------u
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for patch-equivalence tests' '
+	git checkout e &&
+	test_merge u i
+'
+
+test_expect_success "rebase -p re-creates history around dropped commit matching upstream" "
+	reset_rebase &&
+	git rebase -p h u &&
+	test_cmp_rev h HEAD~3 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_failure "rebase -p --onto in merged history does not lose patches in upstream" "
+	reset_rebase &&
+	git rebase -p --onto f h u &&
+	test_cmp_rev f HEAD~3 &&
+	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p --onto in merged history drops patches in onto" "
+	reset_rebase &&
+	git rebase -p --onto h f u &&
+	test_cmp_rev h HEAD~3 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+# a---b---c---g---h
+#      \
+#       d---G---s
+#        \   \ /
+#         \   X
+#          \ / \
+#           e---t
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for dropping whole side' '
+	git checkout G &&
+	test_merge s e &&
+	git checkout e &&
+	test_merge t G
+'
+
+test_expect_failure "rebase -p drops merge commit when entire first-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h s &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+test_expect_success "rebase -p drops merge commit when entire second-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h t &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+# a---b---c
+#      \
+#       d---e
+#        \   \
+#         n---r
+#          \
+#           o
+#
+# r = tree-same with n
+# uppercace = cherry-picked
+test_expect_success 'setup of non-linear-history for empty commits' '
+	git checkout n &&
+	git merge --no-commit e &&
+	git reset n . &&
+	git commit -m r &&
+	git reset --hard &&
+	git clean -f &&
+	git tag r
+'
+
+test_expect_success "rebase -p re-creates empty internal merge commit" "
+	reset_rebase &&
+	git rebase -p c r &&
+	test_revision_subjects 'c d e n r' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p re-creates empty merge commit" "
+	reset_rebase &&
+	git rebase -p o r &&
+	test_revision_subjects 'e o r' HEAD^2 HEAD^ HEAD
+"
+
+test_done
-- 
1.8.2.674.gd17d3d2

Re: [PATCH v2 5/7] add tests for rebasing merged history

[Johannes Sixt]

Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
> +# a---b-----------c
> +#      \           \
> +#       d-------e   \
> +#        \       \   \
> +#         n---o---w---v
> +#              \
> +#               z

> +#TODO: make all flavors of rebase use --topo-order
> +test_run_rebase success 'e n o' ''
> +test_run_rebase success 'e n o' -m
> +test_run_rebase success 'n o e' -i

As test_commit offers predictable timestamps, I think you can work around
this discrepancy by generating commits n and o before e. (That is not a
solution--just a workaround that depends on the current
implementation--because the order in which parents of a merge are listed
is unspecified.)

> +test_expect_success "rebase -p re-creates internal merge" "
> +	reset_rebase &&
> +	git rebase -p c w &&
> +	test_revision_subjects 'c d n e o w' HEAD~4 HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD

Shouldn't this better be

	test_cmp_rev c HEAD~4 &&
	test_revision_subjects 'd n e o w' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD

to ensure that c is not a rewritten commit?

> +"
> +
> +test_expect_success "rebase -p can re-create two branches on onto" "
> +	reset_rebase &&
> +	git rebase -p --onto c d w &&
> +	test_revision_subjects 'c n e o w' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
> +"

Same here.

Time is fleeting. I have to stop here.

-- Hannes

Re: [PATCH v2 5/7] add tests for rebasing merged history

[Martin von Zweigbergk]

On Wed, May 29, 2013 at 12:57 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
>> +# a---b-----------c
>> +#      \           \
>> +#       d-------e   \
>> +#        \       \   \
>> +#         n---o---w---v
>> +#              \
>> +#               z
>
>> +#TODO: make all flavors of rebase use --topo-order
>> +test_run_rebase success 'e n o' ''
>> +test_run_rebase success 'e n o' -m
>> +test_run_rebase success 'n o e' -i
>
> As test_commit offers predictable timestamps, I think you can work around
> this discrepancy by generating commits n and o before e. (That is not a
> solution--just a workaround that depends on the current
> implementation--because the order in which parents of a merge are listed
> is unspecified.)

I actually liked it as documentation of the current inconsistency and
with an explicit TODO.

I have addressed the remainder of your comments in this and the next
message. Thanks again.

Re: [PATCH v2 5/7] add tests for rebasing merged history

[Johannes Sixt]

Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
> +#       f
> +#      /
> +# a---b---c---g---h
> +#      \
> +#       d---G---i
> +#        \       \
> +#         e-------u
> +#
> +# uppercase = cherry-picked
> +# h = reverted g

> +test_expect_failure "rebase -p --onto in merged history does not lose patches in upstream" "
> +	reset_rebase &&
> +	git rebase -p --onto f h u &&
> +	test_cmp_rev f HEAD~3 &&
> +	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
> +"

My expectations are different: When a patch is in upstream, then it is not
to be rebased, even --onto somewhere else than upstream.

But take this with a grain of salt, as I never encounter(ed) this use-case
in practice.

> +test_expect_success "rebase -p --onto in merged history drops patches in onto" "
> +	reset_rebase &&
> +	git rebase -p --onto h f u &&
> +	test_cmp_rev h HEAD~3 &&
> +	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
> +"

And this is just the opposite case, where I think the patch should be kept.

> +# a---b---c
> +#      \
> +#       d---e
> +#        \   \
> +#         n---r
> +#          \
> +#           o
> +#
> +# r = tree-same with n
> +# uppercace = cherry-picked

I do not see any upper-cased letters in this graph. ;)

> +test_expect_success "rebase -p re-creates empty internal merge commit" "
> +	reset_rebase &&
> +	git rebase -p c r &&
> +	test_revision_subjects 'c d e n r' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD

Again, check c with test_cmp_rev.

-- Hannes

[PATCH v2 6/7] t3406: modernize style

[Martin von Zweigbergk]

Update the following:

 - Quote 'setup'
 - Remove blank lines within test case body
 - Use test_commit instead of custom quick_one
 - Create branch "topic" from tag created by test_commit
---
 t/t3406-rebase-message.sh | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index e6a9a0d..fe8c27f 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -4,27 +4,17 @@ test_description='messages from rebase operation'
 
 . ./test-lib.sh
 
-quick_one () {
-	echo "$1" >"file$1" &&
-	git add "file$1" &&
-	test_tick &&
-	git commit -m "$1"
-}
+test_expect_success 'setup' '
+	test_commit O fileO &&
+	test_commit X fileX &&
+	test_commit A fileA &&
+	test_commit B fileB &&
+	test_commit Y fileY &&
 
-test_expect_success setup '
-	quick_one O &&
-	git branch topic &&
-	quick_one X &&
-	quick_one A &&
-	quick_one B &&
-	quick_one Y &&
-
-	git checkout topic &&
-	quick_one A &&
-	quick_one B &&
-	quick_one Z &&
+	git checkout -b topic O &&
+	git cherry-pick A B &&
+	test_commit Z fileZ &&
 	git tag start
-
 '
 
 cat >expect <<\EOF
@@ -34,12 +24,10 @@ Committed: 0003 Z
 EOF
 
 test_expect_success 'rebase -m' '
-
 	git rebase -m master >report &&
 	sed -n -e "/^Already applied: /p" \
 		-e "/^Committed: /p" report >actual &&
 	test_cmp expect actual
-
 '
 
 test_expect_success 'rebase --stat' '
-- 
1.8.2.674.gd17d3d2

[PATCH v2 7/7] tests: move test for rebase messages from t3400 to t3406

[Martin von Zweigbergk]

t3406 is supposed to test "messages from rebase operation", so let's
move tests in t3400 that fit that description into 3406. Most of the
functionality they tested, except for the messages, has now been
subsumed by t3420.
---
 t/t3400-rebase.sh         | 22 ----------------------
 t/t3406-rebase-message.sh | 22 ++++++++++++++++++++++
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b436ef4..45a55e9 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -59,28 +59,6 @@ test_expect_success 'rebase against master' '
 	git rebase master
 '
 
-test_expect_success 'rebase against master twice' '
-	git rebase master >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date" out
-'
-
-test_expect_success 'rebase against master twice with --force' '
-	git rebase --force-rebase master >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date, rebase forced" out
-'
-
-test_expect_success 'rebase against master twice from another branch' '
-	git checkout my-topic-branch^ &&
-	git rebase master my-topic-branch >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date" out
-'
-
-test_expect_success 'rebase fast-forward to master' '
-	git checkout my-topic-branch^ &&
-	git rebase my-topic-branch >out &&
-	test_i18ngrep "Fast-forwarded HEAD to my-topic-branch" out
-'
-
 test_expect_success 'the rebase operation should not have destroyed author information' '
 	! (git log | grep "Author:" | grep "<>")
 '
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index fe8c27f..0392e36 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -30,6 +30,28 @@ test_expect_success 'rebase -m' '
 	test_cmp expect actual
 '
 
+test_expect_success 'rebase against master twice' '
+	git rebase master >out &&
+	test_i18ngrep "Current branch topic is up to date" out
+'
+
+test_expect_success 'rebase against master twice with --force' '
+	git rebase --force-rebase master >out &&
+	test_i18ngrep "Current branch topic is up to date, rebase forced" out
+'
+
+test_expect_success 'rebase against master twice from another branch' '
+	git checkout topic^ &&
+	git rebase master topic >out &&
+	test_i18ngrep "Current branch topic is up to date" out
+'
+
+test_expect_success 'rebase fast-forward to master' '
+	git checkout topic^ &&
+	git rebase topic >out &&
+	test_i18ngrep "Fast-forwarded HEAD to topic" out
+'
+
 test_expect_success 'rebase --stat' '
 	git reset --hard start &&
         git rebase --stat master >diffstat.txt &&
-- 
1.8.2.674.gd17d3d2

Re: [PATCH v2 0/7] Rebase topology test

[Felipe Contreras]

On Wed, May 29, 2013 at 1:39 AM, Martin von Zweigbergk
<martinvonz@gmail.com> wrote:
> After way too long, here is finally a new version of the tests I sent
> at: http://thread.gmane.org/gmane.comp.version-control.git/205796.
>
> I have split the test up into two files. They stil take quite some
> time to run.

This is definitely needed. I'm doing some experiments with 'git
rebase' and I see so many problems in the code that these tests do
exercise.

I think a lot of the functionality of 'git rebase' should move to 'git
cherry-pick', and then all the 'git rebase' code can be simplified
greatly, and tests like these would help a lot.

-- 
Felipe Contreras

Re: [PATCH v2 0/7] Rebase topology test

[Ramkumar Ramachandra]

Felipe Contreras wrote:
> I think a lot of the functionality of 'git rebase' should move to 'git
> cherry-pick', and then all the 'git rebase' code can be simplified
> greatly, and tests like these would help a lot.

What do we do about the leakages?  Want to take on the task of fixing
the merge-recursive machinery?

Cf. $gmane/222887

Re: [PATCH v2 0/7] Rebase topology test

[Felipe Contreras]

On Wed, May 29, 2013 at 7:50 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> Felipe Contreras wrote:
>> I think a lot of the functionality of 'git rebase' should move to 'git
>> cherry-pick', and then all the 'git rebase' code can be simplified
>> greatly, and tests like these would help a lot.
>
> What do we do about the leakages?  Want to take on the task of fixing
> the merge-recursive machinery?
>
> Cf. $gmane/222887

Hmm, I saw that, but I also saw the fix.

Anyway, if this is such a big issue, I'm sure it should be possible to
trigger it in our test frameework.

-- 
Felipe Contreras

[PATCH v3 0/7] Rebase topology test

[Martin von Zweigbergk]

Patches are now expected to be dropped iff they are on upstream. I've
also followed all of Johannes's other suggestions except for the one
about topo-order.

Martin von Zweigbergk (7):
  add simple tests of consistency across rebase types
  add tests for rebasing with patch-equivalence present
  add tests for rebasing of empty commits
  add tests for rebasing root
  add tests for rebasing merged history
  t3406: modernize style
  tests: move test for rebase messages from t3400 to t3406

 t/lib-rebase.sh                   |  32 ++++
 t/t3400-rebase.sh                 |  53 +-----
 t/t3401-rebase-partial.sh         |  69 --------
 t/t3404-rebase-interactive.sh     |  10 +-
 t/t3406-rebase-message.sh         |  50 +++---
 t/t3409-rebase-preserve-merges.sh |  53 ------
 t/t3420-rebase-topology-linear.sh | 350 ++++++++++++++++++++++++++++++++++++++
 t/t3425-rebase-topology-merges.sh | 252 +++++++++++++++++++++++++++
 8 files changed, 666 insertions(+), 203 deletions(-)
 delete mode 100755 t/t3401-rebase-partial.sh
 create mode 100755 t/t3420-rebase-topology-linear.sh
 create mode 100755 t/t3425-rebase-topology-merges.sh

-- 
1.8.2.674.gd17d3d2

[PATCH v3 1/7] add simple tests of consistency across rebase types

[Martin von Zweigbergk]

Helped-by: Johannes Sixt <j6t@kdbg.org>
---
 t/lib-rebase.sh                   | 15 ++++++++
 t/t3420-rebase-topology-linear.sh | 78 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)
 create mode 100755 t/t3420-rebase-topology-linear.sh

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6ccf797..62b3887 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -65,3 +65,18 @@ EOF
 	test_set_editor "$(pwd)/fake-editor.sh"
 	chmod a+x fake-editor.sh
 }
+
+# checks that the revisions in "$2" represent a linear range with the
+# subjects in "$1"
+test_linear_range () {
+	! { git log --format=%p "$2" | sane_grep " " ;} &&
+	expected=$1
+	set -- $(git log --reverse --format=%s "$2")
+	test "$expected" = "$*"
+}
+
+reset_rebase () {
+	git rebase --abort # may fail; ignore exit code
+	git reset --hard &&
+	git clean -f
+}
diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh
new file mode 100755
index 0000000..c4b32db
--- /dev/null
+++ b/t/t3420-rebase-topology-linear.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='basic rebase topology tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+# a---b---c
+#      \
+#       d---e
+test_expect_success 'setup' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git checkout b &&
+	test_commit d &&
+	test_commit e
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "simple rebase $*" "
+		reset_rebase &&
+		git rebase $* c e &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'd e' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* is no-op if upstream is an ancestor" "
+		reset_rebase &&
+		git rebase $* b e &&
+		test_cmp_rev e HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* -f rewrites even if upstream is an ancestor" "
+		reset_rebase &&
+		git rebase $* -f b e &&
+		! test_cmp_rev e HEAD &&
+		test_cmp_rev b HEAD~2 &&
+		test_linear_range 'd e' b..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* fast-forwards if an ancestor of upstream" "
+		reset_rebase &&
+		git rebase $* e b &&
+		test_cmp_rev e HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_done
-- 
1.8.2.674.gd17d3d2

[PATCH v3 2/7] add tests for rebasing with patch-equivalence present

[Martin von Zweigbergk]

---
 t/lib-rebase.sh                   | 17 ++++++++
 t/t3420-rebase-topology-linear.sh | 85 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+)

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 62b3887..16eeb1c 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -80,3 +80,20 @@ reset_rebase () {
 	git reset --hard &&
 	git clean -f
 }
+
+cherry_pick () {
+	git cherry-pick -n "$2" &&
+	git commit -m "$1" &&
+	git tag "$1"
+}
+
+revert () {
+	git revert -n "$2" &&
+	git commit -m "$1" &&
+	git tag "$1"
+}
+
+make_empty () {
+	git commit --allow-empty -m "$1" &&
+	git tag "$1"
+}
diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh
index c4b32db..75cc476 100755
--- a/t/t3420-rebase-topology-linear.sh
+++ b/t/t3420-rebase-topology-linear.sh
@@ -75,4 +75,89 @@ test_run_rebase success -m
 test_run_rebase success -i
 test_run_rebase success -p
 
+#       f
+#      /
+# a---b---c---g---h
+#      \
+#       d---G---i
+#
+# uppercase = cherry-picked
+# h = reverted g
+#
+# Reverted patches are there for tests to be able to check if a commit
+# that introduced the same change as another commit is
+# dropped. Without reverted commits, we could get false positives
+# because applying the patch succeeds, but simply results in no
+# changes.
+test_expect_success 'setup of linear history for range selection tests' '
+	git checkout c &&
+	test_commit g &&
+	revert h g &&
+	git checkout d &&
+	cherry_pick G g &&
+	test_commit i &&
+	git checkout b &&
+	test_commit f
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* drops patches in upstream" "
+		reset_rebase &&
+		git rebase $* h i &&
+		test_cmp_rev h HEAD~2 &&
+		test_linear_range 'd i' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* can drop last patch if in upstream" "
+		reset_rebase &&
+		git rebase $* h G &&
+		test_cmp_rev h HEAD^ &&
+		test_linear_range 'd' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto drops patches in upstream" "
+		reset_rebase &&
+		git rebase $* --onto f h i &&
+		test_cmp_rev f HEAD~2 &&
+		test_linear_range 'd i' f..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto does not drop patches in onto" "
+		reset_rebase &&
+		git rebase $* --onto h f i &&
+		test_cmp_rev h HEAD~3 &&
+		test_linear_range 'd G i' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
 test_done
-- 
1.8.2.674.gd17d3d2

[PATCH v3 3/7] add tests for rebasing of empty commits

[Martin von Zweigbergk]

---
 t/t3401-rebase-partial.sh         | 24 ----------------
 t/t3420-rebase-topology-linear.sh | 58 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 24 deletions(-)

diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
index 58f4823..7ba1797 100755
--- a/t/t3401-rebase-partial.sh
+++ b/t/t3401-rebase-partial.sh
@@ -42,28 +42,4 @@ test_expect_success 'rebase --merge topic branch that was partially merged upstr
 	test_path_is_missing .git/rebase-merge
 '
 
-test_expect_success 'rebase ignores empty commit' '
-	git reset --hard A &&
-	git commit --allow-empty -m empty &&
-	test_commit D &&
-	git rebase C &&
-	test "$(git log --format=%s C..)" = "D"
-'
-
-test_expect_success 'rebase --keep-empty' '
-	git reset --hard D &&
-	git rebase --keep-empty C &&
-	test "$(git log --format=%s C..)" = "D
-empty"
-'
-
-test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' '
-	git reset --hard A &&
-	git commit --allow-empty -m also-empty &&
-	git rebase --keep-empty D &&
-	test "$(git log --format=%s A..)" = "also-empty
-D
-empty"
-'
-
 test_done
diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh
index 75cc476..81e3d59 100755
--- a/t/t3420-rebase-topology-linear.sh
+++ b/t/t3420-rebase-topology-linear.sh
@@ -160,4 +160,62 @@ test_run_rebase success -m
 test_run_rebase success -i
 test_run_rebase success -p
 
+# a---b---c---j!
+#      \
+#       d---k!--l
+#
+# ! = empty
+test_expect_success 'setup of linear history for empty commit tests' '
+	git checkout c &&
+	make_empty j &&
+	git checkout d &&
+	make_empty k &&
+	test_commit l
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* drops empty commit" "
+		reset_rebase &&
+		git rebase $* c l &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'd l' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --keep-empty" "
+		reset_rebase &&
+		git rebase $* --keep-empty c l &&
+		test_cmp_rev c HEAD~3 &&
+		test_linear_range 'd k l' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --keep-empty keeps empty even if already in upstream" "
+		reset_rebase &&
+		git rebase $* --keep-empty j l &&
+		test_cmp_rev j HEAD~3 &&
+		test_linear_range 'd k l' j..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
 test_done
-- 
1.8.2.674.gd17d3d2

[PATCH v3 4/7] add tests for rebasing root

[Martin von Zweigbergk]

---
 t/t3420-rebase-topology-linear.sh | 129 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 129 insertions(+)

diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh
index 81e3d59..659a7b3 100755
--- a/t/t3420-rebase-topology-linear.sh
+++ b/t/t3420-rebase-topology-linear.sh
@@ -218,4 +218,133 @@ test_run_rebase failure -m
 test_run_rebase failure -i
 test_run_rebase failure -p
 
+#       m
+#      /
+# a---b---c---g
+#
+# x---y---B
+#
+# uppercase = cherry-picked
+# m = reverted b
+#
+# Reverted patches are there for tests to be able to check if a commit
+# that introduced the same change as another commit is
+# dropped. Without reverted commits, we could get false positives
+# because applying the patch succeeds, but simply results in no
+# changes.
+test_expect_success 'setup of linear history for test involving root' '
+	git checkout b &&
+	revert m b &&
+	git checkout --orphan disjoint &&
+	git rm -rf . &&
+	test_commit x &&
+	test_commit y &&
+	cherry_pick B b
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root" "
+		reset_rebase &&
+		git rebase $* --onto c --root y &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'x y' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* without --onto --root with disjoint history" "
+		reset_rebase &&
+		git rebase $* c y &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'x y' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root drops patch in onto" "
+		reset_rebase &&
+		git rebase $* --onto m --root B &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'x y' m..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root with merge-base does not go to root" "
+		reset_rebase &&
+		git rebase $* --onto m --root g &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'c g' m..
+	"
+}
+
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* without --onto --root with disjoint history drops patch in onto" "
+		reset_rebase &&
+		git rebase $* m B &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'x y' m..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --root on linear history is a no-op" "
+		reset_rebase &&
+		git rebase $* --root c &&
+		test_cmp_rev c HEAD
+	"
+}
+test_run_rebase failure ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* -f --root on linear history causes re-write" "
+		reset_rebase &&
+		git rebase $* -f --root c &&
+		! test_cmp_rev a HEAD~2 &&
+		test_linear_range 'a b c' HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
 test_done
-- 
1.8.2.674.gd17d3d2

[PATCH v3 5/7] add tests for rebasing merged history

[Martin von Zweigbergk]

---
 t/t3400-rebase.sh                 |  31 +----
 t/t3401-rebase-partial.sh         |  45 -------
 t/t3404-rebase-interactive.sh     |  10 +-
 t/t3409-rebase-preserve-merges.sh |  53 --------
 t/t3425-rebase-topology-merges.sh | 252 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 254 insertions(+), 137 deletions(-)
 delete mode 100755 t/t3401-rebase-partial.sh
 create mode 100755 t/t3425-rebase-topology-merges.sh

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b58fa1a..b436ef4 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -40,13 +40,6 @@ test_expect_success 'prepare repository with topic branches' '
 	echo Side >>C &&
 	git add C &&
 	git commit -m "Add C" &&
-	git checkout -b nonlinear my-topic-branch &&
-	echo Edit >>B &&
-	git add B &&
-	git commit -m "Modify B" &&
-	git merge side &&
-	git checkout -b upstream-merged-nonlinear &&
-	git merge master &&
 	git checkout -f my-topic-branch &&
 	git tag topic
 '
@@ -106,31 +99,9 @@ test_expect_success 'rebase from ambiguous branch name' '
 	git rebase master
 '
 
-test_expect_success 'rebase after merge master' '
-	git checkout --detach refs/tags/topic &&
-	git branch -D topic &&
-	git reset --hard topic &&
-	git merge master &&
-	git rebase master &&
-	! (git show | grep "^Merge:")
-'
-
-test_expect_success 'rebase of history with merges is linearized' '
-	git checkout nonlinear &&
-	test 4 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
-test_expect_success 'rebase of history with merges after upstream merge is linearized' '
-	git checkout upstream-merged-nonlinear &&
-	test 5 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
 test_expect_success 'rebase a single mode change' '
 	git checkout master &&
+	git branch -D topic &&
 	echo 1 >X &&
 	git add X &&
 	test_tick &&
diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
deleted file mode 100755
index 7ba1797..0000000
--- a/t/t3401-rebase-partial.sh
+++ /dev/null
@@ -1,45 +0,0 @@
-#!/bin/sh
-#
-# Copyright (c) 2006 Yann Dirson, based on t3400 by Amos Waterland
-#
-
-test_description='git rebase should detect patches integrated upstream
-
-This test cherry-picks one local change of two into master branch, and
-checks that git rebase succeeds with only the second patch in the
-local branch.
-'
-. ./test-lib.sh
-
-test_expect_success 'prepare repository with topic branch' '
-	test_commit A &&
-	git checkout -b my-topic-branch &&
-	test_commit B &&
-	test_commit C &&
-	git checkout -f master &&
-	test_commit A2 A.t
-'
-
-test_expect_success 'pick top patch from topic branch into master' '
-	git cherry-pick C &&
-	git checkout -f my-topic-branch
-'
-
-test_debug '
-	git cherry master &&
-	git format-patch -k --stdout --full-index master >/dev/null &&
-	gitk --all & sleep 1
-'
-
-test_expect_success 'rebase topic branch against new master and check git am did not get halted' '
-	git rebase master &&
-	test_path_is_missing .git/rebase-apply
-'
-
-test_expect_success 'rebase --merge topic branch that was partially merged upstream' '
-	git reset --hard C &&
-	git rebase --merge master &&
-	test_path_is_missing .git/rebase-merge
-'
-
-test_done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index a58406d..ffcaf02 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -477,19 +477,11 @@ test_expect_success 'interrupted squash works as expected (case 2)' '
 	test $one = $(git rev-parse HEAD~2)
 '
 
-test_expect_success 'ignore patch if in upstream' '
-	HEAD=$(git rev-parse HEAD) &&
-	git checkout -b has-cherry-picked HEAD^ &&
+test_expect_success '--continue tries to commit, even for "edit"' '
 	echo unrelated > file7 &&
 	git add file7 &&
 	test_tick &&
 	git commit -m "unrelated change" &&
-	git cherry-pick $HEAD &&
-	EXPECT_COUNT=1 git rebase -i $HEAD &&
-	test $HEAD = $(git rev-parse HEAD^)
-'
-
-test_expect_success '--continue tries to commit, even for "edit"' '
 	parent=$(git rev-parse HEAD^) &&
 	test_tick &&
 	FAKE_LINES="edit 1" git rebase -i HEAD^ &&
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index 6de4e22..2e0c364 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -11,14 +11,6 @@ Run "git rebase -p" and check that merges are properly carried along
 GIT_AUTHOR_EMAIL=bogus_email_address
 export GIT_AUTHOR_EMAIL
 
-# Clone 1 (trivial merge):
-#
-# A1--A2  <-- origin/master
-#  \   \
-#   B1--M  <-- topic
-#    \
-#     B2  <-- origin/topic
-#
 # Clone 2 (conflicting merge):
 #
 # A1--A2--B3   <-- origin/master
@@ -36,16 +28,6 @@ export GIT_AUTHOR_EMAIL
 #     \--A3    <-- topic2
 #      \
 #       B2     <-- origin/topic
-#
-# Clone 4 (merge using second parent as base):
-#
-# A1--A2--B3   <-- origin/master
-#  \
-#   B1--A3--M  <-- topic
-#    \     /
-#     \--A4    <-- topic2
-#      \
-#       B2     <-- origin/topic
 
 test_expect_success 'setup for merge-preserving rebase' \
 	'echo First > A &&
@@ -58,20 +40,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git checkout -f master &&
 	echo Third >> A &&
 	git commit -a -m "Modify A2" &&
-
-	git clone ./. clone1 &&
-	(cd clone1 &&
-	git checkout -b topic origin/topic &&
-	git merge origin/master
-	) &&
-
-	git clone ./. clone4 &&
-	(
-		cd clone4 &&
-		git checkout -b topic origin/topic &&
-		git merge origin/master
-	) &&
-
 	echo Fifth > B &&
 	git add B &&
 	git commit -m "Add different B" &&
@@ -101,16 +69,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git commit -a -m "Modify B2"
 '
 
-test_expect_success 'rebase -p fakes interactive rebase' '
-	(
-	cd clone1 &&
-	git fetch &&
-	git rebase -p origin/topic &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_expect_success '--continue works after a conflict' '
 	(
 	cd clone2 &&
@@ -138,15 +96,4 @@ test_expect_success 'rebase -p preserves no-ff merges' '
 	)
 '
 
-test_expect_success 'rebase -p works when base inside second parent' '
-	(
-	cd clone4 &&
-	git fetch &&
-	git rebase -p HEAD^2 &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify B" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_done
diff --git a/t/t3425-rebase-topology-merges.sh b/t/t3425-rebase-topology-merges.sh
new file mode 100755
index 0000000..72fa76d
--- /dev/null
+++ b/t/t3425-rebase-topology-merges.sh
@@ -0,0 +1,252 @@
+#!/bin/sh
+
+test_description='rebase topology tests with merges'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+test_revision_subjects () {
+	expected="$1"
+	shift
+	set -- $(git log --format=%s --no-walk=unsorted "$@")
+	test "$expected" = "$*"
+}
+
+# a---b-----------c
+#      \           \
+#       d-------e   \
+#        \       \   \
+#         n---o---w---v
+#              \
+#               z
+test_expect_success 'setup of non-linear-history' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git checkout b &&
+	test_commit d &&
+	test_commit e
+
+	git checkout c &&
+	test_commit g &&
+	revert h g &&
+	git checkout d &&
+	cherry_pick G g &&
+	test_commit i &&
+	git checkout b &&
+	test_commit f
+
+	git checkout d &&
+	test_commit n &&
+	test_commit o &&
+	test_merge w e &&
+	test_merge v c &&
+	git checkout o &&
+	test_commit z
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* after merge from upstream" "
+		reset_rebase &&
+		git rebase $* e w &&
+		test_cmp_rev e HEAD~2 &&
+		test_linear_range 'n o' e..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized in place" "
+		reset_rebase &&
+		git rebase $* d w &&
+		test_cmp_rev d HEAD~3 &&
+		test_linear_range "\'"$expected"\'" d..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'e n o' ''
+test_run_rebase success 'e n o' -m
+test_run_rebase success 'n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized upstream" "
+		reset_rebase &&
+		git rebase $* c w &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history with merges after upstream merge is linearized" "
+		reset_rebase &&
+		git rebase $* c v &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_expect_success "rebase -p is no-op in non-linear history" "
+	reset_rebase &&
+	git rebase -p d w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p is no-op when base inside second parent" "
+	reset_rebase &&
+	git rebase -p e w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_failure "rebase -p --root on non-linear history is a no-op" "
+	reset_rebase &&
+	git rebase -p --root w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p re-creates merge from side branch" "
+	reset_rebase &&
+	git rebase -p z w &&
+	test_cmp_rev z HEAD^ &&
+	test_cmp_rev w^2 HEAD^2
+"
+
+test_expect_success "rebase -p re-creates internal merge" "
+	reset_rebase &&
+	git rebase -p c w &&
+	test_cmp_rev c HEAD~4 &&
+	test_revision_subjects 'd n e o w' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p can re-create two branches on onto" "
+	reset_rebase &&
+	git rebase -p --onto c d w &&
+	test_cmp_rev c HEAD~3 &&
+	test_revision_subjects 'n e o w' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+#       f
+#      /
+# a---b---c---g---h
+#      \
+#       d---G---i
+#        \       \
+#         e-------u
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for patch-equivalence tests' '
+	git checkout e &&
+	test_merge u i
+'
+
+test_expect_success "rebase -p re-creates history around dropped commit matching upstream" "
+	reset_rebase &&
+	git rebase -p h u &&
+	test_cmp_rev h HEAD~3 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p --onto in merged history drops patches in upstream" "
+	reset_rebase &&
+	git rebase -p --onto f h u &&
+	test_cmp_rev f HEAD~3 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p --onto in merged history does not drop patches in onto" "
+	reset_rebase &&
+	git rebase -p --onto h f u &&
+	test_cmp_rev h HEAD~3 &&
+	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
+"
+
+# a---b---c---g---h
+#      \
+#       d---G---s
+#        \   \ /
+#         \   X
+#          \ / \
+#           e---t
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for dropping whole side' '
+	git checkout G &&
+	test_merge s e &&
+	git checkout e &&
+	test_merge t G
+'
+
+test_expect_failure "rebase -p drops merge commit when entire first-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h s &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+test_expect_success "rebase -p drops merge commit when entire second-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h t &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+# a---b---c
+#      \
+#       d---e
+#        \   \
+#         n---r
+#          \
+#           o
+#
+# r = tree-same with n
+test_expect_success 'setup of non-linear-history for empty commits' '
+	git checkout n &&
+	git merge --no-commit e &&
+	git reset n . &&
+	git commit -m r &&
+	git reset --hard &&
+	git clean -f &&
+	git tag r
+'
+
+test_expect_success "rebase -p re-creates empty internal merge commit" "
+	reset_rebase &&
+	git rebase -p c r &&
+	test_cmp_rev c HEAD~3 &&
+	test_revision_subjects 'd e n r' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p re-creates empty merge commit" "
+	reset_rebase &&
+	git rebase -p o r &&
+	test_cmp_rev e HEAD^2 &&
+	test_cmp_rev o HEAD^ &&
+	test_revision_subjects 'r' HEAD
+"
+
+test_done
-- 
1.8.2.674.gd17d3d2

Re: [PATCH v3 5/7] add tests for rebasing merged history

[Johannes Sixt]

Am 31.05.2013 08:49, schrieb Martin von Zweigbergk:
> +#       f
> +#      /
> +# a---b---c---g---h
> +#      \
> +#       d---G---i
> +#        \       \
> +#         e-------u
> +#
> +# uppercase = cherry-picked
> +# h = reverted g
...
> +test_expect_success "rebase -p --onto in merged history drops patches in upstream" "
> +	reset_rebase &&
> +	git rebase -p --onto f h u &&
> +	test_cmp_rev f HEAD~3 &&
> +	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
> +"
> +
> +test_expect_success "rebase -p --onto in merged history does not drop patches in onto" "
> +	reset_rebase &&
> +	git rebase -p --onto h f u &&
> +	test_cmp_rev h HEAD~3 &&
> +	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
> +"

I noticed one new aspect:

The interdiff between v2 and v3 looks like this:

-test_expect_failure "rebase -p --onto in merged history does not lose patches in upstream" "
+test_expect_success "rebase -p --onto in merged history drops patches in upstream" "
 	reset_rebase &&
 	git rebase -p --onto f h u &&
 	test_cmp_rev f HEAD~3 &&
-	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
 "
 
-test_expect_success "rebase -p --onto in merged history drops patches in onto" "
+test_expect_success "rebase -p --onto in merged history does not drop patches in onto" "
 	reset_rebase &&
 	git rebase -p --onto h f u &&
 	test_cmp_rev h HEAD~3 &&
-	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD

The expectations that these two tests check changed from v2 to v3.
Notice that former test goes from expect_failure to expect_success,
as it should, but the latter does not change. Strange, isn't it?

The reason is that this check is incomplete:

	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD

and allowed the latter test in the v2 form to pass.

It should be

	test_revision_subjects 'd i d e u' HEAD^2^ HEAD^2 HEAD~2 HEAD^ HEAD

The check of the latter test should be:

	test_revision_subjects 'd G i d e u' HEAD^2~2 HEAD^2^ HEAD^2 HEAD~2 HEAD^ HEAD

i.e. check all the way back to the mergebase via both branches. This
can be extrapolated to all tests that reconstruct mergy history (not
just these two cases).

-- Hannes

[PATCH v4 5/7] add tests for rebasing merged history

[Martin von Zweigbergk]

---

> The reason is that this check is incomplete:
>
>    test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD

Nice catch! This should fix it. I couldn't use the method you
suggested because of how test_revision_subjects works (repeated
revisions are ignored), but this makes the check stricter anyway.

Junio, all the previous patches are unchanged since v3, so I'm not
resending them. Thanks.

 t/t3400-rebase.sh                 |  31 +----
 t/t3401-rebase-partial.sh         |  45 -------
 t/t3404-rebase-interactive.sh     |  10 +-
 t/t3409-rebase-preserve-merges.sh |  53 --------
 t/t3425-rebase-topology-merges.sh | 258 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 260 insertions(+), 137 deletions(-)
 delete mode 100755 t/t3401-rebase-partial.sh
 create mode 100755 t/t3425-rebase-topology-merges.sh

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b58fa1a..b436ef4 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -40,13 +40,6 @@ test_expect_success 'prepare repository with topic branches' '
 	echo Side >>C &&
 	git add C &&
 	git commit -m "Add C" &&
-	git checkout -b nonlinear my-topic-branch &&
-	echo Edit >>B &&
-	git add B &&
-	git commit -m "Modify B" &&
-	git merge side &&
-	git checkout -b upstream-merged-nonlinear &&
-	git merge master &&
 	git checkout -f my-topic-branch &&
 	git tag topic
 '
@@ -106,31 +99,9 @@ test_expect_success 'rebase from ambiguous branch name' '
 	git rebase master
 '
 
-test_expect_success 'rebase after merge master' '
-	git checkout --detach refs/tags/topic &&
-	git branch -D topic &&
-	git reset --hard topic &&
-	git merge master &&
-	git rebase master &&
-	! (git show | grep "^Merge:")
-'
-
-test_expect_success 'rebase of history with merges is linearized' '
-	git checkout nonlinear &&
-	test 4 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
-test_expect_success 'rebase of history with merges after upstream merge is linearized' '
-	git checkout upstream-merged-nonlinear &&
-	test 5 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
 test_expect_success 'rebase a single mode change' '
 	git checkout master &&
+	git branch -D topic &&
 	echo 1 >X &&
 	git add X &&
 	test_tick &&
diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
deleted file mode 100755
index 7ba1797..0000000
--- a/t/t3401-rebase-partial.sh
+++ /dev/null
@@ -1,45 +0,0 @@
-#!/bin/sh
-#
-# Copyright (c) 2006 Yann Dirson, based on t3400 by Amos Waterland
-#
-
-test_description='git rebase should detect patches integrated upstream
-
-This test cherry-picks one local change of two into master branch, and
-checks that git rebase succeeds with only the second patch in the
-local branch.
-'
-. ./test-lib.sh
-
-test_expect_success 'prepare repository with topic branch' '
-	test_commit A &&
-	git checkout -b my-topic-branch &&
-	test_commit B &&
-	test_commit C &&
-	git checkout -f master &&
-	test_commit A2 A.t
-'
-
-test_expect_success 'pick top patch from topic branch into master' '
-	git cherry-pick C &&
-	git checkout -f my-topic-branch
-'
-
-test_debug '
-	git cherry master &&
-	git format-patch -k --stdout --full-index master >/dev/null &&
-	gitk --all & sleep 1
-'
-
-test_expect_success 'rebase topic branch against new master and check git am did not get halted' '
-	git rebase master &&
-	test_path_is_missing .git/rebase-apply
-'
-
-test_expect_success 'rebase --merge topic branch that was partially merged upstream' '
-	git reset --hard C &&
-	git rebase --merge master &&
-	test_path_is_missing .git/rebase-merge
-'
-
-test_done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index a58406d..ffcaf02 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -477,19 +477,11 @@ test_expect_success 'interrupted squash works as expected (case 2)' '
 	test $one = $(git rev-parse HEAD~2)
 '
 
-test_expect_success 'ignore patch if in upstream' '
-	HEAD=$(git rev-parse HEAD) &&
-	git checkout -b has-cherry-picked HEAD^ &&
+test_expect_success '--continue tries to commit, even for "edit"' '
 	echo unrelated > file7 &&
 	git add file7 &&
 	test_tick &&
 	git commit -m "unrelated change" &&
-	git cherry-pick $HEAD &&
-	EXPECT_COUNT=1 git rebase -i $HEAD &&
-	test $HEAD = $(git rev-parse HEAD^)
-'
-
-test_expect_success '--continue tries to commit, even for "edit"' '
 	parent=$(git rev-parse HEAD^) &&
 	test_tick &&
 	FAKE_LINES="edit 1" git rebase -i HEAD^ &&
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index 6de4e22..2e0c364 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -11,14 +11,6 @@ Run "git rebase -p" and check that merges are properly carried along
 GIT_AUTHOR_EMAIL=bogus_email_address
 export GIT_AUTHOR_EMAIL
 
-# Clone 1 (trivial merge):
-#
-# A1--A2  <-- origin/master
-#  \   \
-#   B1--M  <-- topic
-#    \
-#     B2  <-- origin/topic
-#
 # Clone 2 (conflicting merge):
 #
 # A1--A2--B3   <-- origin/master
@@ -36,16 +28,6 @@ export GIT_AUTHOR_EMAIL
 #     \--A3    <-- topic2
 #      \
 #       B2     <-- origin/topic
-#
-# Clone 4 (merge using second parent as base):
-#
-# A1--A2--B3   <-- origin/master
-#  \
-#   B1--A3--M  <-- topic
-#    \     /
-#     \--A4    <-- topic2
-#      \
-#       B2     <-- origin/topic
 
 test_expect_success 'setup for merge-preserving rebase' \
 	'echo First > A &&
@@ -58,20 +40,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git checkout -f master &&
 	echo Third >> A &&
 	git commit -a -m "Modify A2" &&
-
-	git clone ./. clone1 &&
-	(cd clone1 &&
-	git checkout -b topic origin/topic &&
-	git merge origin/master
-	) &&
-
-	git clone ./. clone4 &&
-	(
-		cd clone4 &&
-		git checkout -b topic origin/topic &&
-		git merge origin/master
-	) &&
-
 	echo Fifth > B &&
 	git add B &&
 	git commit -m "Add different B" &&
@@ -101,16 +69,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git commit -a -m "Modify B2"
 '
 
-test_expect_success 'rebase -p fakes interactive rebase' '
-	(
-	cd clone1 &&
-	git fetch &&
-	git rebase -p origin/topic &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_expect_success '--continue works after a conflict' '
 	(
 	cd clone2 &&
@@ -138,15 +96,4 @@ test_expect_success 'rebase -p preserves no-ff merges' '
 	)
 '
 
-test_expect_success 'rebase -p works when base inside second parent' '
-	(
-	cd clone4 &&
-	git fetch &&
-	git rebase -p HEAD^2 &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify B" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_done
diff --git a/t/t3425-rebase-topology-merges.sh b/t/t3425-rebase-topology-merges.sh
new file mode 100755
index 0000000..efd7dfd
--- /dev/null
+++ b/t/t3425-rebase-topology-merges.sh
@@ -0,0 +1,258 @@
+#!/bin/sh
+
+test_description='rebase topology tests with merges'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+test_revision_subjects () {
+	expected="$1"
+	shift
+	set -- $(git log --format=%s --no-walk=unsorted "$@")
+	test "$expected" = "$*"
+}
+
+# a---b-----------c
+#      \           \
+#       d-------e   \
+#        \       \   \
+#         n---o---w---v
+#              \
+#               z
+test_expect_success 'setup of non-linear-history' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git checkout b &&
+	test_commit d &&
+	test_commit e
+
+	git checkout c &&
+	test_commit g &&
+	revert h g &&
+	git checkout d &&
+	cherry_pick G g &&
+	test_commit i &&
+	git checkout b &&
+	test_commit f
+
+	git checkout d &&
+	test_commit n &&
+	test_commit o &&
+	test_merge w e &&
+	test_merge v c &&
+	git checkout o &&
+	test_commit z
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* after merge from upstream" "
+		reset_rebase &&
+		git rebase $* e w &&
+		test_cmp_rev e HEAD~2 &&
+		test_linear_range 'n o' e..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized in place" "
+		reset_rebase &&
+		git rebase $* d w &&
+		test_cmp_rev d HEAD~3 &&
+		test_linear_range "\'"$expected"\'" d..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'e n o' ''
+test_run_rebase success 'e n o' -m
+test_run_rebase success 'n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized upstream" "
+		reset_rebase &&
+		git rebase $* c w &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history with merges after upstream merge is linearized" "
+		reset_rebase &&
+		git rebase $* c v &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_expect_success "rebase -p is no-op in non-linear history" "
+	reset_rebase &&
+	git rebase -p d w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p is no-op when base inside second parent" "
+	reset_rebase &&
+	git rebase -p e w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_failure "rebase -p --root on non-linear history is a no-op" "
+	reset_rebase &&
+	git rebase -p --root w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p re-creates merge from side branch" "
+	reset_rebase &&
+	git rebase -p z w &&
+	test_cmp_rev z HEAD^ &&
+	test_cmp_rev w^2 HEAD^2
+"
+
+test_expect_success "rebase -p re-creates internal merge" "
+	reset_rebase &&
+	git rebase -p c w &&
+	test_cmp_rev c HEAD~4 &&
+	test_cmp_rev HEAD^2^ HEAD~3 &&
+	test_revision_subjects 'd n e o w' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p can re-create two branches on onto" "
+	reset_rebase &&
+	git rebase -p --onto c d w &&
+	test_cmp_rev c HEAD~3 &&
+	test_cmp_rev c HEAD^2^ &&
+	test_revision_subjects 'n e o w' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+#       f
+#      /
+# a---b---c---g---h
+#      \
+#       d---G---i
+#        \       \
+#         e-------u
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for patch-equivalence tests' '
+	git checkout e &&
+	test_merge u i
+'
+
+test_expect_success "rebase -p re-creates history around dropped commit matching upstream" "
+	reset_rebase &&
+	git rebase -p h u &&
+	test_cmp_rev h HEAD~3 &&
+	test_cmp_rev HEAD^2^ HEAD~2 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p --onto in merged history drops patches in upstream" "
+	reset_rebase &&
+	git rebase -p --onto f h u &&
+	test_cmp_rev f HEAD~3 &&
+	test_cmp_rev HEAD^2^ HEAD~2 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p --onto in merged history does not drop patches in onto" "
+	reset_rebase &&
+	git rebase -p --onto h f u &&
+	test_cmp_rev h HEAD~3 &&
+	test_cmp_rev HEAD^2~2 HEAD~2 &&
+	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
+"
+
+# a---b---c---g---h
+#      \
+#       d---G---s
+#        \   \ /
+#         \   X
+#          \ / \
+#           e---t
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for dropping whole side' '
+	git checkout G &&
+	test_merge s e &&
+	git checkout e &&
+	test_merge t G
+'
+
+test_expect_failure "rebase -p drops merge commit when entire first-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h s &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+test_expect_success "rebase -p drops merge commit when entire second-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h t &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+# a---b---c
+#      \
+#       d---e
+#        \   \
+#         n---r
+#          \
+#           o
+#
+# r = tree-same with n
+test_expect_success 'setup of non-linear-history for empty commits' '
+	git checkout n &&
+	git merge --no-commit e &&
+	git reset n . &&
+	git commit -m r &&
+	git reset --hard &&
+	git clean -f &&
+	git tag r
+'
+
+test_expect_success "rebase -p re-creates empty internal merge commit" "
+	reset_rebase &&
+	git rebase -p c r &&
+	test_cmp_rev c HEAD~3 &&
+	test_cmp_rev HEAD^2^ HEAD~2 &&
+	test_revision_subjects 'd e n r' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p re-creates empty merge commit" "
+	reset_rebase &&
+	git rebase -p o r &&
+	test_cmp_rev e HEAD^2 &&
+	test_cmp_rev o HEAD^ &&
+	test_revision_subjects 'r' HEAD
+"
+
+test_done
-- 
1.8.2.674.gd17d3d2

[PATCH v3 6/7] t3406: modernize style

[Martin von Zweigbergk]

Update the following:

 - Quote 'setup'
 - Remove blank lines within test case body
 - Use test_commit instead of custom quick_one
 - Create branch "topic" from tag created by test_commit
---
 t/t3406-rebase-message.sh | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index e6a9a0d..fe8c27f 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -4,27 +4,17 @@ test_description='messages from rebase operation'
 
 . ./test-lib.sh
 
-quick_one () {
-	echo "$1" >"file$1" &&
-	git add "file$1" &&
-	test_tick &&
-	git commit -m "$1"
-}
+test_expect_success 'setup' '
+	test_commit O fileO &&
+	test_commit X fileX &&
+	test_commit A fileA &&
+	test_commit B fileB &&
+	test_commit Y fileY &&
 
-test_expect_success setup '
-	quick_one O &&
-	git branch topic &&
-	quick_one X &&
-	quick_one A &&
-	quick_one B &&
-	quick_one Y &&
-
-	git checkout topic &&
-	quick_one A &&
-	quick_one B &&
-	quick_one Z &&
+	git checkout -b topic O &&
+	git cherry-pick A B &&
+	test_commit Z fileZ &&
 	git tag start
-
 '
 
 cat >expect <<\EOF
@@ -34,12 +24,10 @@ Committed: 0003 Z
 EOF
 
 test_expect_success 'rebase -m' '
-
 	git rebase -m master >report &&
 	sed -n -e "/^Already applied: /p" \
 		-e "/^Committed: /p" report >actual &&
 	test_cmp expect actual
-
 '
 
 test_expect_success 'rebase --stat' '
-- 
1.8.2.674.gd17d3d2

[PATCH v3 7/7] tests: move test for rebase messages from t3400 to t3406

[Martin von Zweigbergk]

t3406 is supposed to test "messages from rebase operation", so let's
move tests in t3400 that fit that description into 3406. Most of the
functionality they tested, except for the messages, has now been
subsumed by t3420.
---
 t/t3400-rebase.sh         | 22 ----------------------
 t/t3406-rebase-message.sh | 22 ++++++++++++++++++++++
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b436ef4..45a55e9 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -59,28 +59,6 @@ test_expect_success 'rebase against master' '
 	git rebase master
 '
 
-test_expect_success 'rebase against master twice' '
-	git rebase master >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date" out
-'
-
-test_expect_success 'rebase against master twice with --force' '
-	git rebase --force-rebase master >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date, rebase forced" out
-'
-
-test_expect_success 'rebase against master twice from another branch' '
-	git checkout my-topic-branch^ &&
-	git rebase master my-topic-branch >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date" out
-'
-
-test_expect_success 'rebase fast-forward to master' '
-	git checkout my-topic-branch^ &&
-	git rebase my-topic-branch >out &&
-	test_i18ngrep "Fast-forwarded HEAD to my-topic-branch" out
-'
-
 test_expect_success 'the rebase operation should not have destroyed author information' '
 	! (git log | grep "Author:" | grep "<>")
 '
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index fe8c27f..0392e36 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -30,6 +30,28 @@ test_expect_success 'rebase -m' '
 	test_cmp expect actual
 '
 
+test_expect_success 'rebase against master twice' '
+	git rebase master >out &&
+	test_i18ngrep "Current branch topic is up to date" out
+'
+
+test_expect_success 'rebase against master twice with --force' '
+	git rebase --force-rebase master >out &&
+	test_i18ngrep "Current branch topic is up to date, rebase forced" out
+'
+
+test_expect_success 'rebase against master twice from another branch' '
+	git checkout topic^ &&
+	git rebase master topic >out &&
+	test_i18ngrep "Current branch topic is up to date" out
+'
+
+test_expect_success 'rebase fast-forward to master' '
+	git checkout topic^ &&
+	git rebase topic >out &&
+	test_i18ngrep "Fast-forwarded HEAD to topic" out
+'
+
 test_expect_success 'rebase --stat' '
 	git reset --hard start &&
         git rebase --stat master >diffstat.txt &&
-- 
1.8.2.674.gd17d3d2

[PATCH v5 0/7] Rebase topology test

[Martin von Zweigbergk]

The only change since v4 should be that t3420 was renamed t3421.

Martin von Zweigbergk (7):
  add simple tests of consistency across rebase types
  add tests for rebasing with patch-equivalence present
  add tests for rebasing of empty commits
  add tests for rebasing root
  add tests for rebasing merged history
  t3406: modernize style
  tests: move test for rebase messages from t3400 to t3406

 t/lib-rebase.sh                   |  32 ++++
 t/t3400-rebase.sh                 |  53 +-----
 t/t3401-rebase-partial.sh         |  69 --------
 t/t3404-rebase-interactive.sh     |  10 +-
 t/t3406-rebase-message.sh         |  50 +++---
 t/t3409-rebase-preserve-merges.sh |  53 ------
 t/t3421-rebase-topology-linear.sh | 350 ++++++++++++++++++++++++++++++++++++++
 t/t3425-rebase-topology-merges.sh | 258 ++++++++++++++++++++++++++++
 8 files changed, 672 insertions(+), 203 deletions(-)
 delete mode 100755 t/t3401-rebase-partial.sh
 create mode 100755 t/t3421-rebase-topology-linear.sh
 create mode 100755 t/t3425-rebase-topology-merges.sh

-- 
1.8.3.497.g83fddbe

[PATCH v5 1/7] add simple tests of consistency across rebase types

[Martin von Zweigbergk]

Helped-by: Johannes Sixt <j6t@kdbg.org>
---
 t/lib-rebase.sh                   | 15 ++++++++
 t/t3421-rebase-topology-linear.sh | 78 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)
 create mode 100755 t/t3421-rebase-topology-linear.sh

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6ccf797..62b3887 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -65,3 +65,18 @@ EOF
 	test_set_editor "$(pwd)/fake-editor.sh"
 	chmod a+x fake-editor.sh
 }
+
+# checks that the revisions in "$2" represent a linear range with the
+# subjects in "$1"
+test_linear_range () {
+	! { git log --format=%p "$2" | sane_grep " " ;} &&
+	expected=$1
+	set -- $(git log --reverse --format=%s "$2")
+	test "$expected" = "$*"
+}
+
+reset_rebase () {
+	git rebase --abort # may fail; ignore exit code
+	git reset --hard &&
+	git clean -f
+}
diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
new file mode 100755
index 0000000..c4b32db
--- /dev/null
+++ b/t/t3421-rebase-topology-linear.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='basic rebase topology tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+# a---b---c
+#      \
+#       d---e
+test_expect_success 'setup' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git checkout b &&
+	test_commit d &&
+	test_commit e
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "simple rebase $*" "
+		reset_rebase &&
+		git rebase $* c e &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'd e' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* is no-op if upstream is an ancestor" "
+		reset_rebase &&
+		git rebase $* b e &&
+		test_cmp_rev e HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* -f rewrites even if upstream is an ancestor" "
+		reset_rebase &&
+		git rebase $* -f b e &&
+		! test_cmp_rev e HEAD &&
+		test_cmp_rev b HEAD~2 &&
+		test_linear_range 'd e' b..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* fast-forwards if an ancestor of upstream" "
+		reset_rebase &&
+		git rebase $* e b &&
+		test_cmp_rev e HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_done
-- 
1.8.3.497.g83fddbe

Re: [PATCH v5 1/7] add simple tests of consistency across rebase types

[Junio C Hamano]

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> Helped-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  t/lib-rebase.sh                   | 15 ++++++++
>  t/t3421-rebase-topology-linear.sh | 78 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 93 insertions(+)
>  create mode 100755 t/t3421-rebase-topology-linear.sh
>
> diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
> index 6ccf797..62b3887 100644
> --- a/t/lib-rebase.sh
> +++ b/t/lib-rebase.sh
> @@ -65,3 +65,18 @@ EOF
>  	test_set_editor "$(pwd)/fake-editor.sh"
>  	chmod a+x fake-editor.sh
>  }
> +
> +# checks that the revisions in "$2" represent a linear range with the
> +# subjects in "$1"
> +test_linear_range () {
> +	! { git log --format=%p "$2" | sane_grep " " ;} &&

An interesting way to spell:

    test $(git rev-list --merges "$2" | wc -l) = 0

I think I am fine with either, though.

> +	expected=$1
> +	set -- $(git log --reverse --format=%s "$2")
> +	test "$expected" = "$*"

OK.

> +}
> +
> +reset_rebase () {
> +	git rebase --abort # may fail; ignore exit code

test_might_fail to catch unusual exit codes?

> +	git reset --hard &&
> +	git clean -f
> +}
> diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
> new file mode 100755
> index 0000000..c4b32db
> --- /dev/null
> +++ b/t/t3421-rebase-topology-linear.sh
> @@ -0,0 +1,78 @@
> +#!/bin/sh
> +
> +test_description='basic rebase topology tests'
> +. ./test-lib.sh
> +. "$TEST_DIRECTORY"/lib-rebase.sh
> +
> +# a---b---c
> +#      \
> +#       d---e
> +test_expect_success 'setup' '
> +	test_commit a &&
> +	test_commit b &&
> +	test_commit c &&
> +	git checkout b &&
> +	test_commit d &&
> +	test_commit e
> +'
> +
> +test_run_rebase () {
> +	result=$1
> +	shift
> +	test_expect_$result "simple rebase $*" "
> +		reset_rebase &&
> +		git rebase $* c e &&
> +		test_cmp_rev c HEAD~2 &&
> +		test_linear_range 'd e' c..
> +	"
> +}
> +test_run_rebase success ''
> +test_run_rebase success -m
> +test_run_rebase success -i
> +test_run_rebase success -p
> +
> +test_run_rebase () {
> +	result=$1
> +	shift
> +	test_expect_$result "rebase $* is no-op if upstream is an ancestor" "
> +		reset_rebase &&
> +		git rebase $* b e &&
> +		test_cmp_rev e HEAD
> +	"
> +}
> +test_run_rebase success ''
> +test_run_rebase success -m
> +test_run_rebase success -i
> +test_run_rebase success -p
> +
> +test_run_rebase () {
> +	result=$1
> +	shift
> +	test_expect_$result "rebase $* -f rewrites even if upstream is an ancestor" "
> +		reset_rebase &&
> +		git rebase $* -f b e &&

Asking to rebase the history leading to e from b on top of the merge
base (which happens to be b) may be no-op or force-create a new
history that is parallel.  OK.

> +		! test_cmp_rev e HEAD &&
> +		test_cmp_rev b HEAD~2 &&
> +		test_linear_range 'd e' b..
> +	"
> +}
> +test_run_rebase success ''
> +test_run_rebase success -m
> +test_run_rebase success -i
> +test_run_rebase failure -p
> +
> +test_run_rebase () {
> +	result=$1
> +	shift
> +	test_expect_$result "rebase $* fast-forwards if an ancestor of upstream" "

The description is a non-sentence, and while I can tell what it
wants to say, I do not have a good suggestion for rephrasing this.

This is asking to rebase the history leading to b on top of e, but e
already includes everything in b, so it just turns into a no-op of
not moving from e.  So it is not even a fast-forward.

> +		reset_rebase &&
> +		git rebase $* e b &&
> +		test_cmp_rev e HEAD
> +	"
> +}
> +test_run_rebase success ''
> +test_run_rebase success -m
> +test_run_rebase success -i
> +test_run_rebase success -p
> +
> +test_done

Re: [PATCH v5 1/7] add simple tests of consistency across rebase types

[Martin von Zweigbergk]

On Mon, Jun 3, 2013 at 3:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> +
>> +# checks that the revisions in "$2" represent a linear range with the
>> +# subjects in "$1"
>> +test_linear_range () {
>> +     ! { git log --format=%p "$2" | sane_grep " " ;} &&
>
> An interesting way to spell:
>
>     test $(git rev-list --merges "$2" | wc -l) = 0

Heh, true. I'll change that. ("My" version was based on the one in
git-rebase.sh, around line 495.)

>> +reset_rebase () {
>> +     git rebase --abort # may fail; ignore exit code
>
> test_might_fail to catch unusual exit codes?

Will change.

>> +# a---b---c
>> +#      \
>> +#       d---e

>> +test_run_rebase () {
>> +     result=$1
>> +     shift
>> +     test_expect_$result "rebase $* fast-forwards if an ancestor of upstream" "
>
> The description is a non-sentence, and while I can tell what it
> wants to say, I do not have a good suggestion for rephrasing this.

Changing description to "... fast-forwards from an ancestor of upstream".

> This is asking to rebase the history leading to b on top of e, but e
> already includes everything in b, so it just turns into a no-op of
> not moving from e.  So it is not even a fast-forward.
>
>> +             reset_rebase &&
>> +             git rebase $* e b &&
>> +             test_cmp_rev e HEAD

Well, "git rebase e b" is of course a kind of short form of "git
checkout b && git rebase e". While it's true that the implementation
doesn't bother checking out b first, that's just an optimization, but
let me know if you meant something else.

Thanks. Will wait another day or two for further comments before I
send another version.

Re: [PATCH v5 1/7] add simple tests of consistency across rebase types

[Junio C Hamano]

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> Thanks. Will wait another day or two for further comments before I
> send another version.

Thanks; I just noticed that your patches lack S-o-b:.

Re: [PATCH v5 1/7] add simple tests of consistency across rebase types

[Johannes Sixt]

Am 6/4/2013 7:14, schrieb Martin von Zweigbergk:
> On Mon, Jun 3, 2013 at 3:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> +
>>> +# checks that the revisions in "$2" represent a linear range with the
>>> +# subjects in "$1"
>>> +test_linear_range () {
>>> +     ! { git log --format=%p "$2" | sane_grep " " ;} &&
>>
>> An interesting way to spell:
>>
>>     test $(git rev-list --merges "$2" | wc -l) = 0
> 
> Heh, true. I'll change that.

Then I think it would be even better written as

	revlist_merges=$(git rev-list --merges "$2") &&
	test -z "$revlist_merges"

so as not to ignore errors in the git invocation (and at least one less
fork()).

-- Hannes

Re: [PATCH v5 1/7] add simple tests of consistency across rebase types

[Martin von Zweigbergk]

On Mon, Jun 3, 2013 at 11:15 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Am 6/4/2013 7:14, schrieb Martin von Zweigbergk:
>> On Mon, Jun 3, 2013 at 3:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> +
>>>> +# checks that the revisions in "$2" represent a linear range with the
>>>> +# subjects in "$1"
>>>> +test_linear_range () {
>>>> +     ! { git log --format=%p "$2" | sane_grep " " ;} &&
>>>
>>> An interesting way to spell:
>>>
>>>     test $(git rev-list --merges "$2" | wc -l) = 0
>>
>> Heh, true. I'll change that.
>
> Then I think it would be even better written as
>
>         revlist_merges=$(git rev-list --merges "$2") &&
>         test -z "$revlist_merges"
>
> so as not to ignore errors in the git invocation (and at least one less
> fork()).

Done. I'll send it out in a day or two.

[PATCH v5 2/7] add tests for rebasing with patch-equivalence present

[Martin von Zweigbergk]

---
 t/lib-rebase.sh                   | 17 ++++++++
 t/t3421-rebase-topology-linear.sh | 85 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+)

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 62b3887..16eeb1c 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -80,3 +80,20 @@ reset_rebase () {
 	git reset --hard &&
 	git clean -f
 }
+
+cherry_pick () {
+	git cherry-pick -n "$2" &&
+	git commit -m "$1" &&
+	git tag "$1"
+}
+
+revert () {
+	git revert -n "$2" &&
+	git commit -m "$1" &&
+	git tag "$1"
+}
+
+make_empty () {
+	git commit --allow-empty -m "$1" &&
+	git tag "$1"
+}
diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
index c4b32db..75cc476 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -75,4 +75,89 @@ test_run_rebase success -m
 test_run_rebase success -i
 test_run_rebase success -p
 
+#       f
+#      /
+# a---b---c---g---h
+#      \
+#       d---G---i
+#
+# uppercase = cherry-picked
+# h = reverted g
+#
+# Reverted patches are there for tests to be able to check if a commit
+# that introduced the same change as another commit is
+# dropped. Without reverted commits, we could get false positives
+# because applying the patch succeeds, but simply results in no
+# changes.
+test_expect_success 'setup of linear history for range selection tests' '
+	git checkout c &&
+	test_commit g &&
+	revert h g &&
+	git checkout d &&
+	cherry_pick G g &&
+	test_commit i &&
+	git checkout b &&
+	test_commit f
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* drops patches in upstream" "
+		reset_rebase &&
+		git rebase $* h i &&
+		test_cmp_rev h HEAD~2 &&
+		test_linear_range 'd i' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* can drop last patch if in upstream" "
+		reset_rebase &&
+		git rebase $* h G &&
+		test_cmp_rev h HEAD^ &&
+		test_linear_range 'd' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto drops patches in upstream" "
+		reset_rebase &&
+		git rebase $* --onto f h i &&
+		test_cmp_rev f HEAD~2 &&
+		test_linear_range 'd i' f..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto does not drop patches in onto" "
+		reset_rebase &&
+		git rebase $* --onto h f i &&
+		test_cmp_rev h HEAD~3 &&
+		test_linear_range 'd G i' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
 test_done
-- 
1.8.3.497.g83fddbe

[PATCH v5 3/7] add tests for rebasing of empty commits

[Martin von Zweigbergk]

---
 t/t3401-rebase-partial.sh         | 24 ----------------
 t/t3421-rebase-topology-linear.sh | 58 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 24 deletions(-)

diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
index 58f4823..7ba1797 100755
--- a/t/t3401-rebase-partial.sh
+++ b/t/t3401-rebase-partial.sh
@@ -42,28 +42,4 @@ test_expect_success 'rebase --merge topic branch that was partially merged upstr
 	test_path_is_missing .git/rebase-merge
 '
 
-test_expect_success 'rebase ignores empty commit' '
-	git reset --hard A &&
-	git commit --allow-empty -m empty &&
-	test_commit D &&
-	git rebase C &&
-	test "$(git log --format=%s C..)" = "D"
-'
-
-test_expect_success 'rebase --keep-empty' '
-	git reset --hard D &&
-	git rebase --keep-empty C &&
-	test "$(git log --format=%s C..)" = "D
-empty"
-'
-
-test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' '
-	git reset --hard A &&
-	git commit --allow-empty -m also-empty &&
-	git rebase --keep-empty D &&
-	test "$(git log --format=%s A..)" = "also-empty
-D
-empty"
-'
-
 test_done
diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
index 75cc476..81e3d59 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -160,4 +160,62 @@ test_run_rebase success -m
 test_run_rebase success -i
 test_run_rebase success -p
 
+# a---b---c---j!
+#      \
+#       d---k!--l
+#
+# ! = empty
+test_expect_success 'setup of linear history for empty commit tests' '
+	git checkout c &&
+	make_empty j &&
+	git checkout d &&
+	make_empty k &&
+	test_commit l
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* drops empty commit" "
+		reset_rebase &&
+		git rebase $* c l &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'd l' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --keep-empty" "
+		reset_rebase &&
+		git rebase $* --keep-empty c l &&
+		test_cmp_rev c HEAD~3 &&
+		test_linear_range 'd k l' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --keep-empty keeps empty even if already in upstream" "
+		reset_rebase &&
+		git rebase $* --keep-empty j l &&
+		test_cmp_rev j HEAD~3 &&
+		test_linear_range 'd k l' j..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
 test_done
-- 
1.8.3.497.g83fddbe

[PATCH v5 4/7] add tests for rebasing root

[Martin von Zweigbergk]

---
 t/t3421-rebase-topology-linear.sh | 129 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 129 insertions(+)

diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
index 81e3d59..659a7b3 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -218,4 +218,133 @@ test_run_rebase failure -m
 test_run_rebase failure -i
 test_run_rebase failure -p
 
+#       m
+#      /
+# a---b---c---g
+#
+# x---y---B
+#
+# uppercase = cherry-picked
+# m = reverted b
+#
+# Reverted patches are there for tests to be able to check if a commit
+# that introduced the same change as another commit is
+# dropped. Without reverted commits, we could get false positives
+# because applying the patch succeeds, but simply results in no
+# changes.
+test_expect_success 'setup of linear history for test involving root' '
+	git checkout b &&
+	revert m b &&
+	git checkout --orphan disjoint &&
+	git rm -rf . &&
+	test_commit x &&
+	test_commit y &&
+	cherry_pick B b
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root" "
+		reset_rebase &&
+		git rebase $* --onto c --root y &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'x y' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* without --onto --root with disjoint history" "
+		reset_rebase &&
+		git rebase $* c y &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'x y' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root drops patch in onto" "
+		reset_rebase &&
+		git rebase $* --onto m --root B &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'x y' m..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root with merge-base does not go to root" "
+		reset_rebase &&
+		git rebase $* --onto m --root g &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'c g' m..
+	"
+}
+
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* without --onto --root with disjoint history drops patch in onto" "
+		reset_rebase &&
+		git rebase $* m B &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'x y' m..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --root on linear history is a no-op" "
+		reset_rebase &&
+		git rebase $* --root c &&
+		test_cmp_rev c HEAD
+	"
+}
+test_run_rebase failure ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* -f --root on linear history causes re-write" "
+		reset_rebase &&
+		git rebase $* -f --root c &&
+		! test_cmp_rev a HEAD~2 &&
+		test_linear_range 'a b c' HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
 test_done
-- 
1.8.3.497.g83fddbe

[PATCH v5 5/7] add tests for rebasing merged history

[Martin von Zweigbergk]

---
 t/t3400-rebase.sh                 |  31 +----
 t/t3401-rebase-partial.sh         |  45 -------
 t/t3404-rebase-interactive.sh     |  10 +-
 t/t3409-rebase-preserve-merges.sh |  53 --------
 t/t3425-rebase-topology-merges.sh | 258 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 260 insertions(+), 137 deletions(-)
 delete mode 100755 t/t3401-rebase-partial.sh
 create mode 100755 t/t3425-rebase-topology-merges.sh

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b58fa1a..b436ef4 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -40,13 +40,6 @@ test_expect_success 'prepare repository with topic branches' '
 	echo Side >>C &&
 	git add C &&
 	git commit -m "Add C" &&
-	git checkout -b nonlinear my-topic-branch &&
-	echo Edit >>B &&
-	git add B &&
-	git commit -m "Modify B" &&
-	git merge side &&
-	git checkout -b upstream-merged-nonlinear &&
-	git merge master &&
 	git checkout -f my-topic-branch &&
 	git tag topic
 '
@@ -106,31 +99,9 @@ test_expect_success 'rebase from ambiguous branch name' '
 	git rebase master
 '
 
-test_expect_success 'rebase after merge master' '
-	git checkout --detach refs/tags/topic &&
-	git branch -D topic &&
-	git reset --hard topic &&
-	git merge master &&
-	git rebase master &&
-	! (git show | grep "^Merge:")
-'
-
-test_expect_success 'rebase of history with merges is linearized' '
-	git checkout nonlinear &&
-	test 4 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
-test_expect_success 'rebase of history with merges after upstream merge is linearized' '
-	git checkout upstream-merged-nonlinear &&
-	test 5 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
 test_expect_success 'rebase a single mode change' '
 	git checkout master &&
+	git branch -D topic &&
 	echo 1 >X &&
 	git add X &&
 	test_tick &&
diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
deleted file mode 100755
index 7ba1797..0000000
--- a/t/t3401-rebase-partial.sh
+++ /dev/null
@@ -1,45 +0,0 @@
-#!/bin/sh
-#
-# Copyright (c) 2006 Yann Dirson, based on t3400 by Amos Waterland
-#
-
-test_description='git rebase should detect patches integrated upstream
-
-This test cherry-picks one local change of two into master branch, and
-checks that git rebase succeeds with only the second patch in the
-local branch.
-'
-. ./test-lib.sh
-
-test_expect_success 'prepare repository with topic branch' '
-	test_commit A &&
-	git checkout -b my-topic-branch &&
-	test_commit B &&
-	test_commit C &&
-	git checkout -f master &&
-	test_commit A2 A.t
-'
-
-test_expect_success 'pick top patch from topic branch into master' '
-	git cherry-pick C &&
-	git checkout -f my-topic-branch
-'
-
-test_debug '
-	git cherry master &&
-	git format-patch -k --stdout --full-index master >/dev/null &&
-	gitk --all & sleep 1
-'
-
-test_expect_success 'rebase topic branch against new master and check git am did not get halted' '
-	git rebase master &&
-	test_path_is_missing .git/rebase-apply
-'
-
-test_expect_success 'rebase --merge topic branch that was partially merged upstream' '
-	git reset --hard C &&
-	git rebase --merge master &&
-	test_path_is_missing .git/rebase-merge
-'
-
-test_done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index a58406d..ffcaf02 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -477,19 +477,11 @@ test_expect_success 'interrupted squash works as expected (case 2)' '
 	test $one = $(git rev-parse HEAD~2)
 '
 
-test_expect_success 'ignore patch if in upstream' '
-	HEAD=$(git rev-parse HEAD) &&
-	git checkout -b has-cherry-picked HEAD^ &&
+test_expect_success '--continue tries to commit, even for "edit"' '
 	echo unrelated > file7 &&
 	git add file7 &&
 	test_tick &&
 	git commit -m "unrelated change" &&
-	git cherry-pick $HEAD &&
-	EXPECT_COUNT=1 git rebase -i $HEAD &&
-	test $HEAD = $(git rev-parse HEAD^)
-'
-
-test_expect_success '--continue tries to commit, even for "edit"' '
 	parent=$(git rev-parse HEAD^) &&
 	test_tick &&
 	FAKE_LINES="edit 1" git rebase -i HEAD^ &&
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index 6de4e22..2e0c364 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -11,14 +11,6 @@ Run "git rebase -p" and check that merges are properly carried along
 GIT_AUTHOR_EMAIL=bogus_email_address
 export GIT_AUTHOR_EMAIL
 
-# Clone 1 (trivial merge):
-#
-# A1--A2  <-- origin/master
-#  \   \
-#   B1--M  <-- topic
-#    \
-#     B2  <-- origin/topic
-#
 # Clone 2 (conflicting merge):
 #
 # A1--A2--B3   <-- origin/master
@@ -36,16 +28,6 @@ export GIT_AUTHOR_EMAIL
 #     \--A3    <-- topic2
 #      \
 #       B2     <-- origin/topic
-#
-# Clone 4 (merge using second parent as base):
-#
-# A1--A2--B3   <-- origin/master
-#  \
-#   B1--A3--M  <-- topic
-#    \     /
-#     \--A4    <-- topic2
-#      \
-#       B2     <-- origin/topic
 
 test_expect_success 'setup for merge-preserving rebase' \
 	'echo First > A &&
@@ -58,20 +40,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git checkout -f master &&
 	echo Third >> A &&
 	git commit -a -m "Modify A2" &&
-
-	git clone ./. clone1 &&
-	(cd clone1 &&
-	git checkout -b topic origin/topic &&
-	git merge origin/master
-	) &&
-
-	git clone ./. clone4 &&
-	(
-		cd clone4 &&
-		git checkout -b topic origin/topic &&
-		git merge origin/master
-	) &&
-
 	echo Fifth > B &&
 	git add B &&
 	git commit -m "Add different B" &&
@@ -101,16 +69,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git commit -a -m "Modify B2"
 '
 
-test_expect_success 'rebase -p fakes interactive rebase' '
-	(
-	cd clone1 &&
-	git fetch &&
-	git rebase -p origin/topic &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_expect_success '--continue works after a conflict' '
 	(
 	cd clone2 &&
@@ -138,15 +96,4 @@ test_expect_success 'rebase -p preserves no-ff merges' '
 	)
 '
 
-test_expect_success 'rebase -p works when base inside second parent' '
-	(
-	cd clone4 &&
-	git fetch &&
-	git rebase -p HEAD^2 &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify B" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_done
diff --git a/t/t3425-rebase-topology-merges.sh b/t/t3425-rebase-topology-merges.sh
new file mode 100755
index 0000000..efd7dfd
--- /dev/null
+++ b/t/t3425-rebase-topology-merges.sh
@@ -0,0 +1,258 @@
+#!/bin/sh
+
+test_description='rebase topology tests with merges'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+test_revision_subjects () {
+	expected="$1"
+	shift
+	set -- $(git log --format=%s --no-walk=unsorted "$@")
+	test "$expected" = "$*"
+}
+
+# a---b-----------c
+#      \           \
+#       d-------e   \
+#        \       \   \
+#         n---o---w---v
+#              \
+#               z
+test_expect_success 'setup of non-linear-history' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git checkout b &&
+	test_commit d &&
+	test_commit e
+
+	git checkout c &&
+	test_commit g &&
+	revert h g &&
+	git checkout d &&
+	cherry_pick G g &&
+	test_commit i &&
+	git checkout b &&
+	test_commit f
+
+	git checkout d &&
+	test_commit n &&
+	test_commit o &&
+	test_merge w e &&
+	test_merge v c &&
+	git checkout o &&
+	test_commit z
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* after merge from upstream" "
+		reset_rebase &&
+		git rebase $* e w &&
+		test_cmp_rev e HEAD~2 &&
+		test_linear_range 'n o' e..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized in place" "
+		reset_rebase &&
+		git rebase $* d w &&
+		test_cmp_rev d HEAD~3 &&
+		test_linear_range "\'"$expected"\'" d..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'e n o' ''
+test_run_rebase success 'e n o' -m
+test_run_rebase success 'n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized upstream" "
+		reset_rebase &&
+		git rebase $* c w &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history with merges after upstream merge is linearized" "
+		reset_rebase &&
+		git rebase $* c v &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make all flavors of rebase use --topo-order
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_expect_success "rebase -p is no-op in non-linear history" "
+	reset_rebase &&
+	git rebase -p d w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p is no-op when base inside second parent" "
+	reset_rebase &&
+	git rebase -p e w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_failure "rebase -p --root on non-linear history is a no-op" "
+	reset_rebase &&
+	git rebase -p --root w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p re-creates merge from side branch" "
+	reset_rebase &&
+	git rebase -p z w &&
+	test_cmp_rev z HEAD^ &&
+	test_cmp_rev w^2 HEAD^2
+"
+
+test_expect_success "rebase -p re-creates internal merge" "
+	reset_rebase &&
+	git rebase -p c w &&
+	test_cmp_rev c HEAD~4 &&
+	test_cmp_rev HEAD^2^ HEAD~3 &&
+	test_revision_subjects 'd n e o w' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p can re-create two branches on onto" "
+	reset_rebase &&
+	git rebase -p --onto c d w &&
+	test_cmp_rev c HEAD~3 &&
+	test_cmp_rev c HEAD^2^ &&
+	test_revision_subjects 'n e o w' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+#       f
+#      /
+# a---b---c---g---h
+#      \
+#       d---G---i
+#        \       \
+#         e-------u
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for patch-equivalence tests' '
+	git checkout e &&
+	test_merge u i
+'
+
+test_expect_success "rebase -p re-creates history around dropped commit matching upstream" "
+	reset_rebase &&
+	git rebase -p h u &&
+	test_cmp_rev h HEAD~3 &&
+	test_cmp_rev HEAD^2^ HEAD~2 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p --onto in merged history drops patches in upstream" "
+	reset_rebase &&
+	git rebase -p --onto f h u &&
+	test_cmp_rev f HEAD~3 &&
+	test_cmp_rev HEAD^2^ HEAD~2 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p --onto in merged history does not drop patches in onto" "
+	reset_rebase &&
+	git rebase -p --onto h f u &&
+	test_cmp_rev h HEAD~3 &&
+	test_cmp_rev HEAD^2~2 HEAD~2 &&
+	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
+"
+
+# a---b---c---g---h
+#      \
+#       d---G---s
+#        \   \ /
+#         \   X
+#          \ / \
+#           e---t
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for dropping whole side' '
+	git checkout G &&
+	test_merge s e &&
+	git checkout e &&
+	test_merge t G
+'
+
+test_expect_failure "rebase -p drops merge commit when entire first-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h s &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+test_expect_success "rebase -p drops merge commit when entire second-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h t &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+# a---b---c
+#      \
+#       d---e
+#        \   \
+#         n---r
+#          \
+#           o
+#
+# r = tree-same with n
+test_expect_success 'setup of non-linear-history for empty commits' '
+	git checkout n &&
+	git merge --no-commit e &&
+	git reset n . &&
+	git commit -m r &&
+	git reset --hard &&
+	git clean -f &&
+	git tag r
+'
+
+test_expect_success "rebase -p re-creates empty internal merge commit" "
+	reset_rebase &&
+	git rebase -p c r &&
+	test_cmp_rev c HEAD~3 &&
+	test_cmp_rev HEAD^2^ HEAD~2 &&
+	test_revision_subjects 'd e n r' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p re-creates empty merge commit" "
+	reset_rebase &&
+	git rebase -p o r &&
+	test_cmp_rev e HEAD^2 &&
+	test_cmp_rev o HEAD^ &&
+	test_revision_subjects 'r' HEAD
+"
+
+test_done
-- 
1.8.3.497.g83fddbe

Re: [PATCH v5 5/7] add tests for rebasing merged history

[Junio C Hamano]

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> ---
> +#TODO: make all flavors of rebase use --topo-order
> +test_run_rebase success 'e n o' ''
> +test_run_rebase success 'e n o' -m
> +test_run_rebase success 'n o e' -i

I do not quite follow this TODO.

While I think it would be nice to update "rebase" so that all
variants consider replaying the commits in the same order, in this
history you have:

+# a---b-----------c
+#      \           \
+#       d-------e   \
+#        \       \   \
+#         n---o---w---v
+#              \
+#               z

as long as o comes after n and e is shown before n or after o, which
all three expected results satisify, it is in --topo-order, isn't it?

The same comment applies to the other TODO that talks about eno/noe
differences.

> +test_expect_success "rebase -p re-creates merge from side branch" "
> +     reset_rebase &&
> +     git rebase -p z w &&
> +     test_cmp_rev z HEAD^ &&
> +     test_cmp_rev w^2 HEAD^2
> +"

Hmm, turning the left one to the right one?

+#       d-------e               d-------e    
+#        \       \               \       \   
+#         n---o---w     ===>      n---o   \
+#              \                       \   \ 
+#               z                       z---W

If w were a merge of o into e (i.e. w^1 were e not o), what should
happen?  Would we get the same topology?

In other words, when asked to replay w on top of z, how would we
decide which parent to keep (in this case, e is kept)?

> +test_expect_success "rebase -p can re-create two branches on onto" "
> +     reset_rebase &&
> +     git rebase -p --onto c d w &&
> +     test_cmp_rev c HEAD~3 &&
> +     test_cmp_rev c HEAD^2^ &&
> +     test_revision_subjects 'n e o w' HEAD~2 HEAD^2 HEAD^ HEAD
> +"

Nice (so are all the rest).

Re: [PATCH v5 5/7] add tests for rebasing merged history

[Martin von Zweigbergk]

On Tue, Jun 4, 2013 at 10:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> ---
>> +#TODO: make all flavors of rebase use --topo-order
>> +test_run_rebase success 'e n o' ''
>> +test_run_rebase success 'e n o' -m
>> +test_run_rebase success 'n o e' -i
>
> I do not quite follow this TODO.
>
> While I think it would be nice to update "rebase" so that all
> variants consider replaying the commits in the same order, in this
> history you have:
>
> +# a---b-----------c
> +#      \           \
> +#       d-------e   \
> +#        \       \   \
> +#         n---o---w---v
> +#              \
> +#               z
>
> as long as o comes after n and e is shown before n or after o, which
> all three expected results satisify, it is in --topo-order, isn't it?

True, the TODO was too specific. I intended to get the list of commits
to rebase for all kinds of rebase by using 'git rev-list
--topo-order', but it doesn't really matter how the order is decided;
my goal was just to make it consistent. I'll update the TODOs.

>> +test_expect_success "rebase -p re-creates merge from side branch" "
>> +     reset_rebase &&
>> +     git rebase -p z w &&
>> +     test_cmp_rev z HEAD^ &&
>> +     test_cmp_rev w^2 HEAD^2
>> +"
>
> Hmm, turning the left one to the right one?
>
> +#       d-------e               d-------e
> +#        \       \               \       \
> +#         n---o---w     ===>      n---o   \
> +#              \                       \   \
> +#               z                       z---W
>
> If w were a merge of o into e (i.e. w^1 were e not o), what should
> happen?  Would we get the same topology?

Yes, it seems like it does yield the same topology. That seems to be
what I tested at first. Search for "wrong" in [1]. I think Johannes's
point was that it was not realistic, not that he's against it working
in the same way independent of parent order. I don't feel strongly on
whether to include a test for each direction. Unless others do, I
guess I'll leave it as is. (But I did add a test case just now to see,
so it's very little work for me if someone does want it included.)

> In other words, when asked to replay w on top of z, how would we
> decide which parent to keep (in this case, e is kept)?

Keep any parent that is not an ancestor of the new base? Or something like that.


  [1] http://thread.gmane.org/gmane.comp.version-control.git/205796/focus=205806

Re: [PATCH v5 5/7] add tests for rebasing merged history

[Johannes Sixt]

Am 6/4/2013 19:18, schrieb Junio C Hamano:
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
> 
>> ---
>> +#TODO: make all flavors of rebase use --topo-order
>> +test_run_rebase success 'e n o' ''
>> +test_run_rebase success 'e n o' -m
>> +test_run_rebase success 'n o e' -i
> 
> I do not quite follow this TODO.
> 
> While I think it would be nice to update "rebase" so that all
> variants consider replaying the commits in the same order, in this
> history you have:
> 
> +# a---b-----------c
> +#      \           \
> +#       d-------e   \
> +#        \       \   \
> +#         n---o---w---v
> +#              \
> +#               z
> 
> as long as o comes after n and e is shown before n or after o, which
> all three expected results satisify, it is in --topo-order, isn't it?

The comment is really just about the inconsistency, not about a request to
have a guaranteed order among the parents of a merge commit.

Having said that, wouldn't it be useful (generally, not just in this
context) to have a guarantee in which order --topo-order lists parents of
a merge?

>> +test_expect_success "rebase -p re-creates merge from side branch" "
>> +     reset_rebase &&
>> +     git rebase -p z w &&
>> +     test_cmp_rev z HEAD^ &&
>> +     test_cmp_rev w^2 HEAD^2
>> +"
> 
> Hmm, turning the left one to the right one?
> 
> +#       d-------e               d-------e    
> +#        \       \               \       \   
> +#         n---o---w     ===>      n---o   \
> +#              \                       \   \ 
> +#               z                       z---W
> 
> If w were a merge of o into e (i.e. w^1 were e not o), what should
> happen?  Would we get the same topology?

'git rebase -p z w' is a nonsense request in this situation. (I.e., there
is no requirement on the result.) At best, we could detect it and bail out
or warn.

-- Hannes

[PATCH v5 6/7] t3406: modernize style

[Martin von Zweigbergk]

Update the following:

 - Quote 'setup'
 - Remove blank lines within test case body
 - Use test_commit instead of custom quick_one
 - Create branch "topic" from tag created by test_commit
---
 t/t3406-rebase-message.sh | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index e6a9a0d..fe8c27f 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -4,27 +4,17 @@ test_description='messages from rebase operation'
 
 . ./test-lib.sh
 
-quick_one () {
-	echo "$1" >"file$1" &&
-	git add "file$1" &&
-	test_tick &&
-	git commit -m "$1"
-}
+test_expect_success 'setup' '
+	test_commit O fileO &&
+	test_commit X fileX &&
+	test_commit A fileA &&
+	test_commit B fileB &&
+	test_commit Y fileY &&
 
-test_expect_success setup '
-	quick_one O &&
-	git branch topic &&
-	quick_one X &&
-	quick_one A &&
-	quick_one B &&
-	quick_one Y &&
-
-	git checkout topic &&
-	quick_one A &&
-	quick_one B &&
-	quick_one Z &&
+	git checkout -b topic O &&
+	git cherry-pick A B &&
+	test_commit Z fileZ &&
 	git tag start
-
 '
 
 cat >expect <<\EOF
@@ -34,12 +24,10 @@ Committed: 0003 Z
 EOF
 
 test_expect_success 'rebase -m' '
-
 	git rebase -m master >report &&
 	sed -n -e "/^Already applied: /p" \
 		-e "/^Committed: /p" report >actual &&
 	test_cmp expect actual
-
 '
 
 test_expect_success 'rebase --stat' '
-- 
1.8.3.497.g83fddbe

[PATCH v5 7/7] tests: move test for rebase messages from t3400 to t3406

[Martin von Zweigbergk]

t3406 is supposed to test "messages from rebase operation", so let's
move tests in t3400 that fit that description into 3406. Most of the
functionality they tested, except for the messages, has now been
subsumed by t3420.
---
 t/t3400-rebase.sh         | 22 ----------------------
 t/t3406-rebase-message.sh | 22 ++++++++++++++++++++++
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b436ef4..45a55e9 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -59,28 +59,6 @@ test_expect_success 'rebase against master' '
 	git rebase master
 '
 
-test_expect_success 'rebase against master twice' '
-	git rebase master >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date" out
-'
-
-test_expect_success 'rebase against master twice with --force' '
-	git rebase --force-rebase master >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date, rebase forced" out
-'
-
-test_expect_success 'rebase against master twice from another branch' '
-	git checkout my-topic-branch^ &&
-	git rebase master my-topic-branch >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date" out
-'
-
-test_expect_success 'rebase fast-forward to master' '
-	git checkout my-topic-branch^ &&
-	git rebase my-topic-branch >out &&
-	test_i18ngrep "Fast-forwarded HEAD to my-topic-branch" out
-'
-
 test_expect_success 'the rebase operation should not have destroyed author information' '
 	! (git log | grep "Author:" | grep "<>")
 '
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index fe8c27f..0392e36 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -30,6 +30,28 @@ test_expect_success 'rebase -m' '
 	test_cmp expect actual
 '
 
+test_expect_success 'rebase against master twice' '
+	git rebase master >out &&
+	test_i18ngrep "Current branch topic is up to date" out
+'
+
+test_expect_success 'rebase against master twice with --force' '
+	git rebase --force-rebase master >out &&
+	test_i18ngrep "Current branch topic is up to date, rebase forced" out
+'
+
+test_expect_success 'rebase against master twice from another branch' '
+	git checkout topic^ &&
+	git rebase master topic >out &&
+	test_i18ngrep "Current branch topic is up to date" out
+'
+
+test_expect_success 'rebase fast-forward to master' '
+	git checkout topic^ &&
+	git rebase topic >out &&
+	test_i18ngrep "Fast-forwarded HEAD to topic" out
+'
+
 test_expect_success 'rebase --stat' '
 	git reset --hard start &&
         git rebase --stat master >diffstat.txt &&
-- 
1.8.3.497.g83fddbe

[PATCH v6 0/8] Rebase topology test

[Martin von Zweigbergk]

Changes since v5:

 * Improved test_linear_range
 * Changed TODOs to be about consistency, not --topo-order

Martin von Zweigbergk (7):
  add simple tests of consistency across rebase types
  add tests for rebasing with patch-equivalence present
  add tests for rebasing of empty commits
  add tests for rebasing root
  add tests for rebasing merged history
  t3406: modernize style
  tests: move test for rebase messages from t3400 to t3406

 t/lib-rebase.sh                   |  33 ++++
 t/t3400-rebase.sh                 |  53 +-----
 t/t3401-rebase-partial.sh         |  69 --------
 t/t3404-rebase-interactive.sh     |  10 +-
 t/t3406-rebase-message.sh         |  50 +++---
 t/t3409-rebase-preserve-merges.sh |  53 ------
 t/t3421-rebase-topology-linear.sh | 350 ++++++++++++++++++++++++++++++++++++++
 t/t3425-rebase-topology-merges.sh | 258 ++++++++++++++++++++++++++++
 8 files changed, 673 insertions(+), 203 deletions(-)
 delete mode 100755 t/t3401-rebase-partial.sh
 create mode 100755 t/t3421-rebase-topology-linear.sh
 create mode 100755 t/t3425-rebase-topology-merges.sh

-- 
1.8.3.497.g83fddbe

[PATCH v6 1/7] add simple tests of consistency across rebase types

[Martin von Zweigbergk]

Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
 t/lib-rebase.sh                   | 16 ++++++++
 t/t3421-rebase-topology-linear.sh | 78 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+)
 create mode 100755 t/t3421-rebase-topology-linear.sh

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6ccf797..1e0ff28 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -65,3 +65,19 @@ EOF
 	test_set_editor "$(pwd)/fake-editor.sh"
 	chmod a+x fake-editor.sh
 }
+
+# checks that the revisions in "$2" represent a linear range with the
+# subjects in "$1"
+test_linear_range () {
+	revlist_merges=$(git rev-list --merges "$2") &&
+	test -z "$revlist_merges" &&
+	expected=$1
+	set -- $(git log --reverse --format=%s "$2")
+	test "$expected" = "$*"
+}
+
+reset_rebase () {
+	test_might_fail git rebase --abort &&
+	git reset --hard &&
+	git clean -f
+}
diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
new file mode 100755
index 0000000..60365d1
--- /dev/null
+++ b/t/t3421-rebase-topology-linear.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='basic rebase topology tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+# a---b---c
+#      \
+#       d---e
+test_expect_success 'setup' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git checkout b &&
+	test_commit d &&
+	test_commit e
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "simple rebase $*" "
+		reset_rebase &&
+		git rebase $* c e &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'd e' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* is no-op if upstream is an ancestor" "
+		reset_rebase &&
+		git rebase $* b e &&
+		test_cmp_rev e HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* -f rewrites even if upstream is an ancestor" "
+		reset_rebase &&
+		git rebase $* -f b e &&
+		! test_cmp_rev e HEAD &&
+		test_cmp_rev b HEAD~2 &&
+		test_linear_range 'd e' b..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* fast-forwards from ancestor of upstream" "
+		reset_rebase &&
+		git rebase $* e b &&
+		test_cmp_rev e HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_done
-- 
1.8.3.497.g83fddbe

[PATCH v6 2/7] add tests for rebasing with patch-equivalence present

[Martin von Zweigbergk]

Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
 t/lib-rebase.sh                   | 17 ++++++++
 t/t3421-rebase-topology-linear.sh | 85 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+)

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 1e0ff28..4b74ae4 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -81,3 +81,20 @@ reset_rebase () {
 	git reset --hard &&
 	git clean -f
 }
+
+cherry_pick () {
+	git cherry-pick -n "$2" &&
+	git commit -m "$1" &&
+	git tag "$1"
+}
+
+revert () {
+	git revert -n "$2" &&
+	git commit -m "$1" &&
+	git tag "$1"
+}
+
+make_empty () {
+	git commit --allow-empty -m "$1" &&
+	git tag "$1"
+}
diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
index 60365d1..ddcbfc6 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -75,4 +75,89 @@ test_run_rebase success -m
 test_run_rebase success -i
 test_run_rebase success -p
 
+#       f
+#      /
+# a---b---c---g---h
+#      \
+#       d---G---i
+#
+# uppercase = cherry-picked
+# h = reverted g
+#
+# Reverted patches are there for tests to be able to check if a commit
+# that introduced the same change as another commit is
+# dropped. Without reverted commits, we could get false positives
+# because applying the patch succeeds, but simply results in no
+# changes.
+test_expect_success 'setup of linear history for range selection tests' '
+	git checkout c &&
+	test_commit g &&
+	revert h g &&
+	git checkout d &&
+	cherry_pick G g &&
+	test_commit i &&
+	git checkout b &&
+	test_commit f
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* drops patches in upstream" "
+		reset_rebase &&
+		git rebase $* h i &&
+		test_cmp_rev h HEAD~2 &&
+		test_linear_range 'd i' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* can drop last patch if in upstream" "
+		reset_rebase &&
+		git rebase $* h G &&
+		test_cmp_rev h HEAD^ &&
+		test_linear_range 'd' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto drops patches in upstream" "
+		reset_rebase &&
+		git rebase $* --onto f h i &&
+		test_cmp_rev f HEAD~2 &&
+		test_linear_range 'd i' f..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto does not drop patches in onto" "
+		reset_rebase &&
+		git rebase $* --onto h f i &&
+		test_cmp_rev h HEAD~3 &&
+		test_linear_range 'd G i' h..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
 test_done
-- 
1.8.3.497.g83fddbe

[PATCH v6 3/7] add tests for rebasing of empty commits

[Martin von Zweigbergk]

Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
 t/t3401-rebase-partial.sh         | 24 ----------------
 t/t3421-rebase-topology-linear.sh | 58 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 24 deletions(-)

diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
index 58f4823..7ba1797 100755
--- a/t/t3401-rebase-partial.sh
+++ b/t/t3401-rebase-partial.sh
@@ -42,28 +42,4 @@ test_expect_success 'rebase --merge topic branch that was partially merged upstr
 	test_path_is_missing .git/rebase-merge
 '
 
-test_expect_success 'rebase ignores empty commit' '
-	git reset --hard A &&
-	git commit --allow-empty -m empty &&
-	test_commit D &&
-	git rebase C &&
-	test "$(git log --format=%s C..)" = "D"
-'
-
-test_expect_success 'rebase --keep-empty' '
-	git reset --hard D &&
-	git rebase --keep-empty C &&
-	test "$(git log --format=%s C..)" = "D
-empty"
-'
-
-test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' '
-	git reset --hard A &&
-	git commit --allow-empty -m also-empty &&
-	git rebase --keep-empty D &&
-	test "$(git log --format=%s A..)" = "also-empty
-D
-empty"
-'
-
 test_done
diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
index ddcbfc6..f19f0d0 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -160,4 +160,62 @@ test_run_rebase success -m
 test_run_rebase success -i
 test_run_rebase success -p
 
+# a---b---c---j!
+#      \
+#       d---k!--l
+#
+# ! = empty
+test_expect_success 'setup of linear history for empty commit tests' '
+	git checkout c &&
+	make_empty j &&
+	git checkout d &&
+	make_empty k &&
+	test_commit l
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* drops empty commit" "
+		reset_rebase &&
+		git rebase $* c l &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'd l' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --keep-empty" "
+		reset_rebase &&
+		git rebase $* --keep-empty c l &&
+		test_cmp_rev c HEAD~3 &&
+		test_linear_range 'd k l' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --keep-empty keeps empty even if already in upstream" "
+		reset_rebase &&
+		git rebase $* --keep-empty j l &&
+		test_cmp_rev j HEAD~3 &&
+		test_linear_range 'd k l' j..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
 test_done
-- 
1.8.3.497.g83fddbe

[PATCH v6 4/7] add tests for rebasing root

[Martin von Zweigbergk]

Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
 t/t3421-rebase-topology-linear.sh | 129 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 129 insertions(+)

diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
index f19f0d0..e67add6 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -218,4 +218,133 @@ test_run_rebase failure -m
 test_run_rebase failure -i
 test_run_rebase failure -p
 
+#       m
+#      /
+# a---b---c---g
+#
+# x---y---B
+#
+# uppercase = cherry-picked
+# m = reverted b
+#
+# Reverted patches are there for tests to be able to check if a commit
+# that introduced the same change as another commit is
+# dropped. Without reverted commits, we could get false positives
+# because applying the patch succeeds, but simply results in no
+# changes.
+test_expect_success 'setup of linear history for test involving root' '
+	git checkout b &&
+	revert m b &&
+	git checkout --orphan disjoint &&
+	git rm -rf . &&
+	test_commit x &&
+	test_commit y &&
+	cherry_pick B b
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root" "
+		reset_rebase &&
+		git rebase $* --onto c --root y &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'x y' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* without --onto --root with disjoint history" "
+		reset_rebase &&
+		git rebase $* c y &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'x y' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root drops patch in onto" "
+		reset_rebase &&
+		git rebase $* --onto m --root B &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'x y' m..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --onto --root with merge-base does not go to root" "
+		reset_rebase &&
+		git rebase $* --onto m --root g &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'c g' m..
+	"
+}
+
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* without --onto --root with disjoint history drops patch in onto" "
+		reset_rebase &&
+		git rebase $* m B &&
+		test_cmp_rev m HEAD~2 &&
+		test_linear_range 'x y' m..
+	"
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* --root on linear history is a no-op" "
+		reset_rebase &&
+		git rebase $* --root c &&
+		test_cmp_rev c HEAD
+	"
+}
+test_run_rebase failure ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* -f --root on linear history causes re-write" "
+		reset_rebase &&
+		git rebase $* -f --root c &&
+		! test_cmp_rev a HEAD~2 &&
+		test_linear_range 'a b c' HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
 test_done
-- 
1.8.3.497.g83fddbe

[PATCH v6 5/7] add tests for rebasing merged history

[Martin von Zweigbergk]

Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
 t/t3400-rebase.sh                 |  31 +----
 t/t3401-rebase-partial.sh         |  45 -------
 t/t3404-rebase-interactive.sh     |  10 +-
 t/t3409-rebase-preserve-merges.sh |  53 --------
 t/t3425-rebase-topology-merges.sh | 258 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 260 insertions(+), 137 deletions(-)
 delete mode 100755 t/t3401-rebase-partial.sh
 create mode 100755 t/t3425-rebase-topology-merges.sh

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b58fa1a..b436ef4 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -40,13 +40,6 @@ test_expect_success 'prepare repository with topic branches' '
 	echo Side >>C &&
 	git add C &&
 	git commit -m "Add C" &&
-	git checkout -b nonlinear my-topic-branch &&
-	echo Edit >>B &&
-	git add B &&
-	git commit -m "Modify B" &&
-	git merge side &&
-	git checkout -b upstream-merged-nonlinear &&
-	git merge master &&
 	git checkout -f my-topic-branch &&
 	git tag topic
 '
@@ -106,31 +99,9 @@ test_expect_success 'rebase from ambiguous branch name' '
 	git rebase master
 '
 
-test_expect_success 'rebase after merge master' '
-	git checkout --detach refs/tags/topic &&
-	git branch -D topic &&
-	git reset --hard topic &&
-	git merge master &&
-	git rebase master &&
-	! (git show | grep "^Merge:")
-'
-
-test_expect_success 'rebase of history with merges is linearized' '
-	git checkout nonlinear &&
-	test 4 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
-test_expect_success 'rebase of history with merges after upstream merge is linearized' '
-	git checkout upstream-merged-nonlinear &&
-	test 5 = $(git rev-list master.. | wc -l) &&
-	git rebase master &&
-	test 3 = $(git rev-list master.. | wc -l)
-'
-
 test_expect_success 'rebase a single mode change' '
 	git checkout master &&
+	git branch -D topic &&
 	echo 1 >X &&
 	git add X &&
 	test_tick &&
diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
deleted file mode 100755
index 7ba1797..0000000
--- a/t/t3401-rebase-partial.sh
+++ /dev/null
@@ -1,45 +0,0 @@
-#!/bin/sh
-#
-# Copyright (c) 2006 Yann Dirson, based on t3400 by Amos Waterland
-#
-
-test_description='git rebase should detect patches integrated upstream
-
-This test cherry-picks one local change of two into master branch, and
-checks that git rebase succeeds with only the second patch in the
-local branch.
-'
-. ./test-lib.sh
-
-test_expect_success 'prepare repository with topic branch' '
-	test_commit A &&
-	git checkout -b my-topic-branch &&
-	test_commit B &&
-	test_commit C &&
-	git checkout -f master &&
-	test_commit A2 A.t
-'
-
-test_expect_success 'pick top patch from topic branch into master' '
-	git cherry-pick C &&
-	git checkout -f my-topic-branch
-'
-
-test_debug '
-	git cherry master &&
-	git format-patch -k --stdout --full-index master >/dev/null &&
-	gitk --all & sleep 1
-'
-
-test_expect_success 'rebase topic branch against new master and check git am did not get halted' '
-	git rebase master &&
-	test_path_is_missing .git/rebase-apply
-'
-
-test_expect_success 'rebase --merge topic branch that was partially merged upstream' '
-	git reset --hard C &&
-	git rebase --merge master &&
-	test_path_is_missing .git/rebase-merge
-'
-
-test_done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index a58406d..ffcaf02 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -477,19 +477,11 @@ test_expect_success 'interrupted squash works as expected (case 2)' '
 	test $one = $(git rev-parse HEAD~2)
 '
 
-test_expect_success 'ignore patch if in upstream' '
-	HEAD=$(git rev-parse HEAD) &&
-	git checkout -b has-cherry-picked HEAD^ &&
+test_expect_success '--continue tries to commit, even for "edit"' '
 	echo unrelated > file7 &&
 	git add file7 &&
 	test_tick &&
 	git commit -m "unrelated change" &&
-	git cherry-pick $HEAD &&
-	EXPECT_COUNT=1 git rebase -i $HEAD &&
-	test $HEAD = $(git rev-parse HEAD^)
-'
-
-test_expect_success '--continue tries to commit, even for "edit"' '
 	parent=$(git rev-parse HEAD^) &&
 	test_tick &&
 	FAKE_LINES="edit 1" git rebase -i HEAD^ &&
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index 6de4e22..2e0c364 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -11,14 +11,6 @@ Run "git rebase -p" and check that merges are properly carried along
 GIT_AUTHOR_EMAIL=bogus_email_address
 export GIT_AUTHOR_EMAIL
 
-# Clone 1 (trivial merge):
-#
-# A1--A2  <-- origin/master
-#  \   \
-#   B1--M  <-- topic
-#    \
-#     B2  <-- origin/topic
-#
 # Clone 2 (conflicting merge):
 #
 # A1--A2--B3   <-- origin/master
@@ -36,16 +28,6 @@ export GIT_AUTHOR_EMAIL
 #     \--A3    <-- topic2
 #      \
 #       B2     <-- origin/topic
-#
-# Clone 4 (merge using second parent as base):
-#
-# A1--A2--B3   <-- origin/master
-#  \
-#   B1--A3--M  <-- topic
-#    \     /
-#     \--A4    <-- topic2
-#      \
-#       B2     <-- origin/topic
 
 test_expect_success 'setup for merge-preserving rebase' \
 	'echo First > A &&
@@ -58,20 +40,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git checkout -f master &&
 	echo Third >> A &&
 	git commit -a -m "Modify A2" &&
-
-	git clone ./. clone1 &&
-	(cd clone1 &&
-	git checkout -b topic origin/topic &&
-	git merge origin/master
-	) &&
-
-	git clone ./. clone4 &&
-	(
-		cd clone4 &&
-		git checkout -b topic origin/topic &&
-		git merge origin/master
-	) &&
-
 	echo Fifth > B &&
 	git add B &&
 	git commit -m "Add different B" &&
@@ -101,16 +69,6 @@ test_expect_success 'setup for merge-preserving rebase' \
 	git commit -a -m "Modify B2"
 '
 
-test_expect_success 'rebase -p fakes interactive rebase' '
-	(
-	cd clone1 &&
-	git fetch &&
-	git rebase -p origin/topic &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_expect_success '--continue works after a conflict' '
 	(
 	cd clone2 &&
@@ -138,15 +96,4 @@ test_expect_success 'rebase -p preserves no-ff merges' '
 	)
 '
 
-test_expect_success 'rebase -p works when base inside second parent' '
-	(
-	cd clone4 &&
-	git fetch &&
-	git rebase -p HEAD^2 &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify B" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote-tracking branch " | wc -l)
-	)
-'
-
 test_done
diff --git a/t/t3425-rebase-topology-merges.sh b/t/t3425-rebase-topology-merges.sh
new file mode 100755
index 0000000..5400a05
--- /dev/null
+++ b/t/t3425-rebase-topology-merges.sh
@@ -0,0 +1,258 @@
+#!/bin/sh
+
+test_description='rebase topology tests with merges'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+test_revision_subjects () {
+	expected="$1"
+	shift
+	set -- $(git log --format=%s --no-walk=unsorted "$@")
+	test "$expected" = "$*"
+}
+
+# a---b-----------c
+#      \           \
+#       d-------e   \
+#        \       \   \
+#         n---o---w---v
+#              \
+#               z
+test_expect_success 'setup of non-linear-history' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git checkout b &&
+	test_commit d &&
+	test_commit e
+
+	git checkout c &&
+	test_commit g &&
+	revert h g &&
+	git checkout d &&
+	cherry_pick G g &&
+	test_commit i &&
+	git checkout b &&
+	test_commit f
+
+	git checkout d &&
+	test_commit n &&
+	test_commit o &&
+	test_merge w e &&
+	test_merge v c &&
+	git checkout o &&
+	test_commit z
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* after merge from upstream" "
+		reset_rebase &&
+		git rebase $* e w &&
+		test_cmp_rev e HEAD~2 &&
+		test_linear_range 'n o' e..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized in place" "
+		reset_rebase &&
+		git rebase $* d w &&
+		test_cmp_rev d HEAD~3 &&
+		test_linear_range "\'"$expected"\'" d..
+	"
+}
+#TODO: make order consistent across all flavors of rebase
+test_run_rebase success 'e n o' ''
+test_run_rebase success 'e n o' -m
+test_run_rebase success 'n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history is linearized upstream" "
+		reset_rebase &&
+		git rebase $* c w &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make order consistent across all flavors of rebase
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_run_rebase () {
+	result=$1
+	shift
+	expected=$1
+	shift
+	test_expect_$result "rebase $* of non-linear history with merges after upstream merge is linearized" "
+		reset_rebase &&
+		git rebase $* c v &&
+		test_cmp_rev c HEAD~4 &&
+		test_linear_range "\'"$expected"\'" c..
+	"
+}
+#TODO: make order consistent across all flavors of rebase
+test_run_rebase success 'd e n o' ''
+test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' -i
+
+test_expect_success "rebase -p is no-op in non-linear history" "
+	reset_rebase &&
+	git rebase -p d w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p is no-op when base inside second parent" "
+	reset_rebase &&
+	git rebase -p e w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_failure "rebase -p --root on non-linear history is a no-op" "
+	reset_rebase &&
+	git rebase -p --root w &&
+	test_cmp_rev w HEAD
+"
+
+test_expect_success "rebase -p re-creates merge from side branch" "
+	reset_rebase &&
+	git rebase -p z w &&
+	test_cmp_rev z HEAD^ &&
+	test_cmp_rev w^2 HEAD^2
+"
+
+test_expect_success "rebase -p re-creates internal merge" "
+	reset_rebase &&
+	git rebase -p c w &&
+	test_cmp_rev c HEAD~4 &&
+	test_cmp_rev HEAD^2^ HEAD~3 &&
+	test_revision_subjects 'd n e o w' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p can re-create two branches on onto" "
+	reset_rebase &&
+	git rebase -p --onto c d w &&
+	test_cmp_rev c HEAD~3 &&
+	test_cmp_rev c HEAD^2^ &&
+	test_revision_subjects 'n e o w' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+#       f
+#      /
+# a---b---c---g---h
+#      \
+#       d---G---i
+#        \       \
+#         e-------u
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for patch-equivalence tests' '
+	git checkout e &&
+	test_merge u i
+'
+
+test_expect_success "rebase -p re-creates history around dropped commit matching upstream" "
+	reset_rebase &&
+	git rebase -p h u &&
+	test_cmp_rev h HEAD~3 &&
+	test_cmp_rev HEAD^2^ HEAD~2 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p --onto in merged history drops patches in upstream" "
+	reset_rebase &&
+	git rebase -p --onto f h u &&
+	test_cmp_rev f HEAD~3 &&
+	test_cmp_rev HEAD^2^ HEAD~2 &&
+	test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p --onto in merged history does not drop patches in onto" "
+	reset_rebase &&
+	git rebase -p --onto h f u &&
+	test_cmp_rev h HEAD~3 &&
+	test_cmp_rev HEAD^2~2 HEAD~2 &&
+	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
+"
+
+# a---b---c---g---h
+#      \
+#       d---G---s
+#        \   \ /
+#         \   X
+#          \ / \
+#           e---t
+#
+# uppercase = cherry-picked
+# h = reverted g
+test_expect_success 'setup of non-linear-history for dropping whole side' '
+	git checkout G &&
+	test_merge s e &&
+	git checkout e &&
+	test_merge t G
+'
+
+test_expect_failure "rebase -p drops merge commit when entire first-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h s &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+test_expect_success "rebase -p drops merge commit when entire second-parent side is dropped" "
+	reset_rebase &&
+	git rebase -p h t &&
+	test_cmp_rev h HEAD~2 &&
+	test_linear_range 'd e' h..
+"
+
+# a---b---c
+#      \
+#       d---e
+#        \   \
+#         n---r
+#          \
+#           o
+#
+# r = tree-same with n
+test_expect_success 'setup of non-linear-history for empty commits' '
+	git checkout n &&
+	git merge --no-commit e &&
+	git reset n . &&
+	git commit -m r &&
+	git reset --hard &&
+	git clean -f &&
+	git tag r
+'
+
+test_expect_success "rebase -p re-creates empty internal merge commit" "
+	reset_rebase &&
+	git rebase -p c r &&
+	test_cmp_rev c HEAD~3 &&
+	test_cmp_rev HEAD^2^ HEAD~2 &&
+	test_revision_subjects 'd e n r' HEAD~2 HEAD^2 HEAD^ HEAD
+"
+
+test_expect_success "rebase -p re-creates empty merge commit" "
+	reset_rebase &&
+	git rebase -p o r &&
+	test_cmp_rev e HEAD^2 &&
+	test_cmp_rev o HEAD^ &&
+	test_revision_subjects 'r' HEAD
+"
+
+test_done
-- 
1.8.3.497.g83fddbe

[PATCH v6 6/7] t3406: modernize style

[Martin von Zweigbergk]

Update the following:

 - Quote 'setup'
 - Remove blank lines within test case body
 - Use test_commit instead of custom quick_one
 - Create branch "topic" from tag created by test_commit

Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
 t/t3406-rebase-message.sh | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index e6a9a0d..fe8c27f 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -4,27 +4,17 @@ test_description='messages from rebase operation'
 
 . ./test-lib.sh
 
-quick_one () {
-	echo "$1" >"file$1" &&
-	git add "file$1" &&
-	test_tick &&
-	git commit -m "$1"
-}
+test_expect_success 'setup' '
+	test_commit O fileO &&
+	test_commit X fileX &&
+	test_commit A fileA &&
+	test_commit B fileB &&
+	test_commit Y fileY &&
 
-test_expect_success setup '
-	quick_one O &&
-	git branch topic &&
-	quick_one X &&
-	quick_one A &&
-	quick_one B &&
-	quick_one Y &&
-
-	git checkout topic &&
-	quick_one A &&
-	quick_one B &&
-	quick_one Z &&
+	git checkout -b topic O &&
+	git cherry-pick A B &&
+	test_commit Z fileZ &&
 	git tag start
-
 '
 
 cat >expect <<\EOF
@@ -34,12 +24,10 @@ Committed: 0003 Z
 EOF
 
 test_expect_success 'rebase -m' '
-
 	git rebase -m master >report &&
 	sed -n -e "/^Already applied: /p" \
 		-e "/^Committed: /p" report >actual &&
 	test_cmp expect actual
-
 '
 
 test_expect_success 'rebase --stat' '
-- 
1.8.3.497.g83fddbe

[PATCH v6 7/7] tests: move test for rebase messages from t3400 to t3406

[Martin von Zweigbergk]

t3406 is supposed to test "messages from rebase operation", so let's
move tests in t3400 that fit that description into 3406. Most of the
functionality they tested, except for the messages, has now been
subsumed by t3420.

Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
 t/t3400-rebase.sh         | 22 ----------------------
 t/t3406-rebase-message.sh | 22 ++++++++++++++++++++++
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b436ef4..45a55e9 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -59,28 +59,6 @@ test_expect_success 'rebase against master' '
 	git rebase master
 '
 
-test_expect_success 'rebase against master twice' '
-	git rebase master >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date" out
-'
-
-test_expect_success 'rebase against master twice with --force' '
-	git rebase --force-rebase master >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date, rebase forced" out
-'
-
-test_expect_success 'rebase against master twice from another branch' '
-	git checkout my-topic-branch^ &&
-	git rebase master my-topic-branch >out &&
-	test_i18ngrep "Current branch my-topic-branch is up to date" out
-'
-
-test_expect_success 'rebase fast-forward to master' '
-	git checkout my-topic-branch^ &&
-	git rebase my-topic-branch >out &&
-	test_i18ngrep "Fast-forwarded HEAD to my-topic-branch" out
-'
-
 test_expect_success 'the rebase operation should not have destroyed author information' '
 	! (git log | grep "Author:" | grep "<>")
 '
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index fe8c27f..0392e36 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -30,6 +30,28 @@ test_expect_success 'rebase -m' '
 	test_cmp expect actual
 '
 
+test_expect_success 'rebase against master twice' '
+	git rebase master >out &&
+	test_i18ngrep "Current branch topic is up to date" out
+'
+
+test_expect_success 'rebase against master twice with --force' '
+	git rebase --force-rebase master >out &&
+	test_i18ngrep "Current branch topic is up to date, rebase forced" out
+'
+
+test_expect_success 'rebase against master twice from another branch' '
+	git checkout topic^ &&
+	git rebase master topic >out &&
+	test_i18ngrep "Current branch topic is up to date" out
+'
+
+test_expect_success 'rebase fast-forward to master' '
+	git checkout topic^ &&
+	git rebase topic >out &&
+	test_i18ngrep "Fast-forwarded HEAD to topic" out
+'
+
 test_expect_success 'rebase --stat' '
 	git reset --hard start &&
         git rebase --stat master >diffstat.txt &&
-- 
1.8.3.497.g83fddbe

Re: [PATCH v6 0/8] Rebase topology test

[Junio C Hamano]

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> Changes since v5:
>
>  * Improved test_linear_range
>  * Changed TODOs to be about consistency, not --topo-order

Thanks, looked sensible.

Re: [PATCH v6 0/8] Rebase topology test

[Johannes Sixt]

Am 07.06.2013 08:11, schrieb Martin von Zweigbergk:
> Changes since v5:
> 
>  * Improved test_linear_range
>  * Changed TODOs to be about consistency, not --topo-order
> 
> Martin von Zweigbergk (7):
>   add simple tests of consistency across rebase types
>   add tests for rebasing with patch-equivalence present
>   add tests for rebasing of empty commits
>   add tests for rebasing root
>   add tests for rebasing merged history
>   t3406: modernize style
>   tests: move test for rebase messages from t3400 to t3406

I looked at the interdiff to v3 and have nothing to add.

Well done! Thanks.

-- Hannes

[PATCH mz/rebase-tests] rebase topology tests: fix commit names on case-insensitive file systems

[Johannes Sixt]

From: Johannes Sixt <j6t@kdbg.org>

The recently introduced tests used uppercase letters to denote
cherry-picks of commits having the corresponding lowercase letter names.
The helper functions also set up tags with the names of the commits.

But this constellation fails on case-insensitive file systems because
there cannot be distinct tags with names that differ only in case.

Use a less subtle convention for the names of cherry-picked commits.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 Knowing that the tests would take their time to complete on Windows,
 today was the first time I ran them, and there were some unexpected
 issues. Fixed by this patch.

 t/t3421-rebase-topology-linear.sh | 20 ++++++++++----------
 t/t3425-rebase-topology-merges.sh | 16 ++++++++--------
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
index e67add6..9c55cba 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -79,9 +79,9 @@ test_run_rebase success -p
 #      /
 # a---b---c---g---h
 #      \
-#       d---G---i
+#       d---gp--i
 #
-# uppercase = cherry-picked
+# gp = cherry-picked g
 # h = reverted g
 #
 # Reverted patches are there for tests to be able to check if a commit
@@ -94,7 +94,7 @@ test_expect_success 'setup of linear history for range selection tests' '
 	test_commit g &&
 	revert h g &&
 	git checkout d &&
-	cherry_pick G g &&
+	cherry_pick gp g &&
 	test_commit i &&
 	git checkout b &&
 	test_commit f
@@ -120,7 +120,7 @@ test_run_rebase () {
 	shift
 	test_expect_$result "rebase $* can drop last patch if in upstream" "
 		reset_rebase &&
-		git rebase $* h G &&
+		git rebase $* h gp &&
 		test_cmp_rev h HEAD^ &&
 		test_linear_range 'd' h..
 	"
@@ -152,7 +152,7 @@ test_run_rebase () {
 		reset_rebase &&
 		git rebase $* --onto h f i &&
 		test_cmp_rev h HEAD~3 &&
-		test_linear_range 'd G i' h..
+		test_linear_range 'd gp i' h..
 	"
 }
 test_run_rebase success ''
@@ -222,9 +222,9 @@ test_run_rebase failure -p
 #      /
 # a---b---c---g
 #
-# x---y---B
+# x---y---bp
 #
-# uppercase = cherry-picked
+# bp = cherry-picked b
 # m = reverted b
 #
 # Reverted patches are there for tests to be able to check if a commit
@@ -239,7 +239,7 @@ test_expect_success 'setup of linear history for test involving root' '
 	git rm -rf . &&
 	test_commit x &&
 	test_commit y &&
-	cherry_pick B b
+	cherry_pick bp b
 '
 
 test_run_rebase () {
@@ -277,7 +277,7 @@ test_run_rebase () {
 	shift
 	test_expect_$result "rebase $* --onto --root drops patch in onto" "
 		reset_rebase &&
-		git rebase $* --onto m --root B &&
+		git rebase $* --onto m --root bp &&
 		test_cmp_rev m HEAD~2 &&
 		test_linear_range 'x y' m..
 	"
@@ -308,7 +308,7 @@ test_run_rebase () {
 	shift
 	test_expect_$result "rebase $* without --onto --root with disjoint history drops patch in onto" "
 		reset_rebase &&
-		git rebase $* m B &&
+		git rebase $* m bp &&
 		test_cmp_rev m HEAD~2 &&
 		test_linear_range 'x y' m..
 	"
diff --git a/t/t3425-rebase-topology-merges.sh b/t/t3425-rebase-topology-merges.sh
index 5400a05..1d195fb 100755
--- a/t/t3425-rebase-topology-merges.sh
+++ b/t/t3425-rebase-topology-merges.sh
@@ -30,7 +30,7 @@ test_expect_success 'setup of non-linear-history' '
 	test_commit g &&
 	revert h g &&
 	git checkout d &&
-	cherry_pick G g &&
+	cherry_pick gp g &&
 	test_commit i &&
 	git checkout b &&
 	test_commit f
@@ -154,11 +154,11 @@ test_expect_success "rebase -p can re-create two branches on onto" "
 #      /
 # a---b---c---g---h
 #      \
-#       d---G---i
+#       d---gp--i
 #        \       \
 #         e-------u
 #
-# uppercase = cherry-picked
+# gp = cherry-picked g
 # h = reverted g
 test_expect_success 'setup of non-linear-history for patch-equivalence tests' '
 	git checkout e &&
@@ -186,24 +186,24 @@ test_expect_success "rebase -p --onto in merged history does not drop patches in
 	git rebase -p --onto h f u &&
 	test_cmp_rev h HEAD~3 &&
 	test_cmp_rev HEAD^2~2 HEAD~2 &&
-	test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
+	test_revision_subjects 'd gp i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
 "
 
 # a---b---c---g---h
 #      \
-#       d---G---s
+#       d---gp--s
 #        \   \ /
 #         \   X
 #          \ / \
 #           e---t
 #
-# uppercase = cherry-picked
+# gp = cherry-picked g
 # h = reverted g
 test_expect_success 'setup of non-linear-history for dropping whole side' '
-	git checkout G &&
+	git checkout gp &&
 	test_merge s e &&
 	git checkout e &&
-	test_merge t G
+	test_merge t gp
 '
 
 test_expect_failure "rebase -p drops merge commit when entire first-parent side is dropped" "
-- 
1.8.3.1.1749.g223650b

Re: [PATCH mz/rebase-tests] rebase topology tests: fix commit names on case-insensitive file systems

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> From: Johannes Sixt <j6t@kdbg.org>
>
> The recently introduced tests used uppercase letters to denote
> cherry-picks of commits having the corresponding lowercase letter names.
> The helper functions also set up tags with the names of the commits.
>
> But this constellation fails on case-insensitive file systems because
> there cannot be distinct tags with names that differ only in case.
>
> Use a less subtle convention for the names of cherry-picked commits.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  Knowing that the tests would take their time to complete on Windows,
>  today was the first time I ran them, and there were some unexpected
>  issues. Fixed by this patch.

Thanks, will queue on top of that series.

Re: [PATCH mz/rebase-tests] rebase topology tests: fix commit names on case-insensitive file systems

[Martin von Zweigbergk]

On Tue, Jun 18, 2013 at 12:28 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>
> The recently introduced tests used uppercase letters to denote
> cherry-picks of commits having the corresponding lowercase letter names.
> The helper functions also set up tags with the names of the commits.
>
> But this constellation fails on case-insensitive file systems because
> there cannot be distinct tags with names that differ only in case.
>
> Use a less subtle convention for the names of cherry-picked commits.

Makes sense, and the patch looks good. Thanks.

>  Knowing that the tests would take their time to complete on Windows,

I'm curious just how slow it is. Are we talking minutes?

Re: [PATCH mz/rebase-tests] rebase topology tests: fix commit names on case-insensitive file systems

[Johannes Sixt]

Am 6/18/2013 17:53, schrieb Martin von Zweigbergk:
> On Tue, Jun 18, 2013 at 12:28 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>  Knowing that the tests would take their time to complete on Windows,
> 
> I'm curious just how slow it is. Are we talking minutes?

D:\Src\mingw-git\t>bash -c "time make t342[15]*"
...
real    5m7.160s
user    0m21.831s
sys     0m6.376s

The real time is correct, but I don't think that we can trust user or sys
times as the box was otherwise idle.

-- Hannes