Patchwork D6686: unshelve: handle stripping changesets on interactive mode

login
register
mail settings
Submitter phabricator
Date July 24, 2019, 12:48 p.m.
Message ID <differential-rev-PHID-DREV-k66pmcrk5tebposnzxwa-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/41044/
State Superseded
Headers show

Comments

phabricator - July 24, 2019, 12:48 p.m.
navaneeth.suresh created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  On interactive mode, changesets on `nodestoremove` should be
  stripped regardless of the shelve is partial or not. This patch
  modifies `unshelvecontinue()` to do that.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/shelve.py

CHANGE DETAILS




To: navaneeth.suresh, #hg-reviewers
Cc: mercurial-devel
phabricator - July 24, 2019, 2:49 p.m.
pulkit added a comment.


  Can you add tests for this? There is no test which passes or fails because of this change.

REPOSITORY
  rHG Mercurial

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

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

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - July 25, 2019, 4:36 p.m.
navaneeth.suresh added a comment.


  I just found that this change is not required while creating D6694 <https://phab.mercurial-scm.org/D6694>. We will be needing the nodes which we are removing for later in case of a partial unshelve.
  The stripbased approach of unshelve failed on the change in D6694 <https://phab.mercurial-scm.org/D6694> on having this change. Should I abandon this?

REPOSITORY
  rHG Mercurial

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

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

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - July 26, 2019, 2:44 p.m.
pulkit added a comment.


  In D6686#97957 <https://phab.mercurial-scm.org/D6686#97957>, @navaneeth.suresh wrote:
  
  > I just found that this change is not required while creating D6694 <https://phab.mercurial-scm.org/D6694>. We will be needing the nodes which we are removing for later in case of a partial unshelve.
  > The stripbased approach of unshelve failed on the change in D6694 <https://phab.mercurial-scm.org/D6694> on having this change. Should I abandon this?
  
  I think this change is required. A nice way to figure out will be to add the test which you added and not make the change in code and see whether do we remove the extra cset or not.

REPOSITORY
  rHG Mercurial

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

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

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - July 26, 2019, 3:22 p.m.
navaneeth.suresh added a comment.


  In D6686#98006 <https://phab.mercurial-scm.org/D6686#98006>, @pulkit wrote:
  
  > In D6686#97957 <https://phab.mercurial-scm.org/D6686#97957>, @navaneeth.suresh wrote:
  >
  >> I just found that this change is not required while creating D6694 <https://phab.mercurial-scm.org/D6694>. We will be needing the nodes which we are removing for later in case of a partial unshelve.
  >> The stripbased approach of unshelve failed on the change in D6694 <https://phab.mercurial-scm.org/D6694> on having this change. Should I abandon this?
  >
  > I think this change is required. A nice way to figure out will be to add the test which you added and not make the change in code and see whether do we remove the extra cset or not.
  
  Yes, this change is required. D6694 <https://phab.mercurial-scm.org/D6694> is modified and rebased on top of this change.

REPOSITORY
  rHG Mercurial

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

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

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - Aug. 6, 2019, 11:53 a.m.
pulkit added a comment.


  I am not sure why, but `test-shelve.t` fails for me with some hash changes. Can you have a look?

REPOSITORY
  rHG Mercurial

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

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

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

Patch

diff --git a/mercurial/shelve.py b/mercurial/shelve.py
--- a/mercurial/shelve.py
+++ b/mercurial/shelve.py
@@ -747,10 +747,10 @@ 
         mergefiles(ui, repo, state.wctx, shelvectx)
         restorebranch(ui, repo, state.branchtorestore)
 
+        if not phases.supportinternal(repo):
+            repair.strip(ui, repo, state.nodestoremove, backup=False,
+                         topic='shelve')
         if not ispartialunshelve:
-            if not phases.supportinternal(repo):
-                repair.strip(ui, repo, state.nodestoremove, backup=False,
-                            topic='shelve')
             shelvedstate.clear(repo)
             unshelvecleanup(ui, repo, state.name, opts)
         _restoreactivebookmark(repo, state.activebookmark)