making VersionInfo naming more consistent and easier to use

- Renamed constant LIBXML_VERSION to LIBXML_COMPILED_VERSION. - Renamed constant LIBXML_PARSER_VERSION to LIBXML_LOADED_VERSION. - Renamed `VersionInfo#loaded_parser_version` to `#loaded_libxml_version` and changed the return type from String to Gem::Version. - Renamed `VersionInfo#compiled_parser_version` to `#compiled_libxml_version` and changed the return type from String to Gem::Version. - `Nokogiri.uses_libxml?` now accepts an optional requirement string which is interpreted as a Gem::Requirement and tested against the loaded libxml2 version. This greatly simplifies much of the version-dependent branching logic in both the implementation and the tests. - use `uses_libxml?` with appropriate version requirements instead of the ad-hoc comparisons used everywhere related to #1974
sparklemotion · Feb 2, 2020 · f7e47ce · f7e47ce
1 parent 0aaaf55
commit f7e47ce
Show file tree

Hide file tree

Showing 11 changed files with 137 additions and 47 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,18 +14,109 @@ This release ends support for:
 
 #### Self-descriptive version info
 
-This release also changes the information provided in `Nokogiri::VersionInfo`
+This release also changes the information provided in
+`Nokogiri::VersionInfo`, see #1482 and #1974 for background. Note that
+the output of `nokogiri -v` will also reflect these changes.
 
-* Nokogiri::VersionInfo will no longer contain the following keys (previously these were set only when vendored libraries were being used) [#1482]:
+* `Nokogiri::VersionInfo` will no longer contain the following keys (previously these were set only when vendored libraries were being used)
   * `libxml/libxml2_path`
   * `libxml/libxslt_path`
-* `nokogiri -v` will no longer emit these VersionInfo values [#1482]
-* these C macros will no longer be defined [#1482]:
-  * NOKOGIRI_LIBXML2_PATH
-  * NOKOGIRI_LIBXSLT_PATH
-* these global variables will no longer be defined [#1482]:
-  * NOKOGIRI_LIBXML2_PATH
-  * NOKOGIRI_LIBXSLT_PATH
+* `Nokogiri::VersionInfo` now contains version metadata for libxslt:
+  * `libxslt/source` (either "packaged" or "system", similar to `libxml/source`)
+  * `libxslt/compiled` (the version of libxslt compiled at installation time, similar to `libxml/compiled`)
+  * `libxslt/loaded` (the version of libxslt loaded at runtime, similar to `libxml/loaded`)
+  * `libxslt/patches` moved from `libxml/libxslt_patches`
+* `Nokogiri::VersionInfo` key `libxml/libxml2_patches` has been renamed to `libxml/patches`
+* these C macros will no longer be defined:
+  * `NOKOGIRI_LIBXML2_PATH`
+  * `NOKOGIRI_LIBXSLT_PATH`
+* these global variables will no longer be defined:
+  * `NOKOGIRI_LIBXML2_PATH`
+  * `NOKOGIRI_LIBXSLT_PATH`
+* these constants have been renamed:
+  * `Nokogiri::LIBXML_VERSION` is now `Nokogiri::LIBXML_COMPILED_VERSION`
+  * `Nokogiri::LIBXML_PARSER_VERSION` is now `Nokogiri::LIBXML_LOADED_VERSION`
+* these methods have been renamed and the return type changed from `String` to `Gem::Version`:
+  * `VersionInfo#loaded_parser_version` is now `#loaded_libxml_version`
+  * `VersionInfo#compiled_parser_version` is now `#compiled_libxml_version`
+* `Nokogiri.uses_libxml?` now accepts an optional requirement string which is interpreted as a `Gem::Requirement` and tested against the loaded libxml2 version (the value in `VersionInfo` key `libxml/loaded`). This greatly simplifies much of the version-dependent branching logic in both the implementation and the tests.
+
+Maybe to sum this change up, the output from CRuby when using vendored libraries was something like:
+
+```
+# Nokogiri (1.10.7)
+    ---
+    warnings: []
+    nokogiri: 1.10.7
+    ruby:
+      version: 2.7.0
+      platform: x86_64-linux
+      description: ruby 2.7.0p0 (2019-12-25 revision 647ee6f091) [x86_64-linux]
+      engine: ruby
+    libxml:
+      binding: extension
+      source: packaged
+      libxml2_path: "/home/flavorjones/.rvm/gems/ruby-2.7.0/gems/nokogiri-1.10.7/ports/x86_64-pc-linux-gnu/libxml2/2.9.10"
+      libxslt_path: "/home/flavorjones/.rvm/gems/ruby-2.7.0/gems/nokogiri-1.10.7/ports/x86_64-pc-linux-gnu/libxslt/1.1.34"
+      libxml2_patches:
+      - 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
+      libxslt_patches: []
+      compiled: 2.9.10
+      loaded: 2.9.10
+```
+
+but now looks like:
+
+```
+# Nokogiri (1.11.0)
+    ---
+    warnings: []
+    nokogiri: 1.11.0
+    ruby:
+      version: 2.7.0
+      platform: x86_64-linux
+      description: ruby 2.7.0p0 (2019-12-25 revision 647ee6f091) [x86_64-linux]
+      engine: ruby
+    libxml:
+      source: packaged
+      patches:
+      - 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
+      compiled: 2.9.10
+      loaded: 2.9.10
+    libxslt:
+      source: packaged
+      patches: []
+      compiled: 1.1.34
+      loaded: 1.1.34
+```
+
+and the output from using system libraries now looks like:
+
+```
+# Nokogiri (1.11.0)
+    ---
+    warnings: []
+    nokogiri: 1.11.0
+    ruby:
+      version: 2.7.0
+      platform: x86_64-linux
+      description: ruby 2.7.0p0 (2019-12-25 revision 647ee6f091) [x86_64-linux]
+      engine: ruby
+    libxml:
+      source: system
+      compiled: 2.9.4
+      loaded: 2.9.4
+    libxslt:
+      source: system
+      compiled: 1.1.29
+      loaded: 1.1.29
+```
 
 
 ### Features

diff --git a/ext/nokogiri/nokogiri.c b/ext/nokogiri/nokogiri.c
@@ -76,11 +76,11 @@ void Init_nokogiri()
   mNokogiriHtmlSax  = rb_define_module_under(mNokogiriHtml, "SAX");
 
   rb_const_set( mNokogiri,
-                rb_intern("LIBXML_VERSION"),
+                rb_intern("LIBXML_COMPILED_VERSION"),
                 NOKOGIRI_STR_NEW2(LIBXML_DOTTED_VERSION)
               );
   rb_const_set( mNokogiri,
-                rb_intern("LIBXML_PARSER_VERSION"),
+                rb_intern("LIBXML_LOADED_VERSION"),
                 NOKOGIRI_STR_NEW2(xmlParserVersion)
               );
 

diff --git a/lib/nokogiri/version.rb b/lib/nokogiri/version.rb
@@ -12,30 +12,30 @@ def engine
       defined?(RUBY_ENGINE) ? RUBY_ENGINE : "mri"
     end
 
-    def loaded_parser_version
-      LIBXML_PARSER_VERSION.
+    def loaded_libxml_version
+      Gem::Version.new(LIBXML_LOADED_VERSION.
         scan(/^(\d+)(\d\d)(\d\d)(?!\d)/).first.
         collect(&:to_i).
-        join(".")
+        join("."))
     end
 
-    def compiled_parser_version
-      LIBXML_VERSION
+    def compiled_libxml_version
+      Gem::Version.new LIBXML_COMPILED_VERSION
     end
 
     def loaded_libxslt_version
-      LIBXSLT_LOADED_VERSION.
+      Gem::Version.new(LIBXSLT_LOADED_VERSION.
         scan(/^(\d+)(\d\d)(\d\d)(?!\d)/).first.
         collect(&:to_i).
-        join(".")
+        join("."))
     end
 
     def compiled_libxslt_version
-      LIBXSLT_COMPILED_VERSION
+      Gem::Version.new LIBXSLT_COMPILED_VERSION
     end
 
     def libxml2?
-      defined?(LIBXML_VERSION)
+      defined?(LIBXML_COMPILED_VERSION)
     end
 
     def libxml2_using_system?
@@ -50,8 +50,8 @@ def warnings
       warnings = []
 
       if libxml2?
-        if compiled_parser_version != loaded_parser_version
-          warnings << "Nokogiri was built against libxml version #{compiled_parser_version}, but has dynamically loaded #{loaded_parser_version}"
+        if compiled_libxml_version != loaded_libxml_version
+          warnings << "Nokogiri was built against libxml version #{compiled_libxml_version}, but has dynamically loaded #{loaded_libxml_version}"
         end
 
         if compiled_libxslt_version != loaded_libxslt_version
@@ -82,8 +82,8 @@ def to_hash
             else
               libxml["source"] = "system"
             end
-            libxml["compiled"] = compiled_parser_version
-            libxml["loaded"] = loaded_parser_version
+            libxml["compiled"] = compiled_libxml_version.to_s
+            libxml["loaded"] = loaded_libxml_version.to_s
           end
 
           vi["libxslt"] = {}.tap do |libxslt|
@@ -93,8 +93,8 @@ def to_hash
             else
               libxslt["source"] = "system"
             end
-            libxslt["compiled"] = compiled_libxslt_version
-            libxslt["loaded"] = loaded_libxslt_version
+            libxslt["compiled"] = compiled_libxslt_version.to_s
+            libxslt["loaded"] = loaded_libxslt_version.to_s
           end
 
           vi["warnings"] = warnings
@@ -123,8 +123,10 @@ def to_markdown
     def self.instance; @@instance; end
   end
 
-  def self.uses_libxml? # :nodoc:
-    VersionInfo.instance.libxml2?
+  def self.uses_libxml?(requirement = nil) # :nodoc:
+    return false unless VersionInfo.instance.libxml2?
+    return true unless requirement
+    return Gem::Requirement.new(requirement).satisfied_by?(VersionInfo.instance.loaded_libxml_version)
   end
 
   def self.jruby? # :nodoc:

diff --git a/lib/nokogiri/xml/node.rb b/lib/nokogiri/xml/node.rb
@@ -861,7 +861,7 @@ def add_sibling next_or_previous, node_or_tags
       end
 
       # FIXME: used for hacking around the output of broken libxml versions
-      IS_BROKEN_LIBXML_VERSION = (Nokogiri.uses_libxml? && LIBXML_VERSION.start_with?('2.6.')).freeze
+      IS_BROKEN_LIBXML_VERSION = Nokogiri.uses_libxml?("~> 2.6.0").freeze
       private_constant :IS_BROKEN_LIBXML_VERSION
 
       def to_format save_option, options

diff --git a/test/html/sax/test_parser.rb b/test/html/sax/test_parser.rb
@@ -110,11 +110,11 @@ def test_parser_attributes
 
           assert block_called
 
-          noshade_value = if Nokogiri.uses_libxml? && Nokogiri::VERSION_INFO['libxml']['loaded'] < '2.7.7'
-                            ['noshade', 'noshade']
-                          else
-                            ['noshade', nil]
-                          end
+          noshade_value = if Nokogiri.uses_libxml?("< 2.7.7")
+              ["noshade", "noshade"]
+            else
+              ["noshade", nil]
+            end
 
           assert_equal [
             ['html', []],

diff --git a/test/html/test_document.rb b/test/html/test_document.rb
@@ -103,8 +103,7 @@ def test_empty_string_returns_empty_doc
         assert_nil doc.root
       end
 
-      unless Nokogiri.uses_libxml? && %w[2 6] === LIBXML_VERSION.split(".")[0..1]
-        # FIXME: this is a hack around broken libxml versions
+      unless Nokogiri.uses_libxml?("~> 2.6.0")
         def test_to_xhtml_with_indent
           doc = Nokogiri::HTML("<html><body><a>foo</a></body></html>")
           doc = Nokogiri::HTML(doc.to_xhtml(:indent => 2))

diff --git a/test/html/test_document_fragment.rb b/test/html/test_document_fragment.rb
@@ -147,8 +147,7 @@ def test_html_fragment
       def test_html_fragment_has_outer_text
         doc = "a<div>b</div>c"
         fragment = Nokogiri::HTML::Document.new.fragment(doc)
-        if Nokogiri.uses_libxml? &&
-            Nokogiri::VERSION_INFO['libxml']['loaded'] <= "2.6.16"
+        if Nokogiri.uses_libxml?("<= 2.6.16")
           assert_equal "a<div>b</div><p>c</p>", fragment.to_s
         else
           assert_equal "a<div>b</div>c", fragment.to_s
@@ -210,7 +209,7 @@ def test_to_html
       def test_to_xhtml
         doc = "<span>foo<br></span><span>bar</span><p></p>"
         fragment = Nokogiri::HTML::Document.new.fragment(doc)
-        if Nokogiri.jruby? || Nokogiri::VERSION_INFO['libxml']['loaded'] >= "2.7.0"
+        if Nokogiri.jruby? || Nokogiri.uses_libxml?(">= 2.7.0")
           assert_equal "<span>foo<br /></span><span>bar</span><p></p>", fragment.to_xhtml
         else
           # FIXME: why are we doing this ? this violates the spec,
@@ -240,7 +239,7 @@ def test_fragment_with_comment
       end
 
       def test_element_children_counts
-        if Nokogiri.uses_libxml? && Nokogiri::VERSION_INFO['libxml']['loaded'] <= "2.9.1"
+        if Nokogiri.uses_libxml?("<= 2.9.1")
           skip "#elements doesn't work in 2.9.1, see 1793a5a for history"
         end
         doc = Nokogiri::HTML::DocumentFragment.parse("   <div>  </div>\n   ")

diff --git a/test/html/test_element_description.rb b/test/html/test_element_description.rb
@@ -56,7 +56,7 @@ def test_description
 
       def test_subelements
         sub_elements = ElementDescription['body'].sub_elements
-        if Nokogiri.uses_libxml? && Nokogiri::LIBXML_VERSION >= '2.7.7'
+        if Nokogiri.uses_libxml?(">= 2.7.7")
           assert_equal 65, sub_elements.length
         elsif Nokogiri.uses_libxml?
           assert_equal 61, sub_elements.length

diff --git a/test/xml/test_document.rb b/test/xml/test_document.rb
@@ -619,8 +619,7 @@ def test_write_xml_to_with_indent
         assert_indent 5, doc
       end
 
-      # wtf...  osx's libxml sucks.
-      unless !Nokogiri.uses_libxml? || Nokogiri::LIBXML_VERSION =~ /^2\.6\./
+      if ! Nokogiri.uses_libxml?("~> 2.6.0")
         def test_encoding
           xml = Nokogiri::XML(File.read(XML_FILE), XML_FILE, "UTF-8")
           assert_equal "UTF-8", xml.encoding

diff --git a/test/xml/test_document_encoding.rb b/test/xml/test_document_encoding.rb
@@ -17,9 +17,9 @@ def test_encoding
       end
 
       def test_dotted_version
-        if Nokogiri.uses_libxml?
-          assert_equal 'UTF-8', Nokogiri::LIBXML_VERSION.encoding.name
-        end
+        skip "libxml2 is only used for CRuby" unless Nokogiri.uses_libxml?
+        assert_equal "UTF-8", Nokogiri::LIBXML_COMPILED_VERSION.encoding.name
+        assert_equal "UTF-8", Nokogiri::LIBXSLT_COMPILED_VERSION.encoding.name
       end
 
       def test_empty_doc_encoding

diff --git a/test/xml/test_entity_reference.rb b/test/xml/test_entity_reference.rb
@@ -247,7 +247,7 @@ def setup
         reader = Nokogiri::XML::Reader html, path do |cfg|
           cfg.default_xml
         end
-        if Nokogiri.uses_libxml? && Nokogiri::LIBXML_PARSER_VERSION.to_i >= 20900
+        if Nokogiri.uses_libxml?(">= 2.9.0")
           # Unknown entity is not fatal in libxml2 >= 2.9
           assert_equal 8, reader.count
         else