Skip to content

Commit 6104c38

Browse files
committed
cargo-rail: fixing the failed integration tests by adjusting the 'unify'
task to require resolved metadata. We might come back later for an offline mode.
1 parent 8a6e634 commit 6104c38

4 files changed

Lines changed: 15 additions & 36 deletions

File tree

next_phase.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ cargo-rail becomes the default answer to:
6868
- `CargoRailOutputCtx` is initialized once in `src/main.rs` based on global `--quiet` + `--json` / per-command `--format`.
6969
- `WorkspaceContext::build(..., output)` stores the output context and exposes it via `ctx.output()`.
7070
- Commands and subsystems write status/progress/warnings via `ctx.output()` (stdout remains reserved for command results).
71+
- `cargo rail unify` now fails fast with actionable errors if resolved metadata cannot be loaded (e.g., offline with missing cache).
7172
- Deliverable: a single initialization point for output mode; no duplicated init calls in command handlers.
7273
- Docs: `docs/output.md`
7374
- Exit criteria: `just check && just test`

src/cargo/unify_analyzer.rs

Lines changed: 14 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,11 @@ pub struct UnifyAnalyzer {
3838
impl UnifyAnalyzer {
3939
/// Create a new analyzer from workspace context
4040
pub fn new(ctx: &WorkspaceContext) -> RailResult<Self> {
41-
// Get multi-target metadata (lazily loaded on first access)
42-
// This clones the inner data (required for ownership by this analyzer)
43-
let metadata = (*ctx.multi_target_metadata()?).clone();
41+
// Unify requires resolved cargo metadata to be safe and correct.
42+
let metadata = (*ctx.multi_target_metadata().with_context(|| {
43+
"unify requires resolved cargo metadata; run `cargo fetch` (or disable offline) and try again".to_string()
44+
})?)
45+
.clone();
4446

4547
// Parse all manifests once
4648
let workspace_packages = metadata.workspace_packages();
@@ -231,9 +233,7 @@ impl UnifyAnalyzer {
231233
.cloned()
232234
.unwrap_or_else(|| format!("={}", version));
233235
// Try exact version first, fall back to caret format, then ultimate fallback
234-
VersionReq::parse(&exact_version)
235-
.or_else(|_| VersionReq::parse(&format!("^{}", version)))
236-
.unwrap_or_else(|_| VersionReq::default())
236+
VersionReq::parse(&exact_version).unwrap_or_else(|_| VersionReq::default())
237237
} else {
238238
// Try caret format first, fall back to plain version, then ultimate fallback
239239
VersionReq::parse(&format!("^{}", version))
@@ -319,14 +319,15 @@ impl UnifyAnalyzer {
319319
}
320320
};
321321

322-
// Check if target-specific
323-
let targets_with_dep = self.metadata.targets_with_dep(&dep_key.name);
324-
let all_targets = self.metadata.targets();
325-
let target = if targets_with_dep.len() < all_targets.len() {
326-
// This is target-specific, try to infer cfg
327-
Some(self.infer_target_cfg(&targets_with_dep))
328-
} else {
322+
// Best-effort target constraint display.
323+
// Feature computation already excludes target-specific usages from workspace-level features.
324+
let target_cfgs: HashSet<String> = usage_sites.iter().filter_map(|u| u.target.clone()).collect();
325+
let target = if target_cfgs.is_empty() {
329326
None
327+
} else if target_cfgs.len() == 1 {
328+
target_cfgs.into_iter().next()
329+
} else {
330+
Some("multiple target cfgs".to_string())
330331
};
331332

332333
// Get users - convert Arc<str> to String for output
@@ -612,26 +613,6 @@ impl UnifyAnalyzer {
612613
Ok(results)
613614
}
614615

615-
/// Infer a cfg expression from a list of targets
616-
fn infer_target_cfg(&self, targets: &[String]) -> String {
617-
// Simple heuristic: if all Unix-like, use cfg(unix)
618-
if targets.iter().all(|t| t.contains("linux") || t.contains("darwin")) {
619-
"cfg(unix)".to_string()
620-
} else if targets.iter().all(|t| t.contains("windows")) {
621-
"cfg(windows)".to_string()
622-
} else {
623-
// Fall back to listing all targets
624-
format!(
625-
"cfg(any({}))",
626-
targets
627-
.iter()
628-
.map(|t| format!("target = \"{}\"", t))
629-
.collect::<Vec<_>>()
630-
.join(", ")
631-
)
632-
}
633-
}
634-
635616
/// Detect dependencies where resolved features exceed declared features
636617
///
637618
/// This catches cases where a crate relies on Cargo's feature unification to

tests/integration/helpers.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -436,8 +436,6 @@ pub fn run_cargo_rail(cwd: &Path, args: &[&str]) -> Result<Output> {
436436

437437
let output = Command::new(cargo_rail_bin)
438438
.current_dir(cwd)
439-
// Keep integration tests hermetic: prevent Cargo from attempting network access for metadata/resolution.
440-
.env("CARGO_NET_OFFLINE", "true")
441439
.args(args)
442440
.output()
443441
.context("Failed to run cargo-rail")?;

tests/integration/test_cli_api.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ fn run_cargo_rail(cwd: &Path, args: &[&str], envs: &[(&str, &str)]) -> Result<Ou
1919
let cargo_rail_bin = env!("CARGO_BIN_EXE_cargo-rail");
2020
let mut cmd = Command::new(cargo_rail_bin);
2121
cmd.current_dir(cwd).args(args);
22-
cmd.env("CARGO_NET_OFFLINE", "true");
2322
for (k, v) in envs {
2423
cmd.env(k, v);
2524
}

0 commit comments

Comments
 (0)