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

Only use libc++ on when using clang *on apple* #2609

Merged
merged 1 commit into from
Feb 26, 2016

Conversation

Wallacoloo
Copy link
Member

When building LMMS with CC=clang CXX=clang++ on a stock Arch Linux distribution with all LMMS prerequisites plusclang and llvm installed, the build errors early on when trying to #include <algorithm>.

Some research leads me to believe that libc++ is only needed on Apple, due to some licensing issues around GPLv3. Other than that, the system default should be preferred, by omitting the -stdlib=xxx flag.

This change achieves the behavior described in the previous paragraph (thus allowing LMMS to build without errors using clang on Linux systems regardless of which stdlibs the system ships with). Tagging @tresf because it seems that he was the one who added the -stdlib=libc++ flag.

@tresf
Copy link
Member

tresf commented Feb 25, 2016

libc++ is only needed on Apple, due to some licensing issues around GPLv3. Other than that, the system default should be preferred, by omitting the -stdlib=xxx flag.

Thanks for clarification. This shouldn't be a problem. If you look at 2035ff3 it actually adds the LMMS_BUILD_CLANG flag just for this item. I don't think its bad to keep around, but as I weed through compiler issues I find it difficult to know which behaviors are clang induced and which are apple induced. Your investigation has identified this one as clearly the latter. 👍

@Wallacoloo
Copy link
Member Author

@tresf I haven't done much development on macs, but I do believe it's possible to install gcc using macports (see https://trac.macports.org/wiki/UsingTheRightCompiler ), so I think both LMMS_BUILD_CLANG AND LMMS_BUILD_APPLE are necessary.

But maybe using -stdlib=libc++ even on gcc (does it support that?) avoids some runtime pitfalls on OSX?

@tresf
Copy link
Member

tresf commented Feb 25, 2016

@tresf I haven't done much development on macs, but I do believe it's possible to install gcc using macports (see https://trac.macports.org/wiki/UsingTheRightCompiler ), so I think both LMMS_BUILD_CLANG AND LMMS_BUILD_APPLE are necessary.

Possible? Perhaps. Ever going to happen? Not for c++. Here's why https://trac.macports.org/wiki/FAQ#libcpp. To quote the article:

macports.com
After playing whack-a-mole for a while trying to get stuff to coexist, MacPorts has given up and acknowledged that the only thing that works reliably is to go with what is compatible with Apple libraries; that means only older LLVM/clang that uses pre-C++11 interfaces (provided by libstdc++ or an Apple-sourced compatible libc++) on 10.8 and earlier and only newer LLVM/clang that uses C++11 interfaces (provided by modern libc++ but not the libc++ shipped on older OS X) on 10.9 and later. Any other combination might work if you are lucky, but is not guaranteed in any way and has led to many port build failures, and MacPorts no longer attempts to support it.

This also caused some grief in regards to winegcc and we still haven't figured that one out.

P.S. If I'm off the mark here, let me know. This isn't my expertise by any means.

@Wallacoloo
Copy link
Member Author

@tresf Ok. Thanks for clarifying. I went ahead and changed all the conditionals to only use if (LMMS_BUILD_APPLE), as you suggested. I also removed the code that defines LMMS_BUILD_CLANG, as nothing makes use of it anymore.

If you're good with that, then we can merge this after the Travis builds finish.

@tresf
Copy link
Member

tresf commented Feb 26, 2016

👍

tresf added a commit that referenced this pull request Feb 26, 2016
Only use libc++ on when using clang *on apple*
@tresf tresf merged commit e56c31b into LMMS:master Feb 26, 2016
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