diff --git a/src/main.rs b/src/main.rs index 6ad3e0d..13a1c9d 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,6 +875,107 @@ mod tests { ); } + /// Regression test for . + /// + /// `--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] + fn relative_path_arguments_resolve_against_the_working_directory() { + let working_directory = std::env::current_dir().unwrap(); + for value in [".", "./", "./nested/..", "nested/.."] { + assert_eq!( + parse_normalized_path(value).unwrap(), + working_directory, + "--workspacePath {value} should be the working directory" + ); + } + assert_eq!( + parse_normalized_path("nested").unwrap(), + 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()); + } + + /// Regression test for : + /// the symptom users hit. + /// + /// [`BazelOptions::command`] does `command.current_dir(&self.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) + /// ``` + #[cfg(unix)] + #[test] + 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\npwd -P\n").unwrap(); + fs::set_permissions(&stub_bazel, fs::Permissions::from_mode(0o755)).unwrap(); + + 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(). + let output = Command::new(&options.bazel) + .current_dir(&options.workspace) + .arg("version") + .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] fn reads_and_trims_seed_and_repository_files() { let directory = tempfile::tempdir().unwrap();