pulsar-edit / pulsar

A Community-led Hyper-Hackable Text Editor
https://pulsar-edit.dev
Other
3.33k stars 140 forks source link

Cirrus: Install older dotenv gem version ~> 2.8 (< 3) #937

Closed DeeDeeG closed 8 months ago

DeeDeeG commented 9 months ago

Short Summary (from Commit Message)

This indirect dependency has a newer version that requires Ruby 3.0 or newer. We're on Debian 10 "Buster" for now, which is still on Ruby 2.5.

Pin to dotenv 2.8.1, the latest version compatible with our older Ruby, per the error message from CI:

The last version of dotenv (>= 0) to support your Ruby & RubyGems was 2.8.1. Try installing it with gem install dotenv -v 2.8.1 and then running the current command again

dotenv requires Ruby version >= 3.0. The current ruby version is 2.5.0.

Identify the Bug

See: https://github.com/pulsar-edit/pulsar/pull/929#issuecomment-1947759623

Description of the Change

Pin to dotenv 2.8.1, the latest version compatible with our older Ruby, per the error message from CI.

Alternate Designs

Update to newer Ruby. Would be quite the hassle. I think I'd rather wait until Cirrus updates their base images.

(Side note: Cirrus's default base image is currently based on Debian 10 "Buster", the oldest officially-supported Debian release, and the same version we started building our GitHub Actions binaries on recently as well.)

Possible Drawbacks

Stuck on an older version of dotenv gem.

For more info about this package, see:

Verification Process

Worked before. Again, see: https://github.com/pulsar-edit/pulsar/pull/929#issuecomment-1947759623.

Release Notes

N/A

DeeDeeG commented 9 months ago

Please ignore the red "x" for Cirrus in the CI status.

That's from the hotfix run of this commit back on release day, where I had manually cancelled the macOS task for this "hotfix" commit, since this problem only affected the ARM Linux Cirrus tasks and I wanted to save the unnecessary credit use from occurring by canceling what would have been a redundant macOS task.

All CI actually running today should pass. Ignore Cirrus, since that failure is from a manual cancellation a few days ago -- Cirrus doesn't even do actual builds for our PR's anyway. Only tag pushes and cron scheduled builds.

DeeDeeG commented 8 months ago

It slowly occurred to me this version was unbounded before (>=0 meaning effectively "any"), and we're unlikely to remember to check this later -- pinning to an exact version now means we won't get any future patch versions later.

I'd like to receive patch versions, maybe even minor versions if those minor versions might receive patch-level fixes for stuff that aren't ported to older minor versions, as many projects stop patching outdated minor versions when new ones are put out.

So... I think I'd prefer to pin to a range (can receive updates) and just make it less than 3.0, for the reason that that would be the version number "less than broken with our setup" (heh).

Either <3 or ~ 2.8 or ^ 2.8.1, which are all effectively the same. But I forget if gem supports caret ^ semver in gem install?

Edit while drafting: I guess it'd have to be <3 or ~> 2.8 or ~> 2 (all equivalent), because I can't get gem to use caret ^ semver ranges. Caret ^ ranges might just be a NodeJS ecosystem thing. And the tilde has a right arrow bracket next to it (~> AKA the "twiddle-wakka" (???)). Out of these, I personally prefer ~> 2.8, since it says approximately what version we expect, but that's mostly just because I'm used to looking at that format in Gemfiles/Gemfile.lock files. (Maybe <3 is clearer to more people?)

Anywho tl;dr if that's okay with other people I would pin to a range rather than an exact version.

DeeDeeG commented 8 months ago

Validation:

Confirmed that gem install dotenv -v '~> 2.8' works locally to install dotenvpackage specified as a range rather than an exact pinned version.

EDIT: Force pushed to fix a type typo in the commit message title ("Ping" --> "Pin").

confused-Techie commented 8 months ago

Glad you've validated everything here, as I've already approved it, I'll go ahead and merge this one