Constrain Ruby to only supported versions#40
Conversation
| @@ -1,5 +1,5 @@ | |||
| source 'https://rubygems.org' | |||
| ruby '>= 3.0' | |||
| ruby '>= 3.0', '< 3.4' | |||
There was a problem hiding this comment.
Using standard syntax in the Gemfile to prevent execution with a Ruby version higher than 3.4. This already does most of what we want - however, the setup script contains its own version checks and outputs information to the user which could just be more confusing if not updated as well.
Your Ruby version is 3.4.4, but your Gemfile specified >= 3.0, < 3.4
| gemfile = File.read("./Gemfile") | ||
| REQUIRED_RUBY_VERSION = gemfile[/ruby ['"]>=(.+)['"]/, 1] | ||
| ACTUAL_RUBY_VERSION = RUBY_VERSION | ||
| PREFERRED_RUBY_VERSION = File.read("../.ruby-version").strip |
There was a problem hiding this comment.
Since we already have a .ruby-version file used by Intercom to internally maintain this gem, we can use the version defined there as the recommended Ruby version known to work - and not the minimum set in the Gemfile.
| @@ -1 +1,2 @@ | |||
| nodejs 24.10.0 | |||
| ruby 3.2.2 | |||
There was a problem hiding this comment.
We already seem to have a .tools-version file used by the asdf version manager, so we might as well support Ruby in there as well.
Line 1 in a7915b1
| FALLBACK_RUBY_VERSION_MANAGER = "ry" | ||
|
|
||
| def parse_ruby_version_constraints | ||
| File.read("./Gemfile")[/ruby .*/][5..].split(',').map do |constraint| |
There was a problem hiding this comment.
This logic builds on the regex approach already present in this repo and makes some tightly coupled assumptions about the code in this repository (Gemfiles are basically code and parsing them with regex is brittle) - which is not ideal but "fine" and doesn't make the code any worse than before.
| def parse_ruby_version_constraints | ||
| File.read("./Gemfile")[/ruby .*/][5..].split(',').map do |constraint| | ||
| constraint = constraint.gsub(/['"]/, '') | ||
| Gem::Dependency.new('ruby', constraint) |
There was a problem hiding this comment.
Version constraints are much more complex than how we're treating them so far. At the moment we just assume the number set is the minimum version. We could've just continued down that path and also parsed out a max version to achieve what we need.
However, there is already a class that exists to solve this and it's already installed as part of RubyGems - here we start using the Gem::Depencency class to parse all constraints set in the Gemfile to compare the current version of Ruby against any constraints set out. This is simpler and more reliable than what we did before.
e6eda41 to
cd0d3d1
Compare
| @@ -1,6 +1,9 @@ | |||
| #!/bin/bash | |||
|
|
|||
| set -e | |||
There was a problem hiding this comment.
If we don't set the -e option, bash will continue to run the below commands even if one step fails, which makes things confusing to the candidate, i.e. if the version of Ruby doesn't match. We don't want to continue installing things if such a fundamental requirement is not met.
|
I'm not the first to propose a change to this effect, further confirming that this causes friction for candidates: I'll leave it to the maintainers to decide which solution is preferable. |
cd0d3d1 to
da5be66
Compare
This version of Minicom does not work with Ruby version 3.4 or higher - the setup script fails because the
base64gem isrequired but not installed. This is due to Ruby 3.4 splitting said gem out and mandating it to be installed explicitly.The setup script for Rails currently does not constrain Ruby to a maximum version, only the minimum:
minicom-public/rails/Gemfile
Line 2 in a7915b1
The impact is that a candidate using Ruby 3.4 or higher will encounter installation errors if they are following the setup instructions:
Besides updating this project to support newer versions of Ruby, the tool should be more conservative in constraining which versions are supported. This PR adds a Ruby version constraint to ensure the candidate receives a meaningful error message if they are using 3.4 or higher.
Additionally, we add support for
asdfas a Ruby version manager, and ensure the setup script stops if the version check fails.Tagging a few potential reviewers, since I can't assign any reviewers 🙈 Sorry if this is not appropritate!
@liutingdu @patocallaghan @pratik60 @eugeneius