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

Add support for MUSL based Operation Systems #449

Merged
merged 2 commits into from
Mar 23, 2016
Merged

Add support for MUSL based Operation Systems #449

merged 2 commits into from
Mar 23, 2016

Conversation

lostie
Copy link
Contributor

@lostie lostie commented Mar 21, 2016

Addresses #448

Final solution: After some discussion we decided the best way would be to add a __MUSL__ test and get Alpine distributions to add that to the PKG_CXXFLAGS

Using suggestion recommend by the musl F.A.Q.: (Read below)

Q: application XY misbehaves or crashes at runtime when linked against musl
A: Usually this is because the app has hardcoded glibc-specific assumptions or wrong #ifdefs. See Functional differences from glibc. The most common causes are expectations of gnu getopt behaviour, iconv usage on UCS2 with assumptions that BOM is processed and the byte order detected, assuming that off_t is 32 bit, and assumptions that pthread_create will create sufficiently large stacks by default (crash situation). A good approach to solving the issue is to watch out for #if and #ifdefs in the code and placing some debug #warning there to see which code paths are enabled by the preprocessor. Often the logic taken by #ifdef's is to check a blacklist of preprocessor defines #if sun || haiku || irix || qnx portable_code(); #else glibc_code() #endif. The logic should be the other way round: #ifndef GLIBC portable_code() #else glibc_code(). There's also no point in checking GLIBC version numbers without first making sure that GLIBC is defined at all.

http://wiki.musl-libc.org/wiki/FAQ

@kevinushey
Copy link
Contributor

This unfortunately does not compile on OS X (tested on El Capitan).

@kevinushey
Copy link
Contributor

Perhaps instead we need to guard this in https://github.com/RcppCore/Rcpp/blob/master/inst/include/Rcpp/platform/compiler.h, with something like

#define RCPP_DEMANGLING_SUPPORT
#ifdef __ALPINE // a better way of detecting 'just alpine'?
# undef RCPP_DEMANGLING_SUPPORT
#endif

and then guard based on that?

@thirdwing
Copy link
Member

It failed because another change is missed. No __GLIBC__ is defined on OSX. See thirdwing@8db55e0

@kevinushey From the FAQ (http://wiki.musl-libc.org/wiki/FAQ#Q:_why_is_there_no_MUSL_macro_.3F), it seems no way to detect usage of musl.

@eddelbuettel
Copy link
Member

We could put a else-if in there and have the Alpine user (at least for now) define that themselves...

@kevinushey
Copy link
Contributor

@thirdwing but backtraces do currently work on OS X, so there must be a more scoped / targeted change. I loathe the idea of making a configure script to do it, though.

@thirdwing
Copy link
Member

@kevinushey Yes, OSX doesn't define __GLIBC__ and it does have execinfo.h.

The only idea I have now is to detect execinfo.h in the makefile and add something like -DHAVE_EXECINFO into PKG_CPPFLAGS.

@eddelbuettel
Copy link
Member

As I said before, add a test for __ALPINE here and have the Alpine folks add -D__ALPINE=1 to their PKG_CXXFLAGS. This will work for now until they do us the pleasure of defining something in Musl.

I have no intention of breaking OS X, and no appetite for byzantine build mechanisms.

@thirdwing
Copy link
Member

Totally agree.

On Mon, Mar 21, 2016 at 3:04 PM, Dirk Eddelbuettel <notifications@github.com

wrote:

As I said before, add a test for __ALPINE here
https://github.com/RcppCore/Rcpp/blob/0.12.3/src/api.cpp#L37 and have
the Alpine folks add -D__ALPINE=1 to their PKG_CXXFLAGS. This will work
for now until they do us the pleasure of defining something in Musl.

I have no intention of breaking OS X, and no appetite for byzantine build
mechanisms.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#449 (comment)

Qiang Kou
qkou@umail.iu.edu
School of Informatics and Computing, Indiana University

@lostie
Copy link
Contributor Author

lostie commented Mar 23, 2016

As I said before, add a test for __ALPINE here and have the Alpine folks add -D__ALPINE=1 to their PKG_CXXFLAGS. This will work for now until they do us the pleasure of defining something in Musl.

I have no intention of breaking OS X, and no appetite for byzantine build mechanisms

I have added a __MUSL__ test instead of __ALPINE as the issue is directly related with the musl glib and more linux distributions make use of it, not just Alpine.

@lostie lostie changed the title Add support for Alpine linux by checking for GLIBC flag Add support for MUSL based Operation Systems Mar 23, 2016
@eddelbuettel
Copy link
Member

That looks much better. And I presume it works for you, ie __MUSL__ is in fact defined?

(And please don't get me wrong -- I have nothing against Musl or Alpine. I use Docker quite a bit and have quite some sympathy for thinner builds but we got to this right and not break a bazillion existing use cases ...)

@lostie
Copy link
Contributor Author

lostie commented Mar 23, 2016

That looks much better. And I presume it works for you, ie MUSL is in fact defined?

Yes!

For testing purposes (and given I'm using docker) I have added the -D__MUSL__ to the R compilation options (PKG_CXXFLAGS):

x86_64-alpine-linux-musl-g++ -I/usr/lib/R/include -DNDEBUG -D__MUSL__ -I../inst/include -DCOMPILING_DPLYR -Os -fomit-frame-pointer -I"/app/packrat/lib/x86_64-alpine-linux-musl/3.2
.3/Rcpp/include" -I"/app/packrat/lib/x86_64-alpine-linux-musl/3.2.3/BH/include"   -fpic  -Os -fomit-frame-pointer  -c RcppExports.cpp -o RcppExports.o

but once Alpine adds that to the PKG_CXXFLAGS it should work out of the box 👍

@eddelbuettel
Copy link
Member

Err, yes, I understand the PKG_CXXFLAGS trick; after all it was me who suggested it to you 😉

But what is missing here is someone checking with the good folks of the Musl library to please add such an unmistakable identifier as we cannot possibly do this piecemeal package by package.

So let me rephrase: Can you please run a recursive grep over the C library headers for Musl and check that __MUSL__ is there?

@lostie
Copy link
Contributor Author

lostie commented Mar 23, 2016

But what is missing here is someone checking with the good folks of the Musl library to please add such an unmistakable identifier as we cannot possibly do this piecemeal package by package.

So let me rephrase: Can you please run a recursive grep over the C library headers for Musl and check that MUSL is there?

Here's the list of macros: https://gist.github.com/lostie/2b60cb4afb3bbba0b928

__MUSL__ is not present as stated in their F.A.Q.

All I can do is request MUSL community (I've already sent them an e-mail) to set by default in their library.

@eddelbuettel
Copy link
Member

Yes, please liaise with them. They will need this.

I'll fold this in now. We have Rcpp 0.12.4 pending at CRAN so with be for the next release anyway.

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.

4 participants