Python tests fail on filenames with $ signs on non-windows platforms

[Lieven Govaerts <lg...@mobsol.be>]

Hi, 

in r20036 I introduced a new python test, to validate the correct behaviour
of keyword (un)expansion on filenames containing $ signs.

The test works fine on Windows, but when running on linux and mac
buildslaves it fails. 

I see in svntest/main.py that all command-line arguments for svn are wrapped
in double quotes, like this:
CMD: svn.exe "add" "svn-test-work\working_copies\trans_tests-1\id_exp
with_$_sign" "--config-dir"
"C:\devel\subversion\trunk\Debug\subversion\tests\cmdline\svn-test-work\loca
l_tmp\config"

On windows that works fine, but on linux/mac the $_sign will be recognized
as variable and be replaced.

I see two possible solutions:
1. On non-windows platforms, replace all $ signs with \$. Attached patch
will do that.
2. On non-windows platforms, use single-quotes instead of double quotes to
wrap the arguments.

While at first sight both solutions seem to work, I just want to make sure
that it will work on any platform Subversion support. 

Input anyone?

regards,

Lieven.

Re: Python tests fail on filenames with $ signs on non-windows platforms

[Lieven Govaerts <lg...@mobsol.be>]

Quoting Peter Samuelson <pe...@p12n.org>:

>
> [Malcolm Rowe]
> > I like this idea, assuming it works - but if we do do something like
> > this, we probably should also just derive the verbose command line
> > from the argument list when we display it, rather than build up an
> > argument list and verbose string in parallel
>
> Yes, I would've done it that way, except that (a) my patch wasn't
> intended to be applied directly, so I took the easy way out, and (b) I
> don't know python, so this _was_ the easy way out.  (:
>

I'll have a look at it later this week and test it on multiple platforms, if it
works I'll replace my patch with yours.

Lieven.

----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Python tests fail on filenames with $ signs on non-windows platforms

[Peter Samuelson <pe...@p12n.org>]

[Malcolm Rowe]
> I like this idea, assuming it works - but if we do do something like
> this, we probably should also just derive the verbose command line
> from the argument list when we display it, rather than build up an
> argument list and verbose string in parallel

Yes, I would've done it that way, except that (a) my patch wasn't
intended to be applied directly, so I took the easy way out, and (b) I
don't know python, so this _was_ the easy way out.  (:

Re: Python tests fail on filenames with $ signs on non-windows platforms

[Malcolm Rowe <ma...@farside.org.uk>]

On Sun, Jun 11, 2006 at 10:30:41AM -0500, Peter Samuelson wrote:
> IMO, a simpler and better approach would be to let os.popen3() handle
> the quoting instead of trying to do it by hand.  I have no idea how
> intelligent os.popen3 is on the various platforms - would the following
> be sufficiently portable?  It's untested.
> 

I like this idea, assuming it works - but if we do do something like
this, we probably should also just derive the verbose command line from
the argument list when we display it, rather than build up an argument
list and verbose string in parallel (especially as the latter no longer
matches exactly with what we're running).

Regards,
Malcolm

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Python tests fail on filenames with $ signs on non-windows platforms

[Peter Samuelson <pe...@p12n.org>]

[Lieven Govaerts]
> I see in svntest/main.py that all command-line arguments for svn are wrapped
> in double quotes, like this:
> CMD: svn.exe "add" "svn-test-work\working_copies\trans_tests-1\id_exp
> with_$_sign" "--config-dir"
> "C:\devel\subversion\trunk\Debug\subversion\tests\cmdline\svn-test-work\loca
> l_tmp\config"

IMO, a simpler and better approach would be to let os.popen3() handle
the quoting instead of trying to do it by hand.  I have no idea how
intelligent os.popen3 is on the various platforms - would the following
be sufficiently portable?  It's untested.


Index: subversion/tests/cmdline/svntest/main.py
===================================================================
--- subversion/tests/cmdline/svntest/main.py	(revisione 20040)
+++ subversion/tests/cmdline/svntest/main.py	(copia locale)
@@ -16,7 +16,7 @@
 ######################################################################
 
 import sys     # for argv[]
-import os      # for popen2()
+import os      # for popen3()
 import shutil  # for rmtree()
 import re
 import stat    # for ST_MODE
@@ -251,16 +251,16 @@
   Return stdout, stderr as lists of lines.
   If ERROR_EXPECTED is None, any stderr also will be printed."""
 
-  args = ''
-  for arg in varargs:                   # build the command string
+  varargs_verbose = ''
+  for arg in varargs:                   # build the verbose output
     arg = str(arg)
     if os.name != 'nt':
       arg = arg.replace('$', '\$')
-    args = args + ' "' + arg + '"'
+    varargs_verbose = varargs_verbose + ' "' + arg + '"'
 
   # Log the command line
   if verbose_mode:
-    print 'CMD:', os.path.basename(command) + args,
+    print 'CMD:', os.path.basename(command) + varargs_verbose
 
   if binary_mode:
     mode = 'b'
@@ -268,7 +268,7 @@
     mode = 't'
 
   start = time.time()
-  infile, outfile, errfile = os.popen3(command + args, mode)
+  infile, outfile, errfile = os.popen3([command] + varargs, mode)
 
   if stdin_lines:
     map(infile.write, stdin_lines)
@@ -451,17 +451,20 @@
   # require access to the BDB tools, and this doesn't.  Print a fake
   # pipe command so that the displayed CMDs can be run by hand
   create_repos(dst_path)
-  dump_args = ' dump "' + src_path + '"'
-  load_args = ' load "' + dst_path + '"'
+  dump_args = ['dump', src_path]
+  load_args = ['load', dst_path]
+  dump_args_verbose = ' dump "' + src_path + '"'
+  load_args_verbose = ' load "' + dst_path + '"'
 
   if ignore_uuid:
-    load_args = load_args + " --ignore-uuid"
+    load_args.append('--ignore-uuid')
+    load_args_verbose = load_args_verbose + " --ignore-uuid"
   if verbose_mode:
-    print 'CMD:', os.path.basename(svnadmin_binary) + dump_args, \
-          '|', os.path.basename(svnadmin_binary) + load_args,
+    print 'CMD:', os.path.basename(svnadmin_binary) + dump_args_verbose
+          '|', os.path.basename(svnadmin_binary) + load_args_verbose,
   start = time.time()
-  dump_in, dump_out, dump_err = os.popen3(svnadmin_binary + dump_args, 'b')
-  load_in, load_out, load_err = os.popen3(svnadmin_binary + load_args, 'b')
+  dump_in, dump_out, dump_err = os.popen3([svnadmin_binary] + dump_args, 'b')
+  load_in, load_out, load_err = os.popen3([svnadmin_binary] + load_args, 'b')
   stop = time.time()
   if verbose_mode:
     print '<TIME = %.6f>' % (stop - start)