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

Ship precompiled gems for linux #1983

Closed
flavorjones opened this issue Feb 2, 2020 · 24 comments
Closed

Ship precompiled gems for linux #1983

flavorjones opened this issue Feb 2, 2020 · 24 comments

Comments

@flavorjones
Copy link
Member

flavorjones commented Feb 2, 2020

Hello! If you're arriving here to give feedback on the v1.11.0 Release Candidates, please make sure to:

  • tell us which linux distro you're using!
  • (If you can) a docker image in which we can reproduce the failure you're seeing!

Thanks so much for using Nokogiri and for giving us feedback on this experiment!


See #1571 for the PR from @larskanis that enables building precompiled libraries for linux.

This issue is for discussion on how to ship those precompiled libraries in a gem for linux users in a way that won't break the installation experience for people.

Some things that we'll need to figure out were mentioned in #1571 by @larskanis:

  1. Bundler isn't clever at selection of unsupported ruby versions. See Add two specs for choosing platform specific gems rubygems/bundler#6247 . This requires users to explicit opt-out of binary gems, when they want to run nokogiri on ruby-trunk.

  2. There is a known issue on Alpine Linux: Cross building doesn't work for alpine3.7 rake-compiler/rake-compiler-dock#20 . I did a manual test on the alpine docker image and it reproducible fails at Nokogiri::XML::Reader#namespaces somewhere within the tests: https://gist.github.com/larskanis/94920901e6466c9383360ee1346f9f34 . I didn't analyze the reason for the segfault so far, but I think we should before the release.

And he later mentioned:

Issue 1 in the above comment will probably finally be fixed by the next bundler release. See rubygems/bundler#7522 . This way bundler will select the platform=ruby gem automatically, if fat binary gem doesn't fit to the ruby release.

I think we'll probably also be able to fix / work around issue 2, so that I hope we'll be able to ship binary releases some day.

Finally, I'd like for VersionInfo to contain some indication that the user is using precompiled libraries. A patch that worked at one point on the #1571 branch (but now needs to be rewritten) is:

From 20f69908732b50258be0ee18e1d78b841d462dc4 Mon Sep 17 00:00:00 2001
From: Mike Dalessio <mike.dalessio@gmail.com>
Date: Sun, 13 Jan 2019 01:15:06 -0500
Subject: [PATCH] `nokogiri -v` emits info about precompiled binaries

It may be useful information to be able to tell if the libraries were
compiled on installation, or whether they were precompiled in a fat
binary native gem.
---
 Rakefile                | 3 +++
 ext/nokogiri/nokogiri.c | 6 ++++++
 lib/nokogiri/version.rb | 6 ++++++
 3 files changed, 15 insertions(+)

diff --git a/Rakefile b/Rakefile
index 5ef937d..72edb7e 100644
--- a/Rakefile
+++ b/Rakefile
@@ -383,6 +383,9 @@ task :cross do
     raise "rake-compiler has not installed any cross rubies. Use rake-compiler-dock or 'rake gem:windows' for building binary windows gems."
   end
 
+  ENV['CFLAGS'] ||= ""
+  ENV['CFLAGS'] += " -DNOKOGIRI_USE_PRECOMPILED_NATIVE"
+
   CROSS_RUBIES.each do |cross_ruby|
     task "tmp/#{cross_ruby.platform}/nokogiri/#{cross_ruby.ver}/nokogiri.so" do |t|
       # To reduce the gem file size strip mingw32 dlls before packaging
diff --git a/ext/nokogiri/nokogiri.c b/ext/nokogiri/nokogiri.c
index 740b30d..5b32500 100644
--- a/ext/nokogiri/nokogiri.c
+++ b/ext/nokogiri/nokogiri.c
@@ -104,6 +104,12 @@ void Init_nokogiri()
   rb_const_set(mNokogiri, rb_intern("NOKOGIRI_LIBXSLT_PATCHES"), Qnil);
 #endif
 
+#ifdef NOKOGIRI_USE_PRECOMPILED_NATIVE
+  rb_const_set(mNokogiri, rb_intern("NOKOGIRI_USE_PRECOMPILED_NATIVE"), Qtrue);
+#else
+  rb_const_set(mNokogiri, rb_intern("NOKOGIRI_USE_PRECOMPILED_NATIVE"), Qfalse);
+#endif
+
 #ifdef LIBXML_ICONV_ENABLED
   rb_const_set(mNokogiri, rb_intern("LIBXML_ICONV_ENABLED"), Qtrue);
 #else
diff --git a/lib/nokogiri/version.rb b/lib/nokogiri/version.rb
index 44c076a..c05e1a0 100644
--- a/lib/nokogiri/version.rb
+++ b/lib/nokogiri/version.rb
@@ -46,6 +46,10 @@ module Nokogiri
       NOKOGIRI_USE_PACKAGED_LIBRARIES
     end
 
+    def libxml2_using_precompiled_native?
+      NOKOGIRI_USE_PRECOMPILED_NATIVE
+    end
+
     def warnings
       warnings = []
 
@@ -78,6 +82,7 @@ module Nokogiri
           vi["libxml"] = {}.tap do |libxml|
             if libxml2_using_packaged?
               libxml["source"] = "packaged"
+              libxml["precompiled"] = libxml2_using_precompiled_native?
               libxml["patches"] = NOKOGIRI_LIBXML2_PATCHES
             else
               libxml["source"] = "system"
@@ -89,6 +94,7 @@ module Nokogiri
           vi["libxslt"] = {}.tap do |libxslt|
             if libxml2_using_packaged?
               libxslt["source"] = "packaged"
+              libxslt["precompiled"] = libxml2_using_precompiled_native?
               libxslt["patches"] = NOKOGIRI_LIBXSLT_PATCHES
             else
               libxslt["source"] = "system"
-- 
2.17.1

And tagging @tenderlove for visibility because he had some thoughts on the previous #1571 thread.

@larskanis
Copy link
Member

bundler-2.2.0 will fix the issues with binary gems. I already played a bit with it's improved multiplatform support on Windows. I didn't test nokogiri on Linux so far, but a nokogiri pre-release will allow easy testing.

Regarding Alpine Linux (aka musl libc) I did some research and found out that sprintf() doesn't work properly but segfaults as soon as any arguments are used. It is only used once here and can easily get replaced by strcat() / strcpy().

After fixing the sprintf segfault, there is one failing test case on Alpine with binary gem:

Nokogiri::HTML::TestDocumentEncoding#test_encoding_without_charset:
ArgumentError: invalid byte sequence in UTF-8
    /comcard/nokogiri/test/html/test_document_encoding.rb:26:in `test_encoding_without_charset'

The test case succeeds with Shift_JIS encoding but fails with cp932 although they are mostly interchangeable. I'll try to find the root cause.

flavorjones added a commit that referenced this issue Feb 2, 2020
@flavorjones flavorjones changed the title Ship fat binary gems for linux Ship precompiled gems for linux Feb 3, 2020
@flavorjones
Copy link
Member Author

@larskanis Just want to confirm: you think I should ship an RC for precompiled linux? I can do that on Monday if so.

I can add Alpine to the CI pipelines -- that might be the easiest way to tell if something's broken. I'll try to do that in the next day or so.

@flavorjones
Copy link
Member Author

flavorjones commented Feb 3, 2020

@casperisfine
Copy link

Some feedback on precompiled binaries in general, not nokogiri per say.

On paper precompiled binaries are great as they are much faster to install, however they are a huge pain whenever a new Ruby version is released.

I just had this issue with the grpc and google-protobuf gems. I wanted to try the 2.7.0 RCs but couldn't because precompiled gems put a upper requirement on ruby versions.

For instance https://rubygems.org/gems/nokogiri/versions/1.11.0.rc1-x86-linux has:

REQUIRED RUBY VERSION:
>= 2.4, < 2.8.DEV

So let's imagine I use that version in my app, and by the end of 2020 I try to test the Ruby 3 RCs (or the 3.0 release in January 2021).

I won't be able to bundle again because of that ruby version requirement. In theory bundler could fallback to the source gem (https://rubygems.org/gems/nokogiri/versions/1.11.0.rc1), but there's only one global flag which isn't very practical.

Aaron had the same problem with grpc: https://twitter.com/tenderlove/status/1222937689751605248

I realize this is more of a Bundler feedback than a nokogiri one, but my personal opinion is that bundler/rubygems/whatever would need to be improved before popular gems start pushing precompiled binaries.

@flavorjones
Copy link
Member Author

@casperisfine Thanks for the feedback. This should be addressed in Bundler 2.2, see rubygems/bundler#7522 (mentioned above) for more context.

@casperisfine
Copy link

🤦‍♂ Sorry, I read the issue description way too quickly. Great to hear this soon won't be a problem anymore.

@voxik
Copy link
Contributor

voxik commented Feb 4, 2020

Overall, the best way to install nokogiri on Fedora is via dnf install rubygem-nokogiri and Fedora user does not need to care.

Nevertheless, the runtime dependency on mini_portile2 is annoying for pre-built packages.

larskanis added a commit to larskanis/nokogiri that referenced this issue Feb 7, 2020
The binary nokogiri gem built with rake-compiler-dock-1.0.0 segfaults on x86_64-linux-musl.
This happens in:
  lib/nokogiri/xml/reader.rb:92:in `namespaces'
It turned out that any calls to sprintf() with %-arguments fail.

I did not dig deeper on assembly level, since this is the only use of sprintf in nokogiri.
Moreover it can be replaced by calls to ruby string functions easily.

Related to sparklemotion#1983
larskanis added a commit to larskanis/nokogiri that referenced this issue Feb 7, 2020
Musl doesn't recognize cp932 encoding.
It also fails when used on the command line with iconv.
For this test case cp932 can be replaced by Shift_JIS, which is identical for the characters in question and that is handled by musl.

Error was:

  1) Error:
Nokogiri::HTML::TestDocumentEncoding#test_encoding_without_charset:
ArgumentError: invalid byte sequence in UTF-8
    /home/lars/comcard/nokogiri/test/html/test_document_encoding.rb:26:in `test_encoding_without_charset'

Related to sparklemotion#1983
@flavorjones
Copy link
Member Author

@voxik Thanks for your comments!

Regarding Fedora installation instructions, can I ask you to send a PR on this file in the documentation repo?

And -- you're right! We shouldn't have a runtime dependency on mini_portile2 for pre-built packages. I've opened #1991 to fix that in a future RC.

@lamont-granquist
Copy link

Just starting to investigate things, but I'm seeing behavior which suggests that gem install is ignoring --use-system-libraries and env["NOKOGIRI_USE_SYSTEM_LIBRARIES"] = "true" and is instead installing the precompiled linux fat gems. Gonna take some time before I get it boiled down into a usefully minimal example which isn't tightly coupled to our entire build system though. Not sure if this is something that is known or not.

@larskanis
Copy link
Member

@lamont-granquist The behavior you describe is expected. --use-system-libraries and NOKOGIRI_USE_SYSTEM_LIBRARIES are features of the platform=ruby gem only. The binary gems can not link to system libxml2, but always has libxml2 and libxslt built-in.

@lamont-granquist
Copy link

Yeah, anyone know how to override the ruby_platform on a gem install to get the unfat gem back? My google-fu was failing me fairly badly yesterday. I can't recall if that is doable or not or if you've just got to download the gem directly and then install from disk.

@flavorjones
Copy link
Member Author

@lamont-granquist Try gem install --platform=ruby --pre nokogiri

@flavorjones
Copy link
Member Author

flavorjones commented Feb 27, 2020

@lamont-granquist Can you help me understand why you'd want to use the platform=ruby gem if a precompiled version of the libraries is available?

@lamont-granquist
Copy link

lamont-granquist commented Feb 27, 2020

Thanks for the tip on the command.

We build and ship our own precompiled ruby in a nonstandard location and use fixed rpaths in the binary (we also build libxml2 + libxslt and accept that we're entirely responsible for the CVEs in those and have to be responsive to that). In addition our libruby links against our own version of several other libraries like openssl (more fun CVEs) and libz and if anything that tries to link against libruby winds up linking against the system versions that can cause linker issues and hard failures. And the system versions we have to consider go back to RHEL6 for Linux and also include Solaris 11, AIX, FreeBSD and Mac -- our ruby version stays up-to-date so that we can ship e.g. ruby 2.7.0 on RHEL6 consistently and have up-to-date libz/openssl/etc (and then don't have to debug segfaults that have been fixed 6+ years ago in ruby core which still exist on RHEL6).

We have a health check script which audits everything lib that goes into our build and spits out a list of violations. For this current prerelease of nokogiri the relevant bits of the report looks like:

    --> /opt/chef/embedded/lib/ruby/gems/2.7.0/gems/nokogiri-1.11.0.rc1-x86_64-linux/lib/nokogiri/nokogiri.so
    DEPENDS ON: libruby.so.2.5
      COUNT: 1
      PROVIDED BY: not found
      FAILED BECAUSE: Unresolved dependency
    --> /opt/chef/embedded/lib/ruby/gems/2.7.0/gems/nokogiri-1.11.0.rc1-x86_64-linux/lib/nokogiri/nokogiri.so
    DEPENDS ON: libz.so.1
      COUNT: 1
      PROVIDED BY: /lib/x86_64-linux-gnu/libz.so.1 (0x00007fa9fcba3000)
      FAILED BECAUSE: Unsafe dependency

So libz is considered "unsafe" because it needs to be picked up from the version in /opt/chef/embedded/lib/libz* which our libruby is linked against. And then it can't even find the libruby because the prebuilt nokogiri.so doesn't have the right RPATH to find the /opt/chef/embedded/lib/libruby* that it needs. We don't use /etc/ld.so.conf, if I recall correctly, because chef is the tool that needs to manage that file and if you mess that up which messes up chef then you wind up in a chicken-and-egg scenario, so we prefer hardcoded rpaths in the binaries.

I think the TL;DR is that we're like the 0.0001% use case here.

You might find though that you get bug reports from Chef users that attempt to upgrade the nokogiri version in their Chef packages themselves I suppose. The point of my doing this in our client builds is so that they don't have to, but people get stuck on old client versions, and may try to upgrade themselves. I don't see any easy way around that this morning though (inherently if they're on 5+ year old versions of the client I can't patch anything to make their life any easier, and it seems like the fat linux gem approach is vastly better for the 99% use case so those people are just going to need to be told to override the platform -- or just upgrade chef). We've been shipping prebuilt nokogiri for years now, so hopefully not many people are doing it themselves.

@flavorjones
Copy link
Member Author

@lamont-granquist Wow, thanks for that context, I appreciate the level of care you're putting into your environments.

Are you able to work around bundler's behavior of (by default) installing native gems? You may find it helpful to set the bundler config key for force_ruby_platform (see https://bundler.io/v1.15/bundle_config.html) so that native versions of gems are ignored. @larskanis may have more insight into how to work around this behavior.

@lamont-granquist
Copy link

That may come in helpful later. For the client nokogiri is a tacked on extra which we 'gem install'. The bundler tweaks may come in useful though for Chef Workstation (ChefDK) where nokogiri is used by the software we ship so its in the bundle we generate. That's just not the task today and I haven't even thought about that much yet.

Yeah, looks like we bundle install there and expect that to pick up the env["NOKOGIRI_USE_SYSTEM_LIBRARIES"] = "true" setting, so that'll need some work as well.

@flavorjones
Copy link
Member Author

flavorjones commented Feb 28, 2020

@lamont-granquist to be clear, Nokogiri's "system libraries" flags still work, but only during installation of the ruby platform gem. You may want to consider running something like:

NOKOGIRI_USE_SYSTEM_LIBRARIES=t gem install ---platform=ruby nokogiri

or the equivalent:

gem install --platform=ruby nokogiri -- --use-system-libraries

That command will work with past and future Nokogiri releases to compile nokogiri from source against local system libraries.

@lamont-granquist
Copy link

Yep, already done it, which fixed builds. Backcompat seems to work as expected.

@eregon
Copy link
Contributor

eregon commented Mar 29, 2020

To understand better, how does the precompiled nokogiri gem work on Alpine which uses musl?

For context, the sassc gem also had precompiled Linux gems, but they were removed due to this musl/glibc mismatch in sass/sassc-ruby#145, which unfortunately makes it a very slow gem to install: sass/sassc-ruby#189

@flavorjones
Copy link
Member Author

@eregon Please see and follow #1990 for info on musl support.

@eregon
Copy link
Contributor

eregon commented Mar 29, 2020

@flavorjones Thanks for the link, but my question was more about how does the nokogiri binary gem work on Alpine, since I assume it was precompiled against glibc? Does it statically include glibc?

My understanding is using a binary compiled against glibc on alpine doesn't work without installing an extra libc6-compat package (sass/sassc-ruby#141 (comment)). Maybe that's acceptable, I don't know, but it seemed to surprise some people in that issue.
And RubyGems doesn't seem to be able to detect which libc is used/to have one binary gem per libc.

flavorjones pushed a commit that referenced this issue Mar 30, 2020
Musl doesn't recognize cp932 encoding.
It also fails when used on the command line with iconv.
For this test case cp932 can be replaced by Shift_JIS, which is identical for the characters in question and that is handled by musl.

Error was:

  1) Error:
Nokogiri::HTML::TestDocumentEncoding#test_encoding_without_charset:
ArgumentError: invalid byte sequence in UTF-8
    /home/lars/comcard/nokogiri/test/html/test_document_encoding.rb:26:in `test_encoding_without_charset'

Related to #1983
flavorjones pushed a commit that referenced this issue Mar 30, 2020
The binary nokogiri gem built with rake-compiler-dock-1.0.0 segfaults on x86_64-linux-musl.
This happens in:
  lib/nokogiri/xml/reader.rb:92:in `namespaces'
It turned out that any calls to sprintf() with %-arguments fail.

I did not dig deeper on assembly level, since this is the only use of sprintf in nokogiri.
Moreover it can be replaced by calls to ruby string functions easily.

Related to #1983
@flavorjones
Copy link
Member Author

flavorjones commented Mar 30, 2020

@eregon As of right now, the code on master just works on alpine without any compatibility layer. See a3d9438 for a commit that was necessary to limit our usage of libc to what's ABI-compatible with musl. See also https://ci.nokogiri.org/teams/nokogiri-core/pipelines/nokogiri/jobs/cruby-native-gem-test demonstrating that the native gem installs and passes all tests on alpine.

(edited with updated CI URL)

@aenad
Copy link

aenad commented May 6, 2020

hi all, im novice in ruby and i start to install rails and i get very error,and need help.
i read some issues but my error not fixed.
gem install --prerelease
ERROR: While executing gem ... (Gem::CommandLineError)
Please specify at least one gem name (e.g. gem build GEMNAME)
and
gem install rails
ERROR: Error installing rails:
The last version of nokogiri (>= 1.6) to support your Ruby & RubyGems was 1.10.9. Try installing it with gem install nokogiri -v 1.10.9 and then running the current command again
nokogiri requires Ruby version >= 2.3, < 2.7.dev. The current ruby version is 2.7.0.0.
and
gem install nokogiri --platform=ruby -- --use-system-libraries
Temporarily enhancing PATH for MSYS/MINGW...
Building native extensions with: '--use-system-libraries'
This could take a while...
ERROR: Error installing nokogiri:
ERROR: Failed to build gem native extension.

current directory: A:/*/install_ruby/Ruby27-x64/lib/ruby/gems/2.7.0/gems/nokogiri-1.10.9/ext/nokogiri

A:/*\ /install_ruby/Ruby27-x64/bin/ruby.exe -I A:/\ */install_ruby/Ruby27-x64/lib/ruby/2.7.0 -r ./siteconf20200506-16216-1tc60ph.rb extconf.rb --use-system-libraries
checking if the C compiler accepts ... yes
Building nokogiri using system libraries.
pkg-config could not be used to find libxml-2.0
Please install either pkg-config or the pkg-config gem per

gem install pkg-config -v "~> 1.1"

pkg-config could not be used to find libxslt
Please install either pkg-config or the pkg-config gem per

gem install pkg-config -v "~> 1.1"

pkg-config could not be used to find libexslt
Please install either pkg-config or the pkg-config gem per

gem install pkg-config -v "~> 1.1"

ERROR: cannot discover where libxml2 is located on your system. please make sure pkg-config is installed.
*** extconf.rb failed ***
Could not create Makefile due to some reason, probably lack of necessary
libraries and/or headers. Check the mkmf.log file for more details. You may
need configuration options.

Provided configuration options:
--with-opt-dir
--without-opt-dir
--with-opt-include
--without-opt-include=${opt-dir}/include
--with-opt-lib
--without-opt-lib=${opt-dir}/lib
--with-make-prog
--without-make-prog
--srcdir=.
--curdir
--ruby=A:/*/install_ruby/Ruby27-x64/bin/$(RUBY_BASE_NAME)
--help
--clean
--use-system-libraries
--with-zlib-dir
--without-zlib-dir
--with-zlib-include
--without-zlib-include=${zlib-dir}/include
--with-zlib-lib
--without-zlib-lib=${zlib-dir}/lib
--with-xml2-dir
--without-xml2-dir
--with-xml2-include
--without-xml2-include=${xml2-dir}/include
--with-xml2-lib
--without-xml2-lib=${xml2-dir}/lib
--with-libxml-2.0-config
--without-libxml-2.0-config
--with-pkg-config
--without-pkg-config
--with-xslt-dir
--without-xslt-dir
--with-xslt-include
--without-xslt-include=${xslt-dir}/include
--with-xslt-lib
--without-xslt-lib=${xslt-dir}/lib
--with-libxslt-config
--without-libxslt-config
--with-exslt-dir
--without-exslt-dir
--with-exslt-include
--without-exslt-include=${exslt-dir}/include
--with-exslt-lib
--without-exslt-lib=${exslt-dir}/lib
--with-libexslt-config
--without-libexslt-config

To see why this extension failed to compile, please check the mkmf.log which can be found here:

A:/*/install_ruby/Ruby27-x64/lib/ruby/gems/2.7.0/extensions/x64-mingw32/2.7.0/nokogiri-1.10.9/mkmf.log

extconf failed, exit code 1

Gem files will remain installed in A://install_ruby/Ruby27-x64/lib/ruby/gems/2.7.0/gems/nokogiri-1.10.9 for inspection.
Results logged to A:/
/install_ruby/Ruby27-x64/lib/ruby/gems/2.7.0/extensions/x64-mingw32/2.7.0/nokogiri-1.10.9/gem_make.out

@flavorjones
Copy link
Member Author

Closing this issue. RC3 should be out shortly, and I'm pointing people to #2075 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants