This is an archive of the discontinued LLVM Phabricator instance.

Append to CFLAGS_EXTRAS and LD_EXTRAS when building cmdline.
ClosedPublic

Authored by flackr on Apr 9 2015, 5:10 PM.

Details

Summary

When building the command-line for compilations during tests, append to CFLAGS_EXTRAS and LD_EXTRAS to preserve switches set by the environment (i.e. for cross compiling to test on another platform).

Diff Detail

Repository
rL LLVM

Event Timeline

flackr updated this revision to Diff 23546.Apr 9 2015, 5:10 PM
flackr retitled this revision from to Append to CFLAGS_EXTRAS and LD_EXTRAS when building cmdline..
flackr updated this object.
flackr edited the test plan for this revision. (Show Details)
flackr added a reviewer: clayborg.
flackr set the repository for this revision to rL LLVM.
flackr added a subscriber: Unknown Object (MLST).
flackr added inline comments.
test/plugins/builder_base.py
89 ↗(On Diff #23546)

Hmm, I think os.environ[k] might need to be escaped in case it contains ' or ".

flackr updated this revision to Diff 23599.Apr 10 2015, 7:58 AM

Escape quotes and backslashes in environment variable assignments.

labath added a subscriber: labath.Apr 10 2015, 8:26 AM
labath added inline comments.
test/plugins/builder_base.py
101 ↗(On Diff #23599)

What is the agent that is going to be parsing this?

If it is a posix shell, then the single quotes there don't work the way you expect them to:

$ echo 'a string with a \' inside'
> '
a string with a \ inside

I recommend using double quotes and escaping ", `, $ and \.
http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html

flackr updated this revision to Diff 23757.Apr 15 2015, 2:31 AM

Revert first attempt to escape environment variables - need more platform specific investigation.

test/plugins/builder_base.py
101 ↗(On Diff #23599)

Actually, do you mind if I defer on escaping these and commit the first patchset? It's going to be very platform dependent so I'd prefer to investigate how the environment variables are set on Windows as well and make sure I get the right escaping there too.

Sure, go ahead. Quoting is hard. :)

clayborg accepted this revision.Apr 15 2015, 11:08 AM
clayborg edited edge metadata.
This revision is now accepted and ready to land.Apr 15 2015, 11:08 AM
This revision was automatically updated to reflect the committed changes.