Patchwork localrepo: invoke only feature setup functions for enabled extensions

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Oct. 17, 2013, 12:48 p.m.
Message ID <2d01803ef7a896e114e7.1382014109@juju>
Download mbox | patch
Permalink /patch/2787/
State Accepted
Commit d1ac3790e10a44a3e55e7fe2a86e3e2a849382e9
Headers show

Comments

Katsunori FUJIWARA - Oct. 17, 2013, 12:48 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1382013917 -32400
#      Thu Oct 17 21:45:17 2013 +0900
# Node ID 2d01803ef7a896e114e7983022fe40ceb7823346
# Parent  aebfbb68fe9277803d9f68943bf2e63391bd08c5
localrepo: invoke only feature setup functions for enabled extensions

Before this patch, each feature setup functions for localrepository
class should examine whether corresponding extension is enabled or not
by themselves.

This patch invokes only feature setup functions defined in module of
enabled extensions, and it makes implementation of feature setup
functions easier and simpler.
Siddharth Agarwal - Oct. 20, 2013, 1:20 a.m.
On 10/17/2013 05:48 AM, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1382013917 -32400
> #      Thu Oct 17 21:45:17 2013 +0900
> # Node ID 2d01803ef7a896e114e7983022fe40ceb7823346
> # Parent  aebfbb68fe9277803d9f68943bf2e63391bd08c5
> localrepo: invoke only feature setup functions for enabled extensions
>
> Before this patch, each feature setup functions for localrepository
> class should examine whether corresponding extension is enabled or not
> by themselves.
>
> This patch invokes only feature setup functions defined in module of
> enabled extensions, and it makes implementation of feature setup
> functions easier and simpler.

I have a slight preference for making featuresetup a magic function just 
like uisetup or reposetup, but I'm not going to fight over that. Looks 
good to me, unless mpm has a stronger preference.
Matt Mackall - Oct. 20, 2013, 1:27 a.m.
On Thu, 2013-10-17 at 21:48 +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1382013917 -32400
> #      Thu Oct 17 21:45:17 2013 +0900
> # Node ID 2d01803ef7a896e114e7983022fe40ceb7823346
> # Parent  aebfbb68fe9277803d9f68943bf2e63391bd08c5
> localrepo: invoke only feature setup functions for enabled extensions

Queued for default, thanks.

Patch

diff --git a/hgext/largefiles/__init__.py b/hgext/largefiles/__init__.py
--- a/hgext/largefiles/__init__.py
+++ b/hgext/largefiles/__init__.py
@@ -105,7 +105,7 @@ 
 command.
 '''
 
-from mercurial import commands, localrepo, extensions
+from mercurial import commands, localrepo
 
 import lfcommands
 import reposetup
@@ -116,11 +116,8 @@ 
 reposetup = reposetup.reposetup
 
 def featuresetup(ui, supported):
-    for name, module in extensions.extensions(ui):
-        if __name__ == module.__name__:
-            # don't die on seeing a repo with the largefiles requirement
-            supported |= set(['largefiles'])
-            return
+    # don't die on seeing a repo with the largefiles requirement
+    supported |= set(['largefiles'])
 
 def uisetup(ui):
     localrepo.localrepository.featuresetupfuncs.add(featuresetup)
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -153,6 +153,8 @@ 
     requirements = ['revlogv1']
     filtername = None
 
+    # a list of (ui, featureset) functions.
+    # only functions defined in module of enabled extensions are invoked
     featuresetupfuncs = set()
 
     def _baserequirements(self, create):
@@ -181,8 +183,11 @@ 
 
         if self.featuresetupfuncs:
             self.supported = set(self._basesupported) # use private copy
+            extmods = set(m.__name__ for n, m
+                          in extensions.extensions(self.ui))
             for setupfunc in self.featuresetupfuncs:
-                setupfunc(self.ui, self.supported)
+                if setupfunc.__module__ in extmods:
+                    setupfunc(self.ui, self.supported)
         else:
             self.supported = self._basesupported