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

Need help resolving nkf for JRuby #13

Closed
flavorjones opened this issue Jan 15, 2024 · 8 comments · Fixed by #15
Closed

Need help resolving nkf for JRuby #13

flavorjones opened this issue Jan 15, 2024 · 8 comments · Fixed by #15

Comments

@flavorjones
Copy link

For libraries such as https://github.com/sparklemotion/mechanize that rely on nkf, the removal of nkf from CRuby's default gems in 3.3.0 results in a warning:

gems/mechanize-2.9.1/lib/mechanize/util.rb:3: warning: nkf was loaded from the standard library, but will no longer be part of the default gems since Ruby 3.4.0. Add nkf to your Gemfile or gemspec. Also contact author of mechanize-2.9.1 to add nkf into its gemspec

As the maintainer of mechanize, I cannot simply add nkf to the gemspec as that will break the JRuby build (because the nfk gem is a C extension that does not support JRuby). JRuby has NKF support built-in and so no gem is actually needed for that engine.

I could work around this if I cut a java release of mechanize; but I don't think I should have to take on that additional work.

So I'm asking the maintainers of this gem and the maintainers of JRuby to please work together to provide a path forward for library maintainers. Thank you!

cc @headius @hsbt

@headius
Copy link
Contributor

headius commented Jan 15, 2024

We would love to eliminate the NKF library entirely but since it seems that's not possible...

There are two options to make the gem compatible with JRuby.

The first case would be to import JRuby's NKF implementation, which is all of one source file, into the gem. Releases would have a c extension version and a Java version and work like any other dependency.

The second case would be to stub out the gem and release a Java version that includes none of the c extension code.

I would prefer to move our NKF code to the gem so we don't have to maintait as part of our repository. The library has had very few changes over the years and I don't expect there to be many more.

The next major JRuby release will be compatible with 3.3, so this is the right time for us to look at moving NKF out to the gem.

@hsbt
Copy link
Member

hsbt commented Jan 18, 2024

@headius Can you submit your preferred case to this repository? I will support JRuby's dicision.

@headius
Copy link
Contributor

headius commented Jan 19, 2024

I will submit a PR with JRuby's nkf implementation.

headius added a commit to headius/jruby that referenced this issue Jan 19, 2024
JRuby's implementation is moving to the gem:

ruby/nkf#13

This won't happen until 3.4 compatibility and this should be
rebased on our 3.4 branch when available.
headius added a commit to jruby/nkf that referenced this issue Jan 19, 2024
This pulls in the nkf extension implementation from JRuby. The
build and load logic has been updated along the same lines as
ruby/digest and the gem appears to build correctly for the -java
platform.

Fixes ruby#13
headius added a commit to jruby/nkf that referenced this issue Jan 19, 2024
This pulls in the nkf extension implementation from JRuby. The
build and load logic has been updated along the same lines as
ruby/digest and the gem appears to build correctly for the -java
platform.

Fixes ruby#13
@headius
Copy link
Contributor

headius commented Jan 19, 2024

See #15 for the transplanted extension.

@hsbt hsbt closed this as completed in #15 Jan 22, 2024
matzbot pushed a commit to ruby/ruby that referenced this issue Jan 22, 2024
This pulls in the nkf extension implementation from JRuby. The
build and load logic has been updated along the same lines as
ruby/digest and the gem appears to build correctly for the -java
platform.

Fixes ruby/nkf#13

ruby/nkf@18f57f36ed
@flavorjones
Copy link
Author

Thank you, @headius! ❤️

@hsbt
Copy link
Member

hsbt commented Jan 22, 2024

@flavorjones @headius I released https://github.com/ruby/nkf/releases/tag/v0.2.0 with C and Java versions. Please let me know if you have another issues.

@flavorjones
Copy link
Author

I've released https://github.com/sparklemotion/mechanize/releases/tag/v2.10.0 which depends on the nkf gem for both CRuby and JRuby.

@headius
Copy link
Contributor

headius commented Jan 23, 2024

We will likely ship JRuby 9.4.6.0 with nkf moved to a default gem, so it can pick up any updates going forward. I already have a separate a PR in place for the removal of the gem in 3.4.

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

Successfully merging a pull request may close this issue.

3 participants