Patchwork [evolve-ext] inhibit: make rebase see obsolescence even for visible nodes

login
register
mail settings
Submitter Laurent Charignon
Date Nov. 21, 2015, 12:16 a.m.
Message ID <dcd813da374baed56f87.1448065011@lcharignon-mbp.local>
Download mbox | patch
Permalink /patch/11556/
State Changes Requested
Headers show

Comments

Laurent Charignon - Nov. 21, 2015, 12:16 a.m.
# HG changeset patch
# User Laurent Charignon <lcharignon@fb.com>
# Date 1448065000 28800
#      Fri Nov 20 16:16:40 2015 -0800
# Node ID dcd813da374baed56f875bd6f6a03ad4568dd901
# Parent  48547b4c77defdd17c670b1eb0eb94272edf0207
inhibit: make rebase see obsolescence even for visible nodes

Rebase changed recently to take advantage of obsolescence markers to reduce
the number of conflicts to resolve. Inhibit users were unable to leverage this
new feature because none of their visible nodes could be obsolete. This patch
makes the nodes that would be obsolete without inhibit, actually obsolete for
the duration of the rebase to take advantage of the feature mentioned before.
Pierre-Yves David - Nov. 21, 2015, 1:11 a.m.
On 11/20/2015 04:16 PM, Laurent Charignon wrote:
> # HG changeset patch
> # User Laurent Charignon <lcharignon@fb.com>
> # Date 1448065000 28800
> #      Fri Nov 20 16:16:40 2015 -0800
> # Node ID dcd813da374baed56f875bd6f6a03ad4568dd901
> # Parent  48547b4c77defdd17c670b1eb0eb94272edf0207
> inhibit: make rebase see obsolescence even for visible nodes
>
> Rebase changed recently to take advantage of obsolescence markers to reduce
> the number of conflicts to resolve. Inhibit users were unable to leverage this
> new feature because none of their visible nodes could be obsolete. This patch
> makes the nodes that would be obsolete without inhibit, actually obsolete for
> the duration of the rebase to take advantage of the feature mentioned before.

The logic is pretty good, nice job.
I've some feedback on the monkey patching process.

> diff --git a/hgext/inhibit.py b/hgext/inhibit.py
> --- a/hgext/inhibit.py
> +++ b/hgext/inhibit.py
> @@ -176,6 +176,14 @@ def _createmarkers(orig, repo, relations
>       finally:
>           lockmod.release(tr, lock)
>
> +def _computeobsoletenotrebasedwrap(orig, repo, rebasesetrevs, dest):
> +    repo.notinhibited = rebasesetrevs
> +    repo.invalidatevolatilesets()
> +    r = orig(repo, rebasesetrevs, dest)
> +    repo.notinhibited = set()
> +    repo.invalidatevolatilesets()
> +    return r

I would probably use an underscore for the attribute, it is a fairly 
obscure hack for an obscure extension, we do not want that to look too 
official.

I would do a full cleanup of the object namespace by deleting the 
attribute. I would also make sure the cleanup happens:

     repo._notinhibited = rebasesetrevs
     try:
         repo.invalidatevolatilesets()
         r = orig(repo, rebasesetrevs, dest)
     finally:
         del repo._notinhibited
         repo.invalidatevolatilesets()
     return r

>   def transactioncallback(orig, repo, desc, *args, **kwargs):
>       """ Wrap localrepo.transaction to inhibit new obsolete changes """
>       def inhibitposttransaction(transaction):
> @@ -202,8 +210,14 @@ def extsetup(ui):
>           obs = obsfunc(repo)
>           if _inhibitenabled(repo):
>               getrev = repo.changelog.nodemap.get
> +            blacklist = set()
> +            try:
> +                blacklist = repo.notinhibited
> +            except AttributeError as e:
> +                pass

You can use getattr for this purpose

blacklist = getattr(repo, '_notinhibited', set())

Patch

diff --git a/hgext/inhibit.py b/hgext/inhibit.py
--- a/hgext/inhibit.py
+++ b/hgext/inhibit.py
@@ -176,6 +176,14 @@  def _createmarkers(orig, repo, relations
     finally:
         lockmod.release(tr, lock)
 
+def _computeobsoletenotrebasedwrap(orig, repo, rebasesetrevs, dest):
+    repo.notinhibited = rebasesetrevs
+    repo.invalidatevolatilesets()
+    r = orig(repo, rebasesetrevs, dest)
+    repo.notinhibited = set()
+    repo.invalidatevolatilesets()
+    return r
+
 def transactioncallback(orig, repo, desc, *args, **kwargs):
     """ Wrap localrepo.transaction to inhibit new obsolete changes """
     def inhibitposttransaction(transaction):
@@ -202,8 +210,14 @@  def extsetup(ui):
         obs = obsfunc(repo)
         if _inhibitenabled(repo):
             getrev = repo.changelog.nodemap.get
+            blacklist = set()
+            try:
+                blacklist = repo.notinhibited
+            except AttributeError as e:
+                pass
             for n in repo._obsinhibit:
-                obs.discard(getrev(n))
+                if getrev(n) not in blacklist:
+                    obs.discard(getrev(n))
         return obs
     try:
         extensions.find('directaccess')
@@ -232,6 +246,14 @@  def extsetup(ui):
     # wrap both to add inhibition markers.
     extensions.wrapfunction(bookmarks.bmstore, 'recordchange', _bookmarkchanged)
     extensions.wrapfunction(bookmarks.bmstore, 'write', _bookmarkchanged)
+    try:
+        rebase = extensions.find('rebase')
+        if rebase:
+            extensions.wrapfunction(rebase,
+                                    '_computeobsoletenotrebased',
+                                    _computeobsoletenotrebasedwrap)
+    except KeyError:
+        pass
     # Add bookmark -D option
     entry = extensions.wrapcommand(commands.table, 'bookmark', _bookmark)
     entry[1].append(('D','prune',None,
diff --git a/tests/test-inhibit.t b/tests/test-inhibit.t
--- a/tests/test-inhibit.t
+++ b/tests/test-inhibit.t
@@ -700,6 +700,17 @@  Empty commit
   nothing changed
   [1]
 
+Check that the behavior of rebase with obsolescence markers is maintained
+despite inhibit
+
+  $ hg up a438c045eb37
+  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ hg rebase -r 15:: -d 21 --config experimental.rebaseskipobsolete=True
+  note: not rebasing 15:2d66e189f5b5 "add cM", already in destination as 21:721c3c279519 "add cM"
+  rebasing 16:a438c045eb37 "add cN"
+  $ hg up 21
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+
 Directaccess should load after some extensions precised in the conf
 With no extension specified:
 
@@ -714,7 +725,7 @@  With no extension specified:
   > EOF
   $ hg id
   ['rebase', 'strip', 'evolve', 'directaccess', 'inhibit', 'testextension']
-  721c3c279519 tip
+  721c3c279519
 
 With test_extension specified:
   $ cat >> $HGRCPATH << EOF
@@ -723,7 +734,7 @@  With test_extension specified:
   > EOF
   $ hg id
   ['rebase', 'strip', 'evolve', 'inhibit', 'testextension', 'directaccess']
-  721c3c279519 tip
+  721c3c279519
 
 Inhibit should not work without directaccess
   $ cat >> $HGRCPATH <<EOF
@@ -746,6 +757,7 @@  We copy the inhibhit repo to inhibit2 an
   $ pwd=$(pwd)
   $ cd inhibit
   $ mkcommit pk
+  created new head
   $ hg id
   003a4735afde tip
   $ echo "OO" > pk
@@ -767,7 +779,7 @@  Visible commits can still be pushed
   adding changesets
   adding manifests
   adding file changes
-  added 1 changesets with 1 changes to 1 files
+  added 1 changesets with 1 changes to 1 files (+1 heads)
   2 new obsolescence markers
 
 Pulling from a inhibit repo to a non-inhibit repo should work