Skip to content

Added linux-musl support #175

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

Merged
merged 3 commits into from
Jan 22, 2021
Merged

Conversation

petermz
Copy link
Contributor

@petermz petermz commented Jan 15, 2021

Hi,
The only thing that prevents fastr from building on musl is the #include "fpu_control.h" line somewhere in libf2c sources. This header file is missing in musl. I'm adding this (effectively empty) header file as a patch to f2c/libf2c-patch

@steve-s
Copy link
Member

steve-s commented Jan 15, 2021

Hello,

thank you for the PR. How does it work if the extern __fpu_control exposed by the glibc header is not there? Does linux-musl export it also but not in that header?

@petermz
Copy link
Contributor Author

petermz commented Jan 15, 2021

No, musl doesn't have it in its headers. My theory is that __fpu_control use sites are #ifdef'ed out and not compiled with musl

@steve-s
Copy link
Member

steve-s commented Jan 18, 2021

Thanks for the explanation.

Have you signed the Oracle Contributor Agreement? We use it to make the copyright of contributions clear. You can sign it at that link.

@graalvmbot
Copy link
Collaborator

Hello peterz, thanks for contributing a PR to our project!

We use the Oracle Contributor Agreement to make the copyright of contributions clear. We don't have a record of you having signed this yet, based on your email address peterz -(at)- disroot -(dot)- org. You can sign it at that link.

If you think you've already signed it, please comment below and we'll check.

@petermz
Copy link
Contributor Author

petermz commented Jan 22, 2021

Hi,
I believe I should be covered by Bellsoft's umbrella OCA.

I've identified another undefined symbol and added it to unimplemented.c, hence the second commit. (It shows on musl because musl linkers use -z now by default, and libRnative fails to load).

@steve-s
Copy link
Member

steve-s commented Jan 22, 2021

I've identified another undefined symbol and added it to unimplemented.c, hence the second commit. (It shows on musl because musl linkers use -z now by default, and libRnative fails to load).

good, thanks. Could you please also add the standard Oracle GPL3 copyright that we use into the new file. Then I'll spin our internal tests and merge if these changes pass. (Example of the copyright: https://github.com/oracle/fastr/blob/master/com.oracle.truffle.r.test.native/embedded/src/main.c)

I believe I should be covered by Bellsoft's umbrella OCA.

yes that should be OK, could you please give some proof that you're from that company (send me an e-mail from an account on that domain or add secondary e-mail here on GitHub)? Sorry I need to follow the procedures.

@petermz petermz force-pushed the linux-musl-support branch from b08fec0 to d8b70fc Compare January 22, 2021 11:10
@petermz
Copy link
Contributor Author

petermz commented Jan 22, 2021

I already have [email protected] as secondary email here, and I've just sent you an email from that address.
Added copyright header, too. Thanks!

graalvmbot pushed a commit that referenced this pull request Jan 22, 2021
@graalvmbot graalvmbot merged commit d8b70fc into oracle:master Jan 22, 2021
@steve-s
Copy link
Member

steve-s commented Jan 25, 2021

Ah, sorry I forgot that we're using prebuilt F2C in our CI and check that F2C still builts correctly only in post merge checks, which gave us this error:

In file included from ./fpu_control.h:26:
./fpu_control.h:26:2: warning: #include_next in file found relative to primary source file or found by absolute path; will search from start of include path [-Winclude-next-absolute-path]
#include_next "fpu_control.h"
 ^
./fpu_control.h:26:15: error: #include nested too deeply
#include_next "fpu_control.h"
              ^
199 warnings and 1 error generated.
make[2]: *** [makefile:30: uninit.o] Error 1
make[2]: Leaving directory 'fastr/libdownloads/f2c/src/libf2c'
make[1]: *** [Makefile:60: all] Error 2
make[1]: Leaving directory 'fastr/com.oracle.truffle.r.native/f2c'
make: *** [Makefile:70: all] Error 2

Seems that the #include_next did not work as expected?

@petermz
Copy link
Contributor Author

petermz commented Jan 26, 2021

Strange it didn't show in my testing, but now I can reproduce this. Does it sometimes work and sometimes not?

Also, how do you build using prebuilt F2C?

@steve-s
Copy link
Member

steve-s commented Jan 26, 2021

Looking at it now, I think we have a bug in com.oracle.truffle.r.native/f2c/Makefile we do not clean the right f2c copy (there's one in libdownloads that mx downloads for us, and one in f2c top level dir, where we copy the former). Edit: no, actually we wipe out the f2c copy, so that should be alright. So if you do rm -rf f2c and then mx build you should be actually building f2c from scratch (edit: mx clean should do that too).

In the same makefile you can see that we check for environment variable F2C_BINARY and if that's set we assume it points to pre-built f2c and we just copy it over and skip the building altogether.

@petermz
Copy link
Contributor Author

petermz commented Jan 26, 2021

I have a fix: #176

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