Allow overriding env var with --use-system-libraries=false and default to --disable-static on TruffleRuby

[eregon]

See #2191

[eregon]

@flavorjones Could you review and integrate this? It'd be awesome to have a release with it, as then it will be possible to install nokogiri with or without bitcode on TruffleRuby (#2191 (comment)).

[flavorjones]

@eregon Absolutely, sorry for not ACKing earlier, it's been a busy week. Will absolutely look and try to have this merged for a patch release in v1.11.x.

[codeclimate]

Code Climate has analyzed commit 4a3836e and detected 0 issues on this pull request.

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.0% (0.0% change).

View more on Code Climate.

[flavorjones]

@eregon I've finally got some time to take a look, and I rewrote this a bit to clean up the extconf.rb file and how we do configuration (keeping your co-author credit). Notably, I've opted for --enable-system-libraries and --disable-system-libraries rather than checking for a specific value of the NOKOGIRI_USE_SYSTEM_LIBRARIES environment variable.

So now this works OK for me when I have "static" linking on (--enable-static) but I cannot get it to work when I am using "shared" linking (--disable-static), which is the default after this patch is applied. I suspect there's something wrong with my toolchain, but I can't figure out what it is.

You can reproduce what I'm seeing using the flavorjones/truffleruby:nightly docker image, which at this moment is truffleruby 21.1.0-dev-df94e415.

Here's what I'm running and what I see:

# bundle exec rake compile -- --disable-system-libraries
mkdir -p tmp/x86_64-linux/nokogiri/2.7.2
cd tmp/x86_64-linux/nokogiri/2.7.2
/usr/local/bin/truffleruby -I. ../../../../ext/nokogiri/extconf.rb  -- --disable-system-libraries
checking for whether -g is accepted as CFLAGS... yes
checking for whether -Winline is accepted as CFLAGS... yes
checking for whether -Wmissing-noreturn is accepted as CFLAGS... yes
Building nokogiri using packaged libraries.
Static linking is disabled.
Cross build is disabled.
checking for gzdopen() in -lz... yes
Using mini_portile version 2.5.0
checking for iconv... yes
---------- IMPORTANT NOTICE ----------
Building Nokogiri with a packaged version of libxml2-2.9.10.
Configuration options: --host\=x86_64-unknown-linux-gnu --enable-static --disable-shared --libdir\=/nokogiri/ports/x86_64-unknown-linux-gnu/libxml2/2.9.10/lib --with-iconv\=yes --without-python --without-readline --with-c14n --with-debug --with-threads --enable-shared --disable-static CFLAGS\=-O2\ -U_FORTIFY_SOURCE\ -g
The following patches are being applied:
  - 0001-Revert-Do-not-URI-escape-in-server-side-includes.patch
  - 0002-Remove-script-macro-support.patch
  - 0003-Update-entities-to-remove-handling-of-ssi.patch
  - 0004-libxml2.la-is-in-top_builddir.patch
  - 0005-Fix-infinite-loop-in-xmlStringLenDecodeEntities.patch
  - 0006-htmlParseComment-treat-as-if-it-closed-the-comment.patch
  - 0007-use-new-htmlParseLookupCommentEnd-to-find-comment-en.patch
  - 0008-use-glibc-strlen.patch
  - 0009-avoid-isnan-isinf.patch
  - 0010-parser.c-shrink-the-input-buffer-when-appropriate.patch

The Nokogiri maintainers intend to provide timely security updates, but if
this is a concern for you and want to use your OS/distro system library
instead, then abort this installation process and install nokogiri as
instructed at:

  https://nokogiri.org/tutorials/installing_nokogiri.html#installing-using-standard-system-libraries

Note, however, that nokogiri cannot guarantee compatiblity with every
version of libxml2 that may be provided by OS/package vendors.

Extracting libxml2-2.9.10.tar.gz into tmp/x86_64-unknown-linux-gnu/ports/libxml2/2.9.10... OK
Running git apply with /nokogiri/patches/libxml2/0001-Revert-Do-not-URI-escape-in-server-side-includes.patch... OK
Running git apply with /nokogiri/patches/libxml2/0002-Remove-script-macro-support.patch... OK
Running git apply with /nokogiri/patches/libxml2/0003-Update-entities-to-remove-handling-of-ssi.patch... OK
Running git apply with /nokogiri/patches/libxml2/0004-libxml2.la-is-in-top_builddir.patch... OK
Running git apply with /nokogiri/patches/libxml2/0005-Fix-infinite-loop-in-xmlStringLenDecodeEntities.patch... OK
Running git apply with /nokogiri/patches/libxml2/0006-htmlParseComment-treat-as-if-it-closed-the-comment.patch... OK
Running git apply with /nokogiri/patches/libxml2/0007-use-new-htmlParseLookupCommentEnd-to-find-comment-en.patch... OK
Running git apply with /nokogiri/patches/libxml2/0008-use-glibc-strlen.patch... OK
Running git apply with /nokogiri/patches/libxml2/0009-avoid-isnan-isinf.patch... OK
Running git apply with /nokogiri/patches/libxml2/0010-parser.c-shrink-the-input-buffer-when-appropriate.patch... OK
Running 'configure' for libxml2 2.9.10... OK
Running 'compile' for libxml2 2.9.10... ERROR, review '/nokogiri/tmp/x86_64-linux/nokogiri/2.7.2/tmp/x86_64-unknown-linux-gnu/ports/libxml2/2.9.10/compile.log' to see what happened. Last lines are:
========================================================================
  CC       xmlreader.lo
  CC       relaxng.lo
  CC       dict.lo
  CC       SAX2.lo
  CC       xmlwriter.lo
  CC       legacy.lo
  CC       chvalid.lo
  CC       pattern.lo
  CC       xmlsave.lo
  CC       xmlmodule.lo
  CC       schematron.lo
  CC       xzlib.lo
  CCLD     libxml2.la
  CCLD     xmllint
clang-10: error: no such file or directory: './.libs/libxml2.so'
make[2]: *** [Makefile:1152: xmllint] Error 1
make[2]: Leaving directory '/nokogiri/tmp/x86_64-linux/nokogiri/2.7.2/tmp/x86_64-unknown-linux-gnu/ports/libxml2/2.9.10/libxml2-2.9.10'
make[1]: *** [Makefile:1479: all-recursive] Error 1
make[1]: Leaving directory '/nokogiri/tmp/x86_64-linux/nokogiri/2.7.2/tmp/x86_64-unknown-linux-gnu/ports/libxml2/2.9.10/libxml2-2.9.10'
make: *** [Makefile:893: all] Error 2

When I look in the directory /nokogiri/tmp/x86_64-linux/nokogiri/2.7.2/tmp/x86_64-unknown-linux-gnu/ports/libxml2/2.9.10/libxml2-2.9.10/.libs I see that libxml2.so is a symlink to libxml2.so.2.9.10 which doesn't exist:

# ls -lt /nokogiri/tmp/x86_64-linux/nokogiri/2.7.2/tmp/x86_64-unknown-linux-gnu/ports/libxml2/2.9.10/libxml2-2.9.10/.libs
total 9912
lrwxrwxrwx 1 root root      13 Mar  9 03:18 libxml2.la -> ../libxml2.la
-rw-r--r-- 1 root root     975 Mar  9 03:18 libxml2.lai
lrwxrwxrwx 1 root root      17 Mar  9 03:18 libxml2.so -> libxml2.so.2.9.10
lrwxrwxrwx 1 root root      17 Mar  9 03:18 libxml2.so.2 -> libxml2.so.2.9.10
-rw-r--r-- 1 root root   66552 Mar  9 03:18 xzlib.o
...
-rw-r--r-- 1 root root   84512 Mar  9 03:18 entities.o
-rw-r--r-- 1 root root   20260 Mar  9 03:18 SAX.o

I'm wondering if we could omit the commit to disable-static from this PR for now, and treat it as a separate concern? Otherwise this seems good to go.

[eregon]

The issue happens when compiling with bundle exec rake compile -- --disable-system-libraries, I can reproduce it (also on my original branch).
And it doesn't happen on CRuby with --disable-static.

So far I tested by installing the .gem file (produced by bundle exec rake gem) and that works fine.
EDIT: that was on a older version of TruffleRuby, see comment below.

I'll try to figure out why rake compile doesn't work in this case.

[flavorjones]

@eregon Hmm, OK, well since we can't reproduce with a gem, then I'm going to include this in v1.11.2 which should go out today.

[eregon]

Actually, installing from a .gem with gem install --local pkg/nokogiri-1.11.1.gem -- --disable-system-libraries does not work with latest TruffleRuby unfortunately.

It seems due to the fact recently TruffleRuby prepends the toolchain to PATH and libxml2's configure/Makefile not liking it, and in fact disabling that works (as it used to):
TRUFFLERUBYOPT="--experimental-options --cexts-prepend-toolchain-to-path=false" gem install --local pkg/nokogiri-1.11.1.gem -- --disable-system-libraries.
And TRUFFLERUBYOPT="--experimental-options --cexts-prepend-toolchain-to-path=false" bundle exec rake compile -- --disable-system-libraries also works so it's not a gem install vs rake compile thing, it's just about having the toolchain prepended to PATH or not.

--disable-system-libraries --enable-static does indeed seem to work even with the toolchain in PATH.

So it seems best to hold off on this a bit longer until we figure out what's going on, or we decide to go for --enable-static.
Sorry, I did not envision it would break between opening my PR and now.

[flavorjones]

@eregon Thanks for finding the root cause. I'm going to remove the "static" default commit from this PR but pull in the "system libraries" flags to unblock the Shopify folks.

[flavorjones]

And I'll create a new PR with that commit on it!

[eregon]

@flavorjones Agreed, that sounds safe, useful and lets us try all ways to install nokogiri.

[eregon]

I figured it out, the issue was that ld is used by libxml2 and the GraalVM LLVM toolchain's ld wrapper was not properly built in the TruffleRuby standalone. I'll fix that in TruffleRuby, it's trivial. With that fix, --disable-static works well.

@flavorjones So I believe it's fine to include the commit to --disable-static by default on TruffleRuby as well in the nokogiri release. There are several advantages about it so it seems worth it.

[eregon]

oracle/truffleruby@2e22010 merged so now bundle exec rake compile -- --disable-system-libraries/gem install --local nokogiri.gem -- --disable-system-libraries just work when --disable-static is default on TruffleRuby.

[eregon]

@flavorjones I've cherry-picked the commit having --disable-static by default on TruffleRuby, and added an entry to the CHANGELOG about it. It should all be ready now :)

[flavorjones]

@eregon Sweet! Thank you! I'm planning to ship Nokogiri today, and will merge this as soon as I get a few hours.

[flavorjones]

Just kicked CI, it was stuck.