D7824: tests: add test of rebase with conflict in merge commit

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

D7824: tests: add test of rebase with conflict in merge commit

martinvonz (Martin von Zweigbergk)
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  It doesn't seem like we had any tests of this. I think it's pretty
  weird that the two parents we're merging are not the working copy
  parents during the conflict resolution.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D7824

AFFECTED FILES
  tests/test-rebase-conflicts.t

CHANGE DETAILS

diff --git a/tests/test-rebase-conflicts.t b/tests/test-rebase-conflicts.t
--- a/tests/test-rebase-conflicts.t
+++ b/tests/test-rebase-conflicts.t
@@ -429,3 +429,74 @@
   |/
   o  0:draft 'A'
   
+
+Test where the conflict happens when rebasing a merge commit
+
+  $ cd $TESTTMP
+  $ hg init conflict-in-merge
+  $ cd conflict-in-merge
+  $ hg debugdrawdag <<'EOS'
+  > F # F/conflict = foo\n
+  > |\
+  > D E
+  > |/
+  > C B # B/conflict = bar\n
+  > |/
+  > A
+  > EOS
+
+  $ hg co F
+  5 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ hg rebase -d B
+  rebasing 2:dc0947a82db8 "C" (C)
+  rebasing 3:e7b3f00ed42e "D" (D)
+  rebasing 4:03ca77807e91 "E" (E)
+  rebasing 5:9a6b91dc2044 "F" (F tip)
+  merging conflict
+  warning: conflicts while merging conflict! (edit, then use 'hg resolve --mark')
+  unresolved conflicts (see hg resolve, then hg rebase --continue)
+  [1]
+It's weird that the current parents are not 7 and 8 since that's what we're
+merging
+  $ hg tglog
+  @  8:draft 'E'
+  |
+  | o  7:draft 'D'
+  |/
+  o  6:draft 'C'
+  |
+  | @    5:draft 'F'
+  | |\
+  | | o  4:draft 'E'
+  | | |
+  | o |  3:draft 'D'
+  | |/
+  | o  2:draft 'C'
+  | |
+  o |  1:draft 'B'
+  |/
+  o  0:draft 'A'
+  
+  $ echo baz > conflict
+  $ hg resolve -m
+  (no more unresolved files)
+  continue: hg rebase --continue
+  $ hg rebase -c
+  already rebased 2:dc0947a82db8 "C" (C) as 0199610c343e
+  already rebased 3:e7b3f00ed42e "D" (D) as f0dd538aaa63
+  already rebased 4:03ca77807e91 "E" (E) as cbf25af8347d
+  rebasing 5:9a6b91dc2044 "F" (F)
+  saved backup bundle to $TESTTMP/conflict-in-merge/.hg/strip-backup/dc0947a82db8-ca7e7d5b-rebase.hg
+  $ hg tglog
+  @    5:draft 'F'
+  |\
+  | o  4:draft 'E'
+  | |
+  o |  3:draft 'D'
+  |/
+  o  2:draft 'C'
+  |
+  o  1:draft 'B'
+  |
+  o  0:draft 'A'
+  



To: martinvonz, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D7824: tests: add test of rebase with conflict in merge commit

martinvonz (Martin von Zweigbergk)
pulkit added inline comments.

INLINE COMMENTS

> test-rebase-conflicts.t:460
> +It's weird that the current parents are not 7 and 8 since that's what we're
> +merging
> +  $ hg tglog

I think it will be weird to have current parents as 7 and 8. Rebase copies commit, so if there is a merge commit, it copies it instead of recreating one. Also if we have 7 and 8 as parents, we might not have correct conflicts.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7824/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7824

To: martinvonz, #hg-reviewers
Cc: pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D7824: tests: add test of rebase with conflict in merge commit

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
martinvonz added inline comments.

INLINE COMMENTS

> pulkit wrote in test-rebase-conflicts.t:460
> I think it will be weird to have current parents as 7 and 8. Rebase copies commit, so if there is a merge commit, it copies it instead of recreating one. Also if we have 7 and 8 as parents, we might not have correct conflicts.

I made more arguments for changing it in the next patch (D7827 <https://phab.mercurial-scm.org/D7827>). See if those can convince you :)

I'm not sure what you mean about not having correct conflicts. Related to that, I think it's actually weird (with how it's currently done) how you would not see any changes made between the merge base and the rebase base. So if there had been commits between `A` and `C` here and we were rebasing from `C`, we would not see those changes in the working copy. That's of course correct, but the dirstate's two parents suggest, IMO, that we're merging the two commits, so we should see those changes.

I can see the value in having the thing you're grafting in as a working copy parent because it makes it appear in the `hg log` output as a reminder. However, I don't think that's what the purpose of the dirstate parent should be. We could perhaps add another  mechanism for showing the commit that's being rebased, and maybe also the base (which is generally not the merge base).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7824/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7824

To: martinvonz, #hg-reviewers
Cc: pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D7824: tests: add test of rebase with conflict in merge commit

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
martinvonz updated this revision to Diff 19859.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7824?vs=19144&id=19859

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7824/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7824

AFFECTED FILES
  tests/test-rebase-conflicts.t

CHANGE DETAILS

diff --git a/tests/test-rebase-conflicts.t b/tests/test-rebase-conflicts.t
--- a/tests/test-rebase-conflicts.t
+++ b/tests/test-rebase-conflicts.t
@@ -429,3 +429,73 @@
   |/
   o  0:draft 'A'
   
+
+Test where the conflict happens when rebasing a merge commit
+
+  $ cd $TESTTMP
+  $ hg init conflict-in-merge
+  $ cd conflict-in-merge
+  $ hg debugdrawdag <<'EOS'
+  > F # F/conflict = foo\n
+  > |\
+  > D E
+  > |/
+  > C B # B/conflict = bar\n
+  > |/
+  > A
+  > EOS
+
+  $ hg co F
+  5 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ hg rebase -d B
+  rebasing 2:dc0947a82db8 "C" (C)
+  rebasing 3:e7b3f00ed42e "D" (D)
+  rebasing 4:03ca77807e91 "E" (E)
+  rebasing 5:9a6b91dc2044 "F" (F tip)
+  merging conflict
+  warning: conflicts while merging conflict! (edit, then use 'hg resolve --mark')
+  unresolved conflicts (see hg resolve, then hg rebase --continue)
+  [1]
+The current parents are not 7 and 8 even though that's what we're merging
+  $ hg tglog
+  @  8:draft 'E'
+  |
+  | o  7:draft 'D'
+  |/
+  o  6:draft 'C'
+  |
+  | @    5:draft 'F'
+  | |\
+  | | o  4:draft 'E'
+  | | |
+  | o |  3:draft 'D'
+  | |/
+  | o  2:draft 'C'
+  | |
+  o |  1:draft 'B'
+  |/
+  o  0:draft 'A'
+  
+  $ echo baz > conflict
+  $ hg resolve -m
+  (no more unresolved files)
+  continue: hg rebase --continue
+  $ hg rebase -c
+  already rebased 2:dc0947a82db8 "C" (C) as 0199610c343e
+  already rebased 3:e7b3f00ed42e "D" (D) as f0dd538aaa63
+  already rebased 4:03ca77807e91 "E" (E) as cbf25af8347d
+  rebasing 5:9a6b91dc2044 "F" (F)
+  saved backup bundle to $TESTTMP/conflict-in-merge/.hg/strip-backup/dc0947a82db8-ca7e7d5b-rebase.hg
+  $ hg tglog
+  @    5:draft 'F'
+  |\
+  | o  4:draft 'E'
+  | |
+  o |  3:draft 'D'
+  |/
+  o  2:draft 'C'
+  |
+  o  1:draft 'B'
+  |
+  o  0:draft 'A'
+  



To: martinvonz, #hg-reviewers
Cc: pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D7824: tests: add test of rebase with conflict in merge commit

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
martinvonz added inline comments.
martinvonz marked an inline comment as done.

INLINE COMMENTS

> pulkit wrote in test-rebase-conflicts.t:460
> I think it will be weird to have current parents as 7 and 8. Rebase copies commit, so if there is a merge commit, it copies it instead of recreating one. Also if we have 7 and 8 as parents, we might not have correct conflicts.

I've updated the comment to not be opinionated. I hope we can now queue this patch  (and a few more in this series) regardless of what we think of D7827 <https://phab.mercurial-scm.org/D7827>.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7824/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7824

To: martinvonz, #hg-reviewers
Cc: pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D7824: tests: add test of rebase with conflict in merge commit

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
Closed by commit rHG7f7c8521e9bd: tests: add test of rebase with conflict in merge commit (authored by martinvonz).
This revision was automatically updated to reflect the committed changes.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7824?vs=19859&id=20132

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7824/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7824

AFFECTED FILES
  tests/test-rebase-conflicts.t

CHANGE DETAILS

diff --git a/tests/test-rebase-conflicts.t b/tests/test-rebase-conflicts.t
--- a/tests/test-rebase-conflicts.t
+++ b/tests/test-rebase-conflicts.t
@@ -429,3 +429,73 @@
   |/
   o  0:draft 'A'
   
+
+Test where the conflict happens when rebasing a merge commit
+
+  $ cd $TESTTMP
+  $ hg init conflict-in-merge
+  $ cd conflict-in-merge
+  $ hg debugdrawdag <<'EOS'
+  > F # F/conflict = foo\n
+  > |\
+  > D E
+  > |/
+  > C B # B/conflict = bar\n
+  > |/
+  > A
+  > EOS
+
+  $ hg co F
+  5 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ hg rebase -d B
+  rebasing 2:dc0947a82db8 "C" (C)
+  rebasing 3:e7b3f00ed42e "D" (D)
+  rebasing 4:03ca77807e91 "E" (E)
+  rebasing 5:9a6b91dc2044 "F" (F tip)
+  merging conflict
+  warning: conflicts while merging conflict! (edit, then use 'hg resolve --mark')
+  unresolved conflicts (see hg resolve, then hg rebase --continue)
+  [1]
+The current parents are not 7 and 8 even though that's what we're merging
+  $ hg tglog
+  @  8:draft 'E'
+  |
+  | o  7:draft 'D'
+  |/
+  o  6:draft 'C'
+  |
+  | @    5:draft 'F'
+  | |\
+  | | o  4:draft 'E'
+  | | |
+  | o |  3:draft 'D'
+  | |/
+  | o  2:draft 'C'
+  | |
+  o |  1:draft 'B'
+  |/
+  o  0:draft 'A'
+  
+  $ echo baz > conflict
+  $ hg resolve -m
+  (no more unresolved files)
+  continue: hg rebase --continue
+  $ hg rebase -c
+  already rebased 2:dc0947a82db8 "C" (C) as 0199610c343e
+  already rebased 3:e7b3f00ed42e "D" (D) as f0dd538aaa63
+  already rebased 4:03ca77807e91 "E" (E) as cbf25af8347d
+  rebasing 5:9a6b91dc2044 "F" (F)
+  saved backup bundle to $TESTTMP/conflict-in-merge/.hg/strip-backup/dc0947a82db8-ca7e7d5b-rebase.hg
+  $ hg tglog
+  @    5:draft 'F'
+  |\
+  | o  4:draft 'E'
+  | |
+  o |  3:draft 'D'
+  |/
+  o  2:draft 'C'
+  |
+  o  1:draft 'B'
+  |
+  o  0:draft 'A'
+  



To: martinvonz, #hg-reviewers, durin42
Cc: pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel