-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add JRuby extension to the gem #15
Conversation
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
I think you need to confirm @nurse's opinion about this. |
Thank you for mentioning me. I commented that I agree to merge an implementation for JRuby somewhere. |
The pack deprecation does not have a suitable replacement, so it will need some changes in JRuby 9.6(?).
Oops, sorry. Github suggested you but I should have checked actual maintainers. |
Tests are running and green. The only tricky part with these -java gems is that we really need them both to be pushed every time. The JRuby team is happy to maintain the Java code and help with releases. I'm not sure if @nobu or @hsbt have suggestions for how to make sure we always push both (other stdlib gems are doing this too). |
JRuby's implementation is moving to the gem: ruby/nkf#15 This won't happen until 3.4 compatibility and this should be rebased on our 3.4 branch when available.
JRuby's implementation is moving to the gem: ruby/nkf#15 This won't happen until 3.4 compatibility and this should be rebased on our 3.4 branch when available.
@@ -0,0 +1,59 @@ | |||
/***** BEGIN LICENSE BLOCK ***** | |||
* Version: EPL 2.0/GPL 2.0/LGPL 2.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@headius Should we handle GPL 2.0
for JRuby version at https://github.com/ruby/nkf/pull/15/files#diff-981027bf28c71984fb019c36c09b0d12b2c84c185d306c6caede7ccf7a76d4a3R21 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can just drop GPL altogether. EPL (business-friendly) + LGPL (GPL-compatible) should cover every case we need. Would that be ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I'm not lawyer) EPL and LGPL are ok to me. Can you handle them under https://github.com/ruby/nkf/pull/15/files#diff-981027bf28c71984fb019c36c09b0d12b2c84c185d306c6caede7ccf7a76d4a3R34?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed GPL (we know it scares people and have considered removing it from JRuby too) and added the additional JRuby licenses to the java gem.
nkf.gemspec
Outdated
|
||
if Gem::Platform === spec.platform and spec.platform =~ 'java' or RUBY_ENGINE == 'jruby' | ||
spec.platform = 'java' | ||
spec.licenses = ["EPL-2.0", "LGPL-2.1"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spec.licenses = ["EPL-2.0", "LGPL-2.1"] | |
spec.licenses += ["EPL-2.0", "LGPL-2.1"] |
Sorry to nit-picking. gemspec or some files are distributed by Ruby or BSD-2-Clause.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, that was supposed to be +=. Will fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in latest push.
@headius I'll cut of new version from master branch today. |
Thank you all for doing this work! ❤️ |
JRuby's implementation has moved to the gem: ruby/nkf#15 The nkf library has been a default gem in CRuby since 3.0; this aligns 9.4 with that configuration. The gem will either move to bundled or out of stdlib altogether in Ruby 3.4 later this year, so I think we should take this step now.
JRuby's implementation has moved to the gem: ruby/nkf#15 The nkf library has been a default gem in CRuby since 3.0; this aligns 9.4 with that configuration. The gem will either move to bundled or out of stdlib altogether in Ruby 3.4 later this year, so I think we should take this step now.
JRuby's implementation has moved to the gem: ruby/nkf#15 The nkf library has been a default gem in CRuby since 3.0; this aligns 9.4 with that configuration. The gem will either move to bundled or out of stdlib altogether in Ruby 3.4 later this year, so I think we should take this step now.
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 #13