RFC: Stop exporting symbols #2746
Replies: 5 comments 10 replies
-
I'm trying out this approach with the |
Beta Was this translation helpful? Give feedback.
-
https://github.com/instructure/nokogiri-xmlsec-instructure currently uses |
Beta Was this translation helpful? Give feedback.
-
OK, I think we've made a decision, which is:
Obviously we can still learn as we do this work, and may possible change direction, but I'd like to get started. |
Beta Was this translation helpful? Give feedback.
-
Fun and interesting twist: #2898 shows that problems happen to other gems when they accidentally pull in Nokogiri's libxml2. This increases my confidence that we've chosen the right path in deciding to not export symbols in an upcoming minor release. Ugh, what a mess. |
Beta Was this translation helpful? Give feedback.
-
Also note that the YARP project just ran into this issue, and I proposed solving it by hiding symbols in ruby/prism#1069 |
Beta Was this translation helpful? Give feedback.
-
This document is a request for comments, specifically from downstream maintainers of gems that integrate with Nokogiri's C API, or the C API of libxml2, libxslt, or libgumbo.
Proposal
Limit the symbols that are exported by the Nokogiri shared library (extension) to the bare minimum necessary for Nokogiri to operate. This might be the single symbol
Init_nokogiri
, but I need to do some exploration to be sure.Context
Nokogiri has always exported the symbols from libxml2 and libxslt that are statically linked into the extensions. On Linux this has led to one or two issues with symbol collisions (see #2106), but with careful naming we've mostly avoided problems.
In the final days of shipping Nokogiri v1.14.0 with native (precompiled) support for Ruby 3.2, we're struggling a bit with symbol resolution. Ruby 3.2, when compiling on Darwin, now uses the
-bundle_loader
linker flag to resolve symbols against the Ruby executable as if it were a shared library. This means that, when running a Ruby compiled with the--enable-shared
flag, that the extension will fail to resolve Ruby symbols likerb_cObject
. We can work around that with the-flat_namespace
linker flag, which mimics the behavior we already see on Linux and allows us to resolve these symbols at runtime. But for reasons I don't fully understand, many Rubies on Darwin seem to load the libxml2 and libxslt dylibs that ship with XCode commandline tools ("CLT"), and so every libxml2 symbol is a collision and resolves to the wrong libxml2 (not the version we've patched and statically linked into the extension).To work around this last problem, the best solution we know of right now seems to be to avoid exporting those symbols by using the
-load_hidden
flag (or a similar mechanism, there are several we could choose from).The decision I think we've made is to avoid exporting these symbols only in the precompiled Ruby 3.2 Darwin extensions. But that opens up the question of: should we avoid exporting these symbols everywhere, both for consistency and for reliability. That is the question asked by this RFC.
Impact
Good: This would prevent accidental symbol collisions such as the #2106 on Linux, and would ensure that we always pull in the desired version of libxml2, avoiding problems like the ones we're currently experiencing with Ruby 3.2 (see rake-compiler/rake-compiler-dock#87 for extended discussion and more links).
Bad: This would also, however, prevent a small but non-zero number of downstream gems from integrating with Nokogiri's C API, or the C API of libxml2, libxslt, or libgumbo. A notable gem that did this was https://github.com/rubys/nokogumbo (now merged into Nokogiri itself). Another notable gem that I know that does this is
nokogiri-xmlsec
(and the various forks of it, the most popular seems to be https://github.com/instructure/nokogiri-xmlsec-instructure). So this may prevent experimentation and innovation (see Nokogumbo) as well as putting hurdles in front of useful integrations like xmlsec.Feedback
I would like to hear from downstream gem maintainers about how they're using the C APIs exported by Nokogiri's extension today. Please comment below!
References
Beta Was this translation helpful? Give feedback.
All reactions