From 8af85c9024dd9434224e21c12c1523660d6eda49 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 01:33:43 +0000 Subject: [PATCH 1/2] test: add reproducer for relative --workspacePath becoming the empty path Tinder/bazel-diff#470: the Rust CLI parses --workspacePath through parse_normalized_path, which folds away every CurDir component. A path made entirely of such components ("." , "./", "nested/..") normalizes to the empty PathBuf, so BazelOptions::command() chdir's the child into "" and every Bazel spawn dies with ENOENT -- reported against the Bazel binary, which exists, rather than against the workspace. Two ignored tests capture it: one on the parser (root cause), one that spawns a stub Bazel through the same options the CLI builds (symptom). Both are #[ignore]d so CI stays green until the bug is fixed; run them with `cargo test --bin bazel-diff -- --ignored issue_470`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SRpkjiihF9yeX6VPo6xBSa --- src/main.rs | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/src/main.rs b/src/main.rs index 6ad3e0d2..40355342 100644 --- a/src/main.rs +++ b/src/main.rs @@ -859,6 +859,106 @@ mod tests { ); } + /// Reproducer for : the + /// root cause. + /// + /// `--workspacePath` runs through [`parse_normalized_path`], which folds + /// every `CurDir` component away. A path that is *made* of `CurDir` + /// components -- `.`, `./`, or anything that cancels out via `..` -- + /// therefore normalizes to the empty `PathBuf` rather than to the + /// directory the user named. + /// + /// The same parser backs `--cacheDir`, so it has the same hole. + /// + /// Ignored so CI stays green until this is fixed. Run it with + /// `cargo test --bin bazel-diff -- --ignored issue_470`. + #[test] + #[ignore = "reproduces Tinder/bazel-diff#470: relative --workspacePath normalizes to the empty path"] + fn issue_470_relative_workspace_path_survives_normalization() { + for value in [".", "./", "./nested/..", "nested/.."] { + let parsed = parse_normalized_path(value).unwrap(); + assert_ne!( + parsed, + PathBuf::new(), + "--workspacePath {value} normalized to the empty path" + ); + } + + // A relative path that does not cancel out is left relative, and works, + // which is why the empty-path case reads as an unrelated failure. + assert_eq!( + parse_normalized_path("nested").unwrap(), + PathBuf::from("nested") + ); + } + + /// Reproducer for : the + /// symptom a user actually sees. + /// + /// [`BazelOptions::command`] does `command.current_dir(&self.workspace)`. + /// With the empty workspace from the test above, the child's `chdir("")` + /// fails with `ENOENT`, and `std::process::Command` reports that as a spawn + /// failure -- so the error names the *Bazel binary*, which exists and is + /// executable, instead of the workspace: + /// + /// ```text + /// [Error] failed to execute /path/to/bazel: No such file or directory (os error 2) + /// ``` + /// + /// Expected: `-w .` either resolves against the process working directory + /// (as the Kotlin CLI does) or is rejected with a message naming + /// `--workspacePath`. + /// + /// Ignored so CI stays green until this is fixed. Run it with + /// `cargo test --bin bazel-diff -- --ignored issue_470`. + #[cfg(unix)] + #[test] + #[ignore = "reproduces Tinder/bazel-diff#470: `-w .` makes every Bazel spawn fail with ENOENT"] + fn issue_470_relative_workspace_path_spawns_bazel_in_the_workspace() { + use std::os::unix::fs::PermissionsExt; + use std::process::Command; + + let directory = tempfile::tempdir().unwrap(); + let stub_bazel = directory.path().join("bazel"); + fs::write(&stub_bazel, "#!/bin/sh\necho 'Build label: 8.0.0'\n").unwrap(); + fs::set_permissions(&stub_bazel, fs::Permissions::from_mode(0o755)).unwrap(); + assert!(stub_bazel.is_file(), "the stub Bazel exists"); + + let cli = parse(&[ + "bazel-diff", + "generate-hashes", + "-w", + ".", + "-b", + stub_bazel.to_str().unwrap(), + "hashes.json", + ]); + let Commands::GenerateHashes(args) = cli.command else { + panic!("expected generate-hashes"); + }; + let options = bazel_options(&args.hashing, false).unwrap(); + + // Mirrors BazelOptions::command(): the only difference from a working + // run is the workspace the child is chdir'd into. + let spawned = Command::new(&options.bazel) + .current_dir(&options.workspace) + .arg("version") + .output(); + + match spawned { + Ok(output) => assert!( + output.status.success(), + "stub Bazel should have run: {}", + String::from_utf8_lossy(&output.stderr) + ), + Err(error) => panic!( + "`-w .` gave workspace {:?}, so spawning {} failed: {error}", + options.workspace, + options.bazel.display() + ), + } + } + #[test] fn reads_and_trims_seed_and_repository_files() { let directory = tempfile::tempdir().unwrap(); From 933a91e8f50b0e8a73ef18e285dfe7d9c7f6c62b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 01:38:20 +0000 Subject: [PATCH 2/2] fix: resolve relative --workspacePath against the working directory Tinder/bazel-diff#470: the Rust CLI parsed path arguments with normalize_path, which folds away `.` and `..` lexically. On a relative path that is wrong in two ways -- a path made only of such components (".", "./", "nested/..") collapsed to the empty PathBuf, and a leading ".." popped nothing so "../sibling" silently became "sibling". The empty path is the one users hit: BazelOptions::command() does `command.current_dir(&self.workspace)`, so the child's chdir("") failed and std::process::Command reported it as a spawn failure -- naming the Bazel binary, which exists and is executable, rather than the workspace: [Error] failed to execute /path/to/bazel: No such file or directory parse_normalized_path now anchors the argument to the process working directory with std::path::absolute before normalizing, so a relative --workspacePath means what it says, matching the Kotlin CLI. The same parser backs --cacheDir, which had the same hole. An empty path never named a directory and is now rejected up front by clap, with a message naming the flag, instead of reaching chdir. The two ignored reproducers added in the previous commit become regression tests: one on the parser, one spawning a stub Bazel through the options the CLI actually builds. Both fail without this change. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SRpkjiihF9yeX6VPo6xBSa --- src/main.rs | 133 +++++++++++++++++++++++++++++----------------------- 1 file changed, 75 insertions(+), 58 deletions(-) diff --git a/src/main.rs b/src/main.rs index 40355342..13a1c9d9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -325,6 +325,9 @@ fn flatten_options(values: &[String]) -> Vec { .collect() } +/// Folds `.` and `..` away lexically. Only meaningful on an absolute path: +/// applied to a relative one, `.` and a leading `..` have nothing to resolve +/// against and simply vanish, so callers go through [`parse_normalized_path`]. fn normalize_path(path: PathBuf) -> PathBuf { path.components() .fold(PathBuf::new(), |mut normalized, component| { @@ -339,8 +342,21 @@ fn normalize_path(path: PathBuf) -> PathBuf { }) } +/// Resolves a path argument against the process working directory, then +/// normalizes it. +/// +/// Making the path absolute *first* is what keeps a relative argument +/// meaningful. Normalizing `.` on its own drops its only component and leaves +/// the empty path, which then fails every `Command::current_dir` with a +/// `chdir("")` ENOENT that names the Bazel binary rather than the workspace; +/// normalizing `../sibling` on its own pops nothing and silently yields +/// `sibling`. Anchoring to the working directory gives both the directory the +/// user meant, matching the Kotlin CLI. See +/// . fn parse_normalized_path(value: &str) -> Result { - Ok(normalize_path(PathBuf::from(value))) + let absolute = std::path::absolute(value) + .map_err(|error| format!("failed to resolve path {value:?}: {error}"))?; + Ok(normalize_path(absolute)) } fn read_lines(path: Option<&Path>) -> Result> { @@ -859,70 +875,71 @@ mod tests { ); } - /// Reproducer for : the - /// root cause. + /// Regression test for . /// - /// `--workspacePath` runs through [`parse_normalized_path`], which folds - /// every `CurDir` component away. A path that is *made* of `CurDir` - /// components -- `.`, `./`, or anything that cancels out via `..` -- - /// therefore normalizes to the empty `PathBuf` rather than to the - /// directory the user named. - /// - /// The same parser backs `--cacheDir`, so it has the same hole. - /// - /// Ignored so CI stays green until this is fixed. Run it with - /// `cargo test --bin bazel-diff -- --ignored issue_470`. + /// `--workspacePath` and `--cacheDir` are anchored to the process working + /// directory before they are normalized. Without that, a path built only + /// out of `.`/`..` components collapsed to the empty path, and a leading + /// `..` was silently dropped. #[test] - #[ignore = "reproduces Tinder/bazel-diff#470: relative --workspacePath normalizes to the empty path"] - fn issue_470_relative_workspace_path_survives_normalization() { + fn relative_path_arguments_resolve_against_the_working_directory() { + let working_directory = std::env::current_dir().unwrap(); for value in [".", "./", "./nested/..", "nested/.."] { - let parsed = parse_normalized_path(value).unwrap(); - assert_ne!( - parsed, - PathBuf::new(), - "--workspacePath {value} normalized to the empty path" + assert_eq!( + parse_normalized_path(value).unwrap(), + working_directory, + "--workspacePath {value} should be the working directory" ); } - - // A relative path that does not cancel out is left relative, and works, - // which is why the empty-path case reads as an unrelated failure. assert_eq!( parse_normalized_path("nested").unwrap(), - PathBuf::from("nested") + working_directory.join("nested") + ); + assert_eq!( + parse_normalized_path("./nested/inner/..").unwrap(), + working_directory.join("nested") ); + + // A leading `..` has a parent to pop now that the path is absolute. + assert_eq!( + parse_normalized_path("../sibling").unwrap(), + working_directory.parent().unwrap().join("sibling") + ); + + // Absolute arguments keep the normalization they always had. + let absolute = working_directory.join("nested/inner/.."); + assert_eq!( + parse_normalized_path(absolute.to_str().unwrap()).unwrap(), + working_directory.join("nested") + ); + + // The empty path never named a directory; it is now rejected instead of + // being passed to `chdir`. + assert!(parse_normalized_path("").is_err()); } - /// Reproducer for : the - /// symptom a user actually sees. + /// Regression test for : + /// the symptom users hit. /// /// [`BazelOptions::command`] does `command.current_dir(&self.workspace)`. - /// With the empty workspace from the test above, the child's `chdir("")` - /// fails with `ENOENT`, and `std::process::Command` reports that as a spawn - /// failure -- so the error names the *Bazel binary*, which exists and is - /// executable, instead of the workspace: + /// When `-w .` normalized to the empty path the child's `chdir("")` failed, + /// and `std::process::Command` surfaced that as a spawn failure -- so the + /// error named the *Bazel binary*, which exists and is executable, instead + /// of the workspace: /// /// ```text /// [Error] failed to execute /path/to/bazel: No such file or directory (os error 2) /// ``` - /// - /// Expected: `-w .` either resolves against the process working directory - /// (as the Kotlin CLI does) or is rejected with a message naming - /// `--workspacePath`. - /// - /// Ignored so CI stays green until this is fixed. Run it with - /// `cargo test --bin bazel-diff -- --ignored issue_470`. #[cfg(unix)] #[test] - #[ignore = "reproduces Tinder/bazel-diff#470: `-w .` makes every Bazel spawn fail with ENOENT"] - fn issue_470_relative_workspace_path_spawns_bazel_in_the_workspace() { + fn relative_workspace_path_spawns_bazel_in_the_working_directory() { use std::os::unix::fs::PermissionsExt; use std::process::Command; let directory = tempfile::tempdir().unwrap(); let stub_bazel = directory.path().join("bazel"); - fs::write(&stub_bazel, "#!/bin/sh\necho 'Build label: 8.0.0'\n").unwrap(); + fs::write(&stub_bazel, "#!/bin/sh\npwd -P\n").unwrap(); fs::set_permissions(&stub_bazel, fs::Permissions::from_mode(0o755)).unwrap(); - assert!(stub_bazel.is_file(), "the stub Bazel exists"); let cli = parse(&[ "bazel-diff", @@ -938,25 +955,25 @@ mod tests { }; let options = bazel_options(&args.hashing, false).unwrap(); - // Mirrors BazelOptions::command(): the only difference from a working - // run is the workspace the child is chdir'd into. - let spawned = Command::new(&options.bazel) + // Mirrors BazelOptions::command(). + let output = Command::new(&options.bazel) .current_dir(&options.workspace) .arg("version") - .output(); - - match spawned { - Ok(output) => assert!( - output.status.success(), - "stub Bazel should have run: {}", - String::from_utf8_lossy(&output.stderr) - ), - Err(error) => panic!( - "`-w .` gave workspace {:?}, so spawning {} failed: {error}", - options.workspace, - options.bazel.display() - ), - } + .output() + .unwrap_or_else(|error| { + panic!( + "`-w .` gave workspace {:?}, so spawning {} failed: {error}", + options.workspace, + options.bazel.display() + ) + }); + + assert!(output.status.success()); + assert_eq!( + PathBuf::from(String::from_utf8_lossy(&output.stdout).trim()), + std::env::current_dir().unwrap(), + "the child should run in the working directory `.` named" + ); } #[test]