#1447: wxGUI wingrass scripts need whitespace in path
------------------------------+---------------------------------------------
Reporter: mmetz | Owner: grass-dev@…
Type: defect | Status: new
Priority: major | Milestone: 6.4.2
Component: wxGUI | Version: svn-releasebranch64
Keywords: wingrass, spaces | Platform: MSWindows XP
Cpu: All |
------------------------------+---------------------------------------------
Comment(by glynn):
Replying to [comment:11 mmetz]:
> > subprocess.Popen() has to convert the argument list to a command
string according to the rules by which MSVCRT parses the command string
into arguments, so that the program's main() ends up seeing the correct
argv. One of those rules is that strings containing whitespace must be
quoted.
>
> That's all fine for programs that have a main(), but shell scripts don't
have main().
sh.exe does, and it uses argv[i] as the basis for the $<i>. It may perform
conversions (e.g. filename translation) on the individual arguments, but
it doesn't merge argv back into a string then parse it back into
individual arguments. However, with "-c", it will end up parsing the
resulting string into a command and arguments.
> I think that is the reason why the current implementation works for real
programs but not for scripts. MSVCRT rules do not (need to) cater for
strings passed to cmd.exe, then to %GRASS_SH%.
cmd.exe has the same argument parsing rules as MSVCRT. Once an argument
list has been converted to a string, prepending "cmd.exe /c " to the
beginning of the string is supposed to be a no-op, except that the
"program" is no longer restricted to binary executables.
> AFAICT, a file path without whitespaces is passed ok to the Windows
shell (cmd.exe), but then further passed to %GRASS_SH%, yet another shell,
and there the (by now unprotected) backslashes get lost if not quoted
properly.
Aha. The problem is with scripts/windows_launch.bat:
{{{
@"%GRASS_SH%" -c '"%GISBASE%/scripts/SCRIPT_NAME" %*'
}}}
It should be:
{{{
@"%GRASS_SH%" "%GISBASE%/scripts/SCRIPT_NAME" %*
}}}
The former approach can't be made to work. There is no practical way to
write a batch file which will reliably turn a program and a list of
arguments into a Bourne-shell command which will execute that program with
those arguments. Any attempt will work for simple cases but fail for
arguments which contain shell metacharacters (space, backslash, quote,
etc). cmd.exe's string handling is too primitive to do it right.
The latter approach simply runs the shell with the argument list which was
passed to the script, but with the path to the script "pushed" onto the
front (i.e. the original arguments are shifted along one place). This is
exactly how Unix handles a shebang. If this doesn't work, it's because of
"workarounds" elsewhere which try to compensate for this.
> The strings indicating file paths do have escaping under Windows within
the wxGUI, but it's \ that is escaped to \\, looks fine to me.
Are you sure that it isn't Python doing this? Just to be sure, can you add
e.g.:
{{{
f = open('debug.txt', 'w')
f.write(repr((args, kwargs)))
f.close()
}}}
immediately before the subprocess.Popen.__init__ call?
If those doubled backslashes are actually in the strings, that's a bug
which needs to be fixed. subprocess.Popen() does the required quoting, and
it gets it right. Note that backslashes will be doubled by repr(); if
there are quad backslashes, that means they've really been doubled.
> These strings are converted somewhere inside subprocess.Popen to protect
whitespaces, but not to protect backslashes (escapes).
The only conversion which subprocess.Popen() performs is to convert the
argument list to a string according to the MSVCRT/cmd.exe rules. If you
write a C program which just prints out main()'s argv, it will show the
exact argument list which was passed to subprocess.Popen().
> I'm pretty sure that this is the problem for scripts
I'm pretty sure that the core problem is with windows_launch.bat, as
described above. There may well be other bugs which were introduced to
attempt to "cancel out" the bug in windows_launch.bat.
> The fix causes subprocess.Popen to protect everything because the
program and its args are passed as one single string which means that
protection of single arguments, e.g. input="D:\Grass_test_data\IT\It.txt"
must be done manually.
This is an attempt (probably not the first) to "cancel out" the actual
bug.
--
Ticket URL: <https://trac.osgeo.org/grass/ticket/1447#comment:12>
GRASS GIS <http://grass.osgeo.org>