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

Drop support for EOL versions & test against newer ones #58

Merged
merged 10 commits into from
Mar 10, 2022
Merged

Conversation

sambostock
Copy link
Contributor

The most important changes made in this PR are that it drops support for the following:

  • Ruby < 2.6
    Note than 2.6 EOL is scheduled for 31 Mar 2022
    • 2.5 EOL was 31 Mar 2021
    • 2.4 EOL was 31 Mar 2020
  • Active Support < 5.2
    • 5.1 EOL was 25 Aug 2019
    • 5.2 EOL was 09 Apr 2018
    • 4.2 EOL was 27 Apr 2017

Several related changes are made:

  • We now also test against Ruby 3.0 & 3.1
  • We now also test against Active Support 6.1, 7.0, & edge (rails/rail:main)
  • Multiple gemfiles and gemspec are cleaned up
  • All development dependencies are updated

@sambostock sambostock requested a review from rafaelfranca March 8, 2022 00:50
@sambostock
Copy link
Contributor Author

This failure seems to occur on only the Active Support 7.0 CI runs, but be fixed in the edge runs.

...activesupport-7.0.2.2/lib/active_support/xml_mini.rb:184:in `current_thread_backend':
   uninitialized constant ActiveSupport::XmlMini::IsolatedExecutionState (NameError)

        IsolatedExecutionState[:xml_mini_backend]

Using git bisect on rails/rails, I've identified rails/rails#44340 as fixing it. However, that PR mentions

For better or worse, this change will incidentally fix the instances of people requiring some individual files from Active Support without first loading active_support.rb that, while not officially supported, were previously working and recently started failing. (rails/rails#43851 rails/rails#43852 rails/rails#43889 rails/rails#43908 rails/rails#44336) This is not a reversal of that supported-usage policy, and a future change could re-break it at any time.

Based on that, we must have a usage of an Active Support internal file without require "active_support" first. It would probably make sense to track it down and address it before merging this, rather than skipping the 7.0 entries in the test matrix, but that is always an option. Note that (as far as I can tell), this is revealing an existing bug, not introducing one (though I may be wrong).

@sambostock
Copy link
Contributor Author

The issue is that #50 added

require "active_support"
near the bottom of the file, whereas if needs to be the first thing we do, before even
load("tasks/ci_recorder.rake")
because the task code includes
require "active_support/core_ext/hash"
and as described in my previous comment, requiring pieces of Active Support before requiring the main file first is unsupported behaviour.

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link

@rochlefebvre rochlefebvre left a comment

Choose a reason for hiding this comment

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

Nice update and nice cleanup! I learned a few tricks for managing several versioned gemfiles.

CHANGELOG.md Outdated
Comment on lines 5 to 6
* [#58](https://github.com/Shopify/deprecation_toolkit/pull/58): Drop support for Ruby < 2.6 & Active Support < 5.2. (@sambostock)
* [#58](https://github.com/Shopify/deprecation_toolkit/pull/58): `require 'active_support'` earlier to ensure compatibility with v7. (@sambostock)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've only documented the user facing changes. Excluded are:

  • the changes to CI infrastructure
  • the changes to development dependency versions and config

CHANGELOG.md Outdated Show resolved Hide resolved
sambostock and others added 8 commits March 10, 2022 13:58
The rake task code requires a subset of Active Support, but doing this without
having first required the main gem entry point is not supported and breaks in
7.0
We aren't using groups, so we can take a simpler approach by keeping runtime
dependencies in the gemspec, and everything else in Gemfile.

We can also use eval_gemfile to avoid repeating ourselves.

Instance variable approach based on Shopify/maintenance_tasks.
@rafaelfranca rafaelfranca merged commit 87b8cc1 into main Mar 10, 2022
@rafaelfranca rafaelfranca deleted the versions branch March 10, 2022 19:13
@shopify-shipit shopify-shipit bot temporarily deployed to rubygems March 16, 2022 22:06 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants