Patchwork [windows,rfc] dummyssh: just use execvp() to replace ourselves instead of os.system()

login
register
mail settings
Submitter Augie Fackler
Date May 2, 2017, 11:39 p.m.
Message ID <f3fff9ce56510a257772.1493768361@augie-macbookair2.roam.corp.google.com>
Download mbox | patch
Permalink /patch/20361/
State Changes Requested
Headers show

Comments

Augie Fackler - May 2, 2017, 11:39 p.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1492110244 14400
#      Thu Apr 13 15:04:04 2017 -0400
# Node ID f3fff9ce56510a257772e43c3c7971f64f30ccec
# Parent  6cacc271ee0a9385be483314dc73be176a13c891
dummyssh: just use execvp() to replace ourselves instead of os.system()

This might let us avoid some weird Windows quoting problems. On the
other hand, it might be a horrible idea. I did this as part of trying
to diagnose weirdness in sshpeer, and it didn't help, but I kept it
because somehow it feels cleaner.
Augie Fackler - May 3, 2017, 2 a.m.
I have no idea if this is a good patch or not, but it looks like it ought to helming out with some windows weirdness. :)

> On May 2, 2017, at 19:39, Augie Fackler <raf@durin42.com> wrote:
> 
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1492110244 14400
> #      Thu Apr 13 15:04:04 2017 -0400
> # Node ID f3fff9ce56510a257772e43c3c7971f64f30ccec
> # Parent  6cacc271ee0a9385be483314dc73be176a13c891
> dummyssh: just use execvp() to replace ourselves instead of os.system()
> 
> This might let us avoid some weird Windows quoting problems. On the
> other hand, it might be a horrible idea. I did this as part of trying
> to diagnose weirdness in sshpeer, and it didn't help, but I kept it
> because somehow it feels cleaner.
> 
> diff --git a/tests/dummyssh b/tests/dummyssh
> --- a/tests/dummyssh
> +++ b/tests/dummyssh
> @@ -3,6 +3,7 @@
> from __future__ import absolute_import
> 
> import os
> +import shlex
> import sys
> 
> os.chdir(os.getenv('TESTTMP'))
> @@ -19,8 +20,5 @@ for i, arg in enumerate(sys.argv[1:]):
> log.write("\n")
> log.close()
> hgcmd = sys.argv[2]
> -if os.name == 'nt':
> -    # hack to make simple unix single quote quoting work on windows
> -    hgcmd = hgcmd.replace("'", '"')
> -r = os.system(hgcmd)
> -sys.exit(bool(r))
> +args = shlex.split(hgcmd)
> +os.execvp("hg", args)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - May 3, 2017, 2:50 a.m.
On Tue, 2 May 2017 22:00:45 -0400, Augie Fackler wrote:
> I have no idea if this is a good patch or not, but it looks like it ought to helming out with some windows weirdness. :)

> > -if os.name == 'nt':
> > -    # hack to make simple unix single quote quoting work on windows
> > -    hgcmd = hgcmd.replace("'", '"')
> > -r = os.system(hgcmd)
> > -sys.exit(bool(r))
> > +args = shlex.split(hgcmd)
> > +os.execvp("hg", args)

No idea whether this is good or bad, but Windows has no POSIX fork & exec
semantics, so Windows execvp() wouldn't be what we want.

Patch

diff --git a/tests/dummyssh b/tests/dummyssh
--- a/tests/dummyssh
+++ b/tests/dummyssh
@@ -3,6 +3,7 @@ 
 from __future__ import absolute_import
 
 import os
+import shlex
 import sys
 
 os.chdir(os.getenv('TESTTMP'))
@@ -19,8 +20,5 @@  for i, arg in enumerate(sys.argv[1:]):
 log.write("\n")
 log.close()
 hgcmd = sys.argv[2]
-if os.name == 'nt':
-    # hack to make simple unix single quote quoting work on windows
-    hgcmd = hgcmd.replace("'", '"')
-r = os.system(hgcmd)
-sys.exit(bool(r))
+args = shlex.split(hgcmd)
+os.execvp("hg", args)