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

2013-06-05 Thread Johannes Sixt
Am 6/4/2013 19:18, schrieb Junio C Hamano:
 Martin von Zweigbergk martinv...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-06-04 Thread Junio C Hamano
Martin von Zweigbergk martinv...@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).

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-06-04 Thread Martin von Zweigbergk
On Tue, Jun 4, 2013 at 10:18 AM, Junio C Hamano gits...@pobox.com wrote:
 Martin von Zweigbergk martinv...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-06-03 Thread 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..000
--- 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