Merge pull request #2202 from sparklemotion/2167-fix-nokogumbo-linkin…

…g-on-windows fix nokogumbo linking on windows --- **What problem is this PR intended to solve?** Closes #2167 which relates to an issue building Nokogumbo on Windows against the precompiled libraries. This PR does the following: - ensures that libxml2 symbols are exported when built on windows - ensures that nokogiri symbols are exported when built on windows - includes `LDFLAGS` in `Nokogiri::VERSION_INFO` to allow the windows linker to resolve all symbols **Have you included adequate test coverage?** There's pretty good test coverage of this test case in the Nokogumbo CI suite; and I've added some light tests here in `scripts/test-gem-installation` though I'll note that we're not testing gem installation on Windows in the Nokogiri CI suite at the moment (though, see #2153). **Does this change affect the behavior of either the C or the Java implementations?** This should only impact downstream gems like Nokogumbo who are trying to link against the precompiled Nokogiri libraries.
sparklemotion · Mar 8, 2021 · 4830749 · 4830749
2 parents f1aeb9e + 3cff820
commit 4830749
Show file tree

Hide file tree

Showing 6 changed files with 103 additions and 45 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@ Nokogiri follows [Semantic Versioning](https://semver.org/), please see the [REA
 * [CRuby] `NodeSet` may now safely contain `Node` objects from multiple documents. Previously the GC lifecycle of the parent `Document` objects could lead to contained nodes being GCed while still in scope. [[#1952](https://github.com/sparklemotion/nokogiri/issues/1952)]
 * [CRuby] Patch libxml2 to avoid "huge input lookup" errors on large CDATA elements. (See upstream [GNOME/libxml2#200](https://gitlab.gnome.org/GNOME/libxml2/-/issues/200) and [GNOME/libxml2!100](https://gitlab.gnome.org/GNOME/libxml2/-/merge_requests/100).) [[#2132](https://github.com/sparklemotion/nokogiri/issues/2132)].
 * [CRuby] `{XML,HTML}::Document.parse` now invokes `#initialize` exactly once. Previously `#initialize` was invoked twice on each object.
+* [CRuby+Windows] Enable Nokogumbo (and other downstream gems) to compile and link against `nokogiri.so` by including `LDFLAGS` in `Nokogiri::VERSION_INFO`. [[#2167](https://github.com/sparklemotion/nokogiri/issues/2167)]
 * [JRuby] `{XML,HTML}::Document.parse` now invokes `#initialize` exactly once. Previously `#initialize` was not called, which was a problem for subclassing such as done by `Loofah`.
 
 

diff --git a/ext/nokogiri/extconf.rb b/ext/nokogiri/extconf.rb
@@ -239,7 +239,7 @@ def ensure_package_configuration(opt: nil, pc: nil, lib:, func:, headers:)
 end
 
 def ensure_func(func, headers = nil)
-  have_func(func, headers) || abort_could_not_find_library(lib)
+  have_func(func, headers) || abort_could_not_find_library(func)
 end
 
 def preserving_globals
@@ -741,6 +741,10 @@ def compile
       recipe.configure_options += ["RANLIB=/usr/bin/ranlib", "AR=/usr/bin/ar"]
     end
 
+    if windows?
+      cflags = concat_flags(cflags, "-ULIBXML_STATIC", "-DIN_LIBXML")
+    end
+
     recipe.configure_options += [
       "--without-python",
       "--without-readline",

diff --git a/ext/nokogiri/nokogiri.h b/ext/nokogiri/nokogiri.h
@@ -15,6 +15,14 @@
 #  include <windows.h>
 #endif
 
+#if _WIN32
+#  define NOKOPUBFUN __declspec(dllexport)
+#  define NOKOPUBVAR __declspec(dllexport) extern
+#else
+#  define NOKOPUBFUN
+#  define NOKOPUBVAR extern
+#endif
+
 
 #include <stdlib.h>
 #include <string.h>
@@ -83,47 +91,47 @@ xmlNodePtr xmlLastElementChild(xmlNodePtr parent);
 #endif
 
 
-extern VALUE mNokogiri ;
-extern VALUE mNokogiriHtml ;
-extern VALUE mNokogiriHtmlSax ;
-extern VALUE mNokogiriXml ;
-extern VALUE mNokogiriXmlSax ;
-extern VALUE mNokogiriXslt ;
-
-extern VALUE cNokogiriSyntaxError;
-extern VALUE cNokogiriXmlAttr;
-extern VALUE cNokogiriXmlAttributeDecl;
-extern VALUE cNokogiriXmlCData;
-extern VALUE cNokogiriXmlCharacterData;
-extern VALUE cNokogiriXmlComment;
-extern VALUE cNokogiriXmlDocument ;
-extern VALUE cNokogiriXmlDocumentFragment;
-extern VALUE cNokogiriXmlDtd;
-extern VALUE cNokogiriXmlElement ;
-extern VALUE cNokogiriXmlElementContent;
-extern VALUE cNokogiriXmlElementDecl;
-extern VALUE cNokogiriXmlEntityDecl;
-extern VALUE cNokogiriXmlEntityReference;
-extern VALUE cNokogiriXmlNamespace ;
-extern VALUE cNokogiriXmlNode ;
-extern VALUE cNokogiriXmlNodeSet ;
-extern VALUE cNokogiriXmlProcessingInstruction;
-extern VALUE cNokogiriXmlReader;
-extern VALUE cNokogiriXmlRelaxNG;
-extern VALUE cNokogiriXmlSaxParser ;
-extern VALUE cNokogiriXmlSaxParserContext;
-extern VALUE cNokogiriXmlSaxPushParser ;
-extern VALUE cNokogiriXmlSchema;
-extern VALUE cNokogiriXmlSyntaxError;
-extern VALUE cNokogiriXmlText ;
-extern VALUE cNokogiriXmlXpathContext;
-extern VALUE cNokogiriXmlXpathSyntaxError;
-extern VALUE cNokogiriXsltStylesheet ;
-
-extern VALUE cNokogiriHtmlDocument ;
-extern VALUE cNokogiriHtmlSaxPushParser ;
-extern VALUE cNokogiriHtmlElementDescription ;
-extern VALUE cNokogiriHtmlSaxParserContext;
+NOKOPUBVAR VALUE mNokogiri ;
+NOKOPUBVAR VALUE mNokogiriHtml ;
+NOKOPUBVAR VALUE mNokogiriHtmlSax ;
+NOKOPUBVAR VALUE mNokogiriXml ;
+NOKOPUBVAR VALUE mNokogiriXmlSax ;
+NOKOPUBVAR VALUE mNokogiriXslt ;
+
+NOKOPUBVAR VALUE cNokogiriSyntaxError;
+NOKOPUBVAR VALUE cNokogiriXmlAttr;
+NOKOPUBVAR VALUE cNokogiriXmlAttributeDecl;
+NOKOPUBVAR VALUE cNokogiriXmlCData;
+NOKOPUBVAR VALUE cNokogiriXmlCharacterData;
+NOKOPUBVAR VALUE cNokogiriXmlComment;
+NOKOPUBVAR VALUE cNokogiriXmlDocument ;
+NOKOPUBVAR VALUE cNokogiriXmlDocumentFragment;
+NOKOPUBVAR VALUE cNokogiriXmlDtd;
+NOKOPUBVAR VALUE cNokogiriXmlElement ;
+NOKOPUBVAR VALUE cNokogiriXmlElementContent;
+NOKOPUBVAR VALUE cNokogiriXmlElementDecl;
+NOKOPUBVAR VALUE cNokogiriXmlEntityDecl;
+NOKOPUBVAR VALUE cNokogiriXmlEntityReference;
+NOKOPUBVAR VALUE cNokogiriXmlNamespace ;
+NOKOPUBVAR VALUE cNokogiriXmlNode ;
+NOKOPUBVAR VALUE cNokogiriXmlNodeSet ;
+NOKOPUBVAR VALUE cNokogiriXmlProcessingInstruction;
+NOKOPUBVAR VALUE cNokogiriXmlReader;
+NOKOPUBVAR VALUE cNokogiriXmlRelaxNG;
+NOKOPUBVAR VALUE cNokogiriXmlSaxParser ;
+NOKOPUBVAR VALUE cNokogiriXmlSaxParserContext;
+NOKOPUBVAR VALUE cNokogiriXmlSaxPushParser ;
+NOKOPUBVAR VALUE cNokogiriXmlSchema;
+NOKOPUBVAR VALUE cNokogiriXmlSyntaxError;
+NOKOPUBVAR VALUE cNokogiriXmlText ;
+NOKOPUBVAR VALUE cNokogiriXmlXpathContext;
+NOKOPUBVAR VALUE cNokogiriXmlXpathSyntaxError;
+NOKOPUBVAR VALUE cNokogiriXsltStylesheet ;
+
+NOKOPUBVAR VALUE cNokogiriHtmlDocument ;
+NOKOPUBVAR VALUE cNokogiriHtmlSaxPushParser ;
+NOKOPUBVAR VALUE cNokogiriHtmlElementDescription ;
+NOKOPUBVAR VALUE cNokogiriHtmlSaxParserContext;
 
 typedef struct _nokogiriTuple {
   VALUE         doc;
@@ -169,7 +177,7 @@ VALUE noko_xml_node_set_wrap(xmlNodeSetPtr node_set, VALUE document) ;
 
 VALUE noko_xml_document_wrap_with_init_args(VALUE klass, xmlDocPtr doc, int argc, VALUE *argv);
 VALUE noko_xml_document_wrap(VALUE klass, xmlDocPtr doc);
-VALUE Nokogiri_wrap_xml_document(VALUE klass, xmlDocPtr doc); /* deprecated. use noko_xml_document_wrap() instead. */
+NOKOPUBFUN VALUE Nokogiri_wrap_xml_document(VALUE klass, xmlDocPtr doc); /* deprecated. use noko_xml_document_wrap() instead. */
 
 #define DOC_RUBY_OBJECT_TEST(x) ((nokogiriTuplePtr)(x->_private))
 #define DOC_RUBY_OBJECT(x) (((nokogiriTuplePtr)(x->_private))->doc)

diff --git a/lib/nokogiri/version/info.rb b/lib/nokogiri/version/info.rb
@@ -10,6 +10,14 @@ def jruby?
       ::JRUBY_VERSION if ::RUBY_PLATFORM == "java"
     end
 
+    def windows?
+      ::RUBY_PLATFORM =~ /mingw|mswin/
+    end
+
+    def ruby_minor
+      Gem::Version.new(::RUBY_VERSION).segments[0..1].join(".")
+    end
+
     def engine
       defined?(::RUBY_ENGINE) ? ::RUBY_ENGINE : "mri"
     end
@@ -74,18 +82,38 @@ def warnings
 
     def to_hash
       header_directory = File.expand_path(File.join(File.dirname(__FILE__), "../../../ext/nokogiri"))
+
       {}.tap do |vi|
         vi["warnings"] = []
         vi["nokogiri"] = {}.tap do |nokogiri|
           nokogiri["version"] = Nokogiri::VERSION
 
           unless jruby?
+            #  enable gems like nokogumbo to build with the following in their extconf.rb:
+            #
+            #    append_cflags(Nokogiri::VERSION_INFO["nokogiri"]["cppflags"])
+            #    append_ldflags(Nokogiri::VERSION_INFO["nokogiri"]["ldflags"])
+            #
             cppflags = ["-I#{header_directory.shellescape}"]
+            ldflags = []
+
             if libxml2_using_packaged?
               cppflags << "-I#{File.join(header_directory, 'include').shellescape}"
               cppflags << "-I#{File.join(header_directory, 'include/libxml2').shellescape}"
+
+              if windows?
+                # on windows, nokogumbo needs to link against nokogiri.so to resolve symbols. see #2167
+                lib_directory = File.expand_path(File.join(File.dirname(__FILE__), "../#{ruby_minor}"))
+                unless File.exist?(lib_directory)
+                  lib_directory = File.expand_path(File.join(File.dirname(__FILE__), ".."))
+                end
+                ldflags << "-L#{lib_directory.shellescape}"
+                ldflags << "-l:nokogiri.so"
+              end
             end
+
             nokogiri["cppflags"] = cppflags
+            nokogiri["ldflags"] = ldflags
           end
         end
         vi["ruby"] = {}.tap do |ruby|

diff --git a/scripts/test-gem-installation b/scripts/test-gem-installation
@@ -43,6 +43,8 @@ Minitest::Reporters.use!([Minitest::Reporters::SpecReporter.new])
 
 puts "Testing #{gemspec.full_name} installed in #{gemspec.base_dir}"
 describe gemspec.full_name do
+  let(:ruby_maj_min) { Gem::Version.new(::RUBY_VERSION).segments[0..1].join(".") }
+  let(:nokogiri_lib_dir) { File.join(gemspec.gem_dir, "lib/nokogiri") }
   let(:nokogiri_ext_dir) { File.join(gemspec.gem_dir, "ext/nokogiri") }
   let(:nokogiri_include_dir) { File.join(nokogiri_ext_dir, "include") }
 
@@ -108,6 +110,20 @@ describe gemspec.full_name do
             assert(found, "expected to find #{header} in one of: #{headers_dirs.inspect}")
           end
         end
+
+        it "has ldflags pointing to the shared object file" do
+          ldflags = Nokogiri::VERSION_INFO["nokogiri"]["ldflags"]
+          if ::RUBY_PLATFORM =~ /mingw|mswin/
+            if gemspec.platform.cpu
+              assert_includes(ldflags, "-L#{File.join(nokogiri_lib_dir, ruby_maj_min)}")
+            else
+              assert_includes(ldflags, "-L#{nokogiri_lib_dir}")
+            end
+            assert_includes(ldflags, "-l:nokogiri.so")
+          else
+            assert_empty(ldflags)
+          end
+        end
       end if Nokogiri::VersionInfo.instance.libxml2_using_packaged?
 
       describe "using system libraries" do

diff --git a/test/test_version.rb b/test/test_version.rb
@@ -18,8 +18,9 @@ def test_version_info_basics
     if jruby?
       refute(Nokogiri::VERSION_INFO["nokogiri"].has_key?("cppflags"), "did not expect cppflags")
     else
-      # cppflags are more fully tested in scripts/test-gem-installation
-      assert_kind_of(Array, Nokogiri::VERSION_INFO["nokogiri"]["cppflags"], "expected cppflags to be an array")
+      # cppflags/ldflags are more fully tested in scripts/test-gem-installation
+      assert_kind_of(Array, Nokogiri::VERSION_INFO["nokogiri"]["cppflags"], "cppflags should be an array")
+      assert_kind_of(Array, Nokogiri::VERSION_INFO["nokogiri"]["ldflags"], "ldflags should be an array")
     end
 
     assert_equal(::RUBY_VERSION, Nokogiri::VERSION_INFO["ruby"]["version"])