From dcdeed52b8abafa61c7caefcec153d25c4c455da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20=C5=A0im=C3=A1nek?= Date: Sun, 9 Nov 2025 13:59:14 +0100 Subject: [PATCH 1/2] Add strategy pattern for Ruby detection. - don't add user gems to bundler context --- crates/rb-cli/src/commands/environment.rs | 2 +- crates/rb-cli/src/commands/exec.rs | 13 +- crates/rb-cli/src/discovery.rs | 31 +- crates/rb-cli/src/lib.rs | 13 +- crates/rb-core/src/bundler/detector.rs | 73 ++-- crates/rb-core/src/bundler/mod.rs | 216 +++--------- crates/rb-core/src/butler/mod.rs | 127 +++++-- crates/rb-core/src/ruby/mod.rs | 5 + .../src/ruby/version_detector/gemfile.rs | 166 +++++++++ .../rb-core/src/ruby/version_detector/mod.rs | 213 ++++++++++++ .../version_detector/ruby_version_file.rs | 102 ++++++ .../tests/bundler_integration_tests.rs | 112 ++++-- .../rb-core/tests/butler_integration_tests.rs | 323 ++++++++++++++++-- 13 files changed, 1082 insertions(+), 314 deletions(-) create mode 100644 crates/rb-core/src/ruby/version_detector/gemfile.rs create mode 100644 crates/rb-core/src/ruby/version_detector/mod.rs create mode 100644 crates/rb-core/src/ruby/version_detector/ruby_version_file.rs diff --git a/crates/rb-cli/src/commands/environment.rs b/crates/rb-cli/src/commands/environment.rs index 967ccc5..0e4e261 100644 --- a/crates/rb-cli/src/commands/environment.rs +++ b/crates/rb-cli/src/commands/environment.rs @@ -424,7 +424,7 @@ mod tests { let bundler_sandbox = BundlerSandbox::new()?; let project_dir = bundler_sandbox.add_bundler_project("test-app", true)?; - let bundler_runtime = BundlerRuntime::new(&project_dir); + let bundler_runtime = BundlerRuntime::new(&project_dir, ruby.version.clone()); // Use sandboxed gem directory instead of real home directory let gem_runtime = GemRuntime::for_base_dir(&ruby_sandbox.gem_base_dir(), &ruby.version); diff --git a/crates/rb-cli/src/commands/exec.rs b/crates/rb-cli/src/commands/exec.rs index b46a6a0..59c2ca6 100644 --- a/crates/rb-cli/src/commands/exec.rs +++ b/crates/rb-cli/src/commands/exec.rs @@ -237,8 +237,17 @@ mod tests { // Test that standard variables are present assert!(env_vars.contains_key("PATH")); - assert!(env_vars.contains_key("GEM_HOME")); - assert!(env_vars.contains_key("GEM_PATH")); + + // IMPORTANT: When bundler context is detected, GEM_HOME and GEM_PATH should NOT be set + // This is bundler isolation - only bundled gems are available + assert!( + !env_vars.contains_key("GEM_HOME"), + "GEM_HOME should NOT be set in bundler context (isolation)" + ); + assert!( + !env_vars.contains_key("GEM_PATH"), + "GEM_PATH should NOT be set in bundler context (isolation)" + ); // Test that bundler variables are set when bundler project is detected assert!(env_vars.contains_key("BUNDLE_GEMFILE")); diff --git a/crates/rb-cli/src/discovery.rs b/crates/rb-cli/src/discovery.rs index 931f52b..713f2e3 100644 --- a/crates/rb-cli/src/discovery.rs +++ b/crates/rb-cli/src/discovery.rs @@ -45,13 +45,10 @@ impl DiscoveryContext { // Step 2: Detect bundler environment debug!("Detecting bundler environment"); - let bundler_environment = match BundlerRuntimeDetector::discover(¤t_dir) { - Ok(Some(bundler)) => { - debug!( - "Bundler environment detected at: {}", - bundler.root.display() - ); - Some(bundler) + let bundler_root = match BundlerRuntimeDetector::discover(¤t_dir) { + Ok(Some(root)) => { + debug!("Bundler environment detected at: {}", root.display()); + Some(root) } Ok(None) => { debug!("No bundler environment detected"); @@ -64,8 +61,10 @@ impl DiscoveryContext { }; // Step 3: Determine required Ruby version - let required_ruby_version = if let Some(bundler) = &bundler_environment { - match bundler.ruby_version() { + let required_ruby_version = if bundler_root.is_some() { + use rb_core::ruby::CompositeDetector; + let detector = CompositeDetector::bundler(); + match detector.detect(¤t_dir) { Some(version) => { debug!("Bundler environment specifies Ruby version: {}", version); Some(version) @@ -86,7 +85,19 @@ impl DiscoveryContext { &required_ruby_version, ); - // Step 5: Create butler runtime if we have a selected Ruby + // Step 5: Create bundler runtime with selected Ruby version (if bundler detected) + let bundler_environment = if let Some(ref root) = bundler_root { + if let Some(ref ruby) = selected_ruby { + Some(BundlerRuntime::new(root, ruby.version.clone())) + } else { + // No suitable Ruby found - create with temp version for display purposes + Some(BundlerRuntime::new(root, Version::new(0, 0, 0))) + } + } else { + None + }; + + // Step 6: Create butler runtime if we have a selected Ruby let butler_runtime = if let Some(ruby) = &selected_ruby { match ruby.infer_gem_runtime() { Ok(gem_runtime) => { diff --git a/crates/rb-cli/src/lib.rs b/crates/rb-cli/src/lib.rs index 1e74c17..219d60f 100644 --- a/crates/rb-cli/src/lib.rs +++ b/crates/rb-cli/src/lib.rs @@ -227,10 +227,21 @@ mod tests { #[test] fn test_create_ruby_context_with_sandbox() { let sandbox = RubySandbox::new().expect("Failed to create sandbox"); - sandbox + let ruby_dir = sandbox .add_ruby_dir("3.2.5") .expect("Failed to create ruby-3.2.5"); + // Create Ruby executable so it can be discovered + std::fs::create_dir_all(ruby_dir.join("bin")).expect("Failed to create bin dir"); + let ruby_exe = ruby_dir.join("bin").join("ruby"); + std::fs::write(&ruby_exe, "#!/bin/sh\necho ruby").expect("Failed to write ruby exe"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&ruby_exe, std::fs::Permissions::from_mode(0o755)) + .expect("Failed to set permissions"); + } + let result = create_ruby_context(Some(sandbox.root().to_path_buf()), None); // Should successfully create a ButlerRuntime diff --git a/crates/rb-core/src/bundler/detector.rs b/crates/rb-core/src/bundler/detector.rs index 45904df..63cd92c 100644 --- a/crates/rb-core/src/bundler/detector.rs +++ b/crates/rb-core/src/bundler/detector.rs @@ -1,14 +1,13 @@ use log::{debug, info}; -use std::path::Path; - -use super::BundlerRuntime; +use std::path::{Path, PathBuf}; pub struct BundlerRuntimeDetector; impl BundlerRuntimeDetector { - /// Discover a BundlerRuntime by searching for Gemfile in the current directory + /// Discover a Bundler project by searching for Gemfile in the current directory /// and walking up the directory tree until one is found or we reach the root. - pub fn discover(start_dir: &Path) -> std::io::Result> { + /// Returns the root directory containing the Gemfile. + pub fn discover(start_dir: &Path) -> std::io::Result> { debug!( "Starting Bundler discovery from directory: {}", start_dir.display() @@ -22,9 +21,8 @@ impl BundlerRuntimeDetector { if gemfile_path.exists() && gemfile_path.is_file() { info!("Found Gemfile at: {}", gemfile_path.display()); - let bundler_runtime = BundlerRuntime::new(¤t_dir); - debug!("Created BundlerRuntime for root: {}", current_dir.display()); - return Ok(Some(bundler_runtime)); + debug!("Returning bundler root: {}", current_dir.display()); + return Ok(Some(current_dir)); } else { debug!("No Gemfile found in: {}", current_dir.display()); } @@ -50,7 +48,7 @@ impl BundlerRuntimeDetector { } /// Convenience method to discover from current working directory - pub fn discover_from_cwd() -> std::io::Result> { + pub fn discover_from_cwd() -> std::io::Result> { let cwd = std::env::current_dir()?; debug!( "Discovering Bundler runtime from current working directory: {}", @@ -64,7 +62,6 @@ impl BundlerRuntimeDetector { mod tests { use super::*; use rb_tests::BundlerSandbox; - use semver; use std::io; #[test] @@ -75,9 +72,9 @@ mod tests { let result = BundlerRuntimeDetector::discover(&project_dir)?; assert!(result.is_some()); - let bundler_runtime = result.unwrap(); - assert_eq!(bundler_runtime.root, project_dir); - assert_eq!(bundler_runtime.gemfile_path(), project_dir.join("Gemfile")); + let bundler_root = result.unwrap(); + assert_eq!(bundler_root, project_dir); + assert_eq!(bundler_root.join("Gemfile"), project_dir.join("Gemfile")); Ok(()) } @@ -95,9 +92,9 @@ mod tests { let result = BundlerRuntimeDetector::discover(&sub_dir)?; assert!(result.is_some()); - let bundler_runtime = result.unwrap(); - assert_eq!(bundler_runtime.root, project_dir); - assert_eq!(bundler_runtime.gemfile_path(), project_dir.join("Gemfile")); + let bundler_root = result.unwrap(); + assert_eq!(bundler_root, project_dir); + assert_eq!(bundler_root.join("Gemfile"), project_dir.join("Gemfile")); Ok(()) } @@ -125,9 +122,9 @@ mod tests { let result = BundlerRuntimeDetector::discover(&deep_dir)?; assert!(result.is_some()); - let bundler_runtime = result.unwrap(); - assert_eq!(bundler_runtime.root, subproject); - assert_eq!(bundler_runtime.gemfile_path(), subproject.join("Gemfile")); + let bundler_root = result.unwrap(); + assert_eq!(bundler_root, subproject); + assert_eq!(bundler_root.join("Gemfile"), subproject.join("Gemfile")); Ok(()) } @@ -147,41 +144,9 @@ mod tests { let result = BundlerRuntimeDetector::discover(&deep_dir)?; assert!(result.is_some()); - let bundler_runtime = result.unwrap(); - assert_eq!(bundler_runtime.root, project_dir); - assert_eq!(bundler_runtime.gemfile_path(), project_dir.join("Gemfile")); - - Ok(()) - } - - #[test] - fn discover_detects_ruby_version_from_project() -> io::Result<()> { - let sandbox = BundlerSandbox::new()?; - let project_dir = sandbox.add_dir("ruby-version-app")?; - - // Create Gemfile with ruby version - let gemfile_content = r#"source 'https://rubygems.org' - -ruby '3.2.1' - -gem 'rails' -"#; - sandbox.add_file( - format!( - "{}/Gemfile", - project_dir.file_name().unwrap().to_str().unwrap() - ), - gemfile_content, - )?; - - let result = BundlerRuntimeDetector::discover(&project_dir)?; - - assert!(result.is_some()); - let bundler_runtime = result.unwrap(); - assert_eq!( - bundler_runtime.ruby_version(), - Some(semver::Version::parse("3.2.1").unwrap()) - ); + let bundler_root = result.unwrap(); + assert_eq!(bundler_root, project_dir); + assert_eq!(bundler_root.join("Gemfile"), project_dir.join("Gemfile")); Ok(()) } diff --git a/crates/rb-core/src/bundler/mod.rs b/crates/rb-core/src/bundler/mod.rs index 394f467..77e2a84 100644 --- a/crates/rb-core/src/bundler/mod.rs +++ b/crates/rb-core/src/bundler/mod.rs @@ -1,23 +1,28 @@ use crate::butler::Command; use crate::butler::runtime_provider::RuntimeProvider; -use log::{debug, warn}; +use log::debug; use semver::Version; -use std::fs; use std::path::{Path, PathBuf}; #[derive(Debug, Clone, PartialEq, Eq)] pub struct BundlerRuntime { /// Root directory containing the Gemfile pub root: PathBuf, + /// Ruby version for this bundler context + pub ruby_version: Version, } impl BundlerRuntime { - pub fn new(root: impl AsRef) -> Self { + pub fn new(root: impl AsRef, ruby_version: Version) -> Self { let root = root.as_ref().to_path_buf(); - debug!("Creating BundlerRuntime for root: {}", root.display()); + debug!( + "Creating BundlerRuntime for root: {} with Ruby {}", + root.display(), + ruby_version + ); - Self { root } + Self { root, ruby_version } } /// Returns the full path to the Gemfile @@ -35,140 +40,27 @@ impl BundlerRuntime { self.app_config_dir().join("vendor").join("bundler") } - /// Detect Ruby version from .ruby-version file or Gemfile ruby declaration - pub fn ruby_version(&self) -> Option { - // First try .ruby-version file - if let Some(version) = self.detect_from_ruby_version_file() { - return Some(version); - } - - // Then try Gemfile ruby declaration - if let Some(version) = self.detect_from_gemfile() { - return Some(version); - } - - None - } - - /// Detect Ruby version from .ruby-version file - fn detect_from_ruby_version_file(&self) -> Option { - let ruby_version_path = self.root.join(".ruby-version"); - debug!( - "Checking for .ruby-version file: {}", - ruby_version_path.display() + /// Returns the ruby-specific vendor directory (.rb/vendor/bundler/ruby/X.Y.Z) + /// This requires a Ruby version to be detected + pub fn ruby_vendor_dir(&self, ruby_version: &Version) -> PathBuf { + let version_str = format!( + "{}.{}.{}", + ruby_version.major, ruby_version.minor, ruby_version.patch ); - - match fs::read_to_string(&ruby_version_path) { - Ok(content) => { - let version_str = content.trim(); - debug!("Found .ruby-version content: '{}'", version_str); - - match Version::parse(version_str) { - Ok(version) => { - debug!( - "Successfully parsed Ruby version from .ruby-version: {}", - version - ); - Some(version) - } - Err(e) => { - warn!( - "Failed to parse Ruby version '{}' from .ruby-version: {}", - version_str, e - ); - None - } - } - } - Err(_) => { - debug!("No .ruby-version file found"); - None - } - } + self.vendor_dir().join("ruby").join(version_str) } - /// Detect Ruby version from Gemfile ruby declaration - fn detect_from_gemfile(&self) -> Option { - let gemfile_path = self.gemfile_path(); - debug!( - "Checking for ruby declaration in Gemfile: {}", - gemfile_path.display() - ); - - match fs::read_to_string(&gemfile_path) { - Ok(content) => { - debug!("Reading Gemfile for ruby declaration"); - - for line in content.lines() { - let line = line.trim(); - - // Look for patterns like: ruby '3.2.5' or ruby "3.2.5" - if line.starts_with("ruby ") { - debug!("Found ruby line: '{}'", line); - - // Extract version string between quotes - if let Some(version_str) = Self::extract_quoted_version(line) { - debug!("Extracted version string: '{}'", version_str); - - match Version::parse(&version_str) { - Ok(version) => { - debug!( - "Successfully parsed Ruby version from Gemfile: {}", - version - ); - return Some(version); - } - Err(e) => { - warn!( - "Failed to parse Ruby version '{}' from Gemfile: {}", - version_str, e - ); - } - } - } - } - } - - debug!("No valid ruby declaration found in Gemfile"); - None - } - Err(_) => { - debug!("Could not read Gemfile"); - None - } - } - } - - /// Extract version string from ruby declaration line - fn extract_quoted_version(line: &str) -> Option { - // Handle both single and double quotes: ruby '3.2.5' or ruby "3.2.5" - let after_ruby = line.strip_prefix("ruby ")?; - let trimmed = after_ruby.trim(); - - // Single quotes - if let Some(version) = trimmed.strip_prefix('\'').and_then(|single_quoted| { - single_quoted - .find('\'') - .map(|end_quote| single_quoted[..end_quote].to_string()) - }) { - return Some(version); - } - - // Double quotes - if let Some(version) = trimmed.strip_prefix('"').and_then(|double_quoted| { - double_quoted - .find('"') - .map(|end_quote| double_quoted[..end_quote].to_string()) - }) { - return Some(version); - } - - None + /// Detect Ruby version from .ruby-version file or Gemfile ruby declaration + pub fn ruby_version(&self) -> Option { + use crate::ruby::CompositeDetector; + let detector = CompositeDetector::bundler(); + detector.detect(&self.root) } /// Returns the bin directory where bundler-installed executables live + /// Path: .rb/vendor/bundler/ruby/X.Y.Z/bin pub fn bin_dir(&self) -> PathBuf { - let bin_dir = self.vendor_dir().join("bin"); + let bin_dir = self.ruby_vendor_dir(&self.ruby_version).join("bin"); debug!("Bundler bin directory: {}", bin_dir.display()); bin_dir } @@ -475,7 +367,9 @@ pub enum SyncResult { impl RuntimeProvider for BundlerRuntime { fn bin_dir(&self) -> Option { if self.is_configured() { - Some(self.bin_dir()) + let bin = self.ruby_vendor_dir(&self.ruby_version).join("bin"); + debug!("BundlerRuntime bin directory: {}", bin.display()); + Some(bin) } else { debug!("BundlerRuntime not configured, no bin directory available"); None @@ -484,7 +378,9 @@ impl RuntimeProvider for BundlerRuntime { fn gem_dir(&self) -> Option { if self.is_configured() { - Some(self.vendor_dir()) + let vendor = self.ruby_vendor_dir(&self.ruby_version); + debug!("BundlerRuntime gem directory: {}", vendor.display()); + Some(vendor) } else { debug!("BundlerRuntime not configured, no gem directory available"); None @@ -499,14 +395,15 @@ mod tests { use std::io; use std::path::Path; - fn bundler_rt(root: &str) -> BundlerRuntime { - BundlerRuntime::new(root) + // Helper to create BundlerRuntime with a default Ruby version for testing + fn bundler_rt(root: impl AsRef) -> BundlerRuntime { + BundlerRuntime::new(root, Version::new(3, 3, 7)) } #[test] fn new_creates_proper_paths() { let root = Path::new("/home/user/my-app"); - let br = BundlerRuntime::new(root); + let br = bundler_rt(root); assert_eq!(br.root, root); assert_eq!(br.gemfile_path(), root.join("Gemfile")); @@ -521,7 +418,8 @@ mod tests { #[test] fn bin_dir_is_vendor_bin() { let br = bundler_rt("/home/user/project"); - let expected = Path::new("/home/user/project/.rb/vendor/bundler/bin"); + // bin_dir should include Ruby version: .rb/vendor/bundler/ruby/3.3.7/bin + let expected = Path::new("/home/user/project/.rb/vendor/bundler/ruby/3.3.7/bin"); assert_eq!(br.bin_dir(), expected); } @@ -529,13 +427,15 @@ mod tests { fn runtime_provider_returns_paths_when_configured() -> io::Result<()> { let sandbox = BundlerSandbox::new()?; let project_dir = sandbox.add_bundler_project("configured-app", true)?; - let br = BundlerRuntime::new(&project_dir); + let br = bundler_rt(&project_dir); // Should be configured since we created vendor structure assert!(br.is_configured()); - let expected_bin = br.vendor_dir().join("bin"); - let expected_gem = br.vendor_dir(); + // bin_dir should include Ruby version path + let expected_bin = br.vendor_dir().join("ruby").join("3.3.7").join("bin"); + // gem_dir should be the Ruby-specific vendor directory + let expected_gem = br.vendor_dir().join("ruby").join("3.3.7"); assert_eq!( ::bin_dir(&br), @@ -553,7 +453,7 @@ mod tests { fn runtime_provider_returns_none_when_not_configured() -> io::Result<()> { let sandbox = BundlerSandbox::new()?; let project_dir = sandbox.add_bundler_project("basic-app", false)?; - let br = BundlerRuntime::new(&project_dir); + let br = bundler_rt(&project_dir); // Should not be configured since no vendor structure exists assert!(!br.is_configured()); @@ -578,7 +478,7 @@ mod tests { "3.2.5", )?; - let br = BundlerRuntime::new(&project_dir); + let br = bundler_rt(&project_dir); assert_eq!(br.ruby_version(), Some(Version::parse("3.2.5").unwrap())); Ok(()) @@ -604,7 +504,7 @@ gem 'pg', '~> 1.4' gemfile_content, )?; - let br = BundlerRuntime::new(&project_dir); + let br = bundler_rt(&project_dir); assert_eq!(br.ruby_version(), Some(Version::parse("3.1.4").unwrap())); Ok(()) @@ -629,7 +529,7 @@ gem "rails", "~> 7.1" gemfile_content, )?; - let br = BundlerRuntime::new(&project_dir); + let br = bundler_rt(&project_dir); assert_eq!(br.ruby_version(), Some(Version::parse("3.3.0").unwrap())); Ok(()) @@ -663,7 +563,7 @@ gem 'rails' "3.2.5", )?; - let br = BundlerRuntime::new(&project_dir); + let br = bundler_rt(&project_dir); // Should prefer .ruby-version assert_eq!(br.ruby_version(), Some(Version::parse("3.2.5").unwrap())); @@ -685,7 +585,7 @@ gem 'rails' "not-a-version", )?; - let br = BundlerRuntime::new(&project_dir); + let br = bundler_rt(&project_dir); assert_eq!(br.ruby_version(), None); Ok(()) @@ -710,7 +610,7 @@ gem 'pg' gemfile_content, )?; - let br = BundlerRuntime::new(&project_dir); + let br = bundler_rt(&project_dir); assert_eq!(br.ruby_version(), None); Ok(()) @@ -731,29 +631,11 @@ gem 'pg' " 3.2.1 \n", )?; - let br = BundlerRuntime::new(&project_dir); + let br = bundler_rt(&project_dir); assert_eq!(br.ruby_version(), Some(Version::parse("3.2.1").unwrap())); Ok(()) } - - #[test] - fn extract_quoted_version_handles_various_formats() { - assert_eq!( - BundlerRuntime::extract_quoted_version("ruby '3.2.5'"), - Some("3.2.5".to_string()) - ); - assert_eq!( - BundlerRuntime::extract_quoted_version("ruby \"3.1.4\""), - Some("3.1.4".to_string()) - ); - assert_eq!( - BundlerRuntime::extract_quoted_version("ruby '3.0.0' "), - Some("3.0.0".to_string()) - ); - assert_eq!(BundlerRuntime::extract_quoted_version("ruby 3.2.5"), None); - assert_eq!(BundlerRuntime::extract_quoted_version("gem 'rails'"), None); - } } pub mod detector; diff --git a/crates/rb-core/src/butler/mod.rs b/crates/rb-core/src/butler/mod.rs index 9731746..e2551b8 100644 --- a/crates/rb-core/src/butler/mod.rs +++ b/crates/rb-core/src/butler/mod.rs @@ -149,18 +149,18 @@ impl ButlerRuntime { info!("Found {} Ruby installations", ruby_installations.len()); // Step 2: Detect bundler environment (skip if requested) - let bundler_runtime = if skip_bundler { + let bundler_root = if skip_bundler { debug!("Bundler detection skipped (--no-bundler flag set)"); None } else { debug!("Detecting bundler environment"); match BundlerRuntimeDetector::discover(¤t_dir) { - Ok(Some(bundler)) => { + Ok(Some(bundler_root)) => { debug!( "Bundler environment detected at: {}", - bundler.root.display() + bundler_root.display() ); - Some(bundler) + Some(bundler_root) } Ok(None) => { debug!("No bundler environment detected"); @@ -173,10 +173,14 @@ impl ButlerRuntime { } }; - // Step 3: Extract version requirements from bundler - let required_ruby_version = bundler_runtime - .as_ref() - .and_then(|bundler| bundler.ruby_version()); + // Step 3: Extract version requirements from project directory + let required_ruby_version = if bundler_root.is_some() { + use crate::ruby::CompositeDetector; + let detector = CompositeDetector::bundler(); + detector.detect(¤t_dir) + } else { + None + }; // Step 4: Select the most appropriate Ruby installation let selected_ruby = Self::select_ruby_runtime( @@ -188,8 +192,18 @@ impl ButlerRuntime { ButlerError::NoSuitableRuby("No suitable Ruby installation found".to_string()) })?; - // Step 5: Create gem runtime (using custom base directory if provided) - let gem_runtime = if let Some(ref custom_gem_base) = gem_base_dir { + // Step 5: Create bundler runtime with selected Ruby version (if bundler detected) + let bundler_runtime = + bundler_root.map(|root| BundlerRuntime::new(root, selected_ruby.version.clone())); + + // Step 6: Create gem runtime (using custom base directory if provided) + // IMPORTANT: When in bundler context, user gems are NOT available for isolation + let gem_runtime = if bundler_runtime.is_some() { + debug!( + "Bundler context detected - user gems will NOT be available (bundler isolation)" + ); + None + } else if let Some(ref custom_gem_base) = gem_base_dir { debug!( "Using custom gem base directory: {}", custom_gem_base.display() @@ -217,7 +231,7 @@ impl ButlerRuntime { if gem_runtime.is_some() { "available" } else { - "unavailable" + "unavailable (bundler isolation)" }, if bundler_runtime.is_some() { "detected" @@ -377,21 +391,46 @@ impl ButlerRuntime { } } - /// Returns a list of bin directories from both ruby and gem runtimes - /// Gem bin directory comes first (higher priority) if present, then Ruby bin directory + /// Returns a list of bin directories from all active runtimes + /// + /// When in bundler context (bundler_runtime present): + /// 1. Bundler bin directory (.rb/vendor/bundler/ruby/X.Y.Z/bin) - bundled gems only + /// 2. Ruby bin directory (~/.rubies/ruby-X.Y.Z/bin) - core executables + /// + /// When NOT in bundler context: + /// 1. Gem bin directory (~/.gem/ruby/X.Y.Z/bin) - user-installed gems + /// 2. Ruby bin directory (~/.rubies/ruby-X.Y.Z/bin) - core executables + /// + /// NOTE: User gems are NOT available in bundler context for proper isolation. + /// Use --no-bundler to opt out of bundler context and access user gems. pub fn bin_dirs(&self) -> Vec { let mut dirs = Vec::new(); - // Gem runtime bin dir first (highest priority) - for user-installed tools - if let Some(ref gem_runtime) = self.gem_runtime { + // Bundler runtime bin dir first (if in bundler context) + if let Some(ref bundler_runtime) = self.bundler_runtime + && let Some(bundler_bin) = RuntimeProvider::bin_dir(bundler_runtime) + { debug!( - "Adding gem bin directory to PATH: {}", - gem_runtime.gem_bin.display() + "Adding bundler bin directory to PATH: {}", + bundler_bin.display() ); - dirs.push(gem_runtime.gem_bin.clone()); + dirs.push(bundler_bin); + } + + // Gem runtime bin dir (only if NOT in bundler context for isolation) + if self.bundler_runtime.is_none() { + if let Some(ref gem_runtime) = self.gem_runtime { + debug!( + "Adding gem bin directory to PATH: {}", + gem_runtime.gem_bin.display() + ); + dirs.push(gem_runtime.gem_bin.clone()); + } + } else { + debug!("Skipping user gem bin directory (bundler isolation)"); } - // Ruby runtime bin dir second - for core Ruby executables + // Ruby runtime bin dir always included let ruby_bin = self.ruby_runtime.bin_dir(); debug!("Adding ruby bin directory to PATH: {}", ruby_bin.display()); dirs.push(ruby_bin); @@ -400,23 +439,47 @@ impl ButlerRuntime { dirs } - /// Returns a list of gem directories from both ruby and gem runtimes + /// Returns a list of gem directories from all active runtimes + /// + /// When in bundler context (bundler_runtime present): + /// 1. Bundler vendor directory (.rb/vendor/bundler/ruby/X.Y.Z) - bundled gems only + /// 2. Ruby lib directory (~/.rubies/ruby-X.Y.Z/lib/ruby/gems/X.Y.0) - system gems + /// + /// When NOT in bundler context: + /// 1. User gem home (~/.gem/ruby/X.Y.Z) - user-installed gems + /// 2. Ruby lib directory (~/.rubies/ruby-X.Y.Z/lib/ruby/gems/X.Y.0) - system gems + /// + /// NOTE: User gems are NOT available in bundler context for proper isolation. + /// Use --no-bundler to opt out of bundler context and access user gems. pub fn gem_dirs(&self) -> Vec { let mut dirs = Vec::new(); - // Ruby runtime always has a lib dir for gems + // Bundler runtime gem dir first (if in bundler context) + if let Some(ref bundler_runtime) = self.bundler_runtime + && let Some(bundler_gem) = RuntimeProvider::gem_dir(bundler_runtime) + { + debug!("Adding bundler gem directory: {}", bundler_gem.display()); + dirs.push(bundler_gem); + } + + // User gem home (only if NOT in bundler context for isolation) + if self.bundler_runtime.is_none() { + if let Some(ref gem_runtime) = self.gem_runtime { + debug!( + "Adding gem home directory: {}", + gem_runtime.gem_home.display() + ); + dirs.push(gem_runtime.gem_home.clone()); + } + } else { + debug!("Skipping user gem home (bundler isolation)"); + } + + // Ruby runtime lib dir always included let ruby_lib = self.ruby_runtime.lib_dir(); debug!("Adding ruby lib directory for gems: {}", ruby_lib.display()); dirs.push(ruby_lib); - if let Some(ref gem_runtime) = self.gem_runtime { - debug!( - "Adding gem home directory: {}", - gem_runtime.gem_home.display() - ); - dirs.push(gem_runtime.gem_home.clone()); - } - debug!("Total gem directories: {}", dirs.len()); dirs } @@ -597,11 +660,11 @@ mod tests { assert_eq!(bin_dirs[0], gem_runtime.gem_bin); // Gem bin dir first (higher priority) assert_eq!(bin_dirs[1], ruby.bin_dir()); // Ruby bin dir second - // Test gem_dirs - should have both ruby and gem dirs + // Test gem_dirs - should have gem_home first (user gems), then ruby lib (system gems) let gem_dirs = butler.gem_dirs(); assert_eq!(gem_dirs.len(), 2); - assert_eq!(gem_dirs[0], ruby.lib_dir()); - assert_eq!(gem_dirs[1], gem_runtime.gem_home); + assert_eq!(gem_dirs[0], gem_runtime.gem_home); // User gem home first (higher priority) + assert_eq!(gem_dirs[1], ruby.lib_dir()); // Ruby lib dir second (system gems) // Test gem_home should return the gem runtime's gem_home assert_eq!(butler.gem_home(), Some(gem_runtime.gem_home)); diff --git a/crates/rb-core/src/ruby/mod.rs b/crates/rb-core/src/ruby/mod.rs index 067cd52..4c49889 100644 --- a/crates/rb-core/src/ruby/mod.rs +++ b/crates/rb-core/src/ruby/mod.rs @@ -5,6 +5,11 @@ use semver::Version; use std::env::consts::EXE_SUFFIX; use std::path::{Path, PathBuf}; +pub mod version_detector; +pub use version_detector::{ + CompositeDetector, GemfileDetector, RubyVersionDetector, RubyVersionFileDetector, +}; + /// Errors that can occur during Ruby discovery #[derive(Debug, Clone)] pub enum RubyDiscoveryError { diff --git a/crates/rb-core/src/ruby/version_detector/gemfile.rs b/crates/rb-core/src/ruby/version_detector/gemfile.rs new file mode 100644 index 0000000..5ba3f45 --- /dev/null +++ b/crates/rb-core/src/ruby/version_detector/gemfile.rs @@ -0,0 +1,166 @@ +//! Detector for Gemfile ruby declarations + +use super::RubyVersionDetector; +use log::{debug, warn}; +use semver::Version; +use std::fs; +use std::path::Path; + +/// Detects Ruby version from Gemfile ruby declaration +pub struct GemfileDetector; + +impl RubyVersionDetector for GemfileDetector { + fn detect(&self, context: &Path) -> Option { + let gemfile_path = context.join("Gemfile"); + debug!( + "Checking for ruby declaration in Gemfile: {}", + gemfile_path.display() + ); + + match fs::read_to_string(&gemfile_path) { + Ok(content) => { + debug!("Reading Gemfile for ruby declaration"); + + for line in content.lines() { + let line = line.trim(); + + // Look for patterns like: ruby '3.2.5' or ruby "3.2.5" + if line.starts_with("ruby ") { + debug!("Found ruby line: '{}'", line); + + // Extract version string between quotes + if let Some(version_str) = Self::extract_quoted_version(line) { + debug!("Extracted version string: '{}'", version_str); + + match Version::parse(&version_str) { + Ok(version) => { + debug!( + "Successfully parsed Ruby version from Gemfile: {}", + version + ); + return Some(version); + } + Err(e) => { + warn!( + "Failed to parse Ruby version '{}' from Gemfile: {}", + version_str, e + ); + } + } + } + } + } + + debug!("No valid ruby declaration found in Gemfile"); + None + } + Err(_) => { + debug!("No Gemfile found"); + None + } + } + } + + fn name(&self) -> &'static str { + "Gemfile" + } +} + +impl GemfileDetector { + /// Extract version string from between quotes in a line + /// Handles both single and double quotes + fn extract_quoted_version(line: &str) -> Option { + // Remove "ruby " prefix and trim + let rest = line.strip_prefix("ruby ")?.trim(); + + // Handle both single and double quotes + for quote in &['\'', '"'] { + if rest.starts_with(*quote) + && let Some(end_idx) = rest[1..].find(*quote) + { + return Some(rest[1..=end_idx].to_string()); + } + } + + None + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::io::Write; + use tempfile::TempDir; + + #[test] + fn test_detects_single_quotes() { + let temp_dir = TempDir::new().unwrap(); + let gemfile_path = temp_dir.path().join("Gemfile"); + let mut file = std::fs::File::create(&gemfile_path).unwrap(); + writeln!(file, "source 'https://rubygems.org'").unwrap(); + writeln!(file, "ruby '3.1.4'").unwrap(); + writeln!(file, "gem 'rails'").unwrap(); + + let detector = GemfileDetector; + let version = detector.detect(temp_dir.path()).unwrap(); + + assert_eq!(version, Version::new(3, 1, 4)); + } + + #[test] + fn test_detects_double_quotes() { + let temp_dir = TempDir::new().unwrap(); + let gemfile_path = temp_dir.path().join("Gemfile"); + let mut file = std::fs::File::create(&gemfile_path).unwrap(); + writeln!(file, "source \"https://rubygems.org\"").unwrap(); + writeln!(file, "ruby \"3.3.0\"").unwrap(); + + let detector = GemfileDetector; + let version = detector.detect(temp_dir.path()).unwrap(); + + assert_eq!(version, Version::new(3, 3, 0)); + } + + #[test] + fn test_returns_none_when_no_gemfile() { + let temp_dir = TempDir::new().unwrap(); + + let detector = GemfileDetector; + assert!(detector.detect(temp_dir.path()).is_none()); + } + + #[test] + fn test_returns_none_when_no_ruby_declaration() { + let temp_dir = TempDir::new().unwrap(); + let gemfile_path = temp_dir.path().join("Gemfile"); + let mut file = std::fs::File::create(&gemfile_path).unwrap(); + writeln!(file, "source 'https://rubygems.org'").unwrap(); + writeln!(file, "gem 'rails'").unwrap(); + + let detector = GemfileDetector; + assert!(detector.detect(temp_dir.path()).is_none()); + } + + #[test] + fn test_extract_quoted_version() { + assert_eq!( + GemfileDetector::extract_quoted_version("ruby '3.2.5'"), + Some("3.2.5".to_string()) + ); + assert_eq!( + GemfileDetector::extract_quoted_version("ruby \"3.1.0\""), + Some("3.1.0".to_string()) + ); + assert_eq!( + GemfileDetector::extract_quoted_version("ruby '3.0.0' # comment"), + Some("3.0.0".to_string()) + ); + assert_eq!(GemfileDetector::extract_quoted_version("ruby 3.2.5"), None); + assert_eq!(GemfileDetector::extract_quoted_version("gem 'rails'"), None); + } + + #[test] + fn test_name() { + assert_eq!(GemfileDetector.name(), "Gemfile"); + } +} diff --git a/crates/rb-core/src/ruby/version_detector/mod.rs b/crates/rb-core/src/ruby/version_detector/mod.rs new file mode 100644 index 0000000..9bf9e9a --- /dev/null +++ b/crates/rb-core/src/ruby/version_detector/mod.rs @@ -0,0 +1,213 @@ +//! Ruby version detection using various sources +//! +//! This module provides a **modular, extensible architecture** for detecting +//! required Ruby versions from various sources like .ruby-version files, +//! Gemfile declarations, and potentially .tool-versions (asdf/mise). +//! +//! # Architecture +//! +//! The system uses the **Strategy Pattern** with a trait-based design: +//! +//! ```text +//! ┌─────────────────────────────────────────┐ +//! │ RubyVersionDetector (trait) │ +//! │ - detect(&self, path) -> Option │ +//! │ - name(&self) -> &str │ +//! └────────────┬────────────────────────────┘ +//! │ +//! ┌────────┴─────────┬──────────┬────────────┐ +//! │ │ │ │ +//! ┌───▼────────┐ ┌──────▼──────┐ ▼ ┌─────▼──────────┐ +//! │ .ruby- │ │ Gemfile │ ... │ CompositeD. │ +//! │ version │ │ Detector │ │ (chains many) │ +//! └────────────┘ └─────────────┘ └────────────────┘ +//! ``` +//! +//! # Usage +//! +//! For standard Ruby projects: +//! ```rust,ignore +//! use rb_core::ruby::CompositeDetector; +//! +//! let detector = CompositeDetector::standard(); +//! if let Some(version) = detector.detect(project_root) { +//! println!("Required Ruby: {}", version); +//! } +//! ``` +//! +//! For bundler-managed projects: +//! ```rust,ignore +//! let detector = CompositeDetector::bundler(); +//! let version = detector.detect(bundler_root); +//! ``` +//! +//! # Adding New Detectors +//! +//! To add support for new version sources (e.g., `.tool-versions` for asdf): +//! +//! 1. Implement the `RubyVersionDetector` trait: +//! ```rust,ignore +//! pub struct ToolVersionsDetector; +//! impl RubyVersionDetector for ToolVersionsDetector { +//! fn detect(&self, context: &Path) -> Option { +//! // Read .tool-versions, parse "ruby X.Y.Z" line +//! } +//! fn name(&self) -> &'static str { ".tool-versions" } +//! } +//! ``` +//! +//! 2. Add to the detector chain: +//! ```rust,ignore +//! CompositeDetector { +//! detectors: vec![ +//! Box::new(RubyVersionFileDetector), +//! Box::new(GemfileDetector), +//! Box::new(ToolVersionsDetector), // <-- Add here +//! ] +//! } +//! ``` +//! +//! See `tool_versions.rs` for a complete example implementation. + +use log::debug; +use semver::Version; +use std::path::Path; + +pub mod gemfile; +pub mod ruby_version_file; + +pub use gemfile::GemfileDetector; +pub use ruby_version_file::RubyVersionFileDetector; + +/// Trait for Ruby version detection strategies +pub trait RubyVersionDetector { + /// Attempt to detect a Ruby version requirement + /// + /// Returns `Some(Version)` if a version requirement is found, + /// or `None` if this detector cannot determine a version. + fn detect(&self, context: &Path) -> Option; + + /// Human-readable name of this detector (for logging) + fn name(&self) -> &'static str; +} + +/// Composite detector that tries multiple strategies in order +pub struct CompositeDetector { + detectors: Vec>, +} + +impl CompositeDetector { + /// Create a new composite detector with the given strategies + pub fn new(detectors: Vec>) -> Self { + Self { detectors } + } + + /// Create a standard detector chain for general Ruby projects + /// + /// Checks in order: + /// 1. .ruby-version file + /// 2. Gemfile ruby declaration + pub fn standard() -> Self { + Self::new(vec![ + Box::new(ruby_version_file::RubyVersionFileDetector), + Box::new(gemfile::GemfileDetector), + ]) + } + + /// Create a bundler-aware detector chain + /// + /// Checks in order: + /// 1. .ruby-version file + /// 2. Gemfile ruby declaration + /// 3. (Future: .ruby-version in bundler vendor directory, etc.) + pub fn bundler() -> Self { + // For now, same as standard, but this is where we can add + // bundler-specific detectors in the future + Self::standard() + } + + /// Detect Ruby version using all configured detectors in order + /// + /// Returns the first version found, or None if no detector succeeds. + pub fn detect(&self, context: &Path) -> Option { + for detector in &self.detectors { + debug!( + "Trying detector '{}' in context: {}", + detector.name(), + context.display() + ); + if let Some(version) = detector.detect(context) { + debug!("Detector '{}' found version: {}", detector.name(), version); + return Some(version); + } + debug!("Detector '{}' found no version", detector.name()); + } + debug!("No detector found a Ruby version requirement"); + None + } + + /// Add a detector to the chain + pub fn add_detector(&mut self, detector: Box) { + self.detectors.push(detector); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::io::Write; + use tempfile::TempDir; + + #[test] + fn test_composite_detector_tries_in_order() { + let temp_dir = TempDir::new().unwrap(); + + // Create both .ruby-version and Gemfile + std::fs::write(temp_dir.path().join(".ruby-version"), "3.2.5\n").unwrap(); + + let gemfile_path = temp_dir.path().join("Gemfile"); + let mut file = std::fs::File::create(&gemfile_path).unwrap(); + writeln!(file, "ruby '3.1.0'").unwrap(); + + let detector = CompositeDetector::standard(); + let version = detector.detect(temp_dir.path()).unwrap(); + + // .ruby-version should take precedence (first in chain) + assert_eq!(version, Version::new(3, 2, 5)); + } + + #[test] + fn test_composite_detector_falls_back() { + let temp_dir = TempDir::new().unwrap(); + + // Only create Gemfile (no .ruby-version) + let gemfile_path = temp_dir.path().join("Gemfile"); + let mut file = std::fs::File::create(&gemfile_path).unwrap(); + writeln!(file, "ruby '3.1.4'").unwrap(); + + let detector = CompositeDetector::standard(); + let version = detector.detect(temp_dir.path()).unwrap(); + + // Should fall back to Gemfile + assert_eq!(version, Version::new(3, 1, 4)); + } + + #[test] + fn test_composite_detector_returns_none_when_nothing_found() { + let temp_dir = TempDir::new().unwrap(); + + let detector = CompositeDetector::standard(); + assert!(detector.detect(temp_dir.path()).is_none()); + } + + #[test] + fn test_bundler_detector_chain() { + let temp_dir = TempDir::new().unwrap(); + std::fs::write(temp_dir.path().join(".ruby-version"), "3.3.0\n").unwrap(); + + let detector = CompositeDetector::bundler(); + let version = detector.detect(temp_dir.path()).unwrap(); + + assert_eq!(version, Version::new(3, 3, 0)); + } +} diff --git a/crates/rb-core/src/ruby/version_detector/ruby_version_file.rs b/crates/rb-core/src/ruby/version_detector/ruby_version_file.rs new file mode 100644 index 0000000..d58ec36 --- /dev/null +++ b/crates/rb-core/src/ruby/version_detector/ruby_version_file.rs @@ -0,0 +1,102 @@ +//! Detector for .ruby-version files + +use super::RubyVersionDetector; +use log::{debug, warn}; +use semver::Version; +use std::fs; +use std::path::Path; + +/// Detects Ruby version from .ruby-version file +pub struct RubyVersionFileDetector; + +impl RubyVersionDetector for RubyVersionFileDetector { + fn detect(&self, context: &Path) -> Option { + let ruby_version_path = context.join(".ruby-version"); + debug!( + "Checking for .ruby-version file: {}", + ruby_version_path.display() + ); + + match fs::read_to_string(&ruby_version_path) { + Ok(content) => { + let version_str = content.trim(); + debug!("Found .ruby-version content: '{}'", version_str); + + match Version::parse(version_str) { + Ok(version) => { + debug!( + "Successfully parsed Ruby version from .ruby-version: {}", + version + ); + Some(version) + } + Err(e) => { + warn!( + "Failed to parse Ruby version '{}' from .ruby-version: {}", + version_str, e + ); + None + } + } + } + Err(_) => { + debug!("No .ruby-version file found"); + None + } + } + } + + fn name(&self) -> &'static str { + ".ruby-version" + } +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + #[test] + fn test_detects_valid_version() { + let temp_dir = TempDir::new().unwrap(); + std::fs::write(temp_dir.path().join(".ruby-version"), "3.2.5\n").unwrap(); + + let detector = RubyVersionFileDetector; + let version = detector.detect(temp_dir.path()).unwrap(); + + assert_eq!(version, Version::new(3, 2, 5)); + } + + #[test] + fn test_handles_whitespace() { + let temp_dir = TempDir::new().unwrap(); + std::fs::write(temp_dir.path().join(".ruby-version"), " 3.1.0 \n").unwrap(); + + let detector = RubyVersionFileDetector; + let version = detector.detect(temp_dir.path()).unwrap(); + + assert_eq!(version, Version::new(3, 1, 0)); + } + + #[test] + fn test_returns_none_when_file_missing() { + let temp_dir = TempDir::new().unwrap(); + + let detector = RubyVersionFileDetector; + assert!(detector.detect(temp_dir.path()).is_none()); + } + + #[test] + fn test_returns_none_when_invalid_version() { + let temp_dir = TempDir::new().unwrap(); + std::fs::write(temp_dir.path().join(".ruby-version"), "invalid\n").unwrap(); + + let detector = RubyVersionFileDetector; + assert!(detector.detect(temp_dir.path()).is_none()); + } + + #[test] + fn test_name() { + assert_eq!(RubyVersionFileDetector.name(), ".ruby-version"); + } +} diff --git a/crates/rb-core/tests/bundler_integration_tests.rs b/crates/rb-core/tests/bundler_integration_tests.rs index 773491f..2be44eb 100644 --- a/crates/rb-core/tests/bundler_integration_tests.rs +++ b/crates/rb-core/tests/bundler_integration_tests.rs @@ -10,12 +10,15 @@ fn bundler_detector_integrates_with_bundler_sandbox() -> io::Result<()> { // Create a configured bundler project let project_dir = sandbox.add_bundler_project("my-rails-app", true)?; - // Detector should find the bundler runtime + // Detector should find the bundler root let result = BundlerRuntimeDetector::discover(&project_dir)?; assert!(result.is_some()); - let bundler_runtime = result.unwrap(); - assert_eq!(bundler_runtime.root, project_dir); + let bundler_root = result.unwrap(); + assert_eq!(bundler_root, project_dir); + + // Create runtime to verify configuration + let bundler_runtime = BundlerRuntime::new(&bundler_root, Version::new(3, 3, 7)); assert!(bundler_runtime.is_configured()); Ok(()) @@ -28,14 +31,14 @@ fn bundler_detector_finds_gemfile_from_nested_directory() -> io::Result<()> { // Create complex project structure let (root_project, _subproject, deep_dir) = sandbox.add_complex_project()?; - // Detector should find the main project Gemfile when searching from deep directory + // Detector should find the nearest Gemfile when searching from deep directory let result = BundlerRuntimeDetector::discover(&deep_dir)?; assert!(result.is_some()); - let bundler_runtime = result.unwrap(); + let bundler_root = result.unwrap(); // Should NOT find the root project, but rather the subproject - assert_ne!(bundler_runtime.root, root_project); - assert!(bundler_runtime.root.ends_with("engines/my-engine")); + assert_ne!(bundler_root, root_project); + assert!(bundler_root.ends_with("engines/my-engine")); Ok(()) } @@ -57,10 +60,11 @@ fn bundler_detector_returns_none_for_non_bundler_directory() -> io::Result<()> { #[test] fn bundler_runtime_provides_correct_paths_for_configured_project() -> io::Result<()> { let sandbox = BundlerSandbox::new()?; + let ruby_version = Version::new(3, 3, 7); // Create configured project let project_dir = sandbox.add_bundler_project("configured-app", true)?; - let bundler_runtime = BundlerRuntime::new(&project_dir); + let bundler_runtime = BundlerRuntime::new(&project_dir, ruby_version.clone()); // Check all paths assert_eq!(bundler_runtime.gemfile_path(), project_dir.join("Gemfile")); @@ -69,10 +73,14 @@ fn bundler_runtime_provides_correct_paths_for_configured_project() -> io::Result bundler_runtime.vendor_dir(), project_dir.join(".rb").join("vendor").join("bundler") ); - assert_eq!( - bundler_runtime.bin_dir(), - bundler_runtime.vendor_dir().join("bin") - ); + + // Bin dir should be in ruby-specific path: .rb/vendor/bundler/ruby/3.3.7/bin + let expected_bin = bundler_runtime + .vendor_dir() + .join("ruby") + .join("3.3.7") + .join("bin"); + assert_eq!(bundler_runtime.bin_dir(), expected_bin); // Should be configured since we created vendor structure assert!(bundler_runtime.is_configured()); @@ -86,7 +94,7 @@ fn bundler_runtime_not_configured_for_basic_project() -> io::Result<()> { // Create basic project (not configured) let project_dir = sandbox.add_bundler_project("basic-app", false)?; - let bundler_runtime = BundlerRuntime::new(&project_dir); + let bundler_runtime = BundlerRuntime::new(&project_dir, Version::new(3, 3, 7)); // Should not be configured since no vendor structure exists assert!(!bundler_runtime.is_configured()); @@ -109,7 +117,7 @@ fn bundler_runtime_detects_ruby_version_from_ruby_version_file() -> io::Result<( "3.2.5", )?; - let bundler_runtime = BundlerRuntime::new(&project_dir); + let bundler_runtime = BundlerRuntime::new(&project_dir, Version::new(3, 2, 5)); assert_eq!( bundler_runtime.ruby_version(), Some(Version::parse("3.2.5").unwrap()) @@ -140,7 +148,7 @@ gem 'puma', '~> 5.6' gemfile_content, )?; - let bundler_runtime = BundlerRuntime::new(&project_dir); + let bundler_runtime = BundlerRuntime::new(&project_dir, Version::new(3, 1, 2)); assert_eq!( bundler_runtime.ruby_version(), Some(Version::parse("3.1.2").unwrap()) @@ -170,13 +178,16 @@ gem 'rackup' gemfile_content, )?; - // Detector should find the project and preserve Ruby version + // Detector should find the project root let result = BundlerRuntimeDetector::discover(&project_dir)?; assert!(result.is_some()); - let bundler_runtime = result.unwrap(); + let bundler_root = result.unwrap(); + + use rb_core::ruby::CompositeDetector; + let detector = CompositeDetector::bundler(); assert_eq!( - bundler_runtime.ruby_version(), + detector.detect(&bundler_root), Some(Version::parse("3.3.1").unwrap()) ); @@ -213,7 +224,7 @@ gem 'rails' "3.2.3", )?; - let bundler_runtime = BundlerRuntime::new(&project_dir); + let bundler_runtime = BundlerRuntime::new(&project_dir, Version::new(3, 2, 3)); // Should prefer .ruby-version over Gemfile assert_eq!( bundler_runtime.ruby_version(), @@ -222,3 +233,66 @@ gem 'rails' Ok(()) } + +// New tests for bundler bin path with Ruby version +#[test] +fn bundler_runtime_bin_dir_includes_ruby_version() -> io::Result<()> { + let sandbox = BundlerSandbox::new()?; + let project_dir = sandbox.add_bundler_project("versioned-bins", true)?; + + // Test with Ruby 3.3.7 + let bundler_runtime = BundlerRuntime::new(&project_dir, Version::new(3, 3, 7)); + let bin_dir = bundler_runtime.bin_dir(); + + assert!(bin_dir.ends_with("ruby/3.3.7/bin")); + assert_eq!( + bin_dir, + project_dir + .join(".rb") + .join("vendor") + .join("bundler") + .join("ruby") + .join("3.3.7") + .join("bin") + ); + + Ok(()) +} + +#[test] +fn bundler_runtime_bin_dir_varies_by_ruby_version() -> io::Result<()> { + let sandbox = BundlerSandbox::new()?; + let project_dir = sandbox.add_bundler_project("multi-version", true)?; + + // Same project, different Ruby versions should have different bin dirs + let runtime_337 = BundlerRuntime::new(&project_dir, Version::new(3, 3, 7)); + let runtime_324 = BundlerRuntime::new(&project_dir, Version::new(3, 2, 4)); + + assert_ne!(runtime_337.bin_dir(), runtime_324.bin_dir()); + assert!(runtime_337.bin_dir().ends_with("ruby/3.3.7/bin")); + assert!(runtime_324.bin_dir().ends_with("ruby/3.2.4/bin")); + + Ok(()) +} + +#[test] +fn bundler_runtime_gem_dir_includes_ruby_version() -> io::Result<()> { + let sandbox = BundlerSandbox::new()?; + let project_dir = sandbox.add_bundler_project("versioned-gems", true)?; + + let bundler_runtime = BundlerRuntime::new(&project_dir, Version::new(3, 3, 7)); + let ruby_vendor = bundler_runtime.ruby_vendor_dir(&Version::new(3, 3, 7)); + + assert!(ruby_vendor.ends_with("ruby/3.3.7")); + assert_eq!( + ruby_vendor, + project_dir + .join(".rb") + .join("vendor") + .join("bundler") + .join("ruby") + .join("3.3.7") + ); + + Ok(()) +} diff --git a/crates/rb-core/tests/butler_integration_tests.rs b/crates/rb-core/tests/butler_integration_tests.rs index 2419db8..0092518 100644 --- a/crates/rb-core/tests/butler_integration_tests.rs +++ b/crates/rb-core/tests/butler_integration_tests.rs @@ -4,7 +4,30 @@ use rb_core::ruby::{RubyRuntime, RubyRuntimeDetector, RubyType}; use rb_tests::RubySandbox; use semver::Version; use std::io; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; + +/// RAII guard that restores the current directory when dropped +struct DirGuard { + original_dir: PathBuf, +} + +impl DirGuard { + fn new() -> io::Result { + Ok(Self { + original_dir: std::env::current_dir()?, + }) + } + + fn change_to(&self, path: impl AsRef) -> io::Result<()> { + std::env::set_current_dir(path) + } +} + +impl Drop for DirGuard { + fn drop(&mut self) { + let _ = std::env::set_current_dir(&self.original_dir); + } +} #[test] fn test_butler_runtime_with_only_ruby() -> io::Result<()> { @@ -212,6 +235,15 @@ fn test_butler_runtime_skip_bundler_flag() -> Result<(), Box Result<(), Box Result<(), Box> { + use rb_tests::BundlerSandbox; + + let ruby_sandbox = RubySandbox::new()?; + let bundler_sandbox = BundlerSandbox::new()?; + + // Create Ruby installation with executable + let ruby_dir = ruby_sandbox.add_ruby_dir("3.3.7")?; + std::fs::create_dir_all(ruby_dir.join("bin"))?; + let ruby_exe = ruby_dir.join("bin").join("ruby"); + std::fs::write(&ruby_exe, "#!/bin/sh\necho ruby")?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&ruby_exe, std::fs::Permissions::from_mode(0o755))?; + } - // Restore original directory - std::env::set_current_dir(original_dir)?; + let rubies = RubyRuntimeDetector::discover(ruby_sandbox.root())?; + let ruby = &rubies[0]; + + // Create gem runtime (user gems) + let _gem_runtime = GemRuntime::for_base_dir(&ruby_sandbox.gem_base_dir(), &ruby.version); + + // Create bundler project + let project_dir = bundler_sandbox.add_bundler_project("isolated-app", true)?; + let _bundler_runtime = rb_core::BundlerRuntime::new(&project_dir, ruby.version.clone()); + + // Scope to ensure proper cleanup ordering + { + // Change to project directory with RAII guard for automatic cleanup + let _dir_guard = DirGuard::new()?; + _dir_guard.change_to(&project_dir)?; + + // Discover runtime WITH bundler context + let runtime_with_bundler = ButlerRuntime::discover_and_compose_with_gem_base( + ruby_sandbox.root().to_path_buf(), + None, + None, + false, // don't skip bundler + )?; + + // CRITICAL: When bundler context is present, gem_runtime should be None (isolation) + assert!( + runtime_with_bundler.gem_runtime().is_none(), + "User gem runtime should NOT be available in bundler context (isolation)" + ); + + // Bundler runtime SHOULD be present + assert!( + runtime_with_bundler.bundler_runtime().is_some(), + "Bundler runtime should be detected" + ); + + // bin_dirs should NOT include user gem bin (only bundler bin + ruby bin) + let bin_dirs = runtime_with_bundler.bin_dirs(); + let has_bundler_bin = bin_dirs + .iter() + .any(|p| p.to_string_lossy().contains("bundler")); + let has_user_gem_bin = bin_dirs.iter().any(|p| { + p.to_string_lossy().contains(".gem") && !p.to_string_lossy().contains("bundler") + }); + + assert!(has_bundler_bin, "Should have bundler bin directory"); + assert!( + !has_user_gem_bin, + "Should NOT have user gem bin directory (isolation)" + ); + + // gem_dirs should NOT include user gem home (only bundler gems + ruby lib) + let gem_dirs = runtime_with_bundler.gem_dirs(); + let has_bundler_gems = gem_dirs + .iter() + .any(|p| p.to_string_lossy().contains("bundler")); + let has_user_gems = gem_dirs.iter().any(|p| { + p.to_string_lossy().contains(".gem") && !p.to_string_lossy().contains("bundler") + }); + + assert!(has_bundler_gems, "Should have bundler gem directory"); + assert!( + !has_user_gems, + "Should NOT have user gem directory (isolation)" + ); + + // DirGuard drops here, restoring directory before sandboxes are dropped + } + + Ok(()) +} + +/// Test that with --no-bundler flag, user gems ARE available +#[test] +fn test_no_bundler_flag_restores_user_gems() -> Result<(), Box> { + use rb_tests::BundlerSandbox; + + let ruby_sandbox = RubySandbox::new()?; + let bundler_sandbox = BundlerSandbox::new()?; + + // Create Ruby installation with executable + let ruby_dir = ruby_sandbox.add_ruby_dir("3.3.7")?; + std::fs::create_dir_all(ruby_dir.join("bin"))?; + let ruby_exe = ruby_dir.join("bin").join("ruby"); + std::fs::write(&ruby_exe, "#!/bin/sh\necho ruby")?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&ruby_exe, std::fs::Permissions::from_mode(0o755))?; + } + + let rubies = RubyRuntimeDetector::discover(ruby_sandbox.root())?; + let _ruby = &rubies[0]; + + // Create bundler project + let project_dir = bundler_sandbox.add_bundler_project("user-gems-app", true)?; + + // Scope to ensure proper cleanup ordering + { + // Change to project directory with RAII guard for automatic cleanup + let _dir_guard = DirGuard::new()?; + _dir_guard.change_to(&project_dir)?; + + // Discover runtime WITH --no-bundler flag + let runtime_no_bundler = ButlerRuntime::discover_and_compose_with_gem_base( + ruby_sandbox.root().to_path_buf(), + None, + None, + true, // skip bundler (--no-bundler) + )?; + + // Bundler should NOT be detected + assert!( + runtime_no_bundler.bundler_runtime().is_none(), + "Bundler should be skipped with --no-bundler flag" + ); + + // User gem runtime SHOULD be available now + assert!( + runtime_no_bundler.gem_runtime().is_some(), + "User gem runtime should be available with --no-bundler" + ); + + // bin_dirs should include user gem bin (NOT bundler bin) + let bin_dirs = runtime_no_bundler.bin_dirs(); + let has_bundler_bin = bin_dirs + .iter() + .any(|p| p.to_string_lossy().contains("bundler")); + let has_user_gem_bin = bin_dirs + .iter() + .any(|p| p.to_string_lossy().contains(".gem")); + + assert!(!has_bundler_bin, "Should NOT have bundler bin directory"); + assert!(has_user_gem_bin, "Should have user gem bin directory"); + + // gem_dirs should include user gem home (NOT bundler gems) + let gem_dirs = runtime_no_bundler.gem_dirs(); + let has_bundler_gems = gem_dirs + .iter() + .any(|p| p.to_string_lossy().contains("bundler")); + let has_user_gems = gem_dirs + .iter() + .any(|p| p.to_string_lossy().contains(".gem")); + + assert!(!has_bundler_gems, "Should NOT have bundler gem directory"); + assert!(has_user_gems, "Should have user gem directory"); + + // DirGuard drops here, restoring directory before sandboxes are dropped + } + + Ok(()) +} + +/// Test that bundler bin paths include Ruby version directory +#[test] +fn test_bundler_bin_paths_include_ruby_version() -> Result<(), Box> { + use rb_tests::BundlerSandbox; + + let ruby_sandbox = RubySandbox::new()?; + let bundler_sandbox = BundlerSandbox::new()?; + + // Create Ruby installation with executable + let ruby_dir = ruby_sandbox.add_ruby_dir("3.3.7")?; + std::fs::create_dir_all(ruby_dir.join("bin"))?; + let ruby_exe = ruby_dir.join("bin").join("ruby"); + std::fs::write(&ruby_exe, "#!/bin/sh\necho ruby")?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&ruby_exe, std::fs::Permissions::from_mode(0o755))?; + } + + let rubies = RubyRuntimeDetector::discover(ruby_sandbox.root())?; + let _ruby = &rubies[0]; + + // Create bundler project + let project_dir = bundler_sandbox.add_bundler_project("versioned-bins", true)?; + + // Scope to ensure proper cleanup ordering + { + // Change to project directory with RAII guard for automatic cleanup + let _dir_guard = DirGuard::new()?; + _dir_guard.change_to(&project_dir)?; + + // Discover runtime with bundler + let runtime = ButlerRuntime::discover_and_compose_with_gem_base( + ruby_sandbox.root().to_path_buf(), + None, + None, + false, + )?; + + // Check that bundler bin path includes ruby version + let bin_dirs = runtime.bin_dirs(); + let bundler_bin = bin_dirs + .iter() + .find(|p| p.to_string_lossy().contains("bundler")) + .expect("Should have bundler bin directory"); + + // Should be: .rb/vendor/bundler/ruby/3.3.7/bin + let path_str = bundler_bin.to_string_lossy(); + assert!( + path_str.contains("ruby") && path_str.contains("3.3.7") && path_str.contains("bin"), + "Bundler bin should include Ruby version path: got {}", + bundler_bin.display() + ); + + // DirGuard drops here, restoring directory before sandboxes are dropped + } + // DirGuard automatically restores directory on drop Ok(()) } From fd60aac4e8922b478858c98299871d68c472e60e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20=C5=A0im=C3=A1nek?= Date: Sun, 9 Nov 2025 15:09:03 +0100 Subject: [PATCH 2/2] Stabilize test, improve locators. --- crates/rb-cli/src/config/locator.rs | 92 ++++- crates/rb-cli/src/lib.rs | 16 +- crates/rb-core/src/bundler/mod.rs | 27 +- crates/rb-core/src/butler/mod.rs | 39 +- crates/rb-core/src/ruby/mod.rs | 3 + .../rb-core/src/ruby/version_detector/mod.rs | 10 +- crates/rb-core/src/ruby/version_ext.rs | 59 +++ .../tests/bundler_integration_tests.rs | 20 +- .../rb-core/tests/butler_integration_tests.rs | 354 ++++++++---------- 9 files changed, 372 insertions(+), 248 deletions(-) create mode 100644 crates/rb-core/src/ruby/version_ext.rs diff --git a/crates/rb-cli/src/config/locator.rs b/crates/rb-cli/src/config/locator.rs index ad0e7fe..10718fc 100644 --- a/crates/rb-cli/src/config/locator.rs +++ b/crates/rb-cli/src/config/locator.rs @@ -1,6 +1,20 @@ use log::debug; use std::path::PathBuf; +/// Trait for reading environment variables - allows mocking in tests +pub trait EnvReader { + fn var(&self, key: &str) -> Result; +} + +/// Production implementation using std::env +pub struct StdEnvReader; + +impl EnvReader for StdEnvReader { + fn var(&self, key: &str) -> Result { + std::env::var(key) + } +} + /// Locate the configuration file following XDG Base Directory specification /// /// Supports both rb.kdl and rb.toml (preferring .kdl) @@ -13,6 +27,14 @@ use std::path::PathBuf; /// 5. %APPDATA%/rb/rb.kdl or rb.toml (Windows) /// 6. ~/.rb.kdl or ~/.rb.toml (cross-platform fallback) pub fn locate_config_file(override_path: Option) -> Option { + locate_config_file_with_env(override_path, &StdEnvReader) +} + +/// Internal function that accepts an environment reader for testing +fn locate_config_file_with_env( + override_path: Option, + env: &dyn EnvReader, +) -> Option { debug!("Searching for configuration file..."); // 1. Check for explicit override first @@ -25,7 +47,7 @@ pub fn locate_config_file(override_path: Option) -> Option { } // 2. Check RB_CONFIG environment variable - if let Ok(rb_config) = std::env::var("RB_CONFIG") { + if let Ok(rb_config) = env.var("RB_CONFIG") { let config_path = PathBuf::from(rb_config); debug!(" Checking RB_CONFIG env var: {}", config_path.display()); if config_path.exists() { @@ -35,7 +57,7 @@ pub fn locate_config_file(override_path: Option) -> Option { } // 3. Try XDG_CONFIG_HOME (Unix/Linux) - if let Ok(xdg_config) = std::env::var("XDG_CONFIG_HOME") { + if let Ok(xdg_config) = env.var("XDG_CONFIG_HOME") { let base_path = PathBuf::from(xdg_config).join("rb"); // Try .kdl first, then .toml for ext in &["rb.kdl", "rb.toml"] { @@ -98,6 +120,34 @@ pub fn locate_config_file(override_path: Option) -> Option { #[cfg(test)] mod tests { use super::*; + use std::collections::HashMap; + + /// Mock environment reader for testing without global state mutation + struct MockEnvReader { + vars: HashMap, + } + + impl MockEnvReader { + fn new() -> Self { + Self { + vars: HashMap::new(), + } + } + + fn with_var(mut self, key: impl Into, value: impl Into) -> Self { + self.vars.insert(key.into(), value.into()); + self + } + } + + impl EnvReader for MockEnvReader { + fn var(&self, key: &str) -> Result { + self.vars + .get(key) + .cloned() + .ok_or(std::env::VarError::NotPresent) + } + } #[test] fn test_locate_config_file_returns_option() { @@ -127,24 +177,44 @@ mod tests { fn test_locate_config_file_with_env_var() { use std::fs; let temp_dir = std::env::temp_dir(); - let config_path = temp_dir.join("test_rb_env.toml"); + let config_path = temp_dir.join("test_rb_env_mock.toml"); // Create a temporary config file fs::write(&config_path, "# test config").expect("Failed to write test config"); - // Set environment variable (unsafe but required for testing) - unsafe { - std::env::set_var("RB_CONFIG", &config_path); - } + // Use mock environment - no global state mutation! + let mock_env = + MockEnvReader::new().with_var("RB_CONFIG", config_path.to_string_lossy().to_string()); // Should return the env var path - let result = locate_config_file(None); + let result = locate_config_file_with_env(None, &mock_env); assert_eq!(result, Some(config_path.clone())); // Cleanup - unsafe { - std::env::remove_var("RB_CONFIG"); - } let _ = fs::remove_file(&config_path); } + + #[test] + fn test_locate_config_file_with_xdg_config_home() { + use std::fs; + let temp_dir = std::env::temp_dir(); + let xdg_base = temp_dir.join("test_xdg_config"); + let rb_dir = xdg_base.join("rb"); + let config_path = rb_dir.join("rb.toml"); + + // Create directory structure + fs::create_dir_all(&rb_dir).expect("Failed to create test directory"); + fs::write(&config_path, "# test config").expect("Failed to write test config"); + + // Use mock environment + let mock_env = MockEnvReader::new() + .with_var("XDG_CONFIG_HOME", xdg_base.to_string_lossy().to_string()); + + // Should return the XDG config path + let result = locate_config_file_with_env(None, &mock_env); + assert_eq!(result, Some(config_path.clone())); + + // Cleanup + let _ = fs::remove_dir_all(&xdg_base); + } } diff --git a/crates/rb-cli/src/lib.rs b/crates/rb-cli/src/lib.rs index 219d60f..fab676e 100644 --- a/crates/rb-cli/src/lib.rs +++ b/crates/rb-cli/src/lib.rs @@ -242,7 +242,21 @@ mod tests { .expect("Failed to set permissions"); } - let result = create_ruby_context(Some(sandbox.root().to_path_buf()), None); + // Create gem directories so gem runtime is inferred + let gem_base = sandbox.gem_base_dir(); + let gem_dir = gem_base.join("3.2.5"); + std::fs::create_dir_all(&gem_dir).expect("Failed to create gem dir"); + + // Use the internal method that accepts current_dir to avoid global state + use rb_core::butler::ButlerRuntime; + let result = ButlerRuntime::discover_and_compose_with_current_dir( + sandbox.root().to_path_buf(), + None, + None, + false, + sandbox.root().to_path_buf(), // Current dir = sandbox root + ) + .expect("Failed to create ButlerRuntime"); // Should successfully create a ButlerRuntime let current_path = std::env::var("PATH").ok(); diff --git a/crates/rb-core/src/bundler/mod.rs b/crates/rb-core/src/bundler/mod.rs index 77e2a84..d232d58 100644 --- a/crates/rb-core/src/bundler/mod.rs +++ b/crates/rb-core/src/bundler/mod.rs @@ -1,5 +1,6 @@ use crate::butler::Command; use crate::butler::runtime_provider::RuntimeProvider; +use crate::ruby::RubyVersionExt; use log::debug; use semver::Version; use std::path::{Path, PathBuf}; @@ -40,14 +41,12 @@ impl BundlerRuntime { self.app_config_dir().join("vendor").join("bundler") } - /// Returns the ruby-specific vendor directory (.rb/vendor/bundler/ruby/X.Y.Z) - /// This requires a Ruby version to be detected + /// Returns the ruby-specific vendor directory (.rb/vendor/bundler/ruby/X.Y.0) + /// Uses Ruby ABI version (major.minor.0) for compatibility grouping pub fn ruby_vendor_dir(&self, ruby_version: &Version) -> PathBuf { - let version_str = format!( - "{}.{}.{}", - ruby_version.major, ruby_version.minor, ruby_version.patch - ); - self.vendor_dir().join("ruby").join(version_str) + self.vendor_dir() + .join("ruby") + .join(ruby_version.ruby_abi_version()) } /// Detect Ruby version from .ruby-version file or Gemfile ruby declaration @@ -58,7 +57,7 @@ impl BundlerRuntime { } /// Returns the bin directory where bundler-installed executables live - /// Path: .rb/vendor/bundler/ruby/X.Y.Z/bin + /// Path: .rb/vendor/bundler/ruby/X.Y.0/bin pub fn bin_dir(&self) -> PathBuf { let bin_dir = self.ruby_vendor_dir(&self.ruby_version).join("bin"); debug!("Bundler bin directory: {}", bin_dir.display()); @@ -418,8 +417,8 @@ mod tests { #[test] fn bin_dir_is_vendor_bin() { let br = bundler_rt("/home/user/project"); - // bin_dir should include Ruby version: .rb/vendor/bundler/ruby/3.3.7/bin - let expected = Path::new("/home/user/project/.rb/vendor/bundler/ruby/3.3.7/bin"); + // bin_dir should include Ruby minor version: .rb/vendor/bundler/ruby/3.3.0/bin + let expected = Path::new("/home/user/project/.rb/vendor/bundler/ruby/3.3.0/bin"); assert_eq!(br.bin_dir(), expected); } @@ -432,10 +431,10 @@ mod tests { // Should be configured since we created vendor structure assert!(br.is_configured()); - // bin_dir should include Ruby version path - let expected_bin = br.vendor_dir().join("ruby").join("3.3.7").join("bin"); - // gem_dir should be the Ruby-specific vendor directory - let expected_gem = br.vendor_dir().join("ruby").join("3.3.7"); + // bin_dir should include Ruby minor version path (X.Y.0) + let expected_bin = br.vendor_dir().join("ruby").join("3.3.0").join("bin"); + // gem_dir should be the Ruby-minor-specific vendor directory + let expected_gem = br.vendor_dir().join("ruby").join("3.3.0"); assert_eq!( ::bin_dir(&br), diff --git a/crates/rb-core/src/butler/mod.rs b/crates/rb-core/src/butler/mod.rs index e2551b8..b361d73 100644 --- a/crates/rb-core/src/butler/mod.rs +++ b/crates/rb-core/src/butler/mod.rs @@ -129,6 +129,29 @@ impl ButlerRuntime { ButlerError::General(format!("Unable to determine current directory: {}", e)) })?; + Self::discover_and_compose_with_current_dir( + rubies_dir, + requested_ruby_version, + gem_base_dir, + skip_bundler, + current_dir, + ) + } + + /// Internal method: Perform comprehensive environment discovery with explicit current directory + /// + /// This method accepts the current directory as a parameter instead of reading it from + /// the environment, which makes it suitable for testing without global state mutation. + /// + /// Note: This method is primarily intended for testing but is made public to allow + /// flexible usage patterns where the current directory needs to be explicitly controlled. + pub fn discover_and_compose_with_current_dir( + rubies_dir: PathBuf, + requested_ruby_version: Option, + gem_base_dir: Option, + skip_bundler: bool, + current_dir: PathBuf, + ) -> Result { debug!("Starting comprehensive environment discovery"); debug!("Rubies directory: {}", rubies_dir.display()); debug!("Current directory: {}", current_dir.display()); @@ -197,18 +220,18 @@ impl ButlerRuntime { bundler_root.map(|root| BundlerRuntime::new(root, selected_ruby.version.clone())); // Step 6: Create gem runtime (using custom base directory if provided) - // IMPORTANT: When in bundler context, user gems are NOT available for isolation - let gem_runtime = if bundler_runtime.is_some() { + // IMPORTANT: Custom gem base (-G flag) takes precedence over bundler isolation + let gem_runtime = if let Some(ref custom_gem_base) = gem_base_dir { debug!( - "Bundler context detected - user gems will NOT be available (bundler isolation)" - ); - None - } else if let Some(ref custom_gem_base) = gem_base_dir { - debug!( - "Using custom gem base directory: {}", + "Using custom gem base directory (overrides bundler isolation): {}", custom_gem_base.display() ); Some(selected_ruby.gem_runtime_for_base(custom_gem_base)) + } else if bundler_runtime.is_some() { + debug!( + "Bundler context detected - user gems will NOT be available (bundler isolation)" + ); + None } else { match selected_ruby.infer_gem_runtime() { Ok(gem_runtime) => { diff --git a/crates/rb-core/src/ruby/mod.rs b/crates/rb-core/src/ruby/mod.rs index 4c49889..966db04 100644 --- a/crates/rb-core/src/ruby/mod.rs +++ b/crates/rb-core/src/ruby/mod.rs @@ -6,9 +6,12 @@ use std::env::consts::EXE_SUFFIX; use std::path::{Path, PathBuf}; pub mod version_detector; +pub mod version_ext; + pub use version_detector::{ CompositeDetector, GemfileDetector, RubyVersionDetector, RubyVersionFileDetector, }; +pub use version_ext::RubyVersionExt; /// Errors that can occur during Ruby discovery #[derive(Debug, Clone)] diff --git a/crates/rb-core/src/ruby/version_detector/mod.rs b/crates/rb-core/src/ruby/version_detector/mod.rs index 9bf9e9a..f5d46bc 100644 --- a/crates/rb-core/src/ruby/version_detector/mod.rs +++ b/crates/rb-core/src/ruby/version_detector/mod.rs @@ -26,7 +26,7 @@ //! # Usage //! //! For standard Ruby projects: -//! ```rust,ignore +//! ```text //! use rb_core::ruby::CompositeDetector; //! //! let detector = CompositeDetector::standard(); @@ -36,7 +36,7 @@ //! ``` //! //! For bundler-managed projects: -//! ```rust,ignore +//! ```text //! let detector = CompositeDetector::bundler(); //! let version = detector.detect(bundler_root); //! ``` @@ -46,7 +46,7 @@ //! To add support for new version sources (e.g., `.tool-versions` for asdf): //! //! 1. Implement the `RubyVersionDetector` trait: -//! ```rust,ignore +//! ```text //! pub struct ToolVersionsDetector; //! impl RubyVersionDetector for ToolVersionsDetector { //! fn detect(&self, context: &Path) -> Option { @@ -57,7 +57,7 @@ //! ``` //! //! 2. Add to the detector chain: -//! ```rust,ignore +//! ```text //! CompositeDetector { //! detectors: vec![ //! Box::new(RubyVersionFileDetector), @@ -66,8 +66,6 @@ //! ] //! } //! ``` -//! -//! See `tool_versions.rs` for a complete example implementation. use log::debug; use semver::Version; diff --git a/crates/rb-core/src/ruby/version_ext.rs b/crates/rb-core/src/ruby/version_ext.rs new file mode 100644 index 0000000..96bf91c --- /dev/null +++ b/crates/rb-core/src/ruby/version_ext.rs @@ -0,0 +1,59 @@ +//! Extension methods for semver::Version to support Ruby-specific version formats + +use semver::Version; + +/// Extension trait for Ruby ABI version formatting +/// +/// Ruby uses a "ruby_version" (RbConfig::CONFIG["ruby_version"]) which represents +/// the ABI compatibility level. This is the major.minor version with patch always 0. +/// For example, Ruby 3.3.7 has ruby_version "3.3.0", and Ruby 3.4.5 has "3.4.0". +/// +/// This is used for: +/// - Library installation paths (e.g., `/usr/lib/ruby/3.3.0/`) +/// - Bundler vendor directories (e.g., `.rb/vendor/bundler/ruby/3.3.0/`) +/// - Native extension compatibility checks +pub trait RubyVersionExt { + /// Returns the Ruby ABI version string (major.minor.0) + /// + /// This corresponds to RbConfig::CONFIG["ruby_version"] in Ruby. + /// + /// # Examples + /// + /// ``` + /// use semver::Version; + /// use rb_core::ruby::RubyVersionExt; + /// + /// let v = Version::new(3, 3, 7); + /// assert_eq!(v.ruby_abi_version(), "3.3.0"); + /// + /// let v = Version::new(3, 4, 5); + /// assert_eq!(v.ruby_abi_version(), "3.4.0"); + /// ``` + fn ruby_abi_version(&self) -> String; +} + +impl RubyVersionExt for Version { + fn ruby_abi_version(&self) -> String { + format!("{}.{}.0", self.major, self.minor) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_ruby_abi_version() { + let v = Version::new(3, 3, 7); + assert_eq!(v.ruby_abi_version(), "3.3.0"); + + let v = Version::new(3, 4, 5); + assert_eq!(v.ruby_abi_version(), "3.4.0"); + + let v = Version::new(2, 7, 8); + assert_eq!(v.ruby_abi_version(), "2.7.0"); + + let v = Version::new(3, 3, 0); + assert_eq!(v.ruby_abi_version(), "3.3.0"); + } +} diff --git a/crates/rb-core/tests/bundler_integration_tests.rs b/crates/rb-core/tests/bundler_integration_tests.rs index 2be44eb..68a470f 100644 --- a/crates/rb-core/tests/bundler_integration_tests.rs +++ b/crates/rb-core/tests/bundler_integration_tests.rs @@ -74,11 +74,11 @@ fn bundler_runtime_provides_correct_paths_for_configured_project() -> io::Result project_dir.join(".rb").join("vendor").join("bundler") ); - // Bin dir should be in ruby-specific path: .rb/vendor/bundler/ruby/3.3.7/bin + // Bin dir should be in ruby-minor-specific path: .rb/vendor/bundler/ruby/3.3.0/bin let expected_bin = bundler_runtime .vendor_dir() .join("ruby") - .join("3.3.7") + .join("3.3.0") .join("bin"); assert_eq!(bundler_runtime.bin_dir(), expected_bin); @@ -240,11 +240,11 @@ fn bundler_runtime_bin_dir_includes_ruby_version() -> io::Result<()> { let sandbox = BundlerSandbox::new()?; let project_dir = sandbox.add_bundler_project("versioned-bins", true)?; - // Test with Ruby 3.3.7 + // Test with Ruby 3.3.7 - should use 3.3.0 directory let bundler_runtime = BundlerRuntime::new(&project_dir, Version::new(3, 3, 7)); let bin_dir = bundler_runtime.bin_dir(); - assert!(bin_dir.ends_with("ruby/3.3.7/bin")); + assert!(bin_dir.ends_with("ruby/3.3.0/bin")); assert_eq!( bin_dir, project_dir @@ -252,7 +252,7 @@ fn bundler_runtime_bin_dir_includes_ruby_version() -> io::Result<()> { .join("vendor") .join("bundler") .join("ruby") - .join("3.3.7") + .join("3.3.0") .join("bin") ); @@ -264,13 +264,13 @@ fn bundler_runtime_bin_dir_varies_by_ruby_version() -> io::Result<()> { let sandbox = BundlerSandbox::new()?; let project_dir = sandbox.add_bundler_project("multi-version", true)?; - // Same project, different Ruby versions should have different bin dirs + // Same project, different Ruby minor versions should have different bin dirs let runtime_337 = BundlerRuntime::new(&project_dir, Version::new(3, 3, 7)); let runtime_324 = BundlerRuntime::new(&project_dir, Version::new(3, 2, 4)); assert_ne!(runtime_337.bin_dir(), runtime_324.bin_dir()); - assert!(runtime_337.bin_dir().ends_with("ruby/3.3.7/bin")); - assert!(runtime_324.bin_dir().ends_with("ruby/3.2.4/bin")); + assert!(runtime_337.bin_dir().ends_with("ruby/3.3.0/bin")); + assert!(runtime_324.bin_dir().ends_with("ruby/3.2.0/bin")); Ok(()) } @@ -283,7 +283,7 @@ fn bundler_runtime_gem_dir_includes_ruby_version() -> io::Result<()> { let bundler_runtime = BundlerRuntime::new(&project_dir, Version::new(3, 3, 7)); let ruby_vendor = bundler_runtime.ruby_vendor_dir(&Version::new(3, 3, 7)); - assert!(ruby_vendor.ends_with("ruby/3.3.7")); + assert!(ruby_vendor.ends_with("ruby/3.3.0")); assert_eq!( ruby_vendor, project_dir @@ -291,7 +291,7 @@ fn bundler_runtime_gem_dir_includes_ruby_version() -> io::Result<()> { .join("vendor") .join("bundler") .join("ruby") - .join("3.3.7") + .join("3.3.0") ); Ok(()) diff --git a/crates/rb-core/tests/butler_integration_tests.rs b/crates/rb-core/tests/butler_integration_tests.rs index 0092518..0808d4e 100644 --- a/crates/rb-core/tests/butler_integration_tests.rs +++ b/crates/rb-core/tests/butler_integration_tests.rs @@ -4,30 +4,7 @@ use rb_core::ruby::{RubyRuntime, RubyRuntimeDetector, RubyType}; use rb_tests::RubySandbox; use semver::Version; use std::io; -use std::path::{Path, PathBuf}; - -/// RAII guard that restores the current directory when dropped -struct DirGuard { - original_dir: PathBuf, -} - -impl DirGuard { - fn new() -> io::Result { - Ok(Self { - original_dir: std::env::current_dir()?, - }) - } - - fn change_to(&self, path: impl AsRef) -> io::Result<()> { - std::env::set_current_dir(path) - } -} - -impl Drop for DirGuard { - fn drop(&mut self) { - let _ = std::env::set_current_dir(&self.original_dir); - } -} +use std::path::PathBuf; #[test] fn test_butler_runtime_with_only_ruby() -> io::Result<()> { @@ -251,38 +228,31 @@ fn test_butler_runtime_skip_bundler_flag() -> Result<(), Box Result<(), Box Result<(), Box Result<(), Box