From 1b841627d70f2a8c603ff2e005314218bee45710 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Wed, 12 Oct 2022 18:45:07 +0200 Subject: [PATCH 1/5] Upstream truffleruby patches needed when using system libraries * Adapt tests for that configuration. --- ext/nokogiri/nokogiri.h | 3 +++ ext/nokogiri/xml_sax_parser.c | 12 ++++++++++++ ext/nokogiri/xml_xpath_context.c | 17 +++++++++++++++++ ext/nokogiri/xslt_stylesheet.c | 6 ++++++ test/helper.rb | 4 ++++ test/test_xslt_transforms.rb | 15 +++++++++++---- test/xml/sax/test_parser.rb | 6 +++++- test/xml/test_entity_reference.rb | 12 ++++++++++-- 8 files changed, 68 insertions(+), 7 deletions(-) diff --git a/ext/nokogiri/nokogiri.h b/ext/nokogiri/nokogiri.h index 63549ecec5..9e7918cd4d 100644 --- a/ext/nokogiri/nokogiri.h +++ b/ext/nokogiri/nokogiri.h @@ -99,6 +99,9 @@ xmlNodePtr xmlLastElementChild(xmlNodePtr parent); # endif #endif +#if defined(TRUFFLERUBY) && !defined(NOKOGIRI_PACKAGED_LIBRARIES) +# define TRUFFLERUBY_NOKOGIRI_SYSTEM_LIBRARIES +#endif NOKOPUBVAR VALUE mNokogiri ; NOKOPUBVAR VALUE mNokogiriGumbo ; diff --git a/ext/nokogiri/xml_sax_parser.c b/ext/nokogiri/xml_sax_parser.c index 904d8d305a..ea772443f3 100644 --- a/ext/nokogiri/xml_sax_parser.c +++ b/ext/nokogiri/xml_sax_parser.c @@ -203,10 +203,16 @@ warning_func(void *ctx, const char *msg, ...) VALUE doc = rb_iv_get(self, "@document"); VALUE rb_message; +#ifdef TRUFFLERUBY_NOKOGIRI_SYSTEM_LIBRARIES + /* It is not currently possible to pass var args from native + functions to sulong, so we work around the issue here. */ + rb_message = rb_sprintf("warning_func: %s", msg); +#else va_list args; va_start(args, msg); rb_message = rb_vsprintf(msg, args); va_end(args); +#endif rb_funcall(doc, id_warning, 1, rb_message); } @@ -219,10 +225,16 @@ error_func(void *ctx, const char *msg, ...) VALUE doc = rb_iv_get(self, "@document"); VALUE rb_message; +#ifdef TRUFFLERUBY_NOKOGIRI_SYSTEM_LIBRARIES + /* It is not currently possible to pass var args from native + functions to sulong, so we work around the issue here. */ + rb_message = rb_sprintf("error_func: %s", msg); +#else va_list args; va_start(args, msg); rb_message = rb_vsprintf(msg, args); va_end(args); +#endif rb_funcall(doc, id_error, 1, rb_message); } diff --git a/ext/nokogiri/xml_xpath_context.c b/ext/nokogiri/xml_xpath_context.c index b8eb633882..cb6b657461 100644 --- a/ext/nokogiri/xml_xpath_context.c +++ b/ext/nokogiri/xml_xpath_context.c @@ -298,10 +298,16 @@ xpath_generic_exception_handler(void *ctx, const char *msg, ...) { VALUE rb_message; +#ifdef TRUFFLERUBY_NOKOGIRI_SYSTEM_LIBRARIES + /* It is not currently possible to pass var args from native + functions to sulong, so we work around the issue here. */ + rb_message = rb_sprintf("xpath_generic_exception_handler: %s", msg); +#else va_list args; va_start(args, msg); rb_message = rb_vsprintf(msg, args); va_end(args); +#endif rb_exc_raise(rb_exc_new3(rb_eRuntimeError, rb_message)); } @@ -336,7 +342,12 @@ evaluate(int argc, VALUE *argv, VALUE self) } xmlResetLastError(); +#ifdef TRUFFLERUBY_NOKOGIRI_SYSTEM_LIBRARIES + VALUE errors = rb_ary_new(); + xmlSetStructuredErrorFunc(errors, Nokogiri_error_array_pusher); +#else xmlSetStructuredErrorFunc(NULL, Nokogiri_error_raise); +#endif /* For some reason, xmlXPathEvalExpression will blow up with a generic error */ /* when there is a non existent function. */ @@ -346,6 +357,12 @@ evaluate(int argc, VALUE *argv, VALUE self) xmlSetStructuredErrorFunc(NULL, NULL); xmlSetGenericErrorFunc(NULL, NULL); +#ifdef TRUFFLERUBY_NOKOGIRI_SYSTEM_LIBRARIES + if (RARRAY_LEN(errors) > 0) { + rb_exc_raise(rb_ary_entry(errors, 0)); + } +#endif + if (xpath == NULL) { xmlErrorPtr error = xmlGetLastError(); rb_exc_raise(Nokogiri_wrap_xml_syntax_error(error)); diff --git a/ext/nokogiri/xslt_stylesheet.c b/ext/nokogiri/xslt_stylesheet.c index d81a98124a..036bc8ef8d 100644 --- a/ext/nokogiri/xslt_stylesheet.c +++ b/ext/nokogiri/xslt_stylesheet.c @@ -26,10 +26,16 @@ xslt_generic_error_handler(void *ctx, const char *msg, ...) { VALUE message; +#ifdef TRUFFLERUBY_NOKOGIRI_SYSTEM_LIBRARIES + /* It is not currently possible to pass var args from native + functions to sulong, so we work around the issue here. */ + message = rb_sprintf("xslt_generic_error_handler: %s", msg); +#else va_list args; va_start(args, msg); message = rb_vsprintf(msg, args); va_end(args); +#endif rb_str_concat((VALUE)ctx, message); } diff --git a/test/helper.rb b/test/helper.rb index 002bcd6a50..0fe0fe71b3 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -119,6 +119,10 @@ def skip_unless_libxml2_patch(patch_name) def skip_unless_jruby(msg = "this test should only run with jruby") skip(msg) unless Nokogiri.jruby? end + + def truffleruby_system_libraries? + RUBY_ENGINE == "truffleruby" && !Nokogiri::PACKAGED_LIBRARIES + end end class TestBenchmark < Minitest::BenchSpec diff --git a/test/test_xslt_transforms.rb b/test/test_xslt_transforms.rb index 6c75b7d15c..a779465f89 100644 --- a/test/test_xslt_transforms.rb +++ b/test/test_xslt_transforms.rb @@ -349,10 +349,17 @@ def test_non_html_xslt_transform exception = assert_raises(RuntimeError) do xslt.transform(doc) end - assert_match( - /xmlXPathCompOpEval: function decimal not found|java.lang.NoSuchMethodException.*decimal/, - exception.message, - ) + if truffleruby_system_libraries? + assert_equal( + "xslt_generic_error_handler: xmlXPathCompOpEval: function %s not found\nxslt_generic_error_handler: %s", + exception.message + ) + else + assert_match( + /xmlXPathCompOpEval: function decimal not found|java.lang.NoSuchMethodException.*decimal/, + exception.message + ) + end end describe "DEFAULT_XSLT parse options" do diff --git a/test/xml/sax/test_parser.rb b/test/xml/sax/test_parser.rb index 9bf9bfeeb8..1205cedaa1 100644 --- a/test/xml/sax/test_parser.rb +++ b/test/xml/sax/test_parser.rb @@ -484,7 +484,11 @@ def call_parse_io_with_encoding(encoding) XML parser.parse(xml) refute_empty(parser.document.warnings) - assert_match(/URI .* is not absolute/, parser.document.warnings.first) + if truffleruby_system_libraries? + assert_equal("warning_func: %s", parser.document.warnings.first) + else + assert_match(/URI .* is not absolute/, parser.document.warnings.first) + end end end end diff --git a/test/xml/test_entity_reference.rb b/test/xml/test_entity_reference.rb index 998896cf11..8b14711028 100644 --- a/test/xml/test_entity_reference.rb +++ b/test/xml/test_entity_reference.rb @@ -193,7 +193,11 @@ def setup html = File.read(xml_document) @parser.parse(html) refute_nil @parser.document.errors - assert_equal ["Entity 'bar' not defined"], @parser.document.errors.map(&:to_s).map(&:strip) + if truffleruby_system_libraries? + assert_equal ["error_func: %s"], @parser.document.errors.map(&:to_s).map(&:strip) + else + assert_equal ["Entity 'bar' not defined"], @parser.document.errors.map(&:to_s).map(&:strip) + end end test_relative_and_absolute_path :test_more_sax_entity_reference do @@ -206,7 +210,11 @@ def setup ] @parser.parse(html) refute_nil @parser.document.errors - assert_equal ["Entity 'bar' not defined"], @parser.document.errors.map(&:to_s).map(&:strip) + if truffleruby_system_libraries? + assert_equal ["error_func: %s"], @parser.document.errors.map(&:to_s).map(&:strip) + else + assert_equal ["Entity 'bar' not defined"], @parser.document.errors.map(&:to_s).map(&:strip) + end end end From 128fcfae777b19bc42d5d07ef365ec6dfbb82a05 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Thu, 13 Oct 2022 12:59:45 +0200 Subject: [PATCH 2/5] Do not force a full GC between every test on TruffleRuby * That is causing too much overhead. --- .github/workflows/truffle.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/truffle.yml b/.github/workflows/truffle.yml index 05604e889b..621fb45e1c 100644 --- a/.github/workflows/truffle.yml +++ b/.github/workflows/truffle.yml @@ -38,3 +38,5 @@ jobs: - run: bundle install --local || bundle install - run: bundle exec rake compile -- ${{matrix.flags}} - run: bundle exec rake test + env: + NOKOGIRI_TEST_GC_LEVEL: normal # otherwise it takes very long From ceacfbd0c71c5588026cce686c808ca1c15ba3bb Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Thu, 13 Oct 2022 15:05:17 +0200 Subject: [PATCH 3/5] Run tests on truffleruby on push/PR like the ci.yml workflow * It takes about the same time as other CI jobs. --- .github/workflows/truffle.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/truffle.yml b/.github/workflows/truffle.yml index 621fb45e1c..1b06f828b0 100644 --- a/.github/workflows/truffle.yml +++ b/.github/workflows/truffle.yml @@ -5,14 +5,17 @@ concurrency: on: workflow_dispatch: schedule: - - cron: "0 8 * * 1,3,5" # At 08:00 on Monday, Wednesday, and Friday # https://crontab.guru/#0_8_*_*_1,3,5 + - cron: "0 8 * * 3" # At 08:00 on Wednesday # https://crontab.guru/#0_8_*_*_3 push: branches: - - "*truffle*" + - main + - v*.*.x + tags: + - v*.*.* pull_request: types: [opened, synchronize] branches: - - "*truffle*" + - '*' jobs: truffleruby-head: From 6937fb0ad70e40770aaa2b6b119a4189b51fd648 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Thu, 13 Oct 2022 17:35:50 -0400 Subject: [PATCH 4/5] test: truffleruby should always default to normal GC --- .github/workflows/truffle.yml | 2 -- test/helper.rb | 6 +++++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/truffle.yml b/.github/workflows/truffle.yml index 1b06f828b0..abf2340739 100644 --- a/.github/workflows/truffle.yml +++ b/.github/workflows/truffle.yml @@ -41,5 +41,3 @@ jobs: - run: bundle install --local || bundle install - run: bundle exec rake compile -- ${{matrix.flags}} - run: bundle exec rake test - env: - NOKOGIRI_TEST_GC_LEVEL: normal # otherwise it takes very long diff --git a/test/helper.rb b/test/helper.rb index 0fe0fe71b3..a3f029a990 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -155,8 +155,12 @@ def initialize_nokogiri_test_gc_level "compact" elsif (ENV["NOKOGIRI_TEST_GC_LEVEL"] == "verify") && defined?(GC.verify_compaction_references) "verify" + elsif RUBY_ENGINE == "truffleruby" + "normal" + elsif defined?(GC.compact) + "compact" else - defined?(GC.compact) ? "compact" : "major" + "major" end if ["compact", "verify"].include?(@@gc_level) # the only way of detecting an unsupported platform is actually From 7b46d28634d32f24542148d1068c7420380229ce Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Thu, 13 Oct 2022 22:47:19 -0400 Subject: [PATCH 5/5] ci: move truffle tests into the main ci.yml workflow --- .github/workflows/ci.yml | 24 +++++++++++++++++++ .github/workflows/truffle.yml | 43 ----------------------------------- 2 files changed, 24 insertions(+), 43 deletions(-) delete mode 100644 .github/workflows/truffle.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d2d2cc44b1..44b793890f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -279,6 +279,30 @@ jobs: - run: bundle exec rake test - run: bundle exec rake test:bench + truffleruby-head: + needs: ["basic"] + strategy: + fail-fast: false + matrix: + flags: + - "--disable-system-libraries --disable-static" + - "--disable-system-libraries --enable-static" + - "--enable-system-libraries" + runs-on: ubuntu-latest + container: + image: ghcr.io/sparklemotion/nokogiri-test:truffle-nightly + steps: + - uses: actions/checkout@v2 + with: + submodules: true + - uses: actions/cache@v2 + with: + path: ports/archives + key: tarballs-ubuntu-${{hashFiles('dependencies.yml', 'patches/**/*.patch')}} + - run: bundle install --local || bundle install + - run: bundle exec rake compile -- ${{matrix.flags}} + - run: bundle exec rake test + bsd: continue-on-error: true # we're seeing VMs hang and fail the whole workflow needs: ["basic"] diff --git a/.github/workflows/truffle.yml b/.github/workflows/truffle.yml deleted file mode 100644 index abf2340739..0000000000 --- a/.github/workflows/truffle.yml +++ /dev/null @@ -1,43 +0,0 @@ -name: truffle -concurrency: - group: "${{github.workflow}}-${{github.ref}}" - cancel-in-progress: true -on: - workflow_dispatch: - schedule: - - cron: "0 8 * * 3" # At 08:00 on Wednesday # https://crontab.guru/#0_8_*_*_3 - push: - branches: - - main - - v*.*.x - tags: - - v*.*.* - pull_request: - types: [opened, synchronize] - branches: - - '*' - -jobs: - truffleruby-head: - strategy: - fail-fast: false - matrix: - flags: - - "--disable-system-libraries --disable-static" - - "--disable-system-libraries --enable-static" - - "--enable-system-libraries" - continue-on-error: true - runs-on: ubuntu-latest - container: - image: ghcr.io/sparklemotion/nokogiri-test:truffle-nightly - steps: - - uses: actions/checkout@v2 - with: - submodules: true - - uses: actions/cache@v2 - with: - path: ports/archives - key: tarballs-ubuntu-${{hashFiles('dependencies.yml', 'patches/**/*.patch')}} - - run: bundle install --local || bundle install - - run: bundle exec rake compile -- ${{matrix.flags}} - - run: bundle exec rake test