Patchwork [RFC,stateful-chg] dispatch: make request accept additional reposetups

login
register
mail settings
Submitter Jun Wu
Date May 17, 2017, 5:44 p.m.
Message ID <9b7eae7f7d613762884a.1495043069@x1c>
Download mbox | patch
Permalink /patch/20661/
State Accepted
Headers show

Comments

Jun Wu - May 17, 2017, 5:44 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1493527187 25200
#      Sat Apr 29 21:39:47 2017 -0700
# Node ID 9b7eae7f7d613762884ac4880be74b23fe4c7415
# Parent  37bcb4665529f5cc59b8dffb1014ac0cab37492c
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 9b7eae7f7d61
dispatch: make request accept additional reposetups

chg needs special logic around repo object creation (like, collecting and
reporting repo path to the master server). Adding "reposetup" to
dispatch.request seems to be an easy and reasonably clean way to allow that.
Jun Wu - May 17, 2017, 6 p.m.
Background on this:

  - chg worker needs to collect "repo path" information so it could report
    them to the main server (read operation)
  - chg worker needs to mangle "repo" state to fill it with cached objects
    (write operation - should happen before any other reposetups for cache
     to be effective)

Since chgserver.py could not use extensions.wrapfunction to mangle
localrepo.localrepository (or hg.repository), and we tend to avoid global
variables. Adding the extra parameter to a chain of functions seems to be
the only way to achieve the above goals.

The code could be simplified by using module-level variables similar to
hg.wirepeersetupfuncs. However, given the fact that people dislike
module-level variables, I sent this dispatch.py version first for comment.
I don't really like either ways but the feature needs to be implemented.

Excerpts from Jun Wu's message of 2017-05-17 10:44:29 -0700:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1493527187 25200
> #      Sat Apr 29 21:39:47 2017 -0700
> # Node ID 9b7eae7f7d613762884ac4880be74b23fe4c7415
> # Parent  37bcb4665529f5cc59b8dffb1014ac0cab37492c
> # Available At https://bitbucket.org/quark-zju/hg-draft 
> #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r 9b7eae7f7d61
> dispatch: make request accept additional reposetups
> 
> chg needs special logic around repo object creation (like, collecting and
> reporting repo path to the master server). Adding "reposetup" to
> dispatch.request seems to be an easy and reasonably clean way to allow that.
> 
> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
> --- a/mercurial/dispatch.py
> +++ b/mercurial/dispatch.py
> @@ -49,5 +49,5 @@ from . import (
>  class request(object):
>      def __init__(self, args, ui=None, repo=None, fin=None, fout=None,
> -                 ferr=None):
> +                 ferr=None, prereposetups=None):
>          self.args = args
>          self.ui = ui
> @@ -59,4 +59,8 @@ class request(object):
>          self.ferr = ferr
>  
> +        # reposetups which run before extensions, useful for chg to pre-fill
> +        # low-level repo state (for example, changelog) before extensions.
> +        self.prereposetups = prereposetups or []
> +
>      def _runexithandlers(self):
>          exc = None
> @@ -876,5 +880,6 @@ def _dispatch(req):
>              else:
>                  try:
> -                    repo = hg.repository(ui, path=path)
> +                    repo = hg.repository(ui, path=path,
> +                                         presetupfuncs=req.prereposetups)
>                      if not repo.local():
>                          raise error.Abort(_("repository '%s' is not local")
> diff --git a/mercurial/hg.py b/mercurial/hg.py
> --- a/mercurial/hg.py
> +++ b/mercurial/hg.py
> @@ -149,8 +149,10 @@ def openpath(ui, path):
>  wirepeersetupfuncs = []
>  
> -def _peerorrepo(ui, path, create=False):
> +def _peerorrepo(ui, path, create=False, presetupfuncs=None):
>      """return a repository object for the specified path"""
>      obj = _peerlookup(path).instance(ui, path, create)
>      ui = getattr(obj, "ui", ui)
> +    for f in presetupfuncs or []:
> +        f(ui, obj)
>      for name, module in extensions.extensions(ui):
>          hook = getattr(module, 'reposetup', None)
> @@ -162,7 +164,7 @@ def _peerorrepo(ui, path, create=False):
>      return obj
>  
> -def repository(ui, path='', create=False):
> +def repository(ui, path='', create=False, presetupfuncs=None):
>      """return a repository object for the specified path"""
> -    peer = _peerorrepo(ui, path, create)
> +    peer = _peerorrepo(ui, path, create, presetupfuncs=presetupfuncs)
>      repo = peer.local()
>      if not repo:
Yuya Nishihara - May 18, 2017, 2:02 p.m.
On Wed, 17 May 2017 10:44:29 -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1493527187 25200
> #      Sat Apr 29 21:39:47 2017 -0700
> # Node ID 9b7eae7f7d613762884ac4880be74b23fe4c7415
> # Parent  37bcb4665529f5cc59b8dffb1014ac0cab37492c
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 9b7eae7f7d61
> dispatch: make request accept additional reposetups

This seems fine to me.
Augie Fackler - May 21, 2017, 12:18 a.m.
On Thu, May 18, 2017 at 11:02:16PM +0900, Yuya Nishihara wrote:
> On Wed, 17 May 2017 10:44:29 -0700, Jun Wu wrote:
> > # HG changeset patch
> > # User Jun Wu <quark@fb.com>
> > # Date 1493527187 25200
> > #      Sat Apr 29 21:39:47 2017 -0700
> > # Node ID 9b7eae7f7d613762884ac4880be74b23fe4c7415
> > # Parent  37bcb4665529f5cc59b8dffb1014ac0cab37492c
> > # Available At https://bitbucket.org/quark-zju/hg-draft
> > #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 9b7eae7f7d61
> > dispatch: make request accept additional reposetups
>
> This seems fine to me.

Agreed, queued.

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -49,5 +49,5 @@  from . import (
 class request(object):
     def __init__(self, args, ui=None, repo=None, fin=None, fout=None,
-                 ferr=None):
+                 ferr=None, prereposetups=None):
         self.args = args
         self.ui = ui
@@ -59,4 +59,8 @@  class request(object):
         self.ferr = ferr
 
+        # reposetups which run before extensions, useful for chg to pre-fill
+        # low-level repo state (for example, changelog) before extensions.
+        self.prereposetups = prereposetups or []
+
     def _runexithandlers(self):
         exc = None
@@ -876,5 +880,6 @@  def _dispatch(req):
             else:
                 try:
-                    repo = hg.repository(ui, path=path)
+                    repo = hg.repository(ui, path=path,
+                                         presetupfuncs=req.prereposetups)
                     if not repo.local():
                         raise error.Abort(_("repository '%s' is not local")
diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -149,8 +149,10 @@  def openpath(ui, path):
 wirepeersetupfuncs = []
 
-def _peerorrepo(ui, path, create=False):
+def _peerorrepo(ui, path, create=False, presetupfuncs=None):
     """return a repository object for the specified path"""
     obj = _peerlookup(path).instance(ui, path, create)
     ui = getattr(obj, "ui", ui)
+    for f in presetupfuncs or []:
+        f(ui, obj)
     for name, module in extensions.extensions(ui):
         hook = getattr(module, 'reposetup', None)
@@ -162,7 +164,7 @@  def _peerorrepo(ui, path, create=False):
     return obj
 
-def repository(ui, path='', create=False):
+def repository(ui, path='', create=False, presetupfuncs=None):
     """return a repository object for the specified path"""
-    peer = _peerorrepo(ui, path, create)
+    peer = _peerorrepo(ui, path, create, presetupfuncs=presetupfuncs)
     repo = peer.local()
     if not repo: