[GR-17457] Improve building of nokogiri by reducing applied patches.

PullRequest: truffleruby/3298

Author: aardvark179

CHANGELOG.md

@@ -17,6 +17,7 @@ Compatibility:
 * Fix `Marshal.dump` to raise an error when an object has singleton methods (@bjfish).
 * `Exception#full_message` now defaults the order to `:top` like CRuby 3+ (@eregon).
 * Fix `Process.wait2` to return `nil` when the `WNOHANG` flag is given and the child process is still running (@bjfish).
+* Disable most `nokogiri` C extension patches when system libraries are not being used (#2693, @aardvark179).
 
 Performance:
 

lib/cext/ABI_check.txt

@@ -1 +1 @@
-1
+2

lib/mri/mkmf.rb

@@ -32,7 +32,7 @@
 
     for_pipe = proc do |compiler, flags|
       language_flag = '$(CXX)' == compiler ? '-xc++' : '-xc'
-      "#{RbConfig.ruby} #{cext_dir}/preprocess.rb $< | #{compiler} -I$(<D) #{flags} #{language_flag} -"
+      "#{RbConfig.ruby} #{cext_dir}/preprocess.rb $< #{flags} | #{compiler} -I$(<D) #{flags} #{language_flag} -"
     end
 
     c_flags = '$(INCFLAGS) $(CPPFLAGS) $(CFLAGS) $(COUTFLAG)$@ -c'

lib/truffle/truffle/cext_preprocessor.rb

@@ -96,15 +96,18 @@ def self.patch(file, contents, directory)
             # preprocessor macro which _must_ end with a newline and
             # so requires that we preserve the trailing whitespace.
             patched_file[:patches].each do |patch|
-              replacement = patch[:replacement].rstrip
-              last_line = replacement.lines.last || replacement # .lines returns an empty Array if String#empty?
-              last_line = last_line.lstrip
-              contents = contents.gsub(patch[:match],
-                                       if last_line && last_line.start_with?('#')
-                                         patch[:replacement]
-                                       else
-                                         patch[:replacement].rstrip
-                                       end)
+              predicate = patch[:predicate] || -> { true }
+              if predicate.call
+                replacement = patch[:replacement].rstrip
+                last_line = replacement.lines.last || replacement # .lines returns an empty Array if String#empty?
+                last_line = last_line.lstrip
+                contents = contents.gsub(patch[:match],
+                                         if last_line && last_line.start_with?('#')
+                                           patch[:replacement]
+                                         else
+                                           patch[:replacement].rstrip
+                                         end)
+              end
             end
           end
         end

lib/truffle/truffle/patches/nokogiri_patches.rb

@@ -23,14 +23,8 @@ class NokogiriPatches
   xmlDocPtr doc = (xmlDocPtr)c;
 EOF
 
-  def self.patch_for_system_libraries(replacement)
-    <<-EOF
-#ifdef NOKOGIRI_PACKAGED_LIBRARIES
-\\&
-#else
-#{replacement}
-#endif
-EOF
+  def self.using_system_libraries?
+    !ARGV.include?('-DNOKOGIRI_PACKAGED_LIBRARIES')
   end
 
   PATCHES = {
@@ -41,14 +35,16 @@ def self.patch_for_system_libraries(replacement)
           # is called with. This works on MRI but causes an error in
           # TruffleRuby.
           match: 'static VALUE to_array(VALUE self, VALUE rb_node)',
-          replacement: patch_for_system_libraries('static VALUE to_array(VALUE self)')
+          replacement: 'static VALUE to_array(VALUE self)',
+          predicate: -> { using_system_libraries? }
         },
       ],
       'xslt_stylesheet.c' => [
         { # It is not currently possible to pass var args from native
           # functions to sulong, so we work round the issue here.
           match: 'va_list args;',
-          replacement: patch_for_system_libraries('va_list args; rb_str_cat2(ctx, "Generic error"); return;')
+          replacement: 'va_list args; rb_str_cat2(ctx, "Generic error"); return;',
+          predicate: -> { using_system_libraries? }
         }
       ],
       'xml_document.c' => [
@@ -61,31 +57,37 @@ def self.patch_for_system_libraries(replacement)
         { # It is not currently possible to pass var args from native
           # functions to sulong, so we work round the issue here.
           match: /va_list args;[^}]*id_warning, 1, ruby_message\);/,
-          replacement: patch_for_system_libraries('rb_funcall(doc, id_warning, 1, NOKOGIRI_STR_NEW2("Warning."));')
+          replacement: 'rb_funcall(doc, id_warning, 1, NOKOGIRI_STR_NEW2("Warning."));',
+          predicate: -> { using_system_libraries? }
         },
         { # It is not currently possible to pass var args from native
           # functions to sulong, so we work round the issue here.
           match: /va_list args;[^}]*id_error, 1, ruby_message\);/,
-          replacement: patch_for_system_libraries('rb_funcall(doc, id_error, 1, NOKOGIRI_STR_NEW2("Warning."));')
+          replacement: 'rb_funcall(doc, id_error, 1, NOKOGIRI_STR_NEW2("Warning."));',
+          predicate: -> { using_system_libraries? }
         }
       ],
       'xml_xpath_context.c' => [
         { # It is not currently possible to pass var args from native
           # functions to sulong, so we work round the issue here.
           match: 'va_list args;',
-          replacement: patch_for_system_libraries('va_list args; rb_raise(rb_eRuntimeError, "%s", "Exception:"); return;')
+          replacement: 'va_list args; rb_raise(rb_eRuntimeError, "%s", "Exception:"); return;',
+          predicate: -> { using_system_libraries? }
         },
         {
           match: 'VALUE thing = Qnil;',
-          replacement: patch_for_system_libraries("VALUE thing = Qnil;\nVALUE errors = rb_ary_new();")
+          replacement: "VALUE thing = Qnil;\nVALUE errors = rb_ary_new();",
+          predicate: -> { using_system_libraries? }
         },
         {
           match: 'xmlSetStructuredErrorFunc(NULL, Nokogiri_error_raise);',
-          replacement: patch_for_system_libraries('xmlSetStructuredErrorFunc(errors, Nokogiri_error_array_pusher);')
+          replacement: 'xmlSetStructuredErrorFunc(errors, Nokogiri_error_array_pusher);',
+          predicate: -> { using_system_libraries? }
         },
         {
           match: 'if(xpath == NULL)',
-          replacement: patch_for_system_libraries("if (RARRAY_LEN(errors) > 0) { rb_exc_raise(rb_ary_entry(errors, 0)); }\nif(xpath == NULL)")
+          replacement: "if (RARRAY_LEN(errors) > 0) { rb_exc_raise(rb_ary_entry(errors, 0)); }\nif(xpath == NULL)",
+          predicate: -> { using_system_libraries? }
         },
       ],
     }

test/truffle/cexts/test_preprocess.rb

@@ -14,46 +14,17 @@ def test_patch(file, directory, input, expected)
 end
 
 original = <<-EOF
-static void warning_func(void * ctx, const char *msg, ...)
+static int dealloc_node_i(xmlNodePtr key, xmlNodePtr node, xmlDocPtr doc)
 {
-  VALUE self = NOKOGIRI_SAX_SELF(ctx);
-  VALUE doc = rb_iv_get(self, "@document");
-  char * message;
-  VALUE ruby_message;
-
-  va_list args;
-  va_start(args, msg);
-  vasprintf(&message, msg, args);
-  va_end(args);
-
-  ruby_message = NOKOGIRI_STR_NEW2(message);
-  vasprintf_free(message);
-  rb_funcall(doc, id_warning, 1, ruby_message);
 }
 EOF
 
 modified = <<-EOF
-static void warning_func(void * ctx, const char *msg, ...)
+static int dealloc_node_i(st_data_t a, st_data_t b, st_data_t c, int errorState)
 {
-  VALUE self = NOKOGIRI_SAX_SELF(ctx);
-  VALUE doc = rb_iv_get(self, "@document");
-  char * message;
-  VALUE ruby_message;
-
-  #ifdef NOKOGIRI_PACKAGED_LIBRARIES
-va_list args;
-  va_start(args, msg);
-  vasprintf(&message, msg, args);
-  va_end(args);
-
-  ruby_message = NOKOGIRI_STR_NEW2(message);
-  vasprintf_free(message);
-  rb_funcall(doc, id_warning, 1, ruby_message);
-#else
-rb_funcall(doc, id_warning, 1, NOKOGIRI_STR_NEW2("Warning."));
-#endif
-
-}
+  xmlNodePtr key = (xmlNodePtr)a;
+  xmlNodePtr node = (xmlNodePtr)b;
+  xmlDocPtr doc = (xmlDocPtr)c;}
 EOF
 
 json_original = <<-EOF
@@ -68,7 +39,7 @@ def test_patch(file, directory, input, expected)
 # endif
 EOF
 
-test_patch 'xml_sax_parser.c', 'ext/nokogiri', original, modified
+test_patch 'xml_document.c', 'ext/nokogiri', original, modified
 # Should not patch other files or other gems
 test_patch 'other_file.c', 'ext/nokogiri', original, original
 test_patch 'xml_sax_parser.c', 'ext/other_gem', original, original