-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(ruby): reject non-MRI Ruby engines on Windows with clear error #8539
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,7 @@ use crate::toolset::{ToolVersion, Toolset}; | |
| use crate::ui::progress_report::SingleReport; | ||
| use crate::{file, github, plugins}; | ||
| use async_trait::async_trait; | ||
| use eyre::Result; | ||
| use eyre::{Result, bail}; | ||
| use itertools::Itertools; | ||
| use versions::Versioning; | ||
| use xx::regex; | ||
|
|
@@ -205,6 +205,13 @@ impl Backend for RubyPlugin { | |
| ctx: &InstallContext, | ||
| mut tv: ToolVersion, | ||
| ) -> eyre::Result<ToolVersion> { | ||
| if !super::ruby_common::is_mri_version(&tv.version) { | ||
| bail!( | ||
| "Ruby engine '{}' is not supported on Windows.\n\ | ||
| Only standard MRI Ruby versions can be installed via RubyInstaller2.", | ||
| tv.version | ||
| ); | ||
| } | ||
|
Comment on lines
+208
to
+214
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No unit test for the new rejection behavior There is no test that verifies a non-MRI version string (e.g. |
||
| let tarball = self.download(&tv, ctx.pr.as_ref()).await?; | ||
| self.verify_checksum(ctx, &mut tv, &tarball)?; | ||
| self.install(ctx, &tv, &tarball).await?; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation on the second line of this multi-line string literal will be included in the error message, which is likely not intended. This will cause the second line of the error message to be indented with several spaces. It's better to put the string on a single line or remove the indentation on the continued line to fix the output format.