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

Import JRuby implementation #147

Merged
merged 4 commits into from
Oct 7, 2024
Merged

Import JRuby implementation #147

merged 4 commits into from
Oct 7, 2024

Conversation

kou
Copy link
Member

@kou kou commented Sep 28, 2024

Fix GH-104

lib/fiddle/jruby.rb is based on
https://github.com/jruby/jruby/blob/master/lib/ruby/stdlib/fiddle/jruby.rb .

Here are changes for it:

  • Move Fiddle::TYPE_* to Fiddle::Types::*
  • Add Fiddle::Types::VARIADIC
  • Add Fiddle::Types::CONST_STRING
  • Add Fiddle::Types::BOOL
  • Add Fiddle::Types::INT8_T
  • Add Fiddle::Types::UINT8_T
  • Add Fiddle::Types::INT16_T
  • Add Fiddle::Types::UINT16_T
  • Add Fiddle::Types::INT32_T
  • Add Fiddle::Types::UINT32_T
  • Add Fiddle::Types::INT64_T
  • Add Fiddle::Types::UINT64_T
  • Add more Fiddle::ALIGN_* for the above new Fiddle::Types::*
  • Add more Fiddle::SIZEOF_* for the above new Fiddle::Types::*
  • Add support for specifying type as not only Fiddle::Types::* but also Symbol like :int
  • Add support for variable size arguments in Fiddle::Function
  • Add Fiddle::Closure#free
  • Add Fiddle::Closure#freed?
  • Add Fiddle::Error as base the error class
  • Add Fiddle::Pointer#call_free and stop using FFI::AutoPointer in Fiddle::Pointer
  • Add support for Fiddle::Pointer.malloc {} Fiddle::Pointer
  • Add support for Fiddle::Pointer#free=
  • Add Fiddle::Pointer#freed?
  • Use binary string not C string for Fiddle::Pointer#[]
  • Add Fiddle::Handle.sym_defined?
  • Add Fiddle::Handle#sym_defined?
  • Add Fiddle::Handle::DEFAULT
  • Add Fiddle::ClearedReferenceError
  • Add no-op Fiddle::Pinned

Some features are still "not implemented". So there are some "omit"s for JRuby in tests.

Fig GH-104

lib/fiddle/jruby.rb is based on
https://github.com/jruby/jruby/blob/master/lib/ruby/stdlib/fiddle/jruby.rb
.

Here are changes for it:

* Move `Fiddle::TYPE_*` to `Fiddle::Types::*`
* Add `Fiddle::Types::VARIADIC`
* Add `Fiddle::Types::CONST_STRING`
* Add `Fiddle::Types::BOOL`
* Add `Fiddle::Types::INT8_T`
* Add `Fiddle::Types::UINT8_T`
* Add `Fiddle::Types::INT16_T`
* Add `Fiddle::Types::UINT16_T`
* Add `Fiddle::Types::INT32_T`
* Add `Fiddle::Types::UINT32_T`
* Add `Fiddle::Types::INT64_T`
* Add `Fiddle::Types::UINT64_T`
* Add more `Fiddle::ALIGN_*` for the above new `Fiddle::Types::*`
* Add more `Fiddle::SIZEOF_*` for the above new `Fiddle::Types::*`
* Add support for specifying type as not only `Fiddle::Types::*` but also
  `Symbol` like `:int`
* Add support for variable size arguments in `Fiddle::Function`
* Add `Fiddle::Closure#free`
* Add `Fiddle::Closure#freed?`
* Add `Fiddle::Error` as base the error class
* Add `Fiddle::Pointer#call_free` and stop using `FFI::AutoPointer` in
  `Fiddle::Pointer`
* Add support for `Fiddle::Pointer.malloc {}`
  `Fiddle::Pointer`
* Add support for `Fiddle::Pointer#free=`
* Add `Fiddle::Pointer#freed?`
* Use binary string not C string for `Fiddle::Pointer#[]`
* Add `Fiddle::Handle.sym_defined?`
* Add `Fiddle::Handle#sym_defined?`
* Add `Fiddle::Handle::DEFAULT`
* Add `Fiddle::ClearedReferenceError`
* Add no-op `Fiddle::Pinned`

Some features are still "not implemented". So there are some "omit"s
for JRuby in tests.
@kou kou requested a review from headius September 28, 2024 12:37
Copy link
Member Author

@kou kou left a comment

Choose a reason for hiding this comment

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

@headius How about this?

@@ -49,6 +49,11 @@ def test_class_size_with_count
end

def test_set_ctypes
if RUBY_ENGINE == "jruby" and Fiddle::WINDOWS
omit("JRuby's 'l!' pack string is buggy. " +
"See https://github.com/jruby/jruby/issues/8357 for details")
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link

Choose a reason for hiding this comment

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

I will look into it but the workaround is probably going to need to be in place for a while.

Copy link

Choose a reason for hiding this comment

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

@kou yeah for a while likely means JRuby 10 will fix this but 9.4 will continue to use that behavior since it would break any saved pack data for a JRuby app on windows.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK. I've added a workaround for JRuby 9.4 on Windows: b809583

@eregon
Copy link
Member

eregon commented Oct 1, 2024

Taking a quick look at this, since it is implementing Fiddle using FFI that should work on TruffleRuby too, since both JRuby and TruffleRuby ship ffi by default.
One disadvantage is it's slower by going through the FFI gem vs more directly.

@headius
Copy link

headius commented Oct 1, 2024

implementing Fiddle using FFI that should work on TruffleRuby too

It would be helpful to all involved if we could share a single pure-Ruby implementation for this gem that works across all implementations.

One disadvantage is it's slower by going through the FFI gem vs more directly.

Fiddle uses FFI internally. Any overhead introduced by using FFI from Ruby to implement Fiddle can be mitigated along the way, using more direct FFI APIs or by improvements to FFI itself.

@kou
Copy link
Member Author

kou commented Oct 1, 2024

@headius @enebo Can we relicense lib/fiddle/jruby.rb to dual license of Ruby's and BSD-2-Clause like we did for strscan ruby/strscan#25 (comment) ?

@headius
Copy link

headius commented Oct 2, 2024

Can we relicense

I will look into this.

@headius
Copy link

headius commented Oct 2, 2024

Regarding code from https://github.com/jruby/jruby/blob/master/lib/ruby/stdlib/fiddle.rb and https://github.com/jruby/jruby/blob/master/lib/ruby/stdlib/fiddle being imported into this repository, I consent to relicense my contributions under the terms of the Ruby license and the BSD-2-Clause license.

@headius
Copy link

headius commented Oct 2, 2024

@enebo @ahorek @ikaronen-relex You have change lines in the imported versions of these files. Could you reply here giving your consent to relicense the code for Ruby and BSD-2-Clause as I did above?

@headius headius mentioned this pull request Oct 2, 2024
@ikaronen-relex
Copy link

@headius As far as I can tell, all I've contributed to this PR is an empty line, which is not copyrightable. But just in case I've missed something else (and for the avoidance of doubt), I hereby consent to relicense all my past and future contributions to JRuby (and specifically those included in this PR) under the terms of the Ruby license and the BSD-2-Clause license.

@headius
Copy link

headius commented Oct 2, 2024

@ikaronen-relex Ok, I just looked at the history. If you had other lines they may have been overwritten over time. Thanks for the consent in any case!

@ahorek
Copy link

ahorek commented Oct 3, 2024

sure!

I consent to relicense my contributions under the terms of the Ruby license and the BSD-2-Clause license.

@enebo
Copy link

enebo commented Oct 3, 2024

I also consent to relicense my contributions under the terms of the Ruby license and the BSD-2-Clause license.

@headius
Copy link

headius commented Oct 6, 2024

@kou Everyone with lines in these source files has consented to relicense. I think we are good to go there!

@kou
Copy link
Member Author

kou commented Oct 7, 2024

Thanks!
I'll merge this.

@kou kou merged commit 910f48e into master Oct 7, 2024
88 checks passed
@kou kou deleted the jruby branch October 7, 2024 02:03
@headius
Copy link

headius commented Oct 10, 2024

@kou Thank you for the extra work to import JRuby's FFI fiddle! 🙇‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

JRuby support
6 participants