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

Update travis CI adding latest Ruby and JRuby versions; fix failure on JRuby 9.2 #733

Merged
merged 3 commits into from
Jul 6, 2018

Conversation

ivoanjo
Copy link
Contributor

@ivoanjo ivoanjo commented Jun 23, 2018

Hello there, here's a small addition!

I also took a stab at adding truffleruby which is now available in rvm master but was not able to force an rvm upgrade before Travis attempts to install the target Ruby, so I guess we'll need to wait until the next rvm release.

Note: Tests seem to be broken with JRuby 9.2.0.0, I'll submit a separate issue for that.

Copy link
Member

@pitr-ch pitr-ch left a comment

Choose a reason for hiding this comment

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

Thanks! I had couple of small requests.

.travis.yml Outdated
@@ -13,10 +13,13 @@ rvm:
- 2.0.0
- 1.9.3

- jruby-9.2.0.0
Copy link
Member

Choose a reason for hiding this comment

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

jruby-9.2.0.0 should be on line 6, could you move it there please.

- jruby-9.0.5.0
- jruby-1.7.27

- ruby-head
- ruby-2.6.0-preview2
Copy link
Member

Choose a reason for hiding this comment

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

please add it to allowed failure

@pitr-ch
Copy link
Member

pitr-ch commented Jun 29, 2018

Thanks for trying truffleruby. I have an #720 for it. You might want to leave it up to me, since I can more easily solve any issues there. I hope that it will be straight forward after I'll make sure it works for c-r.

@pitr-ch pitr-ch added this to the 1.0.6 milestone Jun 29, 2018
@pitr-ch pitr-ch added the chore Gem maintenance tasks. label Jun 29, 2018
@ivoanjo ivoanjo force-pushed the update-travis-config branch from c2245dc to f36a8d4 Compare June 30, 2018 16:56
@ivoanjo
Copy link
Contributor Author

ivoanjo commented Jun 30, 2018

@pitr-ch fixed, thanks for the feedback!

@pitr-ch pitr-ch mentioned this pull request Jul 6, 2018
1 task
@pitr-ch
Copy link
Member

pitr-ch commented Jul 6, 2018

@ivoanjo would you be interested to look into why is Travis failing for JRuby 9.2?

@pitr-ch pitr-ch added the bug A bug in the library or documentation. label Jul 6, 2018
@pitr-ch pitr-ch changed the title Update travis CI config to add latest Ruby and JRuby versions Update travis CI adding latest Ruby and JRuby versions; fix failure on JRuby 9.2 Jul 6, 2018
@ivoanjo
Copy link
Contributor Author

ivoanjo commented Jul 6, 2018

@pitr-ch I already did! ;)

It's caused by jruby/jruby#5229, which has already been fixed upstream in jruby/jruby#5231.

I was not sure if it was worth adding it to allow_failures or to submit a CR to change concurrent-ruby to avoid the mutation; let me know if you'd like any of those and I'll gladly submit a PR.

@pitr-ch
Copy link
Member

pitr-ch commented Jul 6, 2018

@ianks cool and thanks! Since people will use JRuby 9.2 anyway could you fix it in concurrent-ruby so it does not fail for the users even though it's JRuby's bug.

@pitr-ch pitr-ch removed this from the 1.0.6 milestone Jul 6, 2018
ivoanjo added 3 commits July 6, 2018 19:59
Due to a bug in JRuby 9.2.0.0, mutating the result of `Module#to_s`
actually changes the result of other `Module#to_s` calls to the same
module or class, rather than returning a new `String`.

This broke `AbstractStruct#pr_underscore` which is used when rendering
`#inspect` for structures.

This was already fixed upstream, but as a workaround, let's dup the
String before we mutate it, so as to not trip anyone using this JRuby
version (and to restore Travis to beautiful green).

Issue jruby/jruby#5229
@ivoanjo ivoanjo force-pushed the update-travis-config branch from f36a8d4 to cb4cded Compare July 6, 2018 19:05
@ivoanjo
Copy link
Contributor Author

ivoanjo commented Jul 6, 2018

I've added created the workaround and added it to this PR (rather than creating a separate one), so that we can merge with a clean travis.

Copy link
Member

@pitr-ch pitr-ch left a comment

Choose a reason for hiding this comment

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

Perfect, Thank you!

@pitr-ch pitr-ch merged commit c01a275 into ruby-concurrency:master Jul 6, 2018
@ghost ghost removed the in progress label Jul 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug in the library or documentation. chore Gem maintenance tasks.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants