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

recognise gcc with appended version #5

Merged
merged 1 commit into from Jul 10, 2015
Merged

recognise gcc with appended version #5

merged 1 commit into from Jul 10, 2015

Conversation

PaulPrice
Copy link
Contributor

$CC may be something like "gcc-4.8", which needs to be recognised as gcc.
Note also that "gcc-4.8 --version" produces something like:

gcc-4.8 (Homebrew gcc48 4.8.4) 4.8.4

so we have to deal with that additional "-4.8".

@mjuric
Copy link
Member

mjuric commented Jul 9, 2015

Looks OK to me; merge it!

PS: ... though I'd argue this is a fundamentally wrong way to query compiler properties -- compiling a probe that tests the https://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html (and analogs in other compilers) is probably better

@jhoblitt
Copy link
Member

jhoblitt commented Jul 9, 2015

@PaulPrice No objection to merging this but I think the behavior with respect to env variables is surprising. My general expectation is that if CC or CXX is set in the env that the build tooling would use those values verbatim.

@@ -258,7 +258,7 @@ def ClassifyCc(context):
else:
if env['cc'] != '':
CC = CXX = None
if re.search(r"^gcc(-\d+(\.\d+)*)?( |$)", env['cc']):
if re.search(r"^gcc-(\d+(\.\d+)*)?( |$)", env['cc']):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this regexp no longer matches gcc ? Maybe I'm missing some earlier context.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's not right. Let me check...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed this. Dunno what I was smoking…

@RobertLuptonTheGood
Copy link
Member

scons cc=clang is the preferred way to do this as it sets all appropriate variables, is explicit rather than dependent on the vagaries of your environment, and also turns on and off appropriate flags. I'd be willing to see a warning about trying to use CC and CXX. In fact, if you want to use them enough there's a setenv flag to sconsUtils.

$CC may be something like "gcc-4.8", which needs to be recognised as gcc.
Note also that "gcc-4.8 --version" produces something like:

    gcc-4.8 (Homebrew gcc48 4.8.4) 4.8.4

so we have to deal with that additional "-4.8".
@PaulPrice PaulPrice merged commit 3a39a0e into master Jul 10, 2015
@ktlim ktlim deleted the tickets/DM-3107 branch August 25, 2018 06:16
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

5 participants