-
-
Notifications
You must be signed in to change notification settings - Fork 896
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
WIP: Cross compile for x86_64-darwin and arm64-darwin per rake-compiler-dock #2142
Conversation
@@ -590,6 +590,14 @@ def configure | |||
cflags = concat_flags(ENV["CFLAGS"], "-fPIC", "-g") | |||
execute "configure", ["env", "CHOST=#{host}", "CFLAGS=#{cflags}", "./configure", "--static", configure_prefix] | |||
end | |||
|
|||
def compile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method compile
has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.
Code Climate has analyzed commit 713e723 and detected 1 issue on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 100.0% (80% is the threshold). This pull request will bring the total coverage in the repository to 94.4%. View more on Code Climate. |
@larskanis This is exciting! Thanks for doing this work. I'll spend some time tomorrow trying it out, and I'll ask some questions inline. |
I played a bit with universal libraries to bundle x86_64+arm64 code into one binary, but to no avail. IMHO these universal builds require a lot of additional work compared to a clear separation of CPU architectures. |
I'm kicking the tires today and everything looks good, including the darwin x86_64 gem. I'd like to ship an rc4 in the next day or so so we can get feedback on the arm64 build in particular in time for a v1.11.0 final release by year's end. @larskanis Are you planning to release rake-compiler-dock v1.1.0 soon so I can merge this? |
Yes, I'm planning to release rake-compiler-dock-1.1.0 soon after ruby-3.0 and before end of the year. |
772bd42
to
80010ed
Compare
I've pushed a few related commits. I'll likely keep rebasing this to master HEAD so that I can cut and test release candidates. Sorry for the inconvenience. |
IMHO rake-compiler-dock-1.1.0 is ready to be released. I switched to manylinux for the linux cross builds as sugested here. I'm not fully convinced about this change, but I think the advantages outweigh the disadvantages. Is this OK for you? I would like to add the CrossRuby class of Nokogiri to rake-compiler-dock (in a somewhat more generic way). It helps to build and verify other gems similar to nokogiri. Not in 1.1.0, but in a later release. |
@larskanis Thanks for asking - I think that's fine, I've built those images locally and am trying the builds out tonight. If you'd like to re-use the CrossRuby class, I'd like to suggest changing the file format to be something more easily parseable (like YAML, for instance) because I end up reading it in a few different places (e.g., where I generate the CI pipelines). Maybe we can open up an issue for future discussion? I'm happy to help. |
I used a rebased version of this branch to ship v1.11.0.rc4 just now, and things look like they're going OK! See #2149 (comment) edit: also using rake-compiler-dock 9c6e35e and container images built at that commit |
I released rake-compiler-dock-1.1.0 and all the images some hours ago. I think this PR can be merged now. |
3a4327c
to
046147b
Compare
OK, I've rebased onto master and fixed some of my commits; waiting for this to go green and I'll ship it. |
@larskanis it looks like the move to centos may have introduced an issue running on musl-based systems, see cruby-native-gem-test-32bit #25.1 - Concourse. I need to do a bit more digging to be sure and reproduce locally. |
Oh, that's not good. And it reminds me that I should have done this musl test locally before release. It's too late for today, but I can check this tomorrow. |
This requires latest rake-compiler-dock from master branch.
advapi32.dll is a standard Windows system DLL.
It is now based on manylinux2014 for increased platform compatibility.
because we were seeing these errors when building with 1.6: > error: Source option 6 is no longer supported. Use 7 or later. > error: Target option 6 is no longer supported. Use 7 or later. I think this is because JRuby has been updated in the rake-compiler-dock container since 1.0.0.
also delete unneeded script to set up darwin rake-compiler env
e.g., `rake concourse:task`
b827a1d
to
3036da9
Compare
This will attempt to override the RbConfig flags that would ask for share libraries to be stripped.
@larskanis I've pushed an additional commit that makes extconf.rb support a After running the test suite about a dozen times and examining the stack walkback for each, the crashes are happening while libxml2 is trying to call either Looking at the shared object generated in the centos container, we can see these end up being method calls:
but in a shared object generated in a debian-based container, there are no such symbols, undefined or other. I think this is because it's defined as a macro on musl and debian? I feel like we could work around this if necessary by providing local versions, but I'm curious if you can see an easier way to resolve this issue. |
This patch is a result of rake-compiler-dock using centos 7 (manylinux2014) to cross-compile. Centos, for reasons I have not been able to discern, implements `isnan` and `isinf` as a function and not as a macro. Debian knows how to resolve that function at dynamic-link time (despite using a macro at compile time), but musl-based systems (like alpine) do not. Running `nm` on nokogiri.so created on such a centos system shows: ``` U __isinf@@GLIBC_2.2.5 U __isnan@@GLIBC_2.2.5 ``` (see sparklemotion#2142 for more info) This patch avoids using glibc's `isnan` and `isinf` calls, instead using libxml2's fallback implementation. There's history here, see libxml2 commit 8813f39: commit 8813f39 Author: Nick Wellnhofer <[email protected]> Date: 2017-09-21 00:11:26 +0200 Simplify XPath NaN, inf and -0 handling Use C99 macros NAN, INFINITY, isnan, isinf. If they're not available: - Assume that (0.0 / 0.0) generates a NaN and !(x == x) tests for NaN. - Use C89's HUGE_VAL for INFINITY. Remove manual handling of NaN, infinity and negative zero in functions xmlXPathValueFlipSign and xmlXPathDivValues. Remove xmlXPathGetSign. All the tests for negative zero can be replaced with a test for negative or positive zero. Simplify xmlXPathRoundFunction. Remove Trio dependency. This should work on IEEE 754 compliant implementations even if the C99 macros aren't available, but will likely break some ancient platforms. If problems arise, my plan is to port the relevant trionan.c solution to xpath.c. Note that non-compliant implementations are impossible to fully support, anyway, since XPath requires IEEE 754. This patch would be unnecessary if any of the following was true: * centos implements these as macros, and doesn't generate an unresolved symbol for either in the shared library * we had a way to ensure `__isinf` and `__isnan` resolve on musl (e.g., we implement them locally)
I've pushed a commit that patches libxml2 to avoid calling Edit: that upstream issue is rake-compiler/rake-compiler-dock#42 |
Merged! Thanks again, @larskanis! |
This requires latest rake-compiler-dock from master branch.
What problem is this PR intended to solve?
Fat binary gems for MacOS are currently built on MacOS native. This complicates the build and release cycle. It is desirable to build all binary gems per cross compiler.
Related to #2079
Have you included adequate test coverage?
No CI, but I tested the resulting
x86_64-darwin
gem on MacOS-10.14.4 with both the system ruby-2.3.7 (after extending therequired_ruby_version
of nokogiri) and ruby-2.7.2 installed per rvm. The Mac is running on https://github.com/foxlet/macOS-Simple-KVM .I'm unable to execute the
arm64-darwin
on any computer. I just did some static inspection of the builtnokogiri.bundle
file.Does this change affect the behavior of either the C or the Java implementations?
C only.