Vendor re2 and abseil #67
Conversation
stanhu
force-pushed
the
sh-vendor-re2
branch
4 times, most recently
from
3e756f7 to
b4eb2af
Compare
|
@mudge I've tested this on Linux and macOS. If you're happy with this approach, I suggest:
UPDATE: First two are complete. |
| @@ -29,4 +29,5 @@ Gem::Specification.new do |s| | |||
| ] | |||
| s.add_development_dependency("rake-compiler", "~> 0.9") | |||
| s.add_development_dependency("rspec", "~> 3.2") | |||
| s.add_runtime_dependency("mini_portile2", "~> 2.8.2") # keep version in sync with extconf.rb | |||
There was a problem hiding this comment.
Unfortunately this gem needs Ruby 2.3 to work: https://github.com/mudge/re2/actions/runs/5525303526/jobs/10078757029#step:6:27
stanhu
force-pushed
the
sh-vendor-re2
branch
2 times, most recently
from
c2f0d44 to
a6dec88
Compare
|
This is a huge amount of work; thanks for putting in the effort, @stanhu! Re your checklist:
With the dependency on MiniPortile meaning we drop support for Ruby < 2.3, I'd consider this worthy of a major version bump (i.e. re2 2.0) and, if we're breaking compatibility, perhaps it would make sense for the vendored libraries to be the default. That better justifies including the sources otherwise it seems a little bloated to include dependencies that will never be used. It also makes the precompiled gems make a little more sense: the default expectation is that you can install the gem without having to install re2 or abseil yourself. |
|
@mudge Thanks for the feedback!
I think that's what happening now in https://github.com/mudge/re2/pull/67/checks. Was there something I missed? I would hesitate making the vendored libraries the default until precompiled gems are available for a number of reasons:
I should note that on ARM64 platforms, precompiled gems built with rake-compiler-dock require glibc 2.29 (see rake-compiler/rake-compiler-dock#68). I have a change I'm testing right now that would generate precompiled gems, but then you'll probably want to have some automation to build and push these gems automatically (such as https://github.com/y-crdt/yrb/blob/main/.github/workflows/gem.yml). How would you like to proceed? Do you want to tackle everything in this pull request, or get this merged first? |
No no, just that we agree on what we should be testing. (I suppose it is wishful thinking to only require Ruby >= 2.3 if you're opting into the vendored dependencies as we need the gem dependency at install time.)
Agreed, I was imagining that we'd only make it the default when and only when precompiled gems are available. That way, the common user experience is a fast install but it degrades to a slow compile if a precompiled gem isn't available for your platform.
I'm unfamiliar with precompiled gems, I don't suppose there's a way to only install one if your platform meets more than an CPU architecture requirement?
Agreed.
Would it be helpful for me to make a new |
If `--disable-system-libraries` is specified, this change will download and build abseil and re2. `cmake` and a C++17 compiler is required for this to work. This makes it possible to ensure all the required dependencies are contained within the C extension rather than depend on system libraries, which may break the extension when they are updated. The building of these libraries uses mini_portile2 and techniques borrowed from the nokogiri and ruby-magic gems. Closes mudge#61
mini_portile2 requires >= Ruby 2.3, so we need to drop support for obsolete Ruby versions.
This increases the size of the gem from 32K to 2.4 MB, but this will significantly reduce network transfer and avoid rate limits.
Running `rake gem:native` will attempt to cross-compile gems for the following platforms: - aarch64-linux - arm64-darwin - x86_64-darwin - x86_64-linux `rake gem:<platform>` also will compile one specific platform. For example: `rake gem:arm64-darwin`.
Either way. I'm still ironing out some cross-compiling issues for aarch64. |
mini_portile2 provides `configure` script with the `--host` parameter, and the script figures out the target compilers to use. With CMake, we don't have that benefit so we need to set the C and C++ compilers ourselves. We use a simple heuristic to find the target compilers: 1. For macOS, we use `clang` and `clang++. Otherwise we use `gcc` and `gcc++`. 2. We search the PATH for `<host>-compiler`. Use that if we find it. Otherwise default to the base compilers. To make CMake work with cross-compilation, a number of variables have to be set: - CMAKE_SYSTEM_PROCESSOR - CMAKE_SYSTEM_NAME - CMAKE_C_COMPILER - CMAKE_CXX_COMPILER
|
@mudge Ok, I fixed the cross-compilation issues. Turns out using
Also, the pre-compiled gems only ship with Ruby 2.7 - 3.2 binaries. Do you want to include 2.3 and up? |
Windows builds were previously failing because mini_portile2 adds an unnecessary option to the `cmake` configure step. This commit adds other targets: - arm-linux - arm64-darwin - x64-mingw-ucrt - x86-linux - x86-mingw32
|
@flavorjones Thanks so much for the review and all your work on mini_portile2!
Very true! I just worked out all the kinks with cross-compiling with
Thanks for the links! That will help move this faster.
I was wondering why Nokogiri didn't ship a musl build. Is that because the existing builds are compatible? I attempted to ship a musl gem in another project, but we found we had to require RubyGems >= v3.3.222 because some of our platforms attempted to install the musl version instead of glibc due to rubygems/rubygems#5852.
Would love to help with this. I've spent enough time working on this that I'd appreciate more help. |
This commit also adds an environment variable, `RE2_USE_SYSTEM_LIBRARIES`, to enable system libraries. Add some documentation about mini_portile2 workarounds and improve CMake cross-compilation usage.
To answer my own question:, it looks like on Alpine 3.17: / # ldd ./usr/lib/ruby/gems/3.1.0/gems/nokogiri-1.15.3-x86_64-linux/lib/nokogiri/3.1/nokogiri.so | more
/lib/ld-musl-x86_64.so.1 (0x7f42fbe51000)
libm.so.6 => /lib/ld-musl-x86_64.so.1 (0x7f42fbe51000)
libdl.so.2 => /lib/ld-musl-x86_64.so.1 (0x7f42fbe51000)
Error relocating ./usr/lib/ruby/gems/3.1.0/gems/nokogiri-1.15.3-x86_64-linux/lib/nokogiri/3.1/nokogiri.so libc.so.6 => /lib/ld-musl-x86_64.so.1 (0x7f42fbe51000)
<snip>However, since abseil needs libstdc++ and other libraries, the re2 precompiled gem currently fails miserably: / # ldd /usr/lib/ruby/gems/3.1.0/gems/re2-1.7.0-x86_64-linux/lib/re2.so
/lib/ld-musl-x86_64.so.1 (0x7f4720899000)
librt.so.1 => /lib/ld-musl-x86_64.so.1 (0x7f4720899000)
libruby.so.3.1 => /usr/lib/libruby.so.3.1 (0x7f472026a000)
Error loading shared library libstdc++.so.6: No such file or directory (needed by /usr/lib/ruby/gems/3.1.0/gems/re2-1.7.0-x86_64-linux/lib/re2.so)
Error loading shared library ld-linux-x86-64.so.2: No such file or directory (needed by /usr/lib/ruby/gems/3.1.0/gems/re2-1.7.0-x86_64-linux/lib/re2.so)
Error loading shared library libgcc_s.so.1: No such file or directory (needed by /usr/lib/ruby/gems/3.1.0/gems/re2-1.7.0-x86_64-linux/lib/re2.so)It looks like running Running Still, this isn't a great experience for Alpine users. Do we need to ship a precompiled musl extension, or is there a better way? |
I checked what the |
|
|
||
| Rake::ExtensionTask.new('re2') | ||
| CROSS_RUBY_VERSIONS = %w[3.2.0 3.1.0 3.0.0 2.7.0].join(':') |
There was a problem hiding this comment.
Do we really want to support all the way to Ruby 2.3.0? I'm inclined to keep it at 2.7.0 and set the gemspec to:
spec.required_ruby_version = ">= 2.7.0", That way no one can accidentally install a precompiled gem with the wrong Ruby version.
There was a problem hiding this comment.
If there is no way for us to only ship precompiled binaries for 2.7+ while still supporting system libraries for Rubies older than that then I'm happy to follow @flavorjones' lead with nokogiri and sqlite3 and set the required Ruby for the whole gem to match.
Nokogiri builds are compatible with musl (and we did jump through some hoops to do that), but do require gcompat on some systems. You found the relatively recent feature added that supports non-glibc linux platforms, but I haven't had to play with it yet. Formal musl support has been a low priority compared to some of the other work that needs to be done. 🤷
I recommend only precompiling for supported Rubies. Nokogiri and Sqlite3 have adopted a policy of only shipping 4 versions max (3 in current support, plus the one most recently out-of-support). A precompiled extension is a good "carrot" to encourage people to upgrade Ruby. (Also, my experience has been that a disproportionate number of support issues come from people running old versions of Ruby.) |
Please point me at what you'd like me to do. I'll have some time over the next few days, and I will try to help as much as I can. Just don't want to duplicate anything you've already done! |
@flavorjones I'm going to take a break from this work since I have to focus on other things, but I would love help with all the round-trip tests you mentioned! |
|
@stanhu would love to help ... you want me to just continue this branch in another PR? or add me as a collaborator on your fork? |
|
@flavorjones I'll just add you to my fork. At some point perhaps @mudge can just merge this to the |
|
I'll merge this into |
|
I've raised the issues mentioned above and attached them to the 2.0 milestone: https://github.com/mudge/re2/milestone/1 |
|
👍 Just a note that it may be a few days before I'm able to circle back on the testing issues (I have some other OSS work that is blocking other folks that I need to prioritize), but I will get there. |
|
@stanhu I just tried running Am I missing something? |
These are required dependencies for abseil until a custom musl gem is built: mudge#67 (comment)
These are required dependencies for abseil until a custom musl gem is built: mudge#67 (comment)
These are required dependencies for abseil until a custom musl gem is built: mudge#67 (comment)
These are required dependencies for abseil until a custom musl gem is built: mudge#67 (comment)
These are required dependencies for abseil until a custom musl gem is built: mudge#67 (comment)
These are required dependencies for abseil until a custom musl gem is built: #67 (comment)
These are required dependencies for abseil until a custom musl gem is built: #67 (comment)
This pull request:
Vendors re2 and abseil in the Ruby platform gem.
Builds precompiled native gems for the following platforms:
By default, build and link against the vendored re2 and abseil versions.
cmakeand a C++17 compiler is required for this to work. Users can specify--enable-system-librariesor setRE2_USE_SYSTEM_LIBRARIES.This makes it possible to ensure all the required dependencies are contained within the C extension rather than depend on system libraries, which may break the extension when they are updated.
The building of these libraries uses mini_portile2 and techniques borrowed from the nokogiri and ruby-magic gems.
glibc 2.29 is required for the arm64 Linux installs.
Closes #61