diff --git a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs index ec70c30f0..d0edbb072 100644 --- a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs @@ -1703,7 +1703,8 @@ pub(crate) async fn bundler_loaded_manifest_in( ProjectView::Snapshot(snap) => { // The only files the probe reads: the app config, the global // config and `gems.rb` (the environment it also reads is fixed - // for the run). + // for the run, and the directory identity of a configured + // absolute path is not a file the snapshot fingerprints). let root = snap.root_reading(bundler_config_files(snap.root_reading::<&Path>([]))); bundler_loaded_manifest(root).await } @@ -1713,7 +1714,7 @@ pub(crate) async fn bundler_loaded_manifest_in( let lockfile = config.as_deref().and_then(manifest::config_lockfile); let root = Path::new("/"); let gems_rb = view.is_file("gems.rb"); - let loaded = manifest::classify(root, None, gemfile.as_deref(), None); + let loaded = manifest::classify_lexical(root, gemfile.as_deref()); // `/` only stands in for the project root: bundler opens an // absolute lockfile as is, never the project's own lock, so // it must not compare equal to the pair's lock at `/`. @@ -1725,7 +1726,7 @@ pub(crate) async fn bundler_loaded_manifest_in( }; } } - loaded.with_lockfile(root, None, lockfile.as_deref(), None, gems_rb) + loaded.with_lockfile_lexical(root, lockfile.as_deref(), gems_rb) } } } @@ -3742,6 +3743,41 @@ mod tests { ); } + /// #896: a project reached through a symlinked directory, with + /// `bundle config set --local gemfile "$PWD/Gemfile"` written through the + /// link (or the physical path with `--cwd` through the link): bundler + /// loads the project's own Gemfile, so the app config is followed, not + /// refused as another manifest. + #[cfg(unix)] + #[tokio::test] + async fn loaded_manifest_follows_a_symlinked_spelling_of_the_project() { + use crate::formats::gem::manifest::{GemfileSetting, LoadedManifest}; + let dir = tempfile::tempdir().unwrap(); + let real = dir.path().join("real/app"); + std::fs::create_dir_all(real.join(".bundle")).unwrap(); + std::fs::write(real.join("Gemfile"), "").unwrap(); + std::os::unix::fs::symlink(dir.path().join("real"), dir.path().join("link")).unwrap(); + let link = dir.path().join("link/app"); + for (root, spelled) in [(&real, &link), (&link, &real)] { + std::fs::write( + real.join(".bundle/config"), + format!( + "---\nBUNDLE_GEMFILE: \"{}\"\n", + spelled.join("Gemfile").display() + ), + ) + .unwrap(); + assert_eq!( + bundler_loaded_manifest_with_env(root, BundlerEnv::default()).await, + LoadedManifest::Configured { + manifest: "Gemfile", + by: GemfileSetting::AppConfig + }, + "{root:?}" + ); + } + } + /// #577: a global `gemfile` setting is the third tier: below the local /// config and the environment, and refused like a local one when it /// names a manifest socket-patch does not wire. diff --git a/crates/socket-patch-core/src/formats/gem/manifest.rs b/crates/socket-patch-core/src/formats/gem/manifest.rs index 2b5ef9d1d..41d8ff1c3 100644 --- a/crates/socket-patch-core/src/formats/gem/manifest.rs +++ b/crates/socket-patch-core/src/formats/gem/manifest.rs @@ -43,8 +43,13 @@ //! nothing on disk says which bundler runs: callers refuse it with //! [`twin_manifest_refusal`] (#751). //! -//! The model is pure: -//! the disk and environment reads live in +//! A configured path names a root file when its file name matches and its +//! directory is the root's, compared lexically and, failing that, by the +//! directories' resolved identity: bundler expands the setting lexically +//! and opens it through the OS, so a symlinked spelling of the project +//! directory (macOS `/tmp` → `/private/tmp`) names the same file (#896). +//! That directory lookup is the model's only disk read; the other disk and +//! environment reads live in //! [`crate::crawlers::ruby_crawler::bundler_loaded_manifest`]. use std::ffi::OsStr; @@ -182,6 +187,44 @@ impl LoadedManifest { lockfile_config: Option<&str>, lockfile_global: Option<&str>, gems_rb_present: bool, + ) -> LoadedManifest { + self.with_lockfile_by( + root, + lockfile_env, + lockfile_config, + lockfile_global, + gems_rb_present, + same_dir_on_disk, + ) + } + + /// [`LoadedManifest::with_lockfile`] for a `root` that only stands in + /// for a project not on this disk (an in-memory view): paths compare + /// lexically, never through this host's directories. + pub(crate) fn with_lockfile_lexical( + self, + root: &Path, + lockfile_config: Option<&str>, + gems_rb_present: bool, + ) -> LoadedManifest { + self.with_lockfile_by( + root, + None, + lockfile_config, + None, + gems_rb_present, + |a, b| a == b, + ) + } + + fn with_lockfile_by( + self, + root: &Path, + lockfile_env: Option<&OsStr>, + lockfile_config: Option<&str>, + lockfile_global: Option<&str>, + gems_rb_present: bool, + same_dir: SameDir, ) -> LoadedManifest { let Some((_, lock)) = self.pair(gems_rb_present) else { return self; @@ -200,8 +243,10 @@ impl LoadedManifest { } let target = resolve_against(root, &value); let expected = resolve_against(root, Path::new(lock)); - if target.is_some() && target == expected { - return self; + if let (Some(target), Some(expected)) = (target, expected) { + if same_entry(&target, &expected, same_dir) { + return self; + } } LoadedManifest::UnsupportedLockfile { value: value.display().to_string(), @@ -252,6 +297,27 @@ fn resolve_against(root: &Path, value: &Path) -> Option { .and_then(|p| normalize_lexically(&p)) } +/// How two resolved directories are judged the same directory. +type SameDir = fn(&Path, &Path) -> bool; + +/// Whether two resolved directories are the same directory on this disk: +/// equal, or the same directory by file identity (a symlinked spelling, a +/// bind mount, a Windows junction). +fn same_dir_on_disk(a: &Path, b: &Path) -> bool { + a == b || same_file::is_same_file(a, b).unwrap_or(false) +} + +/// Whether two resolved paths name the same directory entry: the same file +/// name in the `same_dir` directory. Only the directory is resolved, so a +/// root `Gemfile` that is itself a symlink to another manifest stays +/// distinct from that manifest, as bundler's root follows the spelled +/// directory. +fn same_entry(a: &Path, b: &Path, same_dir: SameDir) -> bool { + a == b + || (a.file_name() == b.file_name() + && matches!((a.parent(), b.parent()), (Some(a), Some(b)) if same_dir(a, b))) +} + /// Classify the configured `BUNDLE_GEMFILE` against `root`, which also /// anchors a relative value: the app config value first, then the /// environment — unless the environment names a manifest outside `root`, @@ -263,13 +329,39 @@ pub fn classify( gemfile_env: Option<&OsStr>, config_value: Option<&str>, global_value: Option<&str>, +) -> LoadedManifest { + classify_by( + root, + gemfile_env, + config_value, + global_value, + same_dir_on_disk, + ) +} + +/// [`classify`] of an app config value for a `root` that only stands in +/// for a project not on this disk (an in-memory view): paths compare +/// lexically, never through this host's directories. +pub(crate) fn classify_lexical(root: &Path, config_value: Option<&str>) -> LoadedManifest { + classify_by(root, None, config_value, None, |a, b| a == b) +} + +fn classify_by( + root: &Path, + gemfile_env: Option<&OsStr>, + config_value: Option<&str>, + global_value: Option<&str>, + same_dir: SameDir, ) -> LoadedManifest { let env = gemfile_env.filter(|v| !v.is_empty()).map(PathBuf::from); let config = config_value.filter(|v| !v.is_empty()).map(PathBuf::from); let global = global_value.filter(|v| !v.is_empty()).map(PathBuf::from); let env_keeps_root = |env: &Path| { let dir = resolve_against(root, env).and_then(|p| p.parent().map(Path::to_path_buf)); - dir.is_some() && dir == resolve_against(root, Path::new("")) + match (dir, resolve_against(root, Path::new(""))) { + (Some(dir), Some(root)) => same_dir(&dir, &root), + _ => false, + } }; let (value, by) = match (env, config) { (Some(env), Some(config)) if env_keeps_root(&env) => (config, GemfileSetting::AppConfig), @@ -290,8 +382,11 @@ pub fn classify( let target = resolve_against(root, &value); let root = resolve_against(root, Path::new("")); if let (Some(target), Some(root)) = (target, root) { - for manifest in ["Gemfile", "gems.rb"] { - if target == root.join(manifest) { + let manifest = ["Gemfile", "gems.rb"] + .into_iter() + .find(|m| target.file_name() == Some(OsStr::new(m))); + if let Some(manifest) = manifest { + if same_entry(&target, &root.join(manifest), same_dir) { return LoadedManifest::Configured { manifest, by }; } } @@ -609,6 +704,125 @@ mod tests { } } + /// A project directory reached through a symlink (macOS `/tmp` → + /// `/private/tmp`, a symlinked workspace): bundler expands + /// `BUNDLE_GEMFILE` lexically and opens it through the OS, so a value + /// spelling the root's own `Gemfile` through the link is that Gemfile, + /// whichever side (the setting or `--cwd`) uses the link (#896). + #[cfg(unix)] + #[test] + fn a_symlinked_spelling_of_the_root_is_the_roots_own_manifest() { + let tmp = tempfile::tempdir().unwrap(); + let real = tmp.path().join("real/app"); + std::fs::create_dir_all(&real).unwrap(); + std::fs::write(real.join("Gemfile"), "").unwrap(); + std::os::unix::fs::symlink(tmp.path().join("real"), tmp.path().join("link")).unwrap(); + let link = tmp.path().join("link/app"); + let configured = |by| LoadedManifest::Configured { + manifest: "Gemfile", + by, + }; + for (root, spelled) in [(&real, &link), (&link, &real)] { + let value = spelled.join("Gemfile"); + // `BUNDLE_GEMFILE=$PWD/Gemfile` with a logical `$PWD`. + assert_eq!( + classify(root, Some(value.as_os_str()), None, None), + configured(GemfileSetting::Env), + "{root:?} {value:?}" + ); + // `bundle config set --local gemfile "$PWD/Gemfile"`. + assert_eq!( + classify(root, None, value.to_str(), None), + configured(GemfileSetting::AppConfig) + ); + assert_eq!( + classify(root, None, None, value.to_str()), + configured(GemfileSetting::GlobalConfig) + ); + // The env value keeps bundler's root (same directory), so the + // app config still wins over it. + let next = spelled.join("Gemfile.next"); + assert_eq!( + classify(root, Some(next.as_os_str()), Some("Gemfile"), None), + configured(GemfileSetting::AppConfig) + ); + // `BUNDLE_LOCKFILE` naming the pair's own lock the same way. + let lock = spelled.join("Gemfile.lock"); + assert_eq!( + classify(root, None, None, None).with_lockfile( + root, + Some(lock.as_os_str()), + None, + None, + false + ), + LoadedManifest::Default + ); + // Another file in the same directory is still another manifest. + assert!(matches!( + classify(root, Some(next.as_os_str()), None, None), + LoadedManifest::Unsupported { .. } + )); + } + } + + /// An in-memory view's root only stands in for a project that is not on + /// this disk, so its paths never resolve through this host's + /// directories: a symlinked spelling stays another manifest there. + #[cfg(unix)] + #[test] + fn the_lexical_variants_never_read_this_hosts_directories() { + let tmp = tempfile::tempdir().unwrap(); + let real = tmp.path().join("real/app"); + std::fs::create_dir_all(&real).unwrap(); + std::os::unix::fs::symlink(tmp.path().join("real"), tmp.path().join("link")).unwrap(); + let link = tmp.path().join("link/app"); + let value = link.join("Gemfile"); + assert_eq!( + classify_lexical(&real, value.to_str()), + LoadedManifest::Unsupported { + value: value.display().to_string(), + by: GemfileSetting::AppConfig + } + ); + assert_eq!( + classify_lexical(&real, Some("Gemfile")), + LoadedManifest::Configured { + manifest: "Gemfile", + by: GemfileSetting::AppConfig + } + ); + let lock = link.join("Gemfile.lock"); + assert!(matches!( + LoadedManifest::Default.with_lockfile_lexical(&real, lock.to_str(), false), + LoadedManifest::UnsupportedLockfile { .. } + )); + } + + /// Only the DIRECTORY is resolved: a root `Gemfile` that is itself a + /// symlink to a manifest elsewhere does not make that manifest the + /// project's, since bundler's root (and its lock) follow the spelled + /// directory, not the link target. + #[cfg(unix)] + #[test] + fn a_manifest_the_roots_gemfile_links_to_is_still_another_manifest() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("app"); + let other = tmp.path().join("other"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::create_dir_all(&other).unwrap(); + std::fs::write(other.join("Gemfile"), "").unwrap(); + std::os::unix::fs::symlink(other.join("Gemfile"), root.join("Gemfile")).unwrap(); + let value = other.join("Gemfile"); + assert_eq!( + classify(&root, Some(value.as_os_str()), None, None), + LoadedManifest::Unsupported { + value: value.display().to_string(), + by: GemfileSetting::Env + } + ); + } + #[test] fn config_lockfile_reads_bundlers_own_spelling() { assert_eq!(