Patchwork [2,of,2,V2] chgserver: handle ParseError during validate

login
register
mail settings
Submitter Jun Wu
Date March 14, 2016, 1:01 p.m.
Message ID <4f0a686606863ed15e66.1457960489@x1c>
Download mbox | patch
Permalink /patch/13862/
State Accepted
Delegated to: Yuya Nishihara
Headers show

Comments

Jun Wu - March 14, 2016, 1:01 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1457959955 0
#      Mon Mar 14 12:52:35 2016 +0000
# Node ID 4f0a686606863ed15e660bce515300769ca82cf9
# Parent  e57b54cc488387c44eb85e0ef30f5c87619c20c3
chgserver: handle ParseError during validate

Currently the validate command in chgserver expects config can be loaded
without issues but the config can be broken and chg will print a stacktrace
instead of the parsing error, if a chg server is already running.

This patch adds a handler for ParseError in validate and a new instruction
"exit" to make the client exit without abortmsg. A test is also added to make
sure it will behave as expected.
Yuya Nishihara - March 14, 2016, 3:43 p.m.
On Mon, 14 Mar 2016 13:01:29 +0000, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1457959955 0
> #      Mon Mar 14 12:52:35 2016 +0000
> # Node ID 4f0a686606863ed15e660bce515300769ca82cf9
> # Parent  e57b54cc488387c44eb85e0ef30f5c87619c20c3
> chgserver: handle ParseError during validate

Pushed slightly modified version to the clowncopter, thanks.

> --- a/hgext/chgserver.py
> +++ b/hgext/chgserver.py
> @@ -429,9 +429,17 @@
>                outdated server.
>              - "redirect $path", the client should try to connect to another
>                server instead.
> +            - "exit $n", the client should exit directly with code n.
> +              This may happen if we cannot parse the config.
>          """
>          args = self._readlist()
> -        self.ui = _renewui(self.ui, args)
> +        try:
> +            self.ui = _renewui(self.ui, args)
> +        except error.ParseError as inst:
> +            dispatch._formatparse(self.ui.warn, inst)
> +            self.ui.flush()
> +            self.cresult.write('exit 255\0')

Instructions should be _separated_ by '\0', not terminated. Removed the last
'\0'.

> diff --git a/tests/test-chg.t b/tests/test-chg.t
> new file mode 100644
> --- /dev/null
> +++ b/tests/test-chg.t
> @@ -0,0 +1,17 @@
> +$ cat >> $HGRCPATH <<EOF
> +> [foo]
> +> a=1
> +EOF

These lines are ignored by the test runner and seem unnecessary. Removed.

> +
> +init repo
> +
> +  $ hg init foo
> +  $ cd foo
> +
> +ill-formed config
> +
> +  $ hg status
> +  $ echo '=brokenconfig' >> $HGRCPATH
> +  $ hg status
> +  hg: parse error at * (glob)
> +  [255]

We'll need '#require chg' at some point, but currently this test works with
the pristine "hg", so I've queued as is.

Patch

diff --git a/contrib/chg/chg.c b/contrib/chg/chg.c
--- a/contrib/chg/chg.c
+++ b/contrib/chg/chg.c
@@ -460,6 +460,11 @@ 
 					 "%s", *pinst + 9);
 			if (r < 0 || r >= (int)sizeof(opts->redirectsockname))
 				abortmsg("redirect path is too long (%d)", r);
+		} else if (strncmp(*pinst, "exit ", 5) == 0) {
+			int n = 0;
+			if (sscanf(*pinst + 5, "%d", &n) != 1)
+				abortmsg("cannot read the exit code");
+			exit(n);
 		} else {
 			abortmsg("unknown instruction: %s", *pinst);
 		}
diff --git a/hgext/chgserver.py b/hgext/chgserver.py
--- a/hgext/chgserver.py
+++ b/hgext/chgserver.py
@@ -429,9 +429,17 @@ 
               outdated server.
             - "redirect $path", the client should try to connect to another
               server instead.
+            - "exit $n", the client should exit directly with code n.
+              This may happen if we cannot parse the config.
         """
         args = self._readlist()
-        self.ui = _renewui(self.ui, args)
+        try:
+            self.ui = _renewui(self.ui, args)
+        except error.ParseError as inst:
+            dispatch._formatparse(self.ui.warn, inst)
+            self.ui.flush()
+            self.cresult.write('exit 255\0')
+            return
         newhash = hashstate.fromui(self.ui, self.hashstate.mtimepaths)
         insts = []
         if newhash.mtimehash != self.hashstate.mtimehash:
diff --git a/tests/test-chg.t b/tests/test-chg.t
new file mode 100644
--- /dev/null
+++ b/tests/test-chg.t
@@ -0,0 +1,17 @@ 
+$ cat >> $HGRCPATH <<EOF
+> [foo]
+> a=1
+EOF
+
+init repo
+
+  $ hg init foo
+  $ cd foo
+
+ill-formed config
+
+  $ hg status
+  $ echo '=brokenconfig' >> $HGRCPATH
+  $ hg status
+  hg: parse error at * (glob)
+  [255]