Shrine: Ruby 2.7 compatibility

[janko]

The biggest change in Ruby 2.7 is the step towards separation of keyword arguments and positional arguments – this article explains it really nicely, and here is also the official announcement.

I've noticed many deprecation warnings on Travis builds for ruby-head of type:

/home/travis/build/shrinerb/shrine/lib/shrine/plugins/derivation_endpoint.rb:514: warning: The last argument is used as the keyword parameter

These are still only deprecation warnings, but we should still update Shrine to support the keyword/positional args separation that I heard might be coming in Ruby 3.0. As the article I linked explains, this might in some cases require different logic for Ruby 2.7 and Ruby 2.6 or lower.

We should also update any related gems, including Down, ImageProcessing, tus-ruby-server, uppy-s3_multipart and others. But one gem at a time 😃

[janko]

Progress report:

• http has received PRs (https://github.com/httprb/http/pull/581, https://github.com/httprb/form_data/pull/28)
• ruby-vips has been fixed on master (https://github.com/libvips/ruby-vips/pull/216)
• mini_magick has received a PR (https://github.com/minimagick/minimagick/pull/496)
• image_processing has received a PR (https://github.com/janko/image_processing/pull/63)
• down has been fixed on master (https://github.com/janko/down/commit/e3dcc3bc98e26877af28fd552004407a606be50d), however I'm stuck figuring out why Travis CI cannot find the Docker image necessary for running the tests anymore (failure)

I will take point in getting http, mini_magick, and image_processing fixes released. ruby-vips will probably get released with the release of libvips 8.9 (though I still want to get some changes in for the new streaming changes).

At the moment no one is officially taking care of the shrine gem itself, so that's up for grabs. One person contacted me via email two days ago, saying they would like to help, and I offered they can take care of shrine itself, but they haven't confirmed yet. That's something I would really appreciate help with.

One problematic thing is that there are other gems the test suite is using which still trigger warnings (activesupport, activerecord, rack-test_app, mocha, i18n, dry-initializer). Those might take some time to fix, perhaps we should be sending PRs for them as well. Ruby 2.7 has the ability to turn off these warnings (Warning[:deprecated] = false or something like that), but that would disable warnings for Shrine code as well, so I don't think we can use it. If some fixes take time to get released, we could temporarily pull the fixed branches from GitHub, just for the sake of tests.

[jrochkind]

I am getting some actual test failures on 2.7.0, when I run shrine tests on 2.7.0. Can anyone else confirm they see the same thing?

I am also getting some odd behavior in one of my projects around shrine functionality on 2.7.0, that I haven't dug into yet -- it seemed to me the first step was making sure shrine's own tests passed on 2.7.0. Then I found they do not, although I'm not sure any of the test failures are related.

The deprecation notices in 2.7.0 are intended to warn you what won't work in ruby 3.0; they aren't intended to force you to have no deprecation notices to use 2.7.0, they're just warnings.

So perhaps the first step is just getting all tests _passing_ on 2.7.0, without worrying about deprecation warnings?

The deprecation warnings DO make the test output really hard to read though. This deprecation situation in 2.7.0 may not be very conveniently rolled out. But I still think we should get shrine passing tests on 2.7.0 before worrying about deprecation warnings. Which may mean turning off deprecation notices to begin with, just to make test output readable.

I could try to fix the actual failing tests on 2.7.0 and submit PR's for them... but that probalby doesn't make sense until travis is testing on 2.7.0.

Thoughts?

[janko]

Yes, feel free to add 2.7 to the Travis matrix and start fixing the failures. I know that some failures are coming from Down and http.rb, so if you pull them from master it might get the tests passing:

# Gemfile
# ...
if RUBY_VERSION == "2.7.0"
  gem "http",           github: "janko/http",      branch: "ruby-2-7-compatibility"
  gem "http-form_data", github: "janko/form_data", branch: "ruby-2-7-compatibility"
  gem "down",           github: "janko/down"
end

Getting down released is definitely a priority, as it's causing actual failures (due to Nil#to_s returning a frozen string now). As I mentioned, if you have any ideas how to successfully pull the kennethreiz/httpbin Docker image on Travis, that's the only blocker to getting it released. The image pull just started 404ing for some reason 🤷‍♂

I will nudge http.rb developers again to get my changes released 😃

[dentarg]

As I mentioned, if you have any ideas how to successfully pull the kennethreiz/httpbin Docker image on Travis, that's the only blocker to getting it released.

It works now: https://travis-ci.org/dentarg/down/jobs/634711307 (the failure must have been temporary)

[janko]

Great, thanks for the update. Down 5.1.0 has been released which fixes a FrozenError exception and kwargs warnings. I've also updated the remote_url plugin to not trigger kwargs warnings when interacting with the Down gem.

[janko]

New http version has just been released as well, and I've bumped the version in shrine.gemspec, which fixed the failures.

I've also addressed two Mocha expectation failures caused by how Ruby 2.7 treats double-splatted empty hash argument.

That brings the Shrine test suite back to green 💚. Since actual failures were coming from http-form_data and down gems, there is no need to make any early release, as these gems can be updated separately:

gem "down", "~> 5.1" # Ruby 2.7 compatible
gem "http", "~> 4.3" # Ruby 2.7 compatible

So, we can continue gradually fixing the kwargs warnings now.

[janko]

ImageProcessing 1.10.2 has also been released fixing the kwargs warnings for Ruby 2.7.

[janko]

All Ruby 2.7 warnings related to keyword arguments have been resolved 🎉

There are still some being triggered by Active Record & Active Support when running the test suite, but I expect they will be resolved soon within Rails (I saw that one was already fixed on Rails master).

I will be releasing version 2.2.1 tomorrow.