[Bug binutils/27594] build processes broken by changed space handling

[martin at martin dot st]

https://sourceware.org/bugzilla/show_bug.cgi?id=27594

--- Comment #15 from Martin Storsjö <martin at martin dot st> ---
(In reply to Eli Zaretskii from comment #13)
> So bottom line: the revert reverted the wrong commit.

Yup, exactly.

(In reply to Eli Zaretskii from comment #12)
> That means `windres` will be the only program which requires such strange
> invocation rules.  Every other program which does something similar doesn't
> require such "over-quoting".

Yes, there's no denying that it's an uncommon parameter definition.

> And I don't really understand why you are willing to require such strange
> rules: is that only because of some build scripts that use such broken
> multiple quoting?  IOW, instead of fixing things (and the fixes are really
> simple), we are forever doomed to be bug-compatible with them?  And only on
> Windows on top of that?

None of this is Windows specific; you've been able to call it with
--preprocessor "gcc -xc -E -DRC_INVOKED ..." in the same way on both unix and
Windows for the last 10 years. And if one wanted to specify the tool with a
path with spaces (prior to the 2.36 release), that path would have to be double
quoted - both on unix and Windows. (Admittedly, paths with spaces are much
rarer on unix.)

Regarding arguing for/against this, I think existing cases where projects are
using --preprocessor to pass multiple arguments are much much more common than
cases where one wants to specify the executable with a path that needs to be
quoted. And by going with the historical behaviour of the option, those users
are left working, and there's still one way (which is uncomfortable though) to
pass paths with spaces.

In many cases, it's indeed simple to rewrite things in a way that works with
both old and new windres. In one case,
https://git.videolan.org/?p=ffmpeg.git;a=blob;f=ffbuild/common.mak;h=32f5b997b5979c9799aafdf88d84fb4b6677ec80;hb=f8d910e90f599f338438833dfc92e2f1915ce414#l93,
a makefile variable containing a number of preprocessor options (in most cases
something like "-MMD -MF <arg> -MT <otherarg>") is passed in that way;
rewriting that to use --preprocessor-arg requires a bit more changes.


I'm not dead set against changing it, but if changed, I would prefer that it
isn't swept under the rug as "all these users were misusing the option" but own
up to that the old usage was a seemingly valid way of using the option (the
documentation almost hinted that it was supposed to be used that way) and we're
willingly breaking those users for a clearer definition of the option.

> > Yes I agree that the most sane thing to do would be to have the user specify
> > -DSTRING="quotedstring", but there are projects that have ended up working
> > around this, so fixing this case does break them.
> 
> Those projects should get their act together, if you ask me.  It isn't hard,
> really.  Not my call, obviously, but still...

Not at all actually, so far (in releases) there was no way of doing that that
didn't involve double quoting. But that's for a separate bug report.

(In reply to Eli Zaretskii from comment #14)
> And one more comment: using pre-quoted shell command (as opposed to just the
> preprocessor file name) in --preprocessor will not work with
> --use-temp-file, because the code in `run_cmd` breaks the command into
> argv[] array, and will treat the quoted string ("gcc -E ....") as a file
> name to invoke.  This trick only works in the default pipe mode, because
> then the command is not broken into argv[] elements.

No, I don't think so. Before 2.36, a pre-quoted argument in --preproessor would
have ended up as <<<"path to/gcc.exe" -xc -E ...>>> in the cmd string, and
run_cmd would split it correctly. (In 2.36, such arguments to --preprocessor
don't work though.)

The command string, assembled from --preprocessor arguments, -D/-I and other
options, either gets split into an argv[] array by run_cmd, or passed as such
to popen() (where the shell does the same splitting), and arguments with spaces
in the string need to be quoted the same in both cases.

So the cmd string (which is concatenated from the output of the 'quot'
function, and the 'quot' function quotes things in a platform specific way) is
defined to be escaped/quoted for the current platform shell - and the 'run_cmd'
function would need to mirror those platform specific quoting nuances (e.g.
regarding backslash/quote escaping).

-- 
You are receiving this mail because:
You are on the CC list for the bug.