Merge pull request #2240 from sparklemotion/flavorjones-update-tests-…

…to-work-with-system-libxml-2_9_12

test: adjust tests to pass on system libxml2 >= 2.9.11, and work around windows dll unloading issue

---

**What problem is this PR intended to solve?**

Adjust tests to pass on system libxml2 >= 2.9.11, because the comment parsing improvement was merged upstream. See https://gitlab.gnome.org/GNOME/libxml2/-/merge_requests/82 for context (and the recently removed `htmlParseComment` patch). This will allow us to run tests regularly against upstream master.

Also, work around the libxml2 issue described in #2241 by having libxml2 use its default memory management on windows when system libraries are being used. Note that this also introduces a new value into `Nokogiri::VERSION_INFO`, which is "libxml → memory_management", and the value will be either "ruby" or "default".

ext/nokogiri/nokogiri.c

@@ -196,7 +196,26 @@ Init_nokogiri()
   rb_const_set(mNokogiri, rb_intern("OTHER_LIBRARY_VERSIONS"), NOKOGIRI_STR_NEW2(NOKOGIRI_OTHER_LIBRARY_VERSIONS));
 #endif
 
+#if defined(_WIN32) && !defined(NOKOGIRI_PACKAGED_LIBRARIES)
+  /*
+   *  We choose *not* to do use Ruby's memory management functions with windows DLLs because of this
+   *  issue in libxml 2.9.12:
+   *
+   *    https://github.com/sparklemotion/nokogiri/issues/2241
+   *
+   *  If the atexit() issue gets fixed in a future version of libxml2, then we may be able to skip
+   *  this config only for the specific libxml2 versions 2.9.12.
+   *
+   *  Alternatively, now that Ruby has a generational GC, it might be OK to let libxml2 use its
+   *  default memory management functions (recall that this config was introduced to reduce memory
+   *  bloat and allow Ruby to GC more often); but we should *really* test with production workloads
+   *  before making that kind of a potentially-invasive change.
+   */
+  rb_const_set(mNokogiri, rb_intern("LIBXML_MEMORY_MANAGEMENT"), NOKOGIRI_STR_NEW2("default"));
+#else
+  rb_const_set(mNokogiri, rb_intern("LIBXML_MEMORY_MANAGEMENT"), NOKOGIRI_STR_NEW2("ruby"));
   xmlMemSetup((xmlFreeFunc)ruby_xfree, (xmlMallocFunc)ruby_xmalloc, (xmlReallocFunc)ruby_xrealloc, ruby_strdup);
+#endif
 
   xmlInitParser();
 

lib/nokogiri/version/info.rb

@@ -137,6 +137,7 @@ def to_hash
             else
               libxml["source"] = "system"
             end
+            libxml["memory_management"] = Nokogiri::LIBXML_MEMORY_MANAGEMENT
             libxml["iconv_enabled"] = libxml2_has_iconv?
             libxml["compiled"] = compiled_libxml_version.to_s
             libxml["loaded"] = loaded_libxml_version.to_s

test/html/test_comments.rb

@@ -113,8 +113,7 @@ class TestComment < Nokogiri::TestCase
         let(:subject) { doc.at_css("div#under-test") }
         let(:inner_div) { doc.at_css("div#do-i-exist") }
 
-        if Nokogiri.uses_libxml? && Nokogiri::VersionInfo.instance.libxml2_using_packaged?
-          # see patches/libxml2/0006-htmlParseComment-treat-as-if-it-closed-the-comment.patch
+        if Nokogiri::VersionInfo.instance.libxml2_using_packaged? || (Nokogiri::VersionInfo.instance.libxml2_using_system? && Nokogiri.uses_libxml?(">=2.9.11"))
           it "behaves as if the comment is normally closed" do # COMPLIANT
             assert_equal 3, subject.children.length
             assert subject.children[0].comment?
@@ -128,9 +127,7 @@ class TestComment < Nokogiri::TestCase
           end
         end
 
-        if Nokogiri.jruby? || Nokogiri::VersionInfo.instance.libxml2_using_system?
-          # this behavior may change to the above in libxml v2.9.11 depending on whether
-          # https://gitlab.gnome.org/GNOME/libxml2/-/merge_requests/82 is merged
+        if Nokogiri.jruby? || (Nokogiri::VersionInfo.instance.libxml2_using_system? && Nokogiri.uses_libxml?("<2.9.11"))
           it "behaves as if the comment encompasses the inner div" do # NON-COMPLIANT
             assert_equal 1, subject.children.length
             assert subject.children.first.comment?