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

Compile with cc rather than gcc whenever possible #1505

Merged
merged 2 commits into from
Apr 4, 2018

Conversation

ararslan
Copy link
Contributor

@ararslan ararslan commented Apr 3, 2018

See discussion in #1504 regarding misdetection of the appropriate compiler. This will require some verification, especially from someone familiar with Windows, that the approach taken here is legit.

@ararslan
Copy link
Contributor Author

ararslan commented Apr 4, 2018

The failed Windows build appears to have just timed out.

@martin-frbg
Copy link
Collaborator

martin-frbg commented Apr 4, 2018

Yes, one of the appveyor configurations does this occasionally, probably taking too long to download and install the build environment. Note however that we have no CI tests set up for OSX.
If I read #231 correctly, forcing CC to clang on OSX (which your patch seems to undo) was done to pick up the builtin assembler from llvm rather than the inferior system assembler that gcc would use.
Maybe this could also be achieved by other means, but perhaps it would be safer to move the UNAME_S line up and make the whole "if (origin CC)" thing conditional on "none of the BSDs" ?
Or at least leave the Darwin case in place, as it seems to serve a different purpose...

@ararslan
Copy link
Contributor Author

ararslan commented Apr 4, 2018

Sure, I can certainly reinstate the macOS-specific logic. I assumed it was outdated, as for quite a while now cc on macOS has been Clang rather than GCC. I probably shouldn't be making such assumptions. 🙂

@martin-frbg martin-frbg merged commit 84923de into OpenMathLib:develop Apr 4, 2018
@ararslan ararslan deleted the aa/compiler branch April 4, 2018 20:52
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.

2 participants