Patchwork [07,of,10] chgserver: implement validate command

login
register
mail settings
Submitter Jun Wu
Date March 2, 2016, 10:44 a.m.
Message ID <7a0eaf66c464e1fcab9b.1456915449@x1c>
Download mbox | patch
Permalink /patch/13528/
State Superseded
Commit 8f9661d1637b7207bd3b1e58b60cf13187690b9d
Delegated to: Yuya Nishihara
Headers show

Comments

Jun Wu - March 2, 2016, 10:44 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1456914735 0
#      Wed Mar 02 10:32:15 2016 +0000
# Node ID 7a0eaf66c464e1fcab9b8470811de0d88ef754ff
# Parent  9821d8ed7874129231a85e86131d37d02f01f129
chgserver: implement validate command

validate will load the repo config and check if the server has up-to-date
config to continue serve the client. In case it does not, the server will
send instructions to the client about what to do next, including to retry
with a different address or to unlink an outdated socket file to stop an
old server.
Yuya Nishihara - March 3, 2016, 4:06 p.m.
On Wed, 2 Mar 2016 10:44:09 +0000, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1456914735 0
> #      Wed Mar 02 10:32:15 2016 +0000
> # Node ID 7a0eaf66c464e1fcab9b8470811de0d88ef754ff
> # Parent  9821d8ed7874129231a85e86131d37d02f01f129
> chgserver: implement validate command
> 
> validate will load the repo config and check if the server has up-to-date
> config to continue serve the client. In case it does not, the server will
> send instructions to the client about what to do next, including to retry
> with a different address or to unlink an outdated socket file to stop an
> old server.
> 
> diff --git a/hgext/chgserver.py b/hgext/chgserver.py
> --- a/hgext/chgserver.py
> +++ b/hgext/chgserver.py
> @@ -25,6 +25,9 @@
>  'setumask' command
>      set umask
>  
> +'validate' command
> +    reload the config and check if the server is up to date
> +
>  'SIGHUP' signal
>      reload configuration files
>  
> @@ -339,6 +342,8 @@
>          self._oldios = []  # original (self.ch, ui.fp, fd) before "attachio"
>          self.hashstate = hashstate
>          self.baseaddress = baseaddress
> +        if hashstate is not None:
> +            self.capabilities['validate'] = chgcmdserver.validate

self.capabilities is statically defined. We'll need to copy it before updating.

> +    def validate(self):
> +        """reload the config and check if the server is up to date
> +
> +        read a list of NULL terminated arguments.
> +        write a list of NULL terminated instruction strings.
> +        an instruction string could be either:
> +            - "unlink $path", the client should unlink the path to stop the
> +              outdated server.
> +            - "redirect $path", the client should try to connect to another
> +              server instead.
> +        """
> +        args = self._readlist()
> +        self.ui = _renewui(self.ui, args)
> +        newhash = hashstate.fromui(self.ui, self.hashstate.mtimepaths)
> +        insts = []
> +        if newhash.mtimehash != self.hashstate.mtimehash:
> +            addr = _hashaddress(self.baseaddress, self.hashstate.confighash)
> +            insts.append('unlink %s' % addr)
> +        if newhash.confighash != self.hashstate.confighash:
> +            addr = _hashaddress(self.baseaddress, newhash.confighash)
> +            insts.append('redirect %s' % addr)
> +        _log('validate: %s\n' % insts)
> +        self.cresult.write('\0'.join(insts) + '\0')

It seems awkward that the input has no last '\0', but the output has, though
I know validate() have to return something even if insts is empty.
Jun Wu - March 4, 2016, 11:42 a.m.
On 03/03/2016 04:06 PM, Yuya Nishihara wrote:
> It seems awkward that the input has no last '\0', but the output has, though
> I know validate() have to return something even if insts is empty.

The input is consistent with runcommand and getpager. Since there are 3
commands using a same input, I have considered adding another command for just
passing "args" beforehand. But maintaining compatibility for runcommand is hard
so I gave up.
Yuya Nishihara - March 4, 2016, 1:46 p.m.
On Fri, 4 Mar 2016 11:42:59 +0000, Jun Wu wrote:
> On 03/03/2016 04:06 PM, Yuya Nishihara wrote:
> > It seems awkward that the input has no last '\0', but the output has, though
> > I know validate() have to return something even if insts is empty.
> 
> The input is consistent with runcommand and getpager. Since there are 3
> commands using a same input, I have considered adding another command for just
> passing "args" beforehand. But maintaining compatibility for runcommand is hard
> so I gave up.

I'm okay with "args". My point is that "args" isn't terminated by '\0', so
"insts" shouldn't be terminated by '\0' as well.
Jun Wu - March 4, 2016, 2:17 p.m.
On 03/04/2016 01:46 PM, Yuya Nishihara wrote:
> I'm okay with "args". My point is that "args" isn't terminated by '\0', so
> "insts" shouldn't be terminated by '\0' as well.

I see. But that will require some extra efforts in both client and server to
make it work. I would be happy to keep it as is.
Yuya Nishihara - March 4, 2016, 3:39 p.m.
On Fri, 4 Mar 2016 14:17:06 +0000, Jun Wu wrote:
> On 03/04/2016 01:46 PM, Yuya Nishihara wrote:
> > I'm okay with "args". My point is that "args" isn't terminated by '\0', so
> > "insts" shouldn't be terminated by '\0' as well.
> 
> I see. But that will require some extra efforts in both client and server to
> make it work. I would be happy to keep it as is.

Hmm, I did that extra effort for "getpager" command. Since the client have to
make sure the received data is NULL terminated, there wouldn't be much
difference in C code.

Patch

diff --git a/hgext/chgserver.py b/hgext/chgserver.py
--- a/hgext/chgserver.py
+++ b/hgext/chgserver.py
@@ -25,6 +25,9 @@ 
 'setumask' command
     set umask
 
+'validate' command
+    reload the config and check if the server is up to date
+
 'SIGHUP' signal
     reload configuration files
 
@@ -339,6 +342,8 @@ 
         self._oldios = []  # original (self.ch, ui.fp, fd) before "attachio"
         self.hashstate = hashstate
         self.baseaddress = baseaddress
+        if hashstate is not None:
+            self.capabilities['validate'] = chgcmdserver.validate
 
     def cleanup(self):
         # dispatch._runcatch() does not flush outputs if exception is not
@@ -385,6 +390,30 @@ 
 
         self.cresult.write(struct.pack('>i', len(clientfds)))
 
+    def validate(self):
+        """reload the config and check if the server is up to date
+
+        read a list of NULL terminated arguments.
+        write a list of NULL terminated instruction strings.
+        an instruction string could be either:
+            - "unlink $path", the client should unlink the path to stop the
+              outdated server.
+            - "redirect $path", the client should try to connect to another
+              server instead.
+        """
+        args = self._readlist()
+        self.ui = _renewui(self.ui, args)
+        newhash = hashstate.fromui(self.ui, self.hashstate.mtimepaths)
+        insts = []
+        if newhash.mtimehash != self.hashstate.mtimehash:
+            addr = _hashaddress(self.baseaddress, self.hashstate.confighash)
+            insts.append('unlink %s' % addr)
+        if newhash.confighash != self.hashstate.confighash:
+            addr = _hashaddress(self.baseaddress, newhash.confighash)
+            insts.append('redirect %s' % addr)
+        _log('validate: %s\n' % insts)
+        self.cresult.write('\0'.join(insts) + '\0')
+
     def _saveio(self):
         if self._oldios:
             return False