Patchwork D6169: unshelve: disable unshelve during merge (issue5123)

login
register
mail settings
Submitter phabricator
Date March 25, 2019, 7:17 a.m.
Message ID <differential-rev-PHID-DREV-jb4bzawhm5r5b6g3hvla-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/39378/
State Superseded
Headers show

Comments

phabricator - March 25, 2019, 7:17 a.m.
navaneeth.suresh created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  As stated in the issue5123, unshelve can destroy the second parent of
  the context when tried to unshelve with an uncommitted merge. This
  patch makes unshelve to abort when called with an uncommitted merge.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  hgext/shelve.py
  tests/test-shelve.t

CHANGE DETAILS




To: navaneeth.suresh, #hg-reviewers
Cc: mercurial-devel
Yuya Nishihara - March 25, 2019, 10:28 p.m.
>   As stated in the issue5123, unshelve can destroy the second parent of
>   the context when tried to unshelve with an uncommitted merge. This
>   patch makes unshelve to abort when called with an uncommitted merge.

Is it difficult to fix unshelve to not lose merge parents?

https://bz.mercurial-scm.org/show_bug.cgi?id=5123#c2
phabricator - March 25, 2019, 10:29 p.m.
yuja added a comment.


  >   As stated in the issue5123, unshelve can destroy the second parent of
  >   the context when tried to unshelve with an uncommitted merge. This
  >   patch makes unshelve to abort when called with an uncommitted merge.
  
  Is it difficult to fix unshelve to not lose merge parents?
  
  https://bz.mercurial-scm.org/show_bug.cgi?id=5123#c2

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: yuja, mercurial-devel
phabricator - March 26, 2019, 6:11 p.m.
navaneeth.suresh added a comment.


  > Is it difficult to fix unshelve to not lose merge parents?
  > 
  > https://bz.mercurial-scm.org/show_bug.cgi?id=5123#c2
  
  I'll try to come up with a fix. But, `hg shelve` is aborting during a merge. Can `unshelve` be aborted too? I don't have a strong opinion on this. I am just asking.

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: yuja, mercurial-devel
Yuya Nishihara - March 27, 2019, 10:34 p.m.
>   > Is it difficult to fix unshelve to not lose merge parents?
>   > 
>   > https://bz.mercurial-scm.org/show_bug.cgi?id=5123#c2
>   
>   I'll try to come up with a fix. But, `hg shelve` is aborting during a merge. Can `unshelve` be aborted too?

It could abort, but mpm said there would be no reason to, and I agree with
that. Unshelving on top of a merge seems somewhat useful.
phabricator - March 27, 2019, 10:35 p.m.
yuja added a comment.


  >   > Is it difficult to fix unshelve to not lose merge parents?
  >   > 
  >   > https://bz.mercurial-scm.org/show_bug.cgi?id=5123#c2
  >   
  >   I'll try to come up with a fix. But, `hg shelve` is aborting during a merge. Can `unshelve` be aborted too?
  
  It could abort, but mpm said there would be no reason to, and I agree with
  that. Unshelving on top of a merge seems somewhat useful.

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: yuja, mercurial-devel
phabricator - April 1, 2019, 8:12 a.m.
navaneeth.suresh added a comment.


  I saw some patches[0][1][2] trying to fix this issue. It seems like it's difficult to preserve the second parent.
  
  [0]: http://mercurial.808500.n3.nabble.com/PATCH-shelve-adds-restoring-original-parents-after-unshelve-issue5123-tc4035657.html#none
  [1]: http://mercurial.808500.n3.nabble.com/PATCH-V2-shelve-restore-parents-after-unshelve-issue5123-tc4036093.html#a4037370
  [2]: http://mercurial.808500.n3.nabble.com/PATCH-V3-shelve-restore-parents-after-unshelve-issue5123-tt4036858.html#a4037408

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: yuja, mercurial-devel
Yuya Nishihara - April 1, 2019, 11:03 p.m.
>   I saw some patches[0][1][2] trying to fix this issue. It seems like it's difficult to preserve the second parent.
>   
>   [0]: http://mercurial.808500.n3.nabble.com/PATCH-shelve-adds-restoring-original-parents-after-unshelve-issue5123-tc4035657.html#none
>   [1]: http://mercurial.808500.n3.nabble.com/PATCH-V2-shelve-restore-parents-after-unshelve-issue5123-tc4036093.html#a4037370
>   [2]: http://mercurial.808500.n3.nabble.com/PATCH-V3-shelve-restore-parents-after-unshelve-issue5123-tt4036858.html#a4037408

Okay, let's disable unshelve then. Can you update the commit message to
include why we can't simply allow it?
phabricator - April 1, 2019, 11:04 p.m.
yuja added a comment.


  >   I saw some patches[0][1][2] trying to fix this issue. It seems like it's difficult to preserve the second parent.
  >   
  >   [0]: http://mercurial.808500.n3.nabble.com/PATCH-shelve-adds-restoring-original-parents-after-unshelve-issue5123-tc4035657.html#none
  >   [1]: http://mercurial.808500.n3.nabble.com/PATCH-V2-shelve-restore-parents-after-unshelve-issue5123-tc4036093.html#a4037370
  >   [2]: http://mercurial.808500.n3.nabble.com/PATCH-V3-shelve-restore-parents-after-unshelve-issue5123-tt4036858.html#a4037408
  
  Okay, let's disable unshelve then. Can you update the commit message to
  include why we can't simply allow it?

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: yuja, mercurial-devel
phabricator - April 3, 2019, 3:48 a.m.
navaneeth.suresh added a comment.


  Thanks for queuing Yuya.

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: yuja, mercurial-devel

Patch

diff --git a/tests/test-shelve.t b/tests/test-shelve.t
--- a/tests/test-shelve.t
+++ b/tests/test-shelve.t
@@ -1087,3 +1087,46 @@ 
      test                      (4|13):33f7f61e6c5e (re)
 
   $ cd ..
+
+Abort unshelve while merging (issue5123)
+-----------------------------------------
+  $ hg init issue5123
+  $ cd issue5123
+  $ echo > a
+  $ hg ci -Am a
+  adding a
+  $ hg co null
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ echo > b
+  $ hg ci -Am b
+  adding b
+  created new head
+  $ echo > c
+  $ hg add c
+  $ hg shelve
+  shelved as default
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ hg co 1
+  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ hg merge 0
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  (branch merge, don't forget to commit)
+-- successful merge with two parents
+  $ hg log -G
+  @  changeset:   1:406bf70c274f
+     tag:         tip
+     parent:      -1:000000000000
+     user:        test
+     date:        Thu Jan 01 00:00:00 1970 +0000
+     summary:     b
+  
+  @  changeset:   0:ada8c9eb8252
+     user:        test
+     date:        Thu Jan 01 00:00:00 1970 +0000
+     summary:     a
+  
+-- trying to pull in the shelve bits
+-- unshelve should abort otherwise, it'll eat my second parent.
+  $ hg unshelve
+  abort: cannot unshelve while merging
+  [255]
diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -988,6 +988,12 @@ 
             return unshelvecontinue(ui, repo, state, opts)
     elif len(shelved) > 1:
         raise error.Abort(_('can only unshelve one change at a time'))
+
+    # abort unshelve while merging (issue5123)
+    parents = repo[None].parents()
+    if len(parents) > 1:
+        raise error.Abort(_('cannot unshelve while merging'))
+
     elif not shelved:
         shelved = listshelves(repo)
         if not shelved: