Patchwork chgserver: use global ui instead of repo ui for dispatch.request.ui

login
register
mail settings
Submitter Jun Wu
Date March 20, 2016, 1:39 a.m.
Message ID <cb25e00a2545a07ef9f3.1458437952@x1c>
Download mbox | patch
Permalink /patch/13975/
State Accepted
Delegated to: Yuya Nishihara
Headers show

Comments

Jun Wu - March 20, 2016, 1:39 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1458239530 0
#      Thu Mar 17 18:32:10 2016 +0000
# Node ID cb25e00a2545a07ef9f3842f26450f2a33c0bcd8
# Parent  9949950664cda68757f4312920c6d33095343e03
chgserver: use global ui instead of repo ui for dispatch.request.ui

Before this patch, chgserver will use repo ui as dispatch.request.ui, while
req.ui is designed to be global ui without repo config.

Passing repo ui as dispatch.request.ui leads to repo.ui being incorrect, which
can lead to unwanted results. For example, if the repo config has [extensions],
it could affect which localrepository.featuresetupfuncs get executed and the
repo may have an incorrect list of supported requirements.

This patch changes _renewui to return both global ui and repo ui. The global
ui is passed to req.ui, and the repo ui is used to calculate confighash. It
will make chg pass test-largefiles-misc.t and test-requires.t, which are both
related to repo requirements.
Yuya Nishihara - March 20, 2016, 4:13 p.m.
On Sat, 19 Mar 2016 18:39:12 -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1458239530 0
> #      Thu Mar 17 18:32:10 2016 +0000
> # Node ID cb25e00a2545a07ef9f3842f26450f2a33c0bcd8
> # Parent  9949950664cda68757f4312920c6d33095343e03
> chgserver: use global ui instead of repo ui for dispatch.request.ui

Got pyflakes error. I'll fix this in flight and push to the clowncopter, thanks.

  hgext/chgserver.py:537: undefined name '_renewui'

> @@ -450,13 +451,13 @@
>          """
>          args = self._readlist()
>          try:
> -            self.ui = _renewui(self.ui, args)
> +            self.ui, lui = _loadnewui(self.ui, args)
>          except error.ParseError as inst:
>              dispatch._formatparse(self.ui.warn, inst)
>              self.ui.flush()
>              self.cresult.write('exit 255')
>              return
> -        newhash = hashstate.fromui(self.ui, self.hashstate.mtimepaths)
> +        newhash = hashstate.fromui(lui, self.hashstate.mtimepaths)

This assumes that the initial self.hashstate is calculated from repo.ui, which
implies self.ui was initially a repo.ui (~= lui).

If we change "hg serve --cmdserver chgunix" not to load repo, we'll have to
get "lui" in some way to calculate the initial hash.
Jun Wu - March 20, 2016, 5:59 p.m.
On 03/20/2016 09:13 AM, Yuya Nishihara wrote:
> Got pyflakes error. I'll fix this in flight and push to the clowncopter, thanks.

Sorry. I thought there was only one place calling it and did the rename
without thinking twice. I will improve my workflow to make sure test-check* are
always executed no matter how trivial the change is, right now.

> This assumes that the initial self.hashstate is calculated from repo.ui, which
> implies self.ui was initially a repo.ui (~= lui).

Yes. It looks a bit strange now.

> If we change "hg serve --cmdserver chgunix" not to load repo, we'll have to
> get "lui" in some way to calculate the initial hash.

Since repo config may have [extensions], it must be loaded. While we can skip
constructing the repo object.

An explicit _loadnewui at chgunixservice.init may help to clarify how the code
works, at the cost of 0.01 seconds for startup time. Alternatively, putting
some comments there may also help.

Patch

diff --git a/hgext/chgserver.py b/hgext/chgserver.py
--- a/hgext/chgserver.py
+++ b/hgext/chgserver.py
@@ -267,7 +267,7 @@ 
 
     return chgui(srcui)
 
-def _renewui(srcui, args=None):
+def _loadnewui(srcui, args=None):
     if not args:
         args = []
 
@@ -277,14 +277,7 @@ 
     if util.safehasattr(srcui, '_csystem'):
         newui._csystem = srcui._csystem
 
-    # load wd and repo config, copied from dispatch.py
-    cwds = dispatch._earlygetopt(['--cwd'], args)
-    cwd = cwds and os.path.realpath(cwds[-1]) or None
-    rpath = dispatch._earlygetopt(["-R", "--repository", "--repo"], args)
-    path, newui = dispatch._getlocal(newui, rpath, wd=cwd)
-
     # internal config: extensions.chgserver
-    # copy it. it can only be overrided from command line.
     newui.setconfig('extensions', 'chgserver',
                     srcui.config('extensions', 'chgserver'), '--config')
 
@@ -301,7 +294,15 @@ 
             # ui.configsource returns 'none' by default
             source = ''
         newui.setconfig(section, name, value, source)
-    return newui
+
+    # load wd and repo config, copied from dispatch.py
+    args = args[:]
+    cwds = dispatch._earlygetopt(['--cwd'], args)
+    cwd = cwds and os.path.realpath(cwds[-1]) or None
+    rpath = dispatch._earlygetopt(["-R", "--repository", "--repo"], args)
+    path, newlui = dispatch._getlocal(newui, rpath, wd=cwd)
+
+    return (newui, newlui)
 
 class channeledsystem(object):
     """Propagate ui.system() request in the following format:
@@ -450,13 +451,13 @@ 
         """
         args = self._readlist()
         try:
-            self.ui = _renewui(self.ui, args)
+            self.ui, lui = _loadnewui(self.ui, args)
         except error.ParseError as inst:
             dispatch._formatparse(self.ui.warn, inst)
             self.ui.flush()
             self.cresult.write('exit 255')
             return
-        newhash = hashstate.fromui(self.ui, self.hashstate.mtimepaths)
+        newhash = hashstate.fromui(lui, self.hashstate.mtimepaths)
         insts = []
         if newhash.mtimehash != self.hashstate.mtimehash:
             addr = _hashaddress(self.baseaddress, self.hashstate.confighash)