Apache OpenOffice (AOO) Bugzilla – Issue 81252
Problem running double-quoted command with double-quoted arguments
Last modified: 2013-08-07 15:34:52 UTC
dmake (on Windows XP, at least) seems unable to run a command such as: "command" "arg" (including the double-quotes) which causes problems when the command to be run contains a space in its path and requires an argument that also contains a space (i.e. so that both *need* double-quoting). It isn't the spaces themselves that cause the trouble. It is the use of double-quotes the breaks it. The following simple makefile doesn't require the commands or arguments to be double-quoted, but illustrates the problem: HELLO1 = C:\Temp\hello.exe HELLO2 = C:\Temp\hello.exe arg HELLO3 = C:\Temp\hello.exe "arg" HELLO4 = "C:\Temp\hello.exe" HELLO5 = "C:\Temp\hello.exe" arg HELLO6 = "C:\Temp\hello.exe" "arg" all : @echo $(HELLO1) @$(HELLO1) @echo $(HELLO2) @$(HELLO2) @echo $(HELLO3) @$(HELLO3) @echo $(HELLO4) @$(HELLO4) @echo $(HELLO5) @$(HELLO5) @echo $(HELLO6) @$(HELLO6) When I place that makefile in C:\Temp, together with a compiled C program called hello.exe that simply printf()s "Hello, world.", I get the following output by running "dmake": C:\Temp\hello.exe Hello, world. C:\Temp\hello.exe arg Hello, world. C:\Temp\hello.exe "arg" Hello, world. "C:\Temp\hello.exe" Hello, world. "C:\Temp\hello.exe" arg Hello, world. "C:\Temp\hello.exe" "arg" 'C:\Temp\hello.exe" "arg' is not recognized as an internal or external command, operable program or batch file. dmake: Error code 129, while making 'all' As you can see, the last command, combining the use of double-quotes on the command as well as its argument breaks it. The previous five commands show that this particular example can be fixed by simply not using double-quotes, but that isn't an option if the command and the argument happen to contain spaces. This is currently causing trouble for a user of my CPAN binary distribution of dmake-4.8 for building perl and perl CPAN modules: http://rt.cpan.org/Public/Bug/Display.html?id=28847 The same makefile as above runs OK (producing the expected output) using Microsoft's nmake (VC6, VC7, VC8 or even VC9 Beta 2!), so it must be a dmake thing, rather than something intrinsically wrong with the cmd.exe shell or Win32's process handling generally. PS. The web page http://tools.openoffice.org/dmake/ still refers to dmake-4.7 as the latest version. I thought that 4.8 had been released (and, indeed, the dmake source that I have verified the above bug in identifies itself as 4.9(-to-be, I guess), which also suggests that 4.8 is released).
Hi Steve, I can confirm the problem with a MinGW build dmake. I tried dmake 4.7 and dmake 4.9, both show the problem. I will have a look how easy this is to fix, but it looks like something stupid is happening, note the missing leading and trailing " in: 'C:\Temp\hello.exe" "arg' Oh, yes, dmake 4.8 and 4.9 were "released" and currently dmake 4.11 is in the making. (I skipped 4.10 because of the ambiguity of 4.1 with patch level 0.) Sorry, I didn't send a note as these were only minor bug fix releases.
Hrmpf, thank you Microsoft! Shell recipes are executed by dmake with a command that is created like this: $(SHELL) $(SHELLFLAGS) $(expanded_recipe_command) For the given example this is: cmd.exe /C "C:\Temp\hello.exe" "arg" Now, open a new cmd and try it. :( This follows this rule: > 2. Otherwise, old behavior is to see if the first character is a quote > character and if so, strip the leading character and remove the last quote > character on the command line, preserving any text after the last quote > character. Adding extra quotes would solve this: $(SHELL) $(SHELLFLAGS) "$(expanded_recipe_command)" but this would only be correct for cmd.exe, for any other shell this might create additional problems. (Your problematic example works fine with bash) How about a new speial macro, say SHELLADDQUOTE (and also GROUPSHELLQUOTE), that adds the quotes if set to yes?
Thanks for the quick reply. I've mulled it over a bit, and can't really think of anything better than your suggestion. I was originally thinking that perhaps the $(expanded_recipe_command) could be picked apart into its constituent pieces with proper regard for quoting, and then explicitly stuck back together in some appropriate way once we know what we've got. There is a perl module that plays these sorts of games: see the split_like_sheel() function in ExtUtils::CBuilder::Platform::Windows if you're familiar with perl. However, that all depends on the shell being cmd.exe as well, so it's no better than your (much simpler!) solution. So I think a couple of new special macros would be fine. Would they be set to a sensible default (i.e. assuming cmd.exe behaviour) for Win32 users, e.g. in startup/win95/macros.mk? That way, my binary distributions would work out-of-the-box for users with a cmd.exe shell. Users with other fancy shells would (possibly) have to edit their startup files to set a value appropriate to their shell, but that doesn't really bother me.
Created attachment 48089 [details] Patch for dmake - Add SHELLCMDQUOTE
Hi Steve, the previous patch prepends/appends $(SHELLCMDQUOTE) to the recipe if that macro is defined. Shell recipes are now executed with a command like this: $(SHELL) $(SHELLFLAGS) $(SHELLCMDQUOTE)$(expanded_recipe_command)$(SHELLCMDQUOTE) I briefly tested that it works with your testcase. Some magic in startup/{win96,winnt}/macros.mk should take care of setting the new macro for cmd.exe and command.com. (This is untested ;) ) Have a look, if it works for you I will commit it to dmake411. (Then we only need some documentation for the new macro.)
I added your patch into the latest source and gave it a whirl. It does indeed fix the original testcase that I posted, but testing it some more I've managed to make it crash :-( Try the following makefile: HELLO = "C:\Temp\directory with spaces\hello.exe" "arg" all : @echo $(HELLO) @$(HELLO) where the hello.exe program is as before, just moved into a sub-directory. When I place that makefile in C:\Temp and run "dmake" I get the expected output: "C:\Temp\directory with spaces\hello.exe" "arg" Hello, world. followed by a crash. Rebuilding in debug mode and firing up the debugger, I find that the crash happens here: NTDLL! 7c910f29() NTDLL! 7c910d5c() DMAKE! free + 102 bytes Exec_commands(tcell * 0x0032be70) line 1380 + 9 bytes Make(tcell * 0x0032be70, tcell * 0x00000000) line 750 + 9 bytes Make(tcell * 0x00322400, tcell * 0x00000000) line 580 + 13 bytes Make(tcell * 0x00322360, tcell * 0x00000000) line 580 + 13 bytes Make_targets() line 241 + 14 bytes main(int 0, char * * 0x00320f90) line 441 + 5 bytes DMAKE! mainCRTStartup + 180 bytes KERNEL32! 7c816fd7() The value of "cmnd" that is being passed to FREE() by Exec_commands() looks corrupted: "C:\TempJ [sic!] followed by bytes 0x01, 0x09, 0x00. Trying the same makefile with the unpatched sources, of course, gives an error, but if I change the makefile to what the patch would effectively do to it, namely: HELLO = "C:\Temp\directory with spaces\hello.exe" "arg" all : @echo "$(HELLO)" @"$(HELLO)" then it runs OK without crashing.
Oops, yes I will fix this.
Created attachment 48186 [details] Patch for dmake - Add SHELLCMDQUOTE v2
OK, try this.
Created attachment 48290 [details] New patch, disregard the previous one.
Committed the patch and added documentation to the manpage. Please verify.
Where do I check out the source from to test this? I'm guessing it is the cws_src680_dmake411 branch, so I checked out the source using: cvs -d :pserver:anoncvs@anoncvs.services.openoffice.org:/cvs co -r cws_src680_dmake411 dmake and gave it a whirl. It works fine for me, and the docs look OK too. Should the SHELLCMDQUOTE macro (around line 1954 of man/dmake.tf) be mentioned before SHELLFLAGS, though, just to keep the ordering alphabetical (which it almost is otherwise)? Thanks for fixing this, btw. Any idea when a version (4.11?) containing this will be released?
Yes, cws_src680_dmake411 is the right one. I will fix the alphabetical order later tonight. The CWS is ready, I will do a full OOo build tonight and if nothing comes up I will set it to "Ready for QA" in EIS. <http://eis.services.openoffice.org/EIS2/cws.ShowCWS?Path=SRC680/dmake411> The rest depends on the QA person, and if he finds some other problems, but in principle it should only take a few weeks until it is integrated. ause: Please verify.
I just noticed that the ChangeLog and NEWS files make no mention of this fix. Should they be updated now, or does that come later?
verification looks already done. noticed that there is no testcase for this. it's not mandatory for this version but i think i would like to have one.
@ause: Can you please test the following makefile: --- makefile.mk --- SHELL*:=printf SHELLFLAGS*:= SHELLCMDQUOTE=" all : @+testtest --- makefile.mk --- $ dmake.exe -rf makefile.mk should result in: "testtest" If that works on solaris, I'll make it a testcase. If printf fails, maybe it works with `echo' instead?
yes, works
Testcase committed.
.
dmake 4.11 is in use now