Patchwork [5,of,5,shelve-ext] shelve: move unshelve-finishing logic to a separate function

login
register
mail settings
Submitter Kostia Balytskyi
Date Nov. 13, 2016, 11:39 a.m.
Message ID <c34ee36f4e7fe6b04ea1.1479037178@dev1902.lla1.facebook.com>
Download mbox | patch
Permalink /patch/17547/
State Accepted
Headers show

Comments

Kostia Balytskyi - Nov. 13, 2016, 11:39 a.m.
# HG changeset patch
# User Kostia Balytskyi <ikostia@fb.com>
# Date 1478876487 28800
#      Fri Nov 11 07:01:27 2016 -0800
# Node ID c34ee36f4e7fe6b04ea1b4e2f032e995fe65d6c5
# Parent  dce4581dcae386c3d3b420911350d176c0423520
shelve: move unshelve-finishing logic to a separate function

Finishing unshelve involves two steps now:
- stripping a changelog
- aborting a transaction
Obs-based shelve will not require these things, so isolating this logic
into a separate function where the normal/obs-shelve branching is
going to be implemented seems to be like a nice idea.

Behavior-wise this change moves 'unshelvecleanup' from being between
changelog stripping and transaction abortion to being after them.
I don't think this has any negative effects.
Yuya Nishihara - Nov. 19, 2016, 10:02 a.m.
On Sun, 13 Nov 2016 03:39:38 -0800, Kostia Balytskyi wrote:
> # HG changeset patch
> # User Kostia Balytskyi <ikostia@fb.com>
> # Date 1478876487 28800
> #      Fri Nov 11 07:01:27 2016 -0800
> # Node ID c34ee36f4e7fe6b04ea1b4e2f032e995fe65d6c5
> # Parent  dce4581dcae386c3d3b420911350d176c0423520
> shelve: move unshelve-finishing logic to a separate function
> 
> Finishing unshelve involves two steps now:
> - stripping a changelog
> - aborting a transaction
> Obs-based shelve will not require these things, so isolating this logic
> into a separate function where the normal/obs-shelve branching is
> going to be implemented seems to be like a nice idea.
> 
> Behavior-wise this change moves 'unshelvecleanup' from being between
> changelog stripping and transaction abortion to being after them.
> I don't think this has any negative effects.

Seems fine. shelvedfile isn't a member of transaction.

Patch

diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -708,6 +708,14 @@  def _forgetunknownfiles(repo, shelvectx,
     toforget = (addedafter & shelveunknown) - addedbefore
     repo[None].forget(toforget)
 
+def _finishunshelve(repo, oldtiprev, tr):
+    # The transaction aborting will strip all the commits for us,
+    # but it doesn't update the inmemory structures, so addchangegroup
+    # hooks still fire and try to operate on the missing commits.
+    # Clean up manually to prevent this.
+    repo.unfiltered().changelog.strip(oldtiprev, tr)
+    _aborttransaction(repo)
+
 @command('unshelve',
          [('a', 'abort', None,
            _('abort an incomplete unshelve operation')),
@@ -847,16 +855,8 @@  def _dounshelve(ui, repo, *shelved, **op
         _forgetunknownfiles(repo, shelvectx, addedbefore)
 
         shelvedstate.clear(repo)
-
-        # The transaction aborting will strip all the commits for us,
-        # but it doesn't update the inmemory structures, so addchangegroup
-        # hooks still fire and try to operate on the missing commits.
-        # Clean up manually to prevent this.
-        repo.unfiltered().changelog.strip(oldtiprev, tr)
-
+        _finishunshelve(repo, oldtiprev, tr)
         unshelvecleanup(ui, repo, basename, opts)
-
-        _aborttransaction(repo)
     finally:
         ui.quiet = oldquiet
         if tr: