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: use cc and c++ as defaults on os x #1210

Merged
merged 1 commit into from
Mar 19, 2015

Conversation

bnoordhuis
Copy link
Member

Commit 8b2363d ("configure: use gcc and g++ as CC and CXX defaults")
switches the CC and CXX defaults but it turns out that GYP uses cc
and c++ as defaults on OS X.

It also made the configure script complain about old compilers because
the xcode gcc identifies as v4.2.1, whereas cc is less ambiguous about
it being a clang hybrid.

R=@jbergstroem?

@Fishrock123
Copy link
Contributor

LGTM

@jbergstroem
Copy link
Member

Sigh. The perils of gyp. Some day I have to sit down and actually learn this.

If we're going this route, why not create a list of platforms. Thinking FreeBSD would fit neatly in there (but then again, the v8 toolchain sets g++ down the chain)?

No-one would probably complain if FreeBSD didn't make the list, so whatever you prefer is LGTM to me.

Commit 8b2363d ("configure: use gcc and g++ as CC and CXX defaults")
switches the CC and CXX defaults but it turns out that GYP uses cc
and c++ as defaults on OS X.

It also made the configure script complain about old compilers because
the xcode gcc identifies as v4.2.1, whereas cc is less ambiguous about
it being a clang hybrid.

PR-URL: nodejs#1210
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
@bnoordhuis bnoordhuis closed this Mar 19, 2015
@bnoordhuis bnoordhuis deleted the fix-osx-configure-warning branch March 19, 2015 22:54
@bnoordhuis bnoordhuis merged commit 4c73104 into nodejs:v1.x Mar 19, 2015
@rvagg rvagg mentioned this pull request Mar 20, 2015
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.

3 participants