-
Notifications
You must be signed in to change notification settings - Fork 158
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
Link C/C++ stdlib statically for binary gems #127
Conversation
5923584
to
33f9b8b
Compare
@bolandrm This should fix all the issues reported so far. I don't have access to a Mac though, so please test on one if possible. |
Fails on macOS (10.14.5/Xcode 10.2/Ruby 2.5.0) at runtime because Works fine after bodging the path but I don't know about other platforms or what your intention is here - putting it in |
Putting
This is with Maybe we can create |
Both issues should be fixed now, please try again! |
LGTM. |
I got further this time...
here's the Makefile it generated:
|
@betesh I see from the Makefile you're using JRuby. This is a bug in JRuby, I've reported it here: jruby/jruby#5749 I've worked around the bug in this commit: 2ae4b7c Perhaps your attempt was prior to the work-around commit? |
Look in the output I pasted above -- you can see that the directory it's cloning to contains the SHA 9de3520, so definitely post-workaround. |
I just tried again with 2ae4b7c, same results |
FWIW I'm using jruby 9.1.17.0 (latest in the 9.1 series) |
Ah, the |
Thanks for all the quick responses and the hard work of trying to make this work in every environment. We're getting much closer... Output
Makefile
|
Ah, I see what's going on here. Another bug in JRuby, reported here jruby/jruby#5751 and will push a work-around shortly |
Pushed a work-around, let's see if it works. Thanks for all the testing! |
works, thanks :) |
Works for me too! Thanks @glebm ! |
Also: 1. Compile via `mkmf` instead of libsass's own Makefile. This is necessary to make cross-compilation work (rake-compiler hooks into mkmf). 2. Do not use per-ruby versions of `libsass.so`. It is unnecessary, because `libsass.so` is Ruby-agnostic (the FFI gem is used instead to use it from any Ruby version). 3. Add VERSION file to the non-precompiled gem. 4. Clean before every build.
Thanks for testing! @bolandrm Bugs and commits squashed, should be good to go! 🎉 |
Turns out all of that was enough to get it working on Ubuntu (16.04.6) but not on CentOS (7.5.1804). On a CentOS docker image, I'm hitting this error:
I have a feeling there's a package I'm missing. I'm trying to track it down... |
@betesh Perhaps |
NB: There is an issue with cross-compiled binary gems overriding Ruby versions restrictions. Looking into it. |
-march=native is only enabled when not cross-compiling
Fixed |
No, it's a system package (different on every distro, and one of many options). A pre-requisite for compiling libsass is a working C++ compilation environment (that package is the C++ standard library). On Linux, the pre-compiled gem will be installed by default on x86/64 anyway. |
Version |
This broke installation of the gem on TruffleRuby unfortunately, which I recently just made pass all the Compiling a non-C extension with I guess I should have been faster to add TruffleRuby in |
The easiest way I found to make cross-compilation work correctly was with Can Truffle Ruby check that I've also asked for a better API than |
I think that's not really feasible, because at that point What might help is compiling in TruffleRuby with native + bitcode, but I'm not sure when that's going to land and the Do you think it would make sense to change
Thanks for filing that. |
When compiling libraries for shared linking into ruby, we want to use the same compiler and flags as specified in Then, we'd have to duplicate functionality that Finally, we'd need to build our own replacement for the This seems like a lot of work, and quite error-prone.
Perhaps, but other languages seem to use their own mkmf equivalents for compiling libsass, e.g.:
As a work-around, perhaps TruffleRuby could expose a way to disable bitcode compilation? Then we could do something like this in $truffle_ruby_disable_bitcode_compilation = true
require 'mkmf' |
Thanks for the detailed reply.
What do you mean by "shared linking into ruby"? Isn't I'm not sure about this: why should it be the same compiler and flags than ruby? My thinking is if
That functionality seems to just set I would guess
That might work, I'll need to try. Could be fairly hacky if |
libsass.so itself links C++ and C standard libraries. When the gem is compiled from source on the user's computer, it links them dynamically. If the Ruby installation or another
This is why it's important that the
Not quite, rake-compiler patches The Makefile that ruby generates doesn't accept any external variables (all assignments use
A minimal hack that I can think of is to clone the entire
Hopefully Ruby accepts my feature request for mkmf, and then you can just check mkmf's flag. The "all global variables and constants" design of mkmf does make it a pain to change or customize. 😩 |
I can confirm installing the pre-compiled pre-release works on TruffleRuby, nice. |
* Since libsass is meant to be used with FFI, RbConfig::CONFIG['SOEXT'] would be better, but DLEXT is used as libsass is compiled by mkmf (sass#127). * Use RbConfig::MAKEFILE_CONFIG['DLEXT'] instead of just RbConfig::CONFIG['DLEXT'] so it's also correct on JRuby and not 'jar'. See https://github.com/rake-compiler/rake-compiler/blob/f808dd7a/lib/rake/extensiontask.rb#L41 * This lets Ruby implementations use another file extension than .so/.bundle for C extensions. For example, TruffleRuby 19.3.0 uses .dylib on macOS. * Some OS also use a different file extension, such as HPUX which uses .sl.
Also:
mkmf
instead of libsass's own Makefile. This is necessary to make cross-compilation work (rake-compiler hooks into mkmf).libsass.so
. It is unnecessary, becauselibsass.so
is Ruby-agnostic (the FFI gem is used instead to use it from any Ruby version).