Skip to content

Commit 0528f9f

Browse files
committed
Compare gem manifest dirs by file identity
Address review of the symlink fix: decide directory identity with same-file (device and inode) so a bind mount or Windows junction also counts, check it once per classify, and keep an in-memory project's paths lexical so the host's own directories never decide its verdict. Refs #896 Assisted-by: Claude Code:claude-opus-5-5
1 parent 6fc7454 commit 0528f9f

2 files changed

Lines changed: 114 additions & 16 deletions

File tree

‎crates/socket-patch-core/src/crawlers/ruby_crawler.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1703,7 +1703,8 @@ pub(crate) async fn bundler_loaded_manifest_in(
17031703
ProjectView::Snapshot(snap) => {
17041704
// The only files the probe reads: the app config, the global
17051705
// config and `gems.rb` (the environment it also reads is fixed
1706-
// for the run).
1706+
// for the run, and the directory identity of a configured
1707+
// absolute path is not a file the snapshot fingerprints).
17071708
let root = snap.root_reading(bundler_config_files(snap.root_reading::<&Path>([])));
17081709
bundler_loaded_manifest(root).await
17091710
}
@@ -1713,7 +1714,7 @@ pub(crate) async fn bundler_loaded_manifest_in(
17131714
let lockfile = config.as_deref().and_then(manifest::config_lockfile);
17141715
let root = Path::new("/");
17151716
let gems_rb = view.is_file("gems.rb");
1716-
let loaded = manifest::classify(root, None, gemfile.as_deref(), None);
1717+
let loaded = manifest::classify_lexical(root, gemfile.as_deref());
17171718
// `/` only stands in for the project root: bundler opens an
17181719
// absolute lockfile as is, never the project's own lock, so
17191720
// it must not compare equal to the pair's lock at `/`.
@@ -1725,7 +1726,7 @@ pub(crate) async fn bundler_loaded_manifest_in(
17251726
};
17261727
}
17271728
}
1728-
loaded.with_lockfile(root, None, lockfile.as_deref(), None, gems_rb)
1729+
loaded.with_lockfile_lexical(root, lockfile.as_deref(), gems_rb)
17291730
}
17301731
}
17311732
}

‎crates/socket-patch-core/src/formats/gem/manifest.rs‎

Lines changed: 110 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,44 @@ impl LoadedManifest {
187187
lockfile_config: Option<&str>,
188188
lockfile_global: Option<&str>,
189189
gems_rb_present: bool,
190+
) -> LoadedManifest {
191+
self.with_lockfile_by(
192+
root,
193+
lockfile_env,
194+
lockfile_config,
195+
lockfile_global,
196+
gems_rb_present,
197+
same_dir_on_disk,
198+
)
199+
}
200+
201+
/// [`LoadedManifest::with_lockfile`] for a `root` that only stands in
202+
/// for a project not on this disk (an in-memory view): paths compare
203+
/// lexically, never through this host's directories.
204+
pub(crate) fn with_lockfile_lexical(
205+
self,
206+
root: &Path,
207+
lockfile_config: Option<&str>,
208+
gems_rb_present: bool,
209+
) -> LoadedManifest {
210+
self.with_lockfile_by(
211+
root,
212+
None,
213+
lockfile_config,
214+
None,
215+
gems_rb_present,
216+
|a, b| a == b,
217+
)
218+
}
219+
220+
fn with_lockfile_by(
221+
self,
222+
root: &Path,
223+
lockfile_env: Option<&OsStr>,
224+
lockfile_config: Option<&str>,
225+
lockfile_global: Option<&str>,
226+
gems_rb_present: bool,
227+
same_dir: SameDir,
190228
) -> LoadedManifest {
191229
let Some((_, lock)) = self.pair(gems_rb_present) else {
192230
return self;
@@ -206,7 +244,7 @@ impl LoadedManifest {
206244
let target = resolve_against(root, &value);
207245
let expected = resolve_against(root, Path::new(lock));
208246
if let (Some(target), Some(expected)) = (target, expected) {
209-
if same_entry(&target, &expected) {
247+
if same_entry(&target, &expected, same_dir) {
210248
return self;
211249
}
212250
}
@@ -259,22 +297,22 @@ fn resolve_against(root: &Path, value: &Path) -> Option<PathBuf> {
259297
.and_then(|p| normalize_lexically(&p))
260298
}
261299

262-
/// Whether two resolved directories are the same directory: equal, or both
263-
/// resolving to the same physical path through symlinks.
264-
fn same_dir(a: &Path, b: &Path) -> bool {
265-
a == b
266-
|| matches!(
267-
(std::fs::canonicalize(a), std::fs::canonicalize(b)),
268-
(Ok(a), Ok(b)) if a == b
269-
)
300+
/// How two resolved directories are judged the same directory.
301+
type SameDir = fn(&Path, &Path) -> bool;
302+
303+
/// Whether two resolved directories are the same directory on this disk:
304+
/// equal, or the same directory by file identity (a symlinked spelling, a
305+
/// bind mount, a Windows junction).
306+
fn same_dir_on_disk(a: &Path, b: &Path) -> bool {
307+
a == b || same_file::is_same_file(a, b).unwrap_or(false)
270308
}
271309

272310
/// Whether two resolved paths name the same directory entry: the same file
273-
/// name in the [`same_dir`] directory. Only the directory is resolved, so a
311+
/// name in the `same_dir` directory. Only the directory is resolved, so a
274312
/// root `Gemfile` that is itself a symlink to another manifest stays
275313
/// distinct from that manifest, as bundler's root follows the spelled
276314
/// directory.
277-
fn same_entry(a: &Path, b: &Path) -> bool {
315+
fn same_entry(a: &Path, b: &Path, same_dir: SameDir) -> bool {
278316
a == b
279317
|| (a.file_name() == b.file_name()
280318
&& matches!((a.parent(), b.parent()), (Some(a), Some(b)) if same_dir(a, b)))
@@ -291,6 +329,29 @@ pub fn classify(
291329
gemfile_env: Option<&OsStr>,
292330
config_value: Option<&str>,
293331
global_value: Option<&str>,
332+
) -> LoadedManifest {
333+
classify_by(
334+
root,
335+
gemfile_env,
336+
config_value,
337+
global_value,
338+
same_dir_on_disk,
339+
)
340+
}
341+
342+
/// [`classify`] of an app config value for a `root` that only stands in
343+
/// for a project not on this disk (an in-memory view): paths compare
344+
/// lexically, never through this host's directories.
345+
pub(crate) fn classify_lexical(root: &Path, config_value: Option<&str>) -> LoadedManifest {
346+
classify_by(root, None, config_value, None, |a, b| a == b)
347+
}
348+
349+
fn classify_by(
350+
root: &Path,
351+
gemfile_env: Option<&OsStr>,
352+
config_value: Option<&str>,
353+
global_value: Option<&str>,
354+
same_dir: SameDir,
294355
) -> LoadedManifest {
295356
let env = gemfile_env.filter(|v| !v.is_empty()).map(PathBuf::from);
296357
let config = config_value.filter(|v| !v.is_empty()).map(PathBuf::from);
@@ -321,8 +382,11 @@ pub fn classify(
321382
let target = resolve_against(root, &value);
322383
let root = resolve_against(root, Path::new(""));
323384
if let (Some(target), Some(root)) = (target, root) {
324-
for manifest in ["Gemfile", "gems.rb"] {
325-
if same_entry(&target, &root.join(manifest)) {
385+
let manifest = ["Gemfile", "gems.rb"]
386+
.into_iter()
387+
.find(|m| target.file_name() == Some(OsStr::new(m)));
388+
if let Some(manifest) = manifest {
389+
if same_entry(&target, &root.join(manifest), same_dir) {
326390
return LoadedManifest::Configured { manifest, by };
327391
}
328392
}
@@ -702,6 +766,39 @@ mod tests {
702766
}
703767
}
704768

769+
/// An in-memory view's root only stands in for a project that is not on
770+
/// this disk, so its paths never resolve through this host's
771+
/// directories: a symlinked spelling stays another manifest there.
772+
#[cfg(unix)]
773+
#[test]
774+
fn the_lexical_variants_never_read_this_hosts_directories() {
775+
let tmp = tempfile::tempdir().unwrap();
776+
let real = tmp.path().join("real/app");
777+
std::fs::create_dir_all(&real).unwrap();
778+
std::os::unix::fs::symlink(tmp.path().join("real"), tmp.path().join("link")).unwrap();
779+
let link = tmp.path().join("link/app");
780+
let value = link.join("Gemfile");
781+
assert_eq!(
782+
classify_lexical(&real, value.to_str()),
783+
LoadedManifest::Unsupported {
784+
value: value.display().to_string(),
785+
by: GemfileSetting::AppConfig
786+
}
787+
);
788+
assert_eq!(
789+
classify_lexical(&real, Some("Gemfile")),
790+
LoadedManifest::Configured {
791+
manifest: "Gemfile",
792+
by: GemfileSetting::AppConfig
793+
}
794+
);
795+
let lock = link.join("Gemfile.lock");
796+
assert!(matches!(
797+
LoadedManifest::Default.with_lockfile_lexical(&real, lock.to_str(), false),
798+
LoadedManifest::UnsupportedLockfile { .. }
799+
));
800+
}
801+
705802
/// Only the DIRECTORY is resolved: a root `Gemfile` that is itself a
706803
/// symlink to a manifest elsewhere does not make that manifest the
707804
/// project's, since bundler's root (and its lock) follow the spelled

0 commit comments

Comments
 (0)