Remove ruby-head from Travis matrix & fix jruby failures #5015

Merged
merged 10 commits into from Jun 29, 2016

Conversation

Projects
None yet
3 participants
@parkr
Member

parkr commented Jun 15, 2016

We have had these going for quite a while and we haven't moved forward much with adding stable support for these versions. At this point, having these is just burning time and CPU cycles for us all.

/cc @jekyll/build @jekyll/core

parkr added some commits Jun 15, 2016

@parkr parkr added the internal label Jun 15, 2016

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Jun 15, 2016

Contributor

Ruby-Head can go, but we use JRuby here extensively with Jekyll and we keep an on those tests regardless of the currently failing test (of which our bots know about) and I do plan on doing some work on it when I can find a minute to fix the bugs (actually one of them is already partially fixed by way of Pathutil being added, I've just to get in there and move our globbing to using it instead of Jekyll::Utils version.

Contributor

envygeeks commented Jun 15, 2016

Ruby-Head can go, but we use JRuby here extensively with Jekyll and we keep an on those tests regardless of the currently failing test (of which our bots know about) and I do plan on doing some work on it when I can find a minute to fix the bugs (actually one of them is already partially fixed by way of Pathutil being added, I've just to get in there and move our globbing to using it instead of Jekyll::Utils version.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jun 15, 2016

Member

@envygeeks I'd really like to get the tests passing on jruby before we enable these tests. I would also like them to not take 7 min to run. If they can be pared down to < 3 min of execution time and they pass, I'd be 👍 on testing against them.

@spudowiar I removed ruby-head because it has bugs. Ruby trunk may not be as stable as we'd like.

Member

parkr commented Jun 15, 2016

@envygeeks I'd really like to get the tests passing on jruby before we enable these tests. I would also like them to not take 7 min to run. If they can be pared down to < 3 min of execution time and they pass, I'd be 👍 on testing against them.

@spudowiar I removed ruby-head because it has bugs. Ruby trunk may not be as stable as we'd like.

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Jun 15, 2016

Contributor

@parkr:

I would also like them to not take 7 min to run.

That's impossible because it's RVM and Travis fault it's slow in the first place. It has little to do with the setup and there is little we can do about it. 3/4 of that time has little do with testing and has to do with how terribly RVM does JRuby. I've spent dozens of man hours trying to get JRuby and Travis to play nicely and speedily but their caching ruins attempts to cache through their caching. See any number of my rants about it on Twitter. There are probably thousands of Tweets from me alone over it.

I'd really like to get the tests passing on jruby before we enable these tests.

Technically they were, it was a single pull request that broke it, and that was because JRuby was under review during that time, that's not JRuby's fault, that's the person who sent the pull request, I've offered to fix it, and I will, but that's little reason to remove JRuby.


All of these arguments are not even the fault of JRuby itself, they are either the fault of RVM, and Travis or the person who broke the tests in the first place, forcing me to evetually have to spent dozens of man hours building a lib to fix all of Jekyll's JRuby problems entirely because we rely heavily on JRuby and it's concurrency, I just haven't had time to finish that up, but ultimately whoever broke JRuby should fix it instead of us removing JRuby.

Contributor

envygeeks commented Jun 15, 2016

@parkr:

I would also like them to not take 7 min to run.

That's impossible because it's RVM and Travis fault it's slow in the first place. It has little to do with the setup and there is little we can do about it. 3/4 of that time has little do with testing and has to do with how terribly RVM does JRuby. I've spent dozens of man hours trying to get JRuby and Travis to play nicely and speedily but their caching ruins attempts to cache through their caching. See any number of my rants about it on Twitter. There are probably thousands of Tweets from me alone over it.

I'd really like to get the tests passing on jruby before we enable these tests.

Technically they were, it was a single pull request that broke it, and that was because JRuby was under review during that time, that's not JRuby's fault, that's the person who sent the pull request, I've offered to fix it, and I will, but that's little reason to remove JRuby.


All of these arguments are not even the fault of JRuby itself, they are either the fault of RVM, and Travis or the person who broke the tests in the first place, forcing me to evetually have to spent dozens of man hours building a lib to fix all of Jekyll's JRuby problems entirely because we rely heavily on JRuby and it's concurrency, I just haven't had time to finish that up, but ultimately whoever broke JRuby should fix it instead of us removing JRuby.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jun 15, 2016

Member

I wrote an email to Travis to ask about what we can do to improve the performance on jruby on Travis, and what they can do to improve the performance as well.

Member

parkr commented Jun 15, 2016

I wrote an email to Travis to ask about what we can do to improve the performance on jruby on Travis, and what they can do to improve the performance as well.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jun 21, 2016

Member

Travis got back to me:

It looks like installing jruby takes 2-3 minutes. We're investigating if there is anything we can do here - perhaps point rvm to a cached source (that might be complicated) or use apt installer which might be faster (though the latest version may not be packaged yet).

You have both bundle install and bundle update in your script, both of which take about 30 seconds.

Are you trying to load from cache using install, and then update? If you just do bundle update it could well be faster and would probably cut down 30 seconds. Also, is it important for you to update every time you run tests? If you use your gemfile and run update locally, the build should be faster and our bundle caching would work better for you.

The other thing we noticed that might be taking a lot of time is the use of --profile. It looks like you're using the ruby gem profiler, instead of the built-in jvm profiler (which should be faster). We can investigate this more to see if changing this command will help, but we also wanted to check whether it's essential for you to profile your tests on CI? If not, you can turn the profiler off on Travis, which should help speed up your builds by 3-4 mins.

I'll give some of those a try!

Member

parkr commented Jun 21, 2016

Travis got back to me:

It looks like installing jruby takes 2-3 minutes. We're investigating if there is anything we can do here - perhaps point rvm to a cached source (that might be complicated) or use apt installer which might be faster (though the latest version may not be packaged yet).

You have both bundle install and bundle update in your script, both of which take about 30 seconds.

Are you trying to load from cache using install, and then update? If you just do bundle update it could well be faster and would probably cut down 30 seconds. Also, is it important for you to update every time you run tests? If you use your gemfile and run update locally, the build should be faster and our bundle caching would work better for you.

The other thing we noticed that might be taking a lot of time is the use of --profile. It looks like you're using the ruby gem profiler, instead of the built-in jvm profiler (which should be faster). We can investigate this more to see if changing this command will help, but we also wanted to check whether it's essential for you to profile your tests on CI? If not, you can turn the profiler off on Travis, which should help speed up your builds by 3-4 mins.

I'll give some of those a try!

parkr added some commits Jun 24, 2016

@envygeeks

This comment has been minimized.

Show comment
Hide comment

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jun 28, 2016

Member

Is that something it requires be installed separately?

Member

parkr replied Jun 28, 2016

Is that something it requires be installed separately?

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Jun 28, 2016

Contributor
platforms :jruby do
  gem "jruby-openssl"
end
Contributor

envygeeks replied Jun 28, 2016

platforms :jruby do
  gem "jruby-openssl"
end

parkr added some commits Jun 28, 2016

Merge branch 'master' into remove-jruby-and-ruby-head
* master: (41 commits)
  Fix rubocop offenses on master.
  script/fmt: print Rubocop version
  Update history to reflect merge of #5030 [ci skip]
  rubocop: separate deprecator error messages
  Update history to reflect merge of #5031 [ci skip]
  Update history to reflect merge of #5032 [ci skip]
  rubocop: fix code style
  rubocop: fix code style
  rubocop: fix code style
  Update history to reflect merge of #5024 [ci skip]
  Update history to reflect merge of #5025 [ci skip]
  utils: check that the object is a hash when merging default_proc
  Update history to reflect merge of #5026 [ci skip]
  rubocop: refactor modified? method
  Add a benchmark for capture vs. assign in Liquid. [ci skip]
  Update history to reflect merge of #5027 [ci skip]
  Add generator-jekyllized to third party plugins
  rubocop: fix code style
  rubocop: fix code style
  rubocop: fix code style
  ...
@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jun 29, 2016

Member

Shoot, master contained a commit with [ci skip] in it so nothing happened... ugh. Sending up another one to demonstrate that this is working.

Look OK?

Member

parkr commented Jun 29, 2016

Shoot, master contained a commit with [ci skip] in it so nothing happened... ugh. Sending up another one to demonstrate that this is working.

Look OK?

@parkr parkr changed the title from Remove JRuby and ruby-head from Travis matrix to Remove ruby-head from Travis matrix & fix jruby failures Jun 29, 2016

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jun 29, 2016

Member

If you have further feedback, feel free to leave a comment or open a PR 😄

@jekyllbot: merge +dev

Member

parkr commented Jun 29, 2016

If you have further feedback, feel free to leave a comment or open a PR 😄

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit 325b4c8 into master Jun 29, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jekyllbot jekyllbot deleted the remove-jruby-and-ruby-head branch Jun 29, 2016

jekyllbot added a commit that referenced this pull request Jun 29, 2016

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