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

Source FFI Ruby sources from the gem #6150

Merged
merged 1 commit into from Dec 21, 2020
Merged

Conversation

headius
Copy link
Member

@headius headius commented Mar 29, 2020

This PR will remove FFI Ruby sources from our repository and source them from the gem, so they can be upgraded as any other default gem.

It doesn't pass now because the "java" version of the gem contains no sources.

@headius headius added this to the JRuby 9.3.0.0 milestone Mar 29, 2020
@headius headius force-pushed the ffi_from_gem branch 2 times, most recently from f5fdaaf to 9798741 Compare September 22, 2020 03:56
@headius
Copy link
Member Author

headius commented Sep 22, 2020

I've rebased off current master and this should be ready to go now.

We still have to update specs manually.

@headius
Copy link
Member Author

headius commented Sep 22, 2020

So close! The main ffi.rb file unfortunately uses Gem::Version to parse the JRuby version when checking for >= "9.3.pre", but if we are running with RubyGems disabled it blows up. That prevents JRuby from booting at all, since some internal functionality depends on FFI to be loadable.

I have pushed ffi/ffi#823 to fix this and avoid touching the Gem namespace while loading ffi.rb.

@larskanis
Copy link

ffi/ffi#823 is merged and released as ffi-1.14.0. I also added JRuby tests on github actions, but they fail since this PR is not yet on jruby-head.

@headius
Copy link
Member Author

headius commented Dec 21, 2020

@larskanis Thank you for the update! I will fix this to the release and merge.

This does not include the platform-specific constants files that
appear to only live in JRuby's copy, probably from the original
copy from Rubinius. They are needed for at least syslog and likely
other FFI-based features in the standard library.
@headius
Copy link
Member Author

headius commented Dec 21, 2020

@larskanis I am in the process of working out the last kinks, but noticed that there are platform files only in JRuby for things like syslog constants. I believe these files were generated by Rubinius many years ago and came along with us adding FFI to JRuby. Some of them even have Rubinius property names. I am fine keeping these files versioned in JRuby for now but I will be looking into a way to remove them from the ffi subdirs.

Example: https://github.com/jruby/jruby/tree/862ebf4ea162d44870fc06272020529285842a56/lib/ruby/stdlib/ffi/platform/x86_64-linux

@headius
Copy link
Member Author

headius commented Dec 21, 2020

A quick search seems to indicate that only the syslog files are being used by any libraries in JRuby. The platform.conf files also appear to be unnecessary, likely just another place Rubinius stored platform-specific constants for a few other libraries they implemented with FFI.

@headius headius marked this pull request as ready for review December 21, 2020 19:06
@headius headius merged commit f3614e3 into jruby:master Dec 21, 2020
@headius headius deleted the ffi_from_gem branch December 21, 2020 19:08
@headius
Copy link
Member Author

headius commented Dec 21, 2020

@larskanis I have created #6503 to address the files not found in the FFI gem.

@headius headius mentioned this pull request Sep 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants