Skip to content

Commit 522febb

Browse files
committed
Anchor BUNDLE_GEMFILE at the project root
A relative BUNDLE_GEMFILE was resolved against socket-patch's own working directory, so a --cwd run classified BUNDLE_GEMFILE=Gemfile as unsupported. Bundler expands the value in the directory bundle runs in, which is the project. The refusal also always said to unset the environment variable; for a .bundle/config setting it now gives the matching 'bundle config unset --local gemfile' remedy. Refs #390 Assisted-by: Claude Code:claude-opus-5-5
1 parent ffbe645 commit 522febb

3 files changed

Lines changed: 59 additions & 34 deletions

File tree

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -968,8 +968,7 @@ pub async fn bundler_loaded_manifest_with_env(
968968
.await
969969
.ok()
970970
.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())
971+
crate::formats::gem::manifest::classify(root, gemfile_env, config_value.as_deref())
973972
}
974973

975974
/// Bundler's app-config dir for `root`, following `Bundler.app_config_path`

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

Lines changed: 57 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
//! Bundler's own order (`Bundler::SharedHelpers#default_gemfile` and the CLI's
66
//! `gemfile` setting):
77
//!
8-
//! 1. `BUNDLE_GEMFILE` from the environment (a relative value expands
9-
//! against the process cwd, as `File.expand_path` does);
8+
//! 1. `BUNDLE_GEMFILE` from the environment (a relative value is read
9+
//! against the project root: bundler expands it against the directory
10+
//! `bundle` runs in, which is the project, not socket-patch's own
11+
//! cwd when it runs with `--cwd`);
1012
//! 2. `BUNDLE_GEMFILE:` in the app config file, `$BUNDLE_APP_CONFIG/config`
1113
//! else `<root>/.bundle/config` (what `bundle config set --local gemfile
1214
//! Gemfile.next` writes; relative to the project root);
@@ -85,12 +87,23 @@ impl LoadedManifest {
8587
/// The detail line for a caller that refuses an unsupported manifest.
8688
pub fn unsupported_detail(&self) -> Option<String> {
8789
match self {
88-
LoadedManifest::Unsupported { value, by } => Some(format!(
89-
"bundler loads `{value}` ({}), not the project's Gemfile or gems.rb; \
90-
socket-patch only wires those, so it left the gem manifests untouched (unset \
91-
BUNDLE_GEMFILE, or point it at the project's Gemfile, and re-run)",
92-
by.describe()
93-
)),
90+
LoadedManifest::Unsupported { value, by } => {
91+
let remedy = match by {
92+
GemfileSetting::Env => {
93+
"unset BUNDLE_GEMFILE, or point it at the project's Gemfile"
94+
}
95+
GemfileSetting::AppConfig => {
96+
"run `bundle config unset --local gemfile`, or point it at the \
97+
project's Gemfile"
98+
}
99+
};
100+
Some(format!(
101+
"bundler loads `{value}` ({}), not the project's Gemfile or gems.rb; \
102+
socket-patch only wires those, so it left the gem manifests untouched \
103+
({remedy}, and re-run)",
104+
by.describe()
105+
))
106+
}
94107
_ => None,
95108
}
96109
}
@@ -110,18 +123,16 @@ pub fn config_gemfile(contents: &str) -> Option<String> {
110123
}
111124

112125
/// Classify the configured `BUNDLE_GEMFILE` (environment first, then the
113-
/// app config value) against `root`. `cwd` anchors a relative environment
114-
/// value; a relative config value is anchored at `root`.
126+
/// app config value) against `root`, which also anchors a relative value.
115127
pub fn classify(
116128
root: &Path,
117-
cwd: &Path,
118129
gemfile_env: Option<&OsStr>,
119130
config_value: Option<&str>,
120131
) -> LoadedManifest {
121-
let (value, base, by) = match gemfile_env.filter(|v| !v.is_empty()) {
122-
Some(v) => (PathBuf::from(v), cwd, GemfileSetting::Env),
132+
let (value, by) = match gemfile_env.filter(|v| !v.is_empty()) {
133+
Some(v) => (PathBuf::from(v), GemfileSetting::Env),
123134
None => match config_value.filter(|v| !v.is_empty()) {
124-
Some(v) => (PathBuf::from(v), root, GemfileSetting::AppConfig),
135+
Some(v) => (PathBuf::from(v), GemfileSetting::AppConfig),
125136
None => return LoadedManifest::Default,
126137
},
127138
};
@@ -134,7 +145,7 @@ pub fn classify(
134145
let target = if value.is_absolute() {
135146
absolute(&value)
136147
} else {
137-
absolute(&base.join(&value))
148+
absolute(&root.join(&value))
138149
};
139150
let root = absolute(root);
140151
if let (Some(target), Some(root)) = (target, root) {
@@ -157,20 +168,20 @@ mod tests {
157168

158169
#[test]
159170
fn no_setting_is_bundlers_default_discovery() {
160-
let m = classify(&root(), &root(), None, None);
171+
let m = classify(&root(), None, None);
161172
assert_eq!(m, LoadedManifest::Default);
162173
assert_eq!(m.pair(true), Some(("gems.rb", "gems.locked")));
163174
assert_eq!(m.pair(false), Some(("Gemfile", "Gemfile.lock")));
164175
// An empty value is unset, as in bundler.
165176
assert_eq!(
166-
classify(&root(), &root(), Some(OsStr::new("")), Some("")),
177+
classify(&root(), Some(OsStr::new("")), Some("")),
167178
LoadedManifest::Default
168179
);
169180
}
170181

171182
#[test]
172183
fn config_naming_another_manifest_is_unsupported() {
173-
let m = classify(&root(), &root(), None, Some("Gemfile.next"));
184+
let m = classify(&root(), None, Some("Gemfile.next"));
174185
assert_eq!(
175186
m,
176187
LoadedManifest::Unsupported {
@@ -186,31 +197,30 @@ mod tests {
186197
fn config_naming_the_default_spellings_selects_that_pair() {
187198
// `bundle config set --local gemfile Gemfile` beside a gems.rb:
188199
// bundler loads Gemfile + Gemfile.lock, not gems.rb.
189-
let m = classify(&root(), &root(), None, Some("Gemfile"));
200+
let m = classify(&root(), None, Some("Gemfile"));
190201
assert_eq!(m.pair(true), Some(("Gemfile", "Gemfile.lock")));
191-
let m = classify(&root(), &root(), None, Some("./gems.rb"));
202+
let m = classify(&root(), None, Some("./gems.rb"));
192203
assert_eq!(m.pair(false), Some(("gems.rb", "gems.locked")));
193204
let abs = root().join("Gemfile");
194-
let m = classify(&root(), &root(), None, Some(abs.to_str().unwrap()));
205+
let m = classify(&root(), None, Some(abs.to_str().unwrap()));
195206
assert_eq!(m.pair(true), Some(("Gemfile", "Gemfile.lock")));
196207
}
197208

209+
/// The environment wins over the app config, and a relative value is
210+
/// read against the project root even when socket-patch runs elsewhere
211+
/// with `--cwd` (Bugbot on #431: `BUNDLE_GEMFILE=Gemfile` must select
212+
/// the project's Gemfile, not a file under the process cwd).
198213
#[test]
199-
fn env_wins_over_config_and_expands_against_the_cwd() {
200-
let m = classify(
201-
&root(),
202-
&root().join("sub"),
203-
Some(OsStr::new("../Gemfile")),
204-
Some("Gemfile.next"),
205-
);
214+
fn env_wins_over_config_and_is_anchored_at_the_project_root() {
215+
let m = classify(&root(), Some(OsStr::new("Gemfile")), Some("Gemfile.next"));
206216
assert_eq!(
207217
m,
208218
LoadedManifest::Configured {
209219
manifest: "Gemfile",
210220
by: GemfileSetting::Env
211221
}
212222
);
213-
let m = classify(&root(), &root(), Some(OsStr::new("Gemfile.next")), None);
223+
let m = classify(&root(), Some(OsStr::new("Gemfile.next")), None);
214224
assert!(matches!(
215225
m,
216226
LoadedManifest::Unsupported {
@@ -220,11 +230,27 @@ mod tests {
220230
));
221231
}
222232

233+
/// The refusal names the remedy for the knob that set it: unsetting the
234+
/// environment variable does nothing to a `.bundle/config` setting.
235+
#[test]
236+
fn unsupported_detail_names_the_knob_that_set_it() {
237+
let env = classify(&root(), Some(OsStr::new("Gemfile.next")), None);
238+
let env = env.unsupported_detail().unwrap();
239+
assert!(env.contains("unset BUNDLE_GEMFILE"), "{env}");
240+
let config = classify(&root(), None, Some("Gemfile.next"));
241+
let config = config.unsupported_detail().unwrap();
242+
assert!(
243+
config.contains("bundle config unset --local gemfile"),
244+
"{config}"
245+
);
246+
assert!(!config.contains("unset BUNDLE_GEMFILE"), "{config}");
247+
}
248+
223249
#[test]
224250
fn a_manifest_in_another_directory_is_unsupported() {
225-
let m = classify(&root(), &root(), None, Some("../other/Gemfile"));
251+
let m = classify(&root(), None, Some("../other/Gemfile"));
226252
assert!(matches!(m, LoadedManifest::Unsupported { .. }));
227-
let m = classify(&root(), &root(), None, Some("sub/Gemfile"));
253+
let m = classify(&root(), None, Some("sub/Gemfile"));
228254
assert!(matches!(m, LoadedManifest::Unsupported { .. }));
229255
}
230256

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ async fn keep_bundler_loaded_gem_files(view: &ProjectView<'_>, out: &mut Candida
482482
let config = view.read_text(".bundle/config").await.ok();
483483
let value = config.as_deref().and_then(manifest::config_gemfile);
484484
let root = std::path::Path::new("/");
485-
manifest::classify(root, root, None, value.as_deref())
485+
manifest::classify(root, None, value.as_deref())
486486
}
487487
};
488488
let keep: &[&str] = match &loaded {

0 commit comments

Comments
 (0)