Fix compile_test.py for CXX values that contain spaces.

Fix compile_test.py for CXX values that contain spaces.

One example is CXX="ccache g++".

Using shell=True seems to be a worse option - we'd have to
construct a string to pass to bash -c, instead of building
a nice argv array.

BUG=none
R=mark@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=200910

[Mark Mentovai, 2013-05-10 13:34:12]

LGTM

[qengho, 2013-05-10 16:27:48]

I'm not so sure. There's nothing unusual about spaces in a path, and CXX="/My
Documents/bin/gcc" wouldn't do what anyone expects.

The CXX should be a program, not something to be interpreted in the context of a
sub-shell.  Backslash escaping doesn't make sense here either.

If someone is doing something shell-y, they should be running a shell that they
control.

cat >/usr/local/magicalcxx <<EOF
#!/bin/sh
exec somethingelse somethingelse /usr/bin/gcc "$*"
EOF

CXX=magicalcxx

[Mark Mentovai, 2013-05-10 16:30:20]

Because Makefiles have traditionally played it fast and loose with quoting,
unfortunately, environment variables like CC, CXX, CFLAGS, etc. are passed to
the shell as-is and not treated as single arguments. I don’t like it either.

[Paweł Hajdan Jr., 2013-05-10 17:06:36]

On 2013/05/10 16:27:48, chad.miller wrote:
> I'm not so sure. There's nothing unusual about spaces in a path, and CXX="/My
> Documents/bin/gcc" wouldn't do what anyone expects.

Chad, thanks for raising it. I also hate it when things break because of spaces
in the paths.

I'd like to reach a consensus about this before anything gets committed.

What are your suggestions?

My thought is that the behavior in this patch should be default (issue raised by
Elan is also a valid one), but I can imagine some way to override it, possibly
via an environment variable (CHROMIUM_COMPILE_TEST_USE_CXX_VERBATIM=1?).

> If someone is doing something shell-y, they should be running a shell that
they
> control.

Possibly - note however that the ccache manual
(http://ccache.samba.org/manual.html#_run_modes) says "You can either prefix
your compilation commands with ccache or you can let ccache masquerade as the
compiler by creating a symbolic link (named as the compiler) to ccache." . I see
your point, and it would be great to clean this things up. I just don't want to
argue with the ccache manual for example. I could consider flipping the defaults
though (but for now I'd prefer not to do that).

[Paweł Hajdan Jr., 2013-05-15 17:33:31]

On 2013/05/10 17:06:36, Paweł Hajdan Jr. wrote:
> Chad, thanks for raising it. I also hate it when things break because of
spaces
> in the paths.
> 
> I'd like to reach a consensus about this before anything gets committed.

Chad, I'm going to land this if there are no further objections or comments one
week from your original comment (it is five days already).

I'm listening to review feedback, but generally there shouldn't be response
delays longer than 1-2 working days. Still, feel free to submit patches or
comments later.

[Paweł Hajdan Jr., 2013-05-17 22:46:29]

Committed patchset #1 manually as r200910 (presubmit successful).