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

fiddle fixes #5960

Merged
merged 1 commit into from
Jan 13, 2020
Merged

fiddle fixes #5960

merged 1 commit into from
Jan 13, 2020

Conversation

ahorek
Copy link
Contributor

@ahorek ahorek commented Nov 5, 2019

fixes #5956 and #5955

but this test case doesn't work yet

def test_qsort1

I'm experimanting for now :)

@headius
Copy link
Member

headius commented Nov 7, 2019

Nice! We definitely want to get Fiddle working better. Long term it might make more sense to do a direct implementation atop jnr-ffi, but for now getting the FFI version working is probably best.

@ahorek
Copy link
Contributor Author

ahorek commented Nov 7, 2019

I splited two obvious errors into separate PRs
jruby/ruby#2 jruby/ruby#3

@ahorek
Copy link
Contributor Author

ahorek commented Nov 7, 2019

one more interesting failure

assert_equal("123", str.to_s)

a string pointer #to_s should return a ruby string. That would be tricky to implement :)

@headius
Copy link
Member

headius commented Jan 7, 2020

Do you think there's any work remaining here or could we go ahead with a merge?

@headius
Copy link
Member

headius commented Jan 7, 2020

I have merged jruby/ruby#2 and jruby/ruby#3 across the two active branches jruby-ruby_2_5_7 and jruby-ruby_2_6_5 and incorporated their changes into the JRuby branches master and ruby-2.6.

I also figured out why the libc changes keeps disappearing: we were versioning lib/ruby/stdlib/fiddle/jruby.rb in both places when it's specific to JRuby, without any equivalent in CRuby, and subject to frequent updates. I modified all branches to reflect this, with messages about the proper versioning location added to jruby here and here.

(I see the spelling error now and will fix that).

I've also made sure that all changes from jruby/ruby's jruby-ruby_2_5_7 and jruby/jruby's master branch have been incorporated back into the jruby/ruby jruby-ruby_2_6_5 branch upon which the jruby/jruby ruby-2.6 branch is based. See jruby/ruby@afa0b25.

We need a better way to manage our patch set against CRuby's stdlib.

@ahorek
Copy link
Contributor Author

ahorek commented Jan 12, 2020

Do you think there's any work remaining here or could we go ahead with a merge?

there're still many broken parts, but I think it's ready for review

@ahorek ahorek changed the title WIP: fiddle fixes fiddle fixes Jan 12, 2020
@headius
Copy link
Member

headius commented Jan 13, 2020

Everything here seems appropriate to me. I will merge and we can do other fixes in other PRs.

@headius headius merged commit 797bc70 into jruby:master Jan 13, 2020
@enebo enebo added this to the JRuby 9.2.10.0 milestone Feb 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dlload throws LoadError instead of Fiddle::DLError
3 participants