Skip to content

Commit ffbe645

Browse files
committed
Keep the Bundler manifest model free of I/O
The formats modules are pure by contract, and an architecture test enforces that. Move the disk and environment reads for the manifest Bundler loads into the ruby crawler, next to the BUNDLE_PATH config reader. formats::gem::manifest keeps only the classification. Refs #341, #390 Assisted-by: Claude Code:claude-opus-5-5
1 parent f7dd00f commit ffbe645

4 files changed

Lines changed: 61 additions & 53 deletions

File tree

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -944,6 +944,34 @@ fn expand_tilde(value: &Path, home: Option<&Path>) -> PathBuf {
944944
value.to_path_buf()
945945
}
946946

947+
/// [`crate::formats::gem::manifest::classify`] for `root` on disk: the
948+
/// manifest bundler loads, reading the ambient `BUNDLE_GEMFILE` /
949+
/// `BUNDLE_APP_CONFIG` and the app config file.
950+
pub async fn bundler_loaded_manifest(root: &Path) -> crate::formats::gem::manifest::LoadedManifest {
951+
bundler_loaded_manifest_with_env(
952+
root,
953+
std::env::var_os("BUNDLE_GEMFILE").as_deref(),
954+
std::env::var_os("BUNDLE_APP_CONFIG").as_deref(),
955+
)
956+
.await
957+
}
958+
959+
/// [`bundler_loaded_manifest`] with the environment passed explicitly (hermetic
960+
/// tests).
961+
pub async fn bundler_loaded_manifest_with_env(
962+
root: &Path,
963+
gemfile_env: Option<&OsStr>,
964+
app_config_env: Option<&OsStr>,
965+
) -> crate::formats::gem::manifest::LoadedManifest {
966+
let config = bundler_app_config_dir(root, app_config_env).join("config");
967+
let config_value = crate::utils::fs::read_regular_to_string(&config)
968+
.await
969+
.ok()
970+
.and_then(|text| crate::formats::gem::manifest::config_gemfile(&text));
971+
let cwd = std::env::current_dir().unwrap_or_default();
972+
crate::formats::gem::manifest::classify(root, &cwd, gemfile_env, config_value.as_deref())
973+
}
974+
947975
/// Bundler's app-config dir for `root`, following `Bundler.app_config_path`
948976
/// exactly: `$BUNDLE_APP_CONFIG` when set (a relative value resolves against
949977
/// the project root, NOT the process cwd), else `<root>/.bundle` — e.g. the
@@ -1097,6 +1125,33 @@ fn is_safe_gem_coordinate(name: &str, version: &str) -> bool {
10971125
mod tests {
10981126
use super::*;
10991127

1128+
#[tokio::test]
1129+
async fn loaded_manifest_reads_the_app_config_file() {
1130+
let dir = tempfile::tempdir().unwrap();
1131+
std::fs::create_dir(dir.path().join(".bundle")).unwrap();
1132+
std::fs::write(
1133+
dir.path().join(".bundle/config"),
1134+
"---\nBUNDLE_GEMFILE: \"Gemfile.next\"\n",
1135+
)
1136+
.unwrap();
1137+
let m = bundler_loaded_manifest_with_env(dir.path(), None, None).await;
1138+
assert!(matches!(
1139+
m,
1140+
crate::formats::gem::manifest::LoadedManifest::Unsupported {
1141+
by: crate::formats::gem::manifest::GemfileSetting::AppConfig,
1142+
..
1143+
}
1144+
));
1145+
// BUNDLE_APP_CONFIG moves the config file away from `.bundle`.
1146+
let m = bundler_loaded_manifest_with_env(
1147+
dir.path(),
1148+
None,
1149+
Some(std::ffi::OsStr::new("elsewhere")),
1150+
)
1151+
.await;
1152+
assert_eq!(m, crate::formats::gem::manifest::LoadedManifest::Default);
1153+
}
1154+
11001155
#[test]
11011156
fn test_parse_gem_dir_name() {
11021157
assert_eq!(

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

Lines changed: 4 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@
1919
//! directory, a missing file) is [`LoadedManifest::Unsupported`]: the
2020
//! rewriters and the lock readers only know the two default pairs, so the
2121
//! callers fail closed rather than wire a manifest Bundler never reads.
22-
//! The user-level `~/.bundle/config` is not consulted.
22+
//! The user-level `~/.bundle/config` is not consulted. The model is pure:
23+
//! the disk and environment reads live in
24+
//! [`crate::crawlers::ruby_crawler::bundler_loaded_manifest`].
2325
2426
use std::ffi::OsStr;
2527
use std::path::{Path, PathBuf};
2628

27-
use crate::crawlers::ruby_crawler::{bundler_app_config_dir, unquote_bundle_config_value};
29+
use crate::crawlers::ruby_crawler::unquote_bundle_config_value;
2830
use crate::utils::fs::normalize_lexically;
2931

3032
/// Where a configured `BUNDLE_GEMFILE` came from.
@@ -94,33 +96,6 @@ impl LoadedManifest {
9496
}
9597
}
9698

97-
/// [`classify`] for `root` on disk, reading the ambient `BUNDLE_GEMFILE` /
98-
/// `BUNDLE_APP_CONFIG` and the app config file.
99-
pub async fn loaded_manifest(root: &Path) -> LoadedManifest {
100-
loaded_manifest_with_env(
101-
root,
102-
std::env::var_os("BUNDLE_GEMFILE").as_deref(),
103-
std::env::var_os("BUNDLE_APP_CONFIG").as_deref(),
104-
)
105-
.await
106-
}
107-
108-
/// [`loaded_manifest`] with the environment passed explicitly (hermetic
109-
/// tests).
110-
pub async fn loaded_manifest_with_env(
111-
root: &Path,
112-
gemfile_env: Option<&OsStr>,
113-
app_config_env: Option<&OsStr>,
114-
) -> LoadedManifest {
115-
let config = bundler_app_config_dir(root, app_config_env).join("config");
116-
let config_value = crate::utils::fs::read_regular_to_string(&config)
117-
.await
118-
.ok()
119-
.and_then(|text| config_gemfile(&text));
120-
let cwd = std::env::current_dir().unwrap_or_default();
121-
classify(root, &cwd, gemfile_env, config_value.as_deref())
122-
}
123-
12499
/// The `BUNDLE_GEMFILE:` value of a bundler app config file (flat YAML that
125100
/// bundler writes itself; an empty value counts as unset).
126101
pub fn config_gemfile(contents: &str) -> Option<String> {
@@ -264,26 +239,4 @@ mod tests {
264239
assert_eq!(config_gemfile("---\nBUNDLE_GEMFILE: \"\"\n"), None);
265240
assert_eq!(config_gemfile("---\nBUNDLE_PATH: \"x\"\n"), None);
266241
}
267-
268-
#[tokio::test]
269-
async fn loaded_manifest_reads_the_app_config_file() {
270-
let dir = tempfile::tempdir().unwrap();
271-
std::fs::create_dir(dir.path().join(".bundle")).unwrap();
272-
std::fs::write(
273-
dir.path().join(".bundle/config"),
274-
"---\nBUNDLE_GEMFILE: \"Gemfile.next\"\n",
275-
)
276-
.unwrap();
277-
let m = loaded_manifest_with_env(dir.path(), None, None).await;
278-
assert!(matches!(
279-
m,
280-
LoadedManifest::Unsupported {
281-
by: GemfileSetting::AppConfig,
282-
..
283-
}
284-
));
285-
// BUNDLE_APP_CONFIG moves the config file away from `.bundle`.
286-
let m = loaded_manifest_with_env(dir.path(), None, Some(OsStr::new("elsewhere"))).await;
287-
assert_eq!(m, LoadedManifest::Default);
288-
}
289242
}

‎crates/socket-patch-core/src/hosted/engine.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ async fn keep_bundler_loaded_gem_files(view: &ProjectView<'_>, out: &mut Candida
476476
let loaded = match view {
477477
ProjectView::Disk(root)
478478
| ProjectView::Snapshot(crate::vendor::lock_inventory::DiskSnapshot { root, .. }) => {
479-
manifest::loaded_manifest(root).await
479+
crate::crawlers::ruby_crawler::bundler_loaded_manifest(root).await
480480
}
481481
ProjectView::Memory(_) => {
482482
let config = view.read_text(".bundle/config").await.ok();

‎crates/socket-patch-core/src/vendor/gem.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ struct GemPrelude {
146146
/// its hosted wiring instead of ending up unpatched in both modes.
147147
pub async fn gem_manifest_refusal(project_root: &Path) -> Option<(&'static str, String)> {
148148
use crate::formats::gem::manifest::LoadedManifest;
149-
let loaded = crate::formats::gem::manifest::loaded_manifest(project_root).await;
149+
let loaded = crate::crawlers::ruby_crawler::bundler_loaded_manifest(project_root).await;
150150
let gems_rb_present = tokio::fs::symlink_metadata(project_root.join("gems.rb"))
151151
.await
152152
.is_ok();

0 commit comments

Comments
 (0)