Merge pull request #447 from Shopify/rwstauner/enable-yaml

casperisfine · web-flow · commit 640c12629f77 · 2023-07-03T20:10:31.000+02:00
Enable YAML and JSON CompileCache on TruffleRuby and run the full test suite
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -43,7 +43,7 @@ jobs:
       fail-fast: false
       matrix:
         os: [ubuntu]
-        ruby: ['2.6', '2.7', '3.0', '3.1', '3.2', 'ruby-head', 'debug']
+        ruby: ['2.6', '2.7', '3.0', '3.1', '3.2', 'ruby-head', 'debug', 'truffleruby']
     runs-on: ${{ matrix.os }}-latest
     steps:
       - uses: actions/checkout@v2
@@ -75,7 +75,7 @@ jobs:
       fail-fast: false
       matrix:
         os: [ubuntu]
-        ruby: ['jruby', 'truffleruby']
+        ruby: ['jruby']
     runs-on: ${{ matrix.os }}-latest
     steps:
       - uses: actions/checkout@v2
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,7 @@
 # Unreleased
 
+* Support YAML and JSON CompileCache on TruffleRuby.
+
 # 1.16.0
 
 * Use `RbConfig::CONFIG["rubylibdir"]` instead of `RbConfig::CONFIG["libdir"]` to check for stdlib files. See #431.
diff --git a/ext/bootsnap/extconf.rb b/ext/bootsnap/extconf.rb
@@ -2,7 +2,7 @@
 
 require("mkmf")
 
-if RUBY_ENGINE == "ruby"
+if %w[ruby truffleruby].include?(RUBY_ENGINE)
   $CFLAGS << " -O3 "
   $CFLAGS << " -std=c99"
 
diff --git a/lib/bootsnap/compile_cache.rb b/lib/bootsnap/compile_cache.rb
@@ -54,7 +54,7 @@ def self.permission_error(path)
 
     def self.supported?
       # only enable on 'ruby' (MRI), POSIX (darwin, linux, *bsd), Windows (RubyInstaller2) and >= 2.3.0
-      RUBY_ENGINE == "ruby" && RUBY_PLATFORM.match?(/darwin|linux|bsd|mswin|mingw|cygwin/)
+      %w[ruby truffleruby].include?(RUBY_ENGINE) && RUBY_PLATFORM.match?(/darwin|linux|bsd|mswin|mingw|cygwin/)
     end
   end
 end
diff --git a/lib/bootsnap/compile_cache/iseq.rb b/lib/bootsnap/compile_cache/iseq.rb
@@ -12,6 +12,10 @@ class << self
         def cache_dir=(cache_dir)
           @cache_dir = cache_dir.end_with?("/") ? "#{cache_dir}iseq" : "#{cache_dir}-iseq"
         end
+
+        def supported?
+          CompileCache.supported? && defined?(RubyVM)
+        end
       end
 
       has_ruby_bug_18250 = begin # https://bugs.ruby-lang.org/issues/18250
@@ -103,11 +107,15 @@ def self.compile_option_updated
         crc = Zlib.crc32(option.inspect)
         Bootsnap::CompileCache::Native.compile_option_crc32 = crc
       end
-      compile_option_updated
+      compile_option_updated if supported?
 
       def self.install!(cache_dir)
         Bootsnap::CompileCache::ISeq.cache_dir = cache_dir
+
+        return unless supported?
+
         Bootsnap::CompileCache::ISeq.compile_option_updated
+
         class << RubyVM::InstructionSequence
           prepend(InstructionSequenceMixin)
         end
diff --git a/lib/bootsnap/load_path_cache.rb b/lib/bootsnap/load_path_cache.rb
@@ -50,7 +50,7 @@ def unload!
         @loaded_features_index = nil
         @realpath_cache = nil
         @load_path_cache = nil
-        ChangeObserver.unregister($LOAD_PATH)
+        ChangeObserver.unregister($LOAD_PATH) if supported?
       end
 
       def supported?
diff --git a/test/cli_test.rb b/test/cli_test.rb
@@ -13,30 +13,35 @@ def setup
     end
 
     def test_precompile_single_file
+      skip_unless_iseq
       path = Help.set_file("a.rb", "a = a = 3", 100)
       CompileCache::ISeq.expects(:precompile).with(File.expand_path(path))
       assert_equal 0, CLI.new(["precompile", "-j", "0", path]).run
     end
 
     def test_precompile_rake_files
+      skip_unless_iseq
       path = Help.set_file("a.rake", "a = a = 3", 100)
       CompileCache::ISeq.expects(:precompile).with(File.expand_path(path))
       assert_equal 0, CLI.new(["precompile", "-j", "0", path]).run
     end
 
     def test_precompile_rakefile
+      skip_unless_iseq
       path = Help.set_file("Rakefile", "a = a = 3", 100)
       CompileCache::ISeq.expects(:precompile).with(File.expand_path(path))
       assert_equal 0, CLI.new(["precompile", "-j", "0", path]).run
     end
 
     def test_no_iseq
+      skip_unless_iseq
       path = Help.set_file("a.rb", "a = a = 3", 100)
       CompileCache::ISeq.expects(:precompile).never
       assert_equal 0, CLI.new(["precompile", "-j", "0", "--no-iseq", path]).run
     end
 
     def test_precompile_directory
+      skip_unless_iseq
       path_a = Help.set_file("foo/a.rb", "a = a = 3", 100)
       path_b = Help.set_file("foo/b.rb", "b = b = 3", 100)
 
@@ -46,6 +51,7 @@ def test_precompile_directory
     end
 
     def test_precompile_exclude
+      skip_unless_iseq
       path_a = Help.set_file("foo/a.rb", "a = a = 3", 100)
       Help.set_file("foo/b.rb", "b = b = 3", 100)
 
@@ -68,5 +74,11 @@ def test_no_yaml
       CompileCache::YAML.expects(:precompile).never
       assert_equal 0, CLI.new(["precompile", "-j", "0", "--no-yaml", path]).run
     end
+
+    private
+
+    def skip_unless_iseq
+      skip("Unsupported platform") unless defined?(CompileCache::ISeq) && CompileCache::ISeq.supported?
+    end
   end
 end
diff --git a/test/compile_cache/iseq_cache_test.rb b/test/compile_cache/iseq_cache_test.rb
@@ -3,6 +3,7 @@
 require("test_helper")
 
 class CompileCacheISeqTest < Minitest::Test
+  include(CompileCacheISeqHelper)
   include(TmpdirHelper)
 
   def test_ruby_bug_18250
diff --git a/test/compile_cache/json_test.rb b/test/compile_cache/json_test.rb
@@ -15,6 +15,7 @@ def load_file(_path, symbolize_names: false, freeze: false, fallback: nil)
   end
 
   def setup
+    skip("Unsupported platform") unless Bootsnap::CompileCache.supported?
     super
     Bootsnap::CompileCache::JSON.init!
     FakeJson.singleton_class.prepend(Bootsnap::CompileCache::JSON::Patch)
diff --git a/test/compile_cache/yaml_test.rb b/test/compile_cache/yaml_test.rb
@@ -19,6 +19,7 @@ def unsafe_load_file(_path, symbolize_names: false, freeze: false, fallback: nil
   end
 
   def setup
+    skip("Unsupported platform") unless Bootsnap::CompileCache.supported?
     super
     Bootsnap::CompileCache::YAML.init!
     FakeYaml.singleton_class.prepend(Bootsnap::CompileCache::YAML.patch)
diff --git a/test/compile_cache_handler_errors_test.rb b/test/compile_cache_handler_errors_test.rb
@@ -3,6 +3,7 @@
 require("test_helper")
 
 class CompileCacheHandlerErrorsTest < Minitest::Test
+  include(CompileCacheISeqHelper)
   include(TmpdirHelper)
 
   # now test three failure modes of each handler method:
diff --git a/test/compile_cache_key_format_test.rb b/test/compile_cache_key_format_test.rb
@@ -7,6 +7,7 @@
 
 class CompileCacheKeyFormatTest < Minitest::Test
   FILE = File.expand_path(__FILE__)
+  include(CompileCacheISeqHelper)
   include(TmpdirHelper)
 
   R = {
diff --git a/test/compile_cache_test.rb b/test/compile_cache_test.rb
@@ -3,6 +3,7 @@
 require("test_helper")
 
 class CompileCacheTest < Minitest::Test
+  include(CompileCacheISeqHelper)
   include(TmpdirHelper)
 
   def teardown
diff --git a/test/helper_test.rb b/test/helper_test.rb
@@ -3,6 +3,7 @@
 require("test_helper")
 
 class HelperTest < MiniTest::Test
+  include(CompileCacheISeqHelper)
   include(TmpdirHelper)
 
   def test_validate_cache_path
diff --git a/test/integration/kernel_test.rb b/test/integration/kernel_test.rb
@@ -4,6 +4,7 @@
 
 module Bootsnap
   class KernelTest < Minitest::Test
+    include LoadPathCacheHelper
     include TmpdirHelper
 
     def test_require_symlinked_file_twice
diff --git a/test/load_path_cache/cache_test.rb b/test/load_path_cache/cache_test.rb
@@ -5,7 +5,10 @@
 module Bootsnap
   module LoadPathCache
     class CacheTest < MiniTest::Test
+      include LoadPathCacheHelper
+
       def setup
+        super
         @dir1 = File.realpath(Dir.mktmpdir)
         @dir2 = File.realpath(Dir.mktmpdir)
         FileUtils.touch("#{@dir1}/a.rb")
diff --git a/test/load_path_cache/change_observer_test.rb b/test/load_path_cache/change_observer_test.rb
@@ -5,7 +5,10 @@
 module Bootsnap
   module LoadPathCache
     class ChangeObserverTest < MiniTest::Test
+      include LoadPathCacheHelper
+
       def setup
+        super
         @observer = Object.new
         @arr = []
         ChangeObserver.register(@arr, @observer)
diff --git a/test/load_path_cache/core_ext/kernel_require_test.rb b/test/load_path_cache/core_ext/kernel_require_test.rb
@@ -4,6 +4,8 @@
 
 module Bootsnap
   class KernelRequireTest < Minitest::Test
+    include LoadPathCacheHelper
+
     def test_uses_the_same_duck_type_as_require
       skip("Need a working Process.fork to test in isolation") unless Process.respond_to?(:fork)
       begin
diff --git a/test/load_path_cache/loaded_features_index_test.rb b/test/load_path_cache/loaded_features_index_test.rb
@@ -5,7 +5,10 @@
 module Bootsnap
   module LoadPathCache
     class LoadedFeaturesIndexTest < MiniTest::Test
+      include LoadPathCacheHelper
+
       def setup
+        super
         @index = LoadedFeaturesIndex.new
         # not really necessary but let's just make it a clean slate
         @index.instance_variable_set(:@lfi, {})
diff --git a/test/load_path_cache/path_scanner_test.rb b/test/load_path_cache/path_scanner_test.rb
@@ -5,6 +5,8 @@
 module Bootsnap
   module LoadPathCache
     class PathScannerTest < MiniTest::Test
+      include LoadPathCacheHelper
+
       DLEXT = RbConfig::CONFIG["DLEXT"]
       OTHER_DLEXT = DLEXT == "bundle" ? "so" : "bundle"
 
diff --git a/test/load_path_cache/path_test.rb b/test/load_path_cache/path_test.rb
@@ -6,7 +6,10 @@
 module Bootsnap
   module LoadPathCache
     class PathTest < MiniTest::Test
+      include LoadPathCacheHelper
+
       def setup
+        super
         @cache = Object.new
       end
 
diff --git a/test/load_path_cache/store_test.rb b/test/load_path_cache/store_test.rb
@@ -7,7 +7,10 @@
 module Bootsnap
   module LoadPathCache
     class StoreTest < MiniTest::Test
+      include LoadPathCacheHelper
+
       def setup
+        super
         @dir = Dir.mktmpdir
         @path = "#{@dir}/store"
         @store = Store.new(@path)
diff --git a/test/test_helper.rb b/test/test_helper.rb
@@ -94,24 +94,62 @@ def set_file(path, contents, mtime = nil)
   end
 end
 
+module CompileCacheISeqHelper
+  def setup
+    unless defined?(Bootsnap::CompileCache::ISeq) && Bootsnap::CompileCache::ISeq.supported?
+      skip("Unsupported platform")
+    end
+
+    super
+  end
+end
+
+module LoadPathCacheHelper
+  def setup
+    skip("Unsupported platform") unless Bootsnap::LoadPathCache.supported?
+
+    super
+  end
+end
+
 module TmpdirHelper
   def setup
     super
     @prev_dir = Dir.pwd
     @tmp_dir = Dir.mktmpdir("bootsnap-test")
     Dir.chdir(@tmp_dir)
-    @prev = Bootsnap::CompileCache::ISeq.cache_dir
-    Bootsnap::CompileCache::ISeq.cache_dir = @tmp_dir
-    Bootsnap::CompileCache::YAML.cache_dir = @tmp_dir
-    Bootsnap::CompileCache::JSON.cache_dir = @tmp_dir
+
+    if Bootsnap::CompileCache.supported?
+      set_compile_cache_dir(:ISeq, @tmp_dir)
+      set_compile_cache_dir(:YAML, @tmp_dir)
+      set_compile_cache_dir(:JSON, @tmp_dir)
+    end
   end
 
   def teardown
     super
     Dir.chdir(@prev_dir)
     FileUtils.remove_entry(@tmp_dir)
-    Bootsnap::CompileCache::ISeq.cache_dir = @prev
-    Bootsnap::CompileCache::YAML.cache_dir = @prev
-    Bootsnap::CompileCache::JSON.cache_dir = @prev
+
+    if Bootsnap::CompileCache.supported?
+      restore_compile_cache_dir(:ISeq)
+      restore_compile_cache_dir(:YAML)
+      restore_compile_cache_dir(:JSON)
+    end
+  end
+
+  private
+
+  def restore_compile_cache_dir(mod_name)
+    prev = instance_variable_get("@prev_#{mod_name.downcase}")
+    # Restore directly to instance var to avoid duplication of suffix logic.
+    Bootsnap::CompileCache.const_get(mod_name).instance_variable_set(:@cache_dir, prev) if prev
+  end
+
+  def set_compile_cache_dir(mod_name, dir)
+    mod = Bootsnap::CompileCache.const_get(mod_name)
+    instance_variable_set("@prev_#{mod_name.downcase}", mod.cache_dir)
+    # Use setter method when setting to tmp dir.
+    mod.cache_dir = dir
   end
 end
