Summary: | windres fails on files with spaces | ||
---|---|---|---|
Product: | binutils | Reporter: | Hendrik Sattler <sattler> |
Component: | binutils | Assignee: | unassigned |
Status: | REOPENED --- | ||
Severity: | critical | CC: | bug-binutils, david.macek.0, denpashogai, eliz, gusfl, kai.koehne, nickc, spring, tonyt |
Priority: | P2 | ||
Version: | 2.16 | ||
Target Milestone: | --- | ||
Host: | Target: | ||
Build: | Last reconfirmed: | ||
Attachments: |
Put filenames from command line in quotes to handle spaces correctly
Patch to quote file names for Windows shells |
Description
Hendrik Sattler
2007-04-13 15:51:38 UTC
Subject: Re: New: windres fails on files with spaces Hi HS, > This does not seem to be fixed in latest CVS because the error is most > obviously in binutils/resrc.c (and similar) in function read_rc_file() and > look_for_default(): > sprintf (cmd, "%s %s %s", preprocessor, preprocargs, filename); > > The filename argument MUST be surrounded by "", thus this line should read: > sprintf (cmd, "%s %s \"%s\"", preprocessor, preprocargs, filename); > > Easy fix, I'd say. Would you like to submit a patch that does this then ? > Reproducable on command line using DEFAULT_PREPROCESSOR. Please could you provide an example of such a command line ? Cheers Nick Created attachment 1730 [details] Put filenames from command line in quotes to handle spaces correctly > Please could you provide an example of such a command line? Just specify a full path with spaces in it: C:\>windres "C:\test with spaces\gui.rc" "c:\test with spaces"\gui.obj gcc: C:\test: No such file or directory gcc: with: No such file or directory gcc: spaces\gui.rc: No such file or directory gcc: warning: `-x c' after last input file has no effect gcc: no input files windres: no resources Same for the parameter -I. The patch fixes those issue. NOTE: This is still broken for file names that use special shell characters. This is because you use popen() instead of a combination of pipe(), fork(), a flavour of exec() and fdopen(). The latter one does not have any of those issues because the shell is avoided (hey, it will even be faster). So, see this patch as hotfix for common usage. A bit more work is needed to allow shell specific characters in file name. HS PS: popen() is an _evil_ command! Hi Hendrik, This problem should now be resolved. A recent patch to resrc.c should ensure that all special case filenames are quoted when they are used on generated command lines. Cheers Nick It does not seem to be fixed in 2.21; calling windres with the following: --include-dir "C:\Program Files\Lua\5.1\include" Produces: "gcc: Files\Lua\5.1\include: No such file or directory windres: gcc exited with status 1" Using single quotes instead of double quotes causes a different error; I'm not sure if they have another special meaning. I can work around it with: --include-dir "C:\Progra~1\Lua\5.1\include" But it'd be better if it worked the normal way. The fix is equally simple, though, this time in windres.c: -I%s -> -I\"%s\" The include options are also affected, reopening bug... i hit this bug, too... GNU windres (GNU Binutils) 2.23.1 > "C:\Program Files (x86)\CODEBL~1\MinGW\bin\windres.exe" -O coff D:/spring/rts/icon.rc rts/builds/legacy/CMakeFiles/engine-legacy.dir/__/__/icon.rc.obj
Der Befehl "C:\Program" ist entweder falsch geschrieben oder
konnte nicht gefunden werden.
C:\Program Files (x86)\CODEBL~1\MinGW\bin\windres.exe: preprocessing failed.
windres -I "C:\Program Files\MinGW\include" "manifest.rc" manifest.o -> "Files\MinGW\include" : directory not found At least the -I escaping issue is still valid in binutils 2.34: windres -I"C:/Program Files/OpenSSL-Win64/include" gcc: error: Files/OpenSSL-Win64/include: No such file or directory A workaround is to explicitly mask things twice windres -I\""C:/Program Files/OpenSSL-Win64/include"\" I think that the solutions proposed for this bug are flawed, because they paper over the real problem. The real problem is the function 'quot': it is there to quote special characters that can confuse the shell, but the method it uses to quote -- escape-protect each special character with a backslash -- works only for Posix shells. For Windows shells we need to quote "like this", i.e. enclose the whole string in double quotes (and escape-protect any embedded quotes with a backslash). The attached patch modifies 'quot' to DTRT on MS-Windows. I tested it, and it works in all the test cases I threw on it. Created attachment 13160 [details]
Patch to quote file names for Windows shells
The master branch has been updated by Nick Clifton <nickc@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=cc3edc52747fd8b184ee48f1b0cc1ac0aca7832e commit cc3edc52747fd8b184ee48f1b0cc1ac0aca7832e Author: Eli Zaretskii <eliz@gnu.org> Date: Thu Jan 28 14:57:33 2021 +0000 Improve windres's handling of pathnames containing special characters on Windows platforms. PR 4356 * windres.c (quot): Use double quotes to protect strings on Windows platforms. Hi Eli, Thanks for the patch. I have gone ahead and applied it. Cheers Nick It seems the latest patch has broken this unsupported use case that had worked on Win32: windres "--preprocessor=gcc -E" --use-temp-file [...] If I don't enable temp files or don't specify the preprocessor, it works. See <https://gitlab.haskell.org/ghc/ghc/-/issues/16780#note_340986> for an example of such usage. Given GHC corrected its usage to one that seems more correct, I'd say there's no need for an action here, but at least this note could help someone having the same issue. (In reply to David Macek from comment #15) Hi David, > windres "--preprocessor=gcc -E" --use-temp-file [...] Does this alternative command line work ? windres --preprocessor=gcc --preprocessor-arg=-E --use-temp-file [...] Cheers Nick Just to be clear, the problem is that windres is trying to run "gcc -E" as if that was the full name of the preprocessor, and not a program name plus argument. This happens because internally windres puts double quotes around "gcc -E", preventing it from being split up into program+arguments. I don't think I understand the problem being reported by David Macek. First, the fix alluded to there changed how the function 'quot' quotes file names. The problematic use case is with the --preprocessor= option, but that option doesn't use 'quote' in the sources I have. Moreover, the documentation says that --preprocessor= accepts the PROGRAM, which means no command-line arguments; the arguments are supposed to come via the companion option --preprocessor-arg=. So how exactly could the 'quot' change break some use case that worked before, and how could that use case work in the original windres before the change? Are we talking about the stock windres or about some patched version of windres? Looking at the linked Huskell issue, I think the problem is not with the --preprocessor= option, it is with --preprocessor-arg= option, where 'quot' IS used. The problem is that windres.c quotes the argument if it has whitespace, but then 'read_rc_file' quotes it again. This double quoting is what causes the problem. The problem was exposed by the fix in 'quot': previously it basically didn't quote the argument, so the extra quoting in 'read_rc_file' didn't do any harm. So I think removing the fnquotes thingy from 'read_rc_file' should fix the problem. Please test. (In reply to Eli Zaretskii from comment #19) Hi Eli, > So I think removing the fnquotes thingy from 'read_rc_file' should fix the > problem. Please test. But... the fnquotes thingy is used to quote the filename, not the preprocessor args: sprintf (cmd, "%s %s %s%s%s", preprocessor, preprocargs, fnquotes, filename, fnquotes); Oops, you are right. Sorry for misreading the code. Then I guess I'm back where I started: I don't understand the cause of the problem. The original issue seems to involve Python, maybe it's related. I guess I'm confused as well.
The specific comment I linked said:
> We should probably use --preprocessor-arg instead to pass arguments.
Until we realized that, there were discussions on how to patch windres, but those don't apply anymore AFAICT.
If there is something to be fixed, it might be the inconsistency between `--use-temp-file` and `--no-use-temp-file`. Apparently without a temp file, windres still interprets a multiple word `--preprocessor` value as a command and arguments. (Assuming this is not actually just a quirk of our windres build.)
If you can show an example of a windres command that works without temp files but fails with them (or the other way around), I will try to take a look. But please show an example that doesn't involve any additional programs nor Python, just an example that is invoked from the cmd prompt. Thanks. Here you go. Can you confirm this behavior? $ cat a.rc 1 VERSIONINFO FILEVERSION 1,0,0,0 BEGIN END $ ls a.res ls: a.res: No such file or directory $ windres -o a.res a.rc "--preprocessor=gcc -E -xc" $ echo %ERRORLEVEL% 0 $ ls a.res a.res $ rm a.res $ windres -o a.res a.rc "--preprocessor=gcc -E -xc" --use-temp-file windres: CreateProcess (null): No such file or directory $ ls a.res ls: a.res: No such file or directory Thanks, but this is incorrect usage: --preprocessor= doesn't support command-line arguments. The command-line arguments should be passed via --preprocessor-arg=. Sorry for the confusion, I stand corrected. I assumed the presented inconsistency, although in an incorrect usage, would translate into an inconsistency with correct usage. I tried directing windres to something like "C:\path with spaces\gcc", but I wasn't able to break it regardless of temp files. On the other hand, I was able to break windres by having it itself be in a path with spaces: $ mklink /j "W:\mingw64 root" "C:\msys64\mingw64" Junction created for W:\mingw64 root <<===>> C:\msys64\mingw64 $ set PATH=C:\windows\system32;W:\mingw64 root\bin $ "W:\mingw64 root\bin\windres.exe" a.rc -o a.res --use-temp-file W:\mingw64 root\bin\windres.exe: CreateProcess (null): No such file or directory $ "W:\mingw64 root\bin\windres.exe" a.rc -o a.res 'W:\mingw64' is not recognized as an internal or external command, operable program or batch file. W:\mingw64 root\bin\windres.exe: preprocessing failed. $ "W:\mingw64 root\bin\windres.exe" a.rc -o a.res "--preprocessor=W:\mingw64 root\bin\gcc" --preprocessor-arg=-E --preprocessor-arg=-xc (works) $ "W:\mingw64 root\bin\windres.exe" a.rc -o a.res "--preprocessor=W:\mingw64 root\bin\gcc" --preprocessor-arg=-E --preprocessor-arg=-xc --use-temp-file (works) First, this is unrelated to the original issue. Second, I cannot reproduce this with windres 2.36 that I built myself (which is the stock 2.36 version with the patch for 'quot' discussed in this bug report). So my best guess is that there's something wrong with your windres binary, or maybe it has some local patches which cause that. FTR: I didn't use a junction to reproduce the problem, I actually created a directory with a space and copied there the MinGW installation. So maybe the problem is somehow related to resolving links and junctions, I'm not sure. (In reply to Eli Zaretskii from comment #25) > Thanks, but this is incorrect usage: --preprocessor= doesn't support > command-line arguments. The command-line arguments should be passed via > --preprocessor-arg=. May be a little off-topic, but the windres docs[1] state: "This option may be used to specify the preprocessor to use, including any leading arguments" FFmpeg (probably similar to Haskell) use an invocation like: $(WINDRES) $(IFLAGS) --preprocessor "$(DEPWINDRES) -E -xc-header -DRC_INVOKED $(CC_DEPFLAGS)" I imagine this has been the expected behaviour until a different whitespace fix recently[2]. Clarifying this change and the use of `--preprocessor-arg` going forward would be helpful. Cheers, Tony [1] https://sourceware.org/binutils/docs/binutils/windres.html [2] https://sourceware.org/bugzilla/show_bug.cgi?id=26865 The master branch has been updated by Nick Clifton <nickc@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=749c700282097cf679ff019a9674d7c762f48619 commit 749c700282097cf679ff019a9674d7c762f48619 Author: Thomas Wolff <towo@towo.net> Date: Mon May 10 11:28:15 2021 +0100 Restore old behaviour of windres so that options containing spaces are not enclosed in double quotes. PR 4356 PR 26865 PR 27594 * windres.c (quot): Revert previous delta. Do not use double quotes when spaces are detected in options. * doc/binutils.texi (windres): Remove suggestion that the --preprocessor option can take arguments. Reverting the change in `quot` is IMO a mistake, I will respond in PR 27594 with the details. |