Skip to content

fix(ruby): lock resolved install options #9992

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.

Sign up for GitHub

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

Draft
risu729 wants to merge 6 commits into
base: main
Choose a base branch
from
+221 −9
Draft

fix(ruby): lock resolved install options #9992

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
75d29b4
fix(ruby): include build source options in lock identity
risu729 May 19, 2026
7fb77b1
test(ruby): cover lock option build paths
risu729 May 19, 2026
7931391
Merge branch 'main' into fix/ruby-build-lock-options
risu729 May 31, 2026
2fd50d8
refactor(ruby): inline lockfile option resolution
risu729 May 31, 2026
d5cd217
fix(ruby): avoid default repo lock options
risu729 May 31, 2026
0d98b51
fix(ruby): lock resolved install options
risu729 Jun 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
230 changes: 221 additions & 9 deletions src/plugins/core/ruby.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1023,17 +1023,46 @@ impl Backend for RubyPlugin {
) -> BTreeMap<String, String> {
let mut opts = BTreeMap::new();
let settings = Settings::get();
let ruby = &settings.ruby;
let is_current_platform = target.is_current();
let try_precompiled =
ruby.compile == Some(false) || (settings.experimental && ruby.compile.is_none());

if is_current_platform {
opts.insert("compile".to_string(), (!try_precompiled).to_string());

// Ruby uses ruby-install vs ruby-build. The installer and its options
// can affect the source-built output, including fallback after a
// missing precompiled binary.
opts.insert("ruby_install".to_string(), ruby.ruby_install.to_string());
if ruby.ruby_install {
if let Some(ruby_install_opts) = ruby.ruby_install_opts.clone() {
opts.insert("ruby_install_opts".to_string(), ruby_install_opts);
}
opts.insert(
"ruby_install_repo".to_string(),
ruby.ruby_install_repo.clone(),
);
} else {
if let Some(ruby_build_opts) = ruby.ruby_build_opts.clone() {
opts.insert("ruby_build_opts".to_string(), ruby_build_opts);
}
opts.insert("ruby_build_repo".to_string(), ruby.ruby_build_repo.clone());
}

// Ruby uses ruby-install vs ruby-build (ruby compiles from source either way)
// Only include if using non-default ruby-install tool
let ruby_install = if is_current_platform {
settings.ruby.ruby_install
} else {
false
};
if ruby_install {
opts.insert("ruby_install".to_string(), "true".to_string());
if let Some(apply_patches) = ruby.apply_patches.clone() {
opts.insert("apply_patches".to_string(), apply_patches);
}
}
Comment on lines +1031 to +1056
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 2, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Include resolved source-build defaults for non-current targets too.

For non-current targets this now drops compile, installer choice, repo, build opts, and patches entirely. Those values still affect the fallback/source artifact, and lockfile lookup later requires an exact options-map match, so a cross-platform lock entry can be generated with too little identity and then miss once that platform resolves locally.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/plugins/core/ruby.rs` around lines 1031 - 1056, When building the options
map for non-current targets (i.e. where is_current_platform is false) ensure you
still insert the resolved source-build defaults so lockfile identity is
complete: always set "compile" to (!try_precompiled).to_string(), set
"ruby_install" to ruby.ruby_install.to_string(), and populate the
installer-specific keys by reading ruby.ruby_install_opts/ruby.ruby_install_repo
when ruby.ruby_install is true or ruby.ruby_build_opts/ruby.ruby_build_repo when
false; also insert "apply_patches" from ruby.apply_patches when present. Update
the code around opts insertion (the block that currently runs only under if
is_current_platform) to perform these same inserts for non-current targets so
the options-map includes compile, installer choice, repo, build opts, and
patches for both current and non-current platforms.


if try_precompiled {
opts.insert("precompiled_url".to_string(), ruby.precompiled_url.clone());
if let Some(precompiled_arch) = ruby.precompiled_arch.clone() {
opts.insert("precompiled_arch".to_string(), precompiled_arch);
}
if let Some(precompiled_os) = ruby.precompiled_os.clone() {
opts.insert("precompiled_os".to_string(), precompiled_os);
}
}

opts
Expand Down Expand Up @@ -1119,9 +1148,41 @@ fn parse_gemfile(body: &str) -> String {
#[cfg(test)]
mod tests {
use super::*;
use crate::config::settings::SettingsPartial;
use crate::toolset::ToolSource;
use confique::Layer;
use indoc::indoc;
use pretty_assertions::assert_eq;

static TEST_SETTINGS_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
const DEFAULT_RUBY_BUILD_REPO: &str = "https://github.com/rbenv/ruby-build.git";
const DEFAULT_RUBY_INSTALL_REPO: &str = "https://github.com/postmodern/ruby-install.git";
const DEFAULT_RUBY_PRECOMPILED_URL: &str = "jdx/ruby";

struct SettingsResetGuard {
_lock: std::sync::MutexGuard<'static, ()>,
}

impl Drop for SettingsResetGuard {
fn drop(&mut self) {
Settings::reset(None);
}
}

fn resolve_ruby_lockfile_options(
configure_settings: impl FnOnce(&mut SettingsPartial),
) -> BTreeMap<String, String> {
let lock = TEST_SETTINGS_LOCK.lock().unwrap();
let mut settings = SettingsPartial::empty();
configure_settings(&mut settings);
Settings::reset(Some(settings));
let _guard = SettingsResetGuard { _lock: lock };

let backend = RubyPlugin::new();
let request = ToolRequest::new(backend.ba().clone(), "3.3.0", ToolSource::Unknown).unwrap();
backend.resolve_lockfile_options(&request, &PlatformTarget::from_current())
}

#[test]
fn test_tag_to_version() {
// Standard versions
Expand Down Expand Up @@ -1188,4 +1249,155 @@ mod tests {
""
);
}

#[test]
fn test_ruby_lockfile_options_include_precompiled_inputs() {
let opts = resolve_ruby_lockfile_options(|settings| {
settings.ruby.compile = Some(false);
settings.ruby.precompiled_url = Some("acme/ruby".to_string());
settings.ruby.precompiled_arch = Some("arm64".to_string());
settings.ruby.precompiled_os = Some("linux".to_string());
});

assert_eq!(
opts,
BTreeMap::from([
("compile".to_string(), "false".to_string()),
("precompiled_arch".to_string(), "arm64".to_string()),
("precompiled_os".to_string(), "linux".to_string()),
("precompiled_url".to_string(), "acme/ruby".to_string()),
(
"ruby_build_repo".to_string(),
DEFAULT_RUBY_BUILD_REPO.to_string(),
),
("ruby_install".to_string(), "false".to_string()),
])
);
}

#[test]
fn test_ruby_lockfile_options_include_source_build_inputs() {
let opts = resolve_ruby_lockfile_options(|settings| {
settings.ruby.compile = Some(true);
settings.ruby.ruby_build_opts = Some("--enable-yjit".to_string());
settings.ruby.apply_patches = Some("https://example.com/ruby.patch".to_string());
});

assert_eq!(
opts,
BTreeMap::from([
(
"apply_patches".to_string(),
"https://example.com/ruby.patch".to_string(),
),
("compile".to_string(), "true".to_string()),
(
"ruby_build_repo".to_string(),
DEFAULT_RUBY_BUILD_REPO.to_string(),
),
("ruby_build_opts".to_string(), "--enable-yjit".to_string()),
("ruby_install".to_string(), "false".to_string()),
])
);
}

#[test]
fn test_ruby_lockfile_options_include_experimental_precompiled_default() {
let opts = resolve_ruby_lockfile_options(|settings| {
settings.ruby.precompiled_url = Some("acme/ruby".to_string());
settings.ruby.precompiled_arch = Some("arm64".to_string());
settings.ruby.precompiled_os = Some("linux".to_string());
});

assert_eq!(
opts,
BTreeMap::from([
("compile".to_string(), "false".to_string()),
("precompiled_arch".to_string(), "arm64".to_string()),
("precompiled_os".to_string(), "linux".to_string()),
("precompiled_url".to_string(), "acme/ruby".to_string()),
(
"ruby_build_repo".to_string(),
DEFAULT_RUBY_BUILD_REPO.to_string(),
),
("ruby_install".to_string(), "false".to_string()),
])
);
Comment on lines +1305 to +1325
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 2, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

These assertions encode precompiled-by-default behavior too early.

Both tests expect compile=false/precompiled_url with ruby.compile unset, but should_try_precompiled() and install_version_() still require either ruby.compile = Some(false) or settings.experimental = true. As written, the “experimental default” case never enables experimental, and the “resolved defaults” case asserts the future default instead of the current one.

Also applies to: 1329-1346

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/plugins/core/ruby.rs` around lines 1305 - 1325, The tests in
test_ruby_lockfile_options_include_experimental_precompiled_default incorrectly
assume precompiled-by-default; update the test cases that call
resolve_ruby_lockfile_options so they mirror the runtime gating used by
should_try_precompiled() and install_version_: for the “experimental default”
variant explicitly set settings.experimental = true (so precompiled behavior is
enabled), and for the “resolved defaults” variant either set
settings.ruby.compile = Some(false) to opt into precompiled behavior or else
change the expected BTreeMap to the current (non-precompiled) defaults; locate
these changes around the test function
test_ruby_lockfile_options_include_experimental_precompiled_default and the
sibling test referenced at lines 1329-1346 and adjust expectations to match the
presence or absence of settings.experimental and settings.ruby.compile.

}

#[test]
fn test_ruby_lockfile_options_include_resolved_defaults() {
let opts = resolve_ruby_lockfile_options(|_| {});

assert_eq!(
opts,
BTreeMap::from([
("compile".to_string(), "false".to_string()),
(
"precompiled_url".to_string(),
DEFAULT_RUBY_PRECOMPILED_URL.to_string(),
),
(
"ruby_build_repo".to_string(),
DEFAULT_RUBY_BUILD_REPO.to_string(),
),
("ruby_install".to_string(), "false".to_string()),
])
);
}

#[test]
fn test_ruby_lockfile_options_include_source_fallback_inputs() {
let opts = resolve_ruby_lockfile_options(|settings| {
settings.ruby.compile = Some(false);
settings.ruby.ruby_build_opts = Some("--enable-yjit".to_string());
settings.ruby.apply_patches = Some("https://example.com/ruby.patch".to_string());
});

assert_eq!(
opts,
BTreeMap::from([
(
"apply_patches".to_string(),
"https://example.com/ruby.patch".to_string(),
),
("compile".to_string(), "false".to_string()),
(
"precompiled_url".to_string(),
DEFAULT_RUBY_PRECOMPILED_URL.to_string(),
),
(
"ruby_build_repo".to_string(),
DEFAULT_RUBY_BUILD_REPO.to_string(),
),
("ruby_build_opts".to_string(), "--enable-yjit".to_string()),
("ruby_install".to_string(), "false".to_string()),
])
);
}

#[test]
fn test_ruby_lockfile_options_include_ruby_install_inputs() {
let opts = resolve_ruby_lockfile_options(|settings| {
settings.ruby.compile = Some(true);
settings.ruby.ruby_install = Some(true);
settings.ruby.ruby_install_opts = Some("--no-reinstall".to_string());
});

assert_eq!(
opts,
BTreeMap::from([
("compile".to_string(), "true".to_string()),
("ruby_install".to_string(), "true".to_string()),
(
"ruby_install_opts".to_string(),
"--no-reinstall".to_string()
),
(
"ruby_install_repo".to_string(),
DEFAULT_RUBY_INSTALL_REPO.to_string(),
),
])
);
}
}
Loading