Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use JRuby implementation for TruffleRuby #149

Merged
merged 6 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@ jobs:
- '3.2'
- debug
- jruby
- truffleruby
include:
- { os: windows-latest , ruby: mingw }
- { os: windows-latest , ruby: mswin }
exclude:
- { os: macos-14 , ruby: '2.5' }
- { os: windows-latest , ruby: '3.0' }
- { os: windows-latest , ruby: debug }
- { os: windows-latest , ruby: truffleruby }

steps:
- uses: actions/checkout@v4
Expand Down
2 changes: 1 addition & 1 deletion ext/fiddle/extconf.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true
require 'mkmf'

if RUBY_ENGINE == "jruby"
unless RUBY_ENGINE == "ruby"
File.write('Makefile', dummy_makefile("").join)
return
end
Expand Down
1 change: 1 addition & 0 deletions fiddle.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ Gem::Specification.new do |spec|
"lib/fiddle/pack.rb",
"lib/fiddle/ruby.rb",
"lib/fiddle/struct.rb",
"lib/fiddle/truffleruby.rb",
"lib/fiddle/types.rb",
"lib/fiddle/value.rb",
"lib/fiddle/version.rb",
Expand Down
11 changes: 11 additions & 0 deletions lib/fiddle/jruby.rb
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,17 @@ def call(*args, &block);
end
Copy link
Member

@eregon eregon Oct 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to rename this file to ffi_backend.rb or so and avoid a JRuby namespace since it is effectively independent from JRuby but it doesn't like that currently due to file naming and things like Fiddle::JRuby.
It's helpful to have a small diff at this point though, so I think that's best done later, maybe even in a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we don't need to do it because this is a short-term workaround for TruffleRuby.
This file will be used by only JRuby eventually.

Copy link
Member

@eregon eregon Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the FFI backend seems to work well I think we should just use it long term, for both JRuby and TruffleRuby.
It's also nice to have 2 backends instead of 3.

args[i] = Fiddle::JRuby.__ffi_type__(args[i])
end
else
args.size.times do |i|
arg = args[i]
next unless arg.respond_to?(:to_ptr)
loop do
arg = arg.to_ptr
break if arg.is_a?(FFI::Pointer)
break unless arg.respond_to?(:to_ptr)
end
args[i] = arg
end
end
result = @function.call(*args, &block)
result = Pointer.new(result) if result.is_a?(FFI::Pointer)
Expand Down
1 change: 1 addition & 0 deletions lib/fiddle/truffleruby.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
require_relative 'jruby'
6 changes: 6 additions & 0 deletions test/fiddle/test_closure.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@

module Fiddle
class TestClosure < Fiddle::TestCase
def setup
if RUBY_ENGINE == "truffleruby"
omit("FFI::Function don't accept #call-able object with TruffleRuby")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look into this, TruffleRuby does pass most FFI gem tests so it's surprising, maybe this is not well tested or is not an officially supported API.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 41c21f5 so all such tests pass now

end
end

def teardown
super
# We can't use ObjectSpace with JRuby.
Expand Down
11 changes: 11 additions & 0 deletions test/fiddle/test_fiddle.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ def test_nil_true_etc
if RUBY_ENGINE == "jruby"
omit("Fiddle::Q* aren't supported with JRuby")
end
if RUBY_ENGINE == "truffleruby"
omit("Fiddle::Q* aren't supported with TruffleRuby")
end

assert_equal Fiddle::Qtrue, Fiddle.dlwrap(true)
assert_equal Fiddle::Qfalse, Fiddle.dlwrap(false)
Expand All @@ -30,6 +33,10 @@ def test_dlopen_linker_script_input_linux
if Dir.glob("/usr/lib/*/libncurses.so").empty?
omit("libncurses.so is needed")
end
if RUBY_ENGINE == "truffleruby"
omit("Fiddle::Handle#file_name doesn't exist in TruffleRuby")
end

# libncurses.so uses INPUT() on Debian GNU/Linux
# $ cat /usr/lib/x86_64-linux-gnu/libncurses.so
# INPUT(libncurses.so.6 -ltinfo)
Expand All @@ -44,6 +51,10 @@ def test_dlopen_linker_script_input_linux

def test_dlopen_linker_script_group_linux
omit("This is only for Linux") unless RUBY_PLATFORM.match?("linux")
if RUBY_ENGINE == "truffleruby"
omit("Fiddle::Handle#file_name doesn't exist in TruffleRuby")
end

# libc.so uses GROUP() on Debian GNU/Linux
# $ cat /usr/lib/x86_64-linux-gnu/libc.so
# /* GNU ld script
Expand Down
4 changes: 4 additions & 0 deletions test/fiddle/test_func.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ def test_strtod
end

def test_qsort1
if RUBY_ENGINE == "truffleruby"
omit("TruffleRuby's FFI::Function don't accept #call-able object")
end

closure_class = Class.new(Closure) do
def call(x, y)
Pointer.new(x)[0] <=> Pointer.new(y)[0]
Expand Down
13 changes: 13 additions & 0 deletions test/fiddle/test_function.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ def test_need_gvl?
if RUBY_ENGINE == "jruby"
omit("rb_str_dup() doesn't exist in JRuby")
end
if RUBY_ENGINE == "truffleruby"
omit("rb_str_dup() doesn't work with TruffleRuby")
end

libruby = Fiddle.dlopen(nil)
rb_str_dup = Function.new(libruby['rb_str_dup'],
Expand Down Expand Up @@ -91,6 +94,10 @@ def test_call
end

def test_argument_count
if RUBY_ENGINE == "truffleruby"
omit("TruffleRuby's FFI::Function don't accept #call-able object")
end

closure_class = Class.new(Closure) do
def call one
10 + one
Expand All @@ -112,6 +119,9 @@ def test_last_error
if RUBY_ENGINE == "jruby"
omit("Fiddle.last_error doesn't work with JRuby")
end
if RUBY_ENGINE == "truffleruby"
omit("Fiddle.last_error doesn't work with TruffleRuby")
end

func = Function.new(@libc['strcpy'], [TYPE_VOIDP, TYPE_VOIDP], TYPE_VOIDP)

Expand Down Expand Up @@ -219,6 +229,9 @@ def test_no_memory_leak
if RUBY_ENGINE == "jruby"
omit("rb_obj_frozen_p() doesn't exist in JRuby")
end
if RUBY_ENGINE == "truffleruby"
omit("memory leak detection is fragile with TruffleRuby")
end

if respond_to?(:assert_nothing_leaked_memory)
rb_obj_frozen_p_symbol = Fiddle.dlopen(nil)["rb_obj_frozen_p"]
Expand Down
15 changes: 15 additions & 0 deletions test/fiddle/test_handle.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ def test_to_i
if RUBY_ENGINE == "jruby"
omit("Fiddle::Handle#to_i is unavailable with JRuby")
end
if RUBY_ENGINE == "truffleruby"
omit("Fiddle::Handle#to_i is unavailable with TruffleRuby")
end

handle = Fiddle::Handle.new(LIBC_SO)
assert_kind_of Integer, handle.to_i
Expand All @@ -21,6 +24,9 @@ def test_to_ptr
if RUBY_ENGINE == "jruby"
omit("Fiddle::Handle#to_i is unavailable with JRuby")
end
if RUBY_ENGINE == "truffleruby"
omit("Fiddle::Handle#to_i is unavailable with TruffleRuby")
end

handle = Fiddle::Handle.new(LIBC_SO)
ptr = handle.to_ptr
Expand All @@ -37,6 +43,9 @@ def test_static_sym
if RUBY_ENGINE == "jruby"
omit("We can't assume static symbols with JRuby")
end
if RUBY_ENGINE == "truffleruby"
omit("We can't assume static symbols with TruffleRuby")
end

begin
# Linux / Darwin / FreeBSD
Expand Down Expand Up @@ -136,6 +145,9 @@ def test_file_name
if RUBY_ENGINE == "jruby"
omit("Fiddle::Handle#file_name doesn't exist in JRuby")
end
if RUBY_ENGINE == "truffleruby"
omit("Fiddle::Handle#file_name doesn't exist in TruffleRuby")
end

file_name = Handle.new(LIBC_SO).file_name
if file_name
Expand All @@ -158,6 +170,9 @@ def test_NEXT
if RUBY_ENGINE == "jruby"
omit("Fiddle::Handle::NEXT doesn't exist in JRuby")
end
if RUBY_ENGINE == "truffleruby"
omit("Fiddle::Handle::NEXT doesn't exist in TruffleRuby")
end

begin
# Linux / Darwin
Expand Down
3 changes: 3 additions & 0 deletions test/fiddle/test_import.rb
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@ def test_io()
if RUBY_ENGINE == "jruby"
omit("BUILD_RUBY_PLATFORM doesn't exist in JRuby")
end
if RUBY_ENGINE == "truffleruby"
omit("BUILD_RUBY_PLATFORM doesn't exist in TruffleRuby")
end

if( RUBY_PLATFORM != BUILD_RUBY_PLATFORM ) || !defined?(LIBC.fprintf)
return
Expand Down
15 changes: 15 additions & 0 deletions test/fiddle/test_pointer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ def test_can_read_write_memory
if RUBY_ENGINE == "jruby"
omit("Fiddle::Pointer.{read,write} don't exist in JRuby")
end
if RUBY_ENGINE == "truffleruby"
omit("Fiddle::Pointer.{read,write} don't exist in TruffleRuby")
end

# Allocate some memory
Fiddle::Pointer.malloc(Fiddle::SIZEOF_VOIDP, Fiddle::RUBY_FREE) do |pointer|
Expand Down Expand Up @@ -116,6 +119,9 @@ def test_inspect
if RUBY_ENGINE == "jruby"
omit("Fiddle::Pointer#inspect is incompatible on JRuby")
end
if RUBY_ENGINE == "truffleruby"
omit("Fiddle::Pointer#inspect is incompatible on TruffleRuby")
end

ptr = Pointer.new(0)
inspect = ptr.inspect
Expand All @@ -135,6 +141,9 @@ def test_to_ptr_io
if RUBY_ENGINE == "jruby"
omit("Fiddle::Pointer.to_ptr(IO) isn't supported with JRuby")
end
if RUBY_ENGINE == "truffleruby"
omit("Fiddle::Pointer.to_ptr(IO) isn't supported with TruffleRuby")
end

Pointer.malloc(10, Fiddle::RUBY_FREE) do |buf|
File.open(__FILE__, 'r') do |f|
Expand Down Expand Up @@ -186,6 +195,9 @@ def test_ref_ptr
if RUBY_ENGINE == "jruby"
omit("Fiddle.dlwrap([]) isn't supported with JRuby")
end
if RUBY_ENGINE == "truffleruby"
omit("Fiddle.dlwrap([]) isn't supported with TruffleRuby")
end

ary = [0,1,2,4,5]
addr = Pointer.new(dlwrap(ary))
Expand All @@ -198,6 +210,9 @@ def test_to_value
if RUBY_ENGINE == "jruby"
omit("Fiddle.dlwrap([]) isn't supported with JRuby")
end
if RUBY_ENGINE == "truffleruby"
omit("Fiddle.dlwrap([]) isn't supported with TruffleRuby")
end

ary = [0,1,2,4,5]
addr = Pointer.new(dlwrap(ary))
Expand Down
Loading