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

Add JRuby 9.2 to the test matrix #228

Merged
merged 63 commits into from
Jan 26, 2020
Merged

Add JRuby 9.2 to the test matrix #228

merged 63 commits into from
Jan 26, 2020

Conversation

pkuczynski
Copy link
Member

No description provided.

Copy link
Contributor

@rdubya rdubya left a comment

Choose a reason for hiding this comment

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

I think the changes are ok, but it looks like travis is using too old of a version of rvm so it isn't recognizing some of them

@rdubya
Copy link
Contributor

rdubya commented Jun 20, 2019

Would it make things easier if we pulled the bundler dependency out of the gemspec so we aren't forcing a specific version of bundler?

@pkuczynski
Copy link
Member Author

Yeah, good idea!

@rdubya
Copy link
Contributor

rdubya commented Jun 20, 2019

I realized the JRuby specs will fail because sqlite3 isn't compatible with jruby, we need to conditionally include activerecord-jdbcsqlite3-adapter in that case.

@pkuczynski
Copy link
Member Author

pkuczynski commented Jun 20, 2019

Do you wanna update this PR? Ideally, we should get rid of ActiveRecord dependency. It doesn't really make sense for this gem, don't you think?

All we rely on is active_support/core_ext/module/attribute_accessors - can this be easily replaced by something else?

@pkuczynski
Copy link
Member Author

If I understand correctly https://stackoverflow.com/questions/185573/what-is-mattr-accessor-in-a-rails-module the attribute_accessors are only needed to provide mattr_accessor, so maybe we can just replace it with explicit setters/getters definition? What do you think?

@pkuczynski pkuczynski modified the milestone: 2.0 Jun 20, 2019
@pkuczynski
Copy link
Member Author

@eregon seems that your jruby is built without or with wrong libyaml: https://github.com/rubyconfig/config/pull/228/checks?check_run_id=380015086

Can you help?

@eregon
Copy link

eregon commented Jan 8, 2020

It looks like a bug of JRuby when running in GitHub Actions.
Not sure what's causing it.
I found this issue: rubygems/bundler#6878 (comment)
But it would good to file a new issue at https://github.com/jruby/jruby

@eregon
Copy link

eregon commented Jan 8, 2020

JRuby is not using libyaml AFAIK, they have their own version of SnakeYAML AFAIK.

@pkuczynski
Copy link
Member Author

Hmm, JRuby seems to be very flaky. Worked for 8ae80e9 and failed for 74b650f

This is really strange...

@eregon
Copy link

eregon commented Jan 8, 2020

I filed jruby/jruby#6023

@headius
Copy link

headius commented Jan 8, 2020

Hmm, JRuby seems to be very flaky.

I assume you mean JRuby support on Github actions is flaky. If JRuby works once it should work every time. I strongly suspect there's some change in the Github actions environment that's messing with how we locate the Psych library, like it's picking up a GEM_HOME pointing at CRuby gems.

@headius
Copy link

headius commented Jan 8, 2020

Also confirming @eregon's statement: JRuby does not use libyaml; we use SnakeYAML, which is shipped in-the-box with our psych extension as part of the JRuby distribution.

Literally any system with a working JDK should be able to run JRuby from that tarball with no additional dependencies or build steps. This is an environment issue.

@pkuczynski
Copy link
Member Author

@headius yeah, that's what I mean :)

I also posted an example on jruby/jruby#6023

If you have any suggestions how can we get this build working, I am very keen to hear about it! Maybe it's connected with the gem cache from GHA?

@headius
Copy link

headius commented Jan 8, 2020

Maybe it's connected with the gem cache from GHA?

I think you nailed it. I just noticed that the failed build was able to restore a cache and the successful one was not. I suspect it's assuming the CRuby cached gems are ok for JRuby and loading the CRuby psych somewhere in the pipeline.

@pkuczynski
Copy link
Member Author

That's odd as I include jruby in my cache key, so it should be only relevant to jruby...

@headius
Copy link

headius commented Jan 8, 2020

See also rubygems/bundler#6878 (comment) where @enebo describes a very similar problem. In that case, the wrong version of SnakeYAML was getting picked up, which causes Psych to fail rather far along in its boot process because of an incompatible SnakeYAML API.

Edit: wrong link

@pkuczynski
Copy link
Member Author

@headius it can not be a cache issue. Now the build had no-cache (I intentionally changed the key to test it) and it still failed the same way: https://github.com/rubyconfig/config/runs/380132707

@headius
Copy link

headius commented Jan 8, 2020

@pkuczynski See jruby/jruby#6023 (comment)

This is a horrible thing to have set in a default CLASSPATH. GHA needs to remove that. For now I'd suggest clearing that var.

@pkuczynski
Copy link
Member Author

Waiting for actions/runner-images#242 to be solved

@pkuczynski pkuczynski merged commit 9a670d4 into master Jan 26, 2020
@pkuczynski pkuczynski deleted the jruby branch January 26, 2020 17:08
@pkuczynski pkuczynski modified the milestones: 2.3.0, 2.2.2 Dec 8, 2020
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.

None yet

4 participants