Patchwork [2,of,2] commandserver: cut import cycle by itself

login
register
mail settings
Submitter Yuya Nishihara
Date Dec. 13, 2015, 2:49 p.m.
Message ID <374b960b42acf96be105.1450018143@mimosa>
Download mbox | patch
Permalink /patch/12012/
State Accepted
Headers show

Comments

Yuya Nishihara - Dec. 13, 2015, 2:49 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1448373834 -32400
#      Tue Nov 24 23:03:54 2015 +0900
# Node ID 374b960b42acf96be105d8eb429fd041a30929c7
# Parent  d5bbc726e292099326891eedd2e5d1c309d84b8f
commandserver: cut import cycle by itself

We generally make modules importable from the front-end layer, dispatch ->
commands -> x. So the import cycle to dispatch should be resolved by the
commandserver module.
Augie Fackler - Dec. 15, 2015, 6:10 p.m.
On Sun, Dec 13, 2015 at 11:49:03PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1448373834 -32400
> #      Tue Nov 24 23:03:54 2015 +0900
> # Node ID 374b960b42acf96be105d8eb429fd041a30929c7
> # Parent  d5bbc726e292099326891eedd2e5d1c309d84b8f
> commandserver: cut import cycle by itself

Queued these (thanks for the review indygreg), though at some point
I'd like to sit down and figure out how we can split a module
(probably commandserver?) such that it doesn't perform this kind of
layering violation.

>
> We generally make modules importable from the front-end layer, dispatch ->
> commands -> x. So the import cycle to dispatch should be resolved by the
> commandserver module.
>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -23,6 +23,7 @@ import setdiscovery, treediscovery, dagu
>  import phases, obsolete, exchange, bundle2, repair, lock as lockmod
>  import ui as uimod
>  import streamclone
> +import commandserver
>
>  table = {}
>
> @@ -6185,7 +6186,6 @@ def serve(ui, repo, **opts):
>          s.serve_forever()
>
>      if opts["cmdserver"]:
> -        import commandserver
>          service = commandserver.createservice(ui, repo, opts)
>      else:
>          service = hgweb.createservice(ui, repo, opts)
> diff --git a/mercurial/commandserver.py b/mercurial/commandserver.py
> --- a/mercurial/commandserver.py
> +++ b/mercurial/commandserver.py
> @@ -16,7 +16,6 @@ import traceback
>
>  from .i18n import _
>  from . import (
> -    dispatch,
>      encoding,
>      error,
>      util,
> @@ -186,6 +185,7 @@ class server(object):
>      def runcommand(self):
>          """ reads a list of \0 terminated arguments, executes
>          and writes the return code to the result channel """
> +        from . import dispatch  # avoid cycle
>
>          length = struct.unpack('>I', self._read(4))[0]
>          if not length:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -23,6 +23,7 @@  import setdiscovery, treediscovery, dagu
 import phases, obsolete, exchange, bundle2, repair, lock as lockmod
 import ui as uimod
 import streamclone
+import commandserver
 
 table = {}
 
@@ -6185,7 +6186,6 @@  def serve(ui, repo, **opts):
         s.serve_forever()
 
     if opts["cmdserver"]:
-        import commandserver
         service = commandserver.createservice(ui, repo, opts)
     else:
         service = hgweb.createservice(ui, repo, opts)
diff --git a/mercurial/commandserver.py b/mercurial/commandserver.py
--- a/mercurial/commandserver.py
+++ b/mercurial/commandserver.py
@@ -16,7 +16,6 @@  import traceback
 
 from .i18n import _
 from . import (
-    dispatch,
     encoding,
     error,
     util,
@@ -186,6 +185,7 @@  class server(object):
     def runcommand(self):
         """ reads a list of \0 terminated arguments, executes
         and writes the return code to the result channel """
+        from . import dispatch  # avoid cycle
 
         length = struct.unpack('>I', self._read(4))[0]
         if not length: