Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

configure.py/bootstrap.py — account for CXXFLAGS. #676

Closed
wants to merge 1 commit into from

Conversation

geoff-nixon
Copy link

Take into account CXXFLAGS in the environment in the bootstrap and configure scripts. I've tried to ensure this doesn't interfere with existing workflows which depend on using CFLAGS (only) by appending to, rather than replacing, CFLAGS; this may lead to some extraneous duplicates, but it seems better than the alternative.

Also, CXXFLAGS (and CFLAGS) are added to LDFLAGS with this change; this is to account for some special flags (like clang's -stdlib) which modify the behavior of both the compiler and the linker.

Take into account CXXFLAGS in the environment. I've tried to ensure
this doesn't interfere with existing workflows which depend on using
CFLAGS only by appending rather than replacing CFLAGS; this may lead to
some extraneous duplicates, but it seems better than the alternative.
Also, CXXFLAGS (and CFLAGS) are added to LDFLAGS with this change; this
is to account for some special flags (like clang's -stdlib) which
modify the behavior of both the compiler and the linker.
@buildhive
Copy link

Evan Martin » ninja #636 SUCCESS
This pull request looks good
(what's this?)

@ryandesign
Copy link
Contributor

Can this, or any other fix for the issue, be merged, please?

@ryandesign
Copy link
Contributor

Here is an updated/simplified version of this change which works for me with ninja 1.8.2 and which I have added to MacPorts:

https://github.com/macports/macports-ports/blob/53f57e242a7dc2bc4692db47aad18d4e35bd82a1/devel/ninja/files/patch-configure.py-CXXFLAGS.diff

@nico
Copy link
Collaborator

nico commented Apr 5, 2018

@ryandesign If you send me a pull request for the diff you linked to, I'll merge it.

Closing this one out in the meantime since it doesn't apply anymore (and looked a bit more complex).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants