[PATCH] ssh: use shlex to parse command line

Brendan Cully brendan at kublai.com
Fri Sep 7 12:33:34 CDT 2007


On Friday, 07 September 2007 at 10:57, Steve Borho wrote:
> On Thu, 2007-09-06 at 20:03 -0700, Brendan Cully wrote:
> > On Thursday, 06 September 2007 at 21:33, Steve Borho wrote:
> > > On Thu, 2007-09-06 at 18:00 -0700, Brendan Cully wrote:
> > > > On Thursday, 06 September 2007 at 15:44, Steve Borho wrote:
> > > > > # HG changeset patch
> > > > > # User Steve Borho <steve at borho.org>
> > > > > # Date 1189111299 18000
> > > > > # Node ID 2cb669d7c6f6b52ff816e5ea2a0e89513a2c8044
> > > > > # Parent  f8c36b215281a7e8f3aaed632206d3627ee21e6e
> > > > > ssh: use shlex to parse command line
> > > > 
> > > > This seems to break test-init here. I haven't looked into why.
> > > 
> > > I tracked it down to the switch from os.system() to os.popen2().
> > > The i.close() is supposed to return the same status that os.system()
> > > returned, but it appears to always be None, thus allowing failed create
> > > commands to continue (potentially dangerous).
> > > 
> > > I'll see if I can find a better fix.
> > 
> > os.popen returns the result code, but os.popen2 does not. Probably the
> > simplest thing to do is to use util.shellquote to produce a string for
> > system. Something like ' '.join([util.shellquote(x) for x in args])
> > might work? I'm not clear on what windows expects in terms of quoting
> > though.
> 
> Aw crap.  os.popen3() on Windows doesn't support a list for the command
> argument, which makes all the rest of this a moot point.  I think I need
> to use the new subprocess module to get this to work on Windows, but
> that's not compatible with python 2.3 AFAIK.
> 
> So maybe it'll be ok to include a patch for this in my installer patch
> queue since I have control over which python it gets bundled with.

It seems like the shellquote method I mentioned above should still
work to produce a good string for system and popen3. Can you try it on
windows?


More information about the Mercurial-devel mailing list