From d07ac80ac6a18770acb1726609ab95d58d89482f Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Tue, 10 Sep 2024 14:30:20 -0700 Subject: [PATCH] Update `runfiles::rlocation!` to return `Option` instead of panicing (#2847) closes https://github.com/bazelbuild/rules_rust/issues/2653 --- crate_universe/src/api/lockfile.rs | 2 +- crate_universe/src/config.rs | 3 +- crate_universe/src/splicing.rs | 15 +- .../src/splicing/crate_index_lookup.rs | 5 +- crate_universe/src/splicing/splicer.rs | 4 +- .../tests/cargo_integration_test.rs | 8 +- examples/hello_runfiles/hello_runfiles.rs | 8 +- examples/proto/helloworld/helloworld_test.rs | 4 +- test/bzl_version/bzl_version_test.rs | 2 +- test/bzlmod_repo_mapping/module_b/lib.rs | 4 +- test/process_wrapper/rustc_quit_on_rmeta.rs | 6 +- .../bootstrap_process_wrapper_test.rs | 3 +- test/rust/src/greeter.rs | 10 +- tools/runfiles/runfiles.rs | 337 ++++++++++++------ tools/rust_analyzer/lib.rs | 3 +- tools/rustfmt/src/lib.rs | 8 +- tools/upstream_wrapper/src/main.rs | 2 +- 17 files changed, 272 insertions(+), 152 deletions(-) diff --git a/crate_universe/src/api/lockfile.rs b/crate_universe/src/api/lockfile.rs index 62fbe378a2..0cbf017419 100644 --- a/crate_universe/src/api/lockfile.rs +++ b/crate_universe/src/api/lockfile.rs @@ -145,7 +145,7 @@ mod test { let runfiles = runfiles::Runfiles::create().unwrap(); let path = runfiles::rlocation!( - runfiles, "rules_rust/crate_universe/test_data/cargo_bazel_lockfile/multi_package-cargo-bazel-lock.json"); + runfiles, "rules_rust/crate_universe/test_data/cargo_bazel_lockfile/multi_package-cargo-bazel-lock.json").unwrap(); let parsed = parse(&path).unwrap(); assert_eq!(parsed.workspace_members(), want_workspace_member_names); diff --git a/crate_universe/src/config.rs b/crate_universe/src/config.rs index 25598771b1..8236ee9952 100644 --- a/crate_universe/src/config.rs +++ b/crate_universe/src/config.rs @@ -922,7 +922,8 @@ mod test { let path = runfiles::rlocation!( runfiles, "rules_rust/crate_universe/test_data/serialized_configs/config.json" - ); + ) + .unwrap(); let content = std::fs::read_to_string(path).unwrap(); diff --git a/crate_universe/src/splicing.rs b/crate_universe/src/splicing.rs index eb5e31420b..7f0560f47d 100644 --- a/crate_universe/src/splicing.rs +++ b/crate_universe/src/splicing.rs @@ -487,7 +487,8 @@ mod test { let path = runfiles::rlocation!( runfiles, "rules_rust/crate_universe/test_data/serialized_configs/splicing_manifest.json" - ); + ) + .unwrap(); let content = std::fs::read_to_string(path).unwrap(); @@ -571,7 +572,8 @@ mod test { let path = runfiles::rlocation!( runfiles, "rules_rust/crate_universe/test_data/serialized_configs/splicing_manifest.json" - ); + ) + .unwrap(); let content = std::fs::read_to_string(path).unwrap(); @@ -616,16 +618,19 @@ mod test { let workspace_manifest_path = runfiles::rlocation!( runfiles, "rules_rust/crate_universe/test_data/metadata/workspace_path/Cargo.toml" - ); + ) + .unwrap(); let workspace_path = workspace_manifest_path.parent().unwrap().to_path_buf(); let child_a_manifest_path = runfiles::rlocation!( runfiles, "rules_rust/crate_universe/test_data/metadata/workspace_path/child_a/Cargo.toml" - ); + ) + .unwrap(); let child_b_manifest_path = runfiles::rlocation!( runfiles, "rules_rust/crate_universe/test_data/metadata/workspace_path/child_b/Cargo.toml" - ); + ) + .unwrap(); let manifest = SplicingManifest { direct_packages: BTreeMap::new(), manifests: BTreeMap::from([ diff --git a/crate_universe/src/splicing/crate_index_lookup.rs b/crate_universe/src/splicing/crate_index_lookup.rs index 72f404b78b..05bd34f79f 100644 --- a/crate_universe/src/splicing/crate_index_lookup.rs +++ b/crate_universe/src/splicing/crate_index_lookup.rs @@ -69,7 +69,8 @@ mod test { runfiles::rlocation!( runfiles, "rules_rust/crate_universe/test_data/crate_indexes/lazy_static/cargo_home" - ), + ) + .unwrap(), ); let index = CrateIndexLookup::Http( @@ -97,7 +98,7 @@ mod test { } { let _e = EnvVarResetter::set("CARGO_HOME", - runfiles::rlocation!(runfiles, "rules_rust/crate_universe/test_data/crate_indexes/rewritten_lazy_static/cargo_home")); + runfiles::rlocation!(runfiles, "rules_rust/crate_universe/test_data/crate_indexes/rewritten_lazy_static/cargo_home").unwrap()); let index = CrateIndexLookup::Http( crates_index::SparseIndex::from_url("sparse+https://index.crates.io/").unwrap(), diff --git a/crate_universe/src/splicing/splicer.rs b/crate_universe/src/splicing/splicer.rs index bd6cbcae54..8b4b7ea84f 100644 --- a/crate_universe/src/splicing/splicer.rs +++ b/crate_universe/src/splicing/splicer.rs @@ -765,8 +765,8 @@ mod test { #[cfg(not(feature = "cargo"))] fn get_cargo_and_rustc_paths() -> (PathBuf, PathBuf) { let r = runfiles::Runfiles::create().unwrap(); - let cargo_path = runfiles::rlocation!(r, concat!("rules_rust/", env!("CARGO"))); - let rustc_path = runfiles::rlocation!(r, concat!("rules_rust/", env!("RUSTC"))); + let cargo_path = runfiles::rlocation!(r, concat!("rules_rust/", env!("CARGO"))).unwrap(); + let rustc_path = runfiles::rlocation!(r, concat!("rules_rust/", env!("RUSTC"))).unwrap(); (cargo_path, rustc_path) } diff --git a/crate_universe/tests/cargo_integration_test.rs b/crate_universe/tests/cargo_integration_test.rs index 7682798823..5631839b7b 100644 --- a/crate_universe/tests/cargo_integration_test.rs +++ b/crate_universe/tests/cargo_integration_test.rs @@ -103,7 +103,7 @@ fn run(repository_name: &str, manifests: HashMap, lockfile: &str splice(SpliceOptions { splicing_manifest, - cargo_lockfile: Some(runfiles::rlocation!(runfiles, lockfile)), + cargo_lockfile: Some(runfiles::rlocation!(runfiles, lockfile).unwrap()), repin: None, workspace_dir: None, output_dir: scratch.path().join("out"), @@ -139,6 +139,7 @@ fn feature_generator() { r, "rules_rust/crate_universe/test_data/metadata/target_features/Cargo.toml" ) + .unwrap() .to_string_lossy() .to_string(), "//:test_input".to_string(), @@ -256,6 +257,7 @@ fn feature_generator_cfg_features() { r, "rules_rust/crate_universe/test_data/metadata/target_cfg_features/Cargo.toml" ) + .unwrap() .to_string_lossy() .to_string(), "//:test_input".to_string(), @@ -325,6 +327,7 @@ fn feature_generator_workspace() { r, "rules_rust/crate_universe/test_data/metadata/workspace/Cargo.toml" ) + .unwrap() .to_string_lossy() .to_string(), "//:test_input".to_string(), @@ -334,6 +337,7 @@ fn feature_generator_workspace() { r, "rules_rust/crate_universe/test_data/metadata/workspace/child/Cargo.toml" ) + .unwrap() .to_string_lossy() .to_string(), "//crate_universe:test_data/metadata/workspace/child/Cargo.toml".to_string(), @@ -360,6 +364,7 @@ fn feature_generator_crate_combined_features() { r, "rules_rust/crate_universe/test_data/metadata/crate_combined_features/Cargo.toml" ) + .unwrap() .to_string_lossy() .to_string(), "//:test_input".to_string(), @@ -400,6 +405,7 @@ fn resolver_2_deps() { r, "rules_rust/crate_universe/test_data/metadata/resolver_2_deps/Cargo.toml" ) + .unwrap() .to_string_lossy() .to_string(), "//:test_input".to_string(), diff --git a/examples/hello_runfiles/hello_runfiles.rs b/examples/hello_runfiles/hello_runfiles.rs index 93604a38f4..376c074e7a 100644 --- a/examples/hello_runfiles/hello_runfiles.rs +++ b/examples/hello_runfiles/hello_runfiles.rs @@ -6,11 +6,9 @@ use runfiles::Runfiles; fn main() { let r = Runfiles::create().unwrap(); - let mut f = File::open(runfiles::rlocation!( - r, - "examples/hello_runfiles/hello_runfiles.rs" - )) - .unwrap(); + let mut f = + File::open(runfiles::rlocation!(r, "examples/hello_runfiles/hello_runfiles.rs").unwrap()) + .unwrap(); let mut buffer = String::new(); f.read_to_string(&mut buffer).unwrap(); diff --git a/examples/proto/helloworld/helloworld_test.rs b/examples/proto/helloworld/helloworld_test.rs index 827a0a942e..186b487b4f 100644 --- a/examples/proto/helloworld/helloworld_test.rs +++ b/examples/proto/helloworld/helloworld_test.rs @@ -39,7 +39,7 @@ impl ServerInfo { let mut c = Command::new(runfiles::rlocation!( r, "examples/proto/helloworld/greeter_server/greeter_server" - )) + ).unwrap()) .arg("0") .stdout(Stdio::piped()) .spawn() @@ -70,7 +70,7 @@ impl ServerInfo { let mut cmd0 = Command::new(runfiles::rlocation!( r, "examples/proto/helloworld/greeter_client/greeter_client" - )); + ).unwrap()); let cmd = cmd0.arg(format!("-p={}", self.port)); let output = if let Some(s) = arg { cmd.arg(s) } else { cmd } diff --git a/test/bzl_version/bzl_version_test.rs b/test/bzl_version/bzl_version_test.rs index b551afc21d..85b296b0df 100644 --- a/test/bzl_version/bzl_version_test.rs +++ b/test/bzl_version/bzl_version_test.rs @@ -27,7 +27,7 @@ fn module_bzl_has_correct_version() { let version = std::env::var("VERSION").unwrap(); let module_bazel_text = { let r = Runfiles::create().unwrap(); - let path = runfiles::rlocation!(r, std::env::var("MODULE_BAZEL").unwrap()); + let path = runfiles::rlocation!(r, std::env::var("MODULE_BAZEL").unwrap()).unwrap(); std::fs::read_to_string(path).unwrap() }; diff --git a/test/bzlmod_repo_mapping/module_b/lib.rs b/test/bzlmod_repo_mapping/module_b/lib.rs index 3a555115d9..16400130ee 100644 --- a/test/bzlmod_repo_mapping/module_b/lib.rs +++ b/test/bzlmod_repo_mapping/module_b/lib.rs @@ -2,5 +2,5 @@ use runfiles::Runfiles; pub fn read_file_from_module_c() -> String { let r = Runfiles::create().unwrap(); - std::fs::read_to_string(runfiles::rlocation!(r, "aliased_c/MODULE.bazel")).unwrap() -} \ No newline at end of file + std::fs::read_to_string(runfiles::rlocation!(r, "aliased_c/MODULE.bazel").unwrap()).unwrap() +} diff --git a/test/process_wrapper/rustc_quit_on_rmeta.rs b/test/process_wrapper/rustc_quit_on_rmeta.rs index 9d7b38c5d5..5ea3e2e9d2 100644 --- a/test/process_wrapper/rustc_quit_on_rmeta.rs +++ b/test/process_wrapper/rustc_quit_on_rmeta.rs @@ -29,7 +29,8 @@ mod test { ] .iter() .collect::() - ); + ) + .unwrap(); let process_wrapper = runfiles::rlocation!( r, @@ -45,7 +46,8 @@ mod test { ] .iter() .collect::() - ); + ) + .unwrap(); let output = Command::new(process_wrapper) .args(process_wrapper_args) diff --git a/test/process_wrapper_bootstrap/bootstrap_process_wrapper_test.rs b/test/process_wrapper_bootstrap/bootstrap_process_wrapper_test.rs index 4e5230f14b..c2924b5de0 100644 --- a/test/process_wrapper_bootstrap/bootstrap_process_wrapper_test.rs +++ b/test/process_wrapper_bootstrap/bootstrap_process_wrapper_test.rs @@ -13,7 +13,8 @@ fn test_shebang() { let script = runfiles::rlocation!( rfiles, "rules_rust/util/process_wrapper/private/process_wrapper.sh" - ); + ) + .unwrap(); let content = read_to_string(script).unwrap(); assert!( diff --git a/test/rust/src/greeter.rs b/test/rust/src/greeter.rs index 1bc99ed5c4..886a2bf888 100644 --- a/test/rust/src/greeter.rs +++ b/test/rust/src/greeter.rs @@ -43,13 +43,13 @@ impl Greeter { /// /// let greeter = Greeter::from_txt_file()?; /// ``` - pub fn from_txt_file() -> std::io::Result { + pub fn from_txt_file() -> runfiles::Result { let r = runfiles::Runfiles::create()?; Ok(Greeter { - greeting: std::fs::read_to_string(runfiles::rlocation!( - r, - "rules_rust/test/rust/greeting.txt" - ))?, + greeting: std::fs::read_to_string( + runfiles::rlocation!(r, "rules_rust/test/rust/greeting.txt").unwrap(), + ) + .map_err(runfiles::RunfilesError::RunfileIoError)?, }) } diff --git a/tools/runfiles/runfiles.rs b/tools/runfiles/runfiles.rs index 0df98a2223..a3b690e658 100644 --- a/tools/runfiles/runfiles.rs +++ b/tools/runfiles/runfiles.rs @@ -2,34 +2,33 @@ //! //! USAGE: //! -//! 1. Depend on this runfiles library from your build rule: -//! ```python -//! rust_binary( -//! name = "my_binary", -//! ... -//! data = ["//path/to/my/data.txt"], -//! deps = ["@rules_rust//tools/runfiles"], -//! ) -//! ``` +//! 1. Depend on this runfiles library from your build rule: +//! ```python +//! rust_binary( +//! name = "my_binary", +//! ... +//! data = ["//path/to/my/data.txt"], +//! deps = ["@rules_rust//tools/runfiles"], +//! ) +//! ``` //! -//! 2. Import the runfiles library. -//! ```ignore -//! extern crate runfiles; +//! 2. Import the runfiles library. +//! ```ignore +//! use runfiles::Runfiles; +//! ``` //! -//! use runfiles::Runfiles; -//! ``` +//! 3. Create a Runfiles object and use `rlocation!`` to look up runfile paths: +//! ```ignore //! -//! 3. Create a Runfiles object and use `rlocation!`` to look up runfile paths: -//! ```ignore -- This doesn't work under rust_doc_test because argv[0] is not what we expect. +//! use runfiles::{Runfiles, rlocation}; //! -//! use runfiles::{Runfiles, rlocation}; +//! let r = Runfiles::create().unwrap(); +//! let path = rlocation!(r, "my_workspace/path/to/my/data.txt").expect("Failed to locate runfile"); //! -//! let r = Runfiles::create().unwrap(); -//! let path = rlocation!(r, "my_workspace/path/to/my/data.txt"); +//! let f = File::open(path).unwrap(); //! -//! let f = File::open(path).unwrap(); -//! // ... -//! ``` +//! // ... +//! ``` use std::collections::HashMap; use std::env; @@ -49,15 +48,81 @@ macro_rules! rlocation { }; } +/// The error type for [Runfiles] construction. +#[derive(Debug)] +pub enum RunfilesError { + /// Directory based runfiles could not be found. + RunfilesDirNotFound, + + /// An [I/O Error](https://doc.rust-lang.org/std/io/struct.Error.html) + /// which occurred during the creation of directory-based runfiles. + RunfilesDirIoError(io::Error), + + /// An [I/O Error](https://doc.rust-lang.org/std/io/struct.Error.html) + /// which occurred during the creation of manifest-file-based runfiles. + RunfilesManifestIoError(io::Error), + + /// A manifest file could not be parsed. + RunfilesManifestInvalidFormat, + + /// The bzlmod repo-mapping file could not be found. + RepoMappingNotFound, + + /// The bzlmod repo-mapping file could not be parsed. + RepoMappingInvalidFormat, + + /// An [I/O Error](https://doc.rust-lang.org/std/io/struct.Error.html) + /// which occurred during the parsing of a repo-mapping file. + RepoMappingIoError(io::Error), + + /// An error indicating a specific Runfile was not found. + RunfileNotFound(PathBuf), + + /// An [I/O Error](https://doc.rust-lang.org/std/io/struct.Error.html) + /// which occurred when operating with a particular runfile. + RunfileIoError(io::Error), +} + +impl std::fmt::Display for RunfilesError { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + match self { + RunfilesError::RunfilesDirNotFound => write!(f, "RunfilesDirNotFound"), + RunfilesError::RunfilesDirIoError(err) => write!(f, "RunfilesDirIoError: {:?}", err), + RunfilesError::RunfilesManifestIoError(err) => { + write!(f, "RunfilesManifestIoError: {:?}", err) + } + RunfilesError::RunfilesManifestInvalidFormat => write!(f, "RepoMappingInvalidFormat"), + RunfilesError::RepoMappingNotFound => write!(f, "RepoMappingInvalidFormat"), + RunfilesError::RepoMappingInvalidFormat => write!(f, "RepoMappingInvalidFormat"), + RunfilesError::RepoMappingIoError(err) => write!(f, "RepoMappingIoError: {:?}", err), + RunfilesError::RunfileNotFound(path) => { + write!(f, "RunfileNotFound: {}", path.display()) + } + RunfilesError::RunfileIoError(err) => write!(f, "RunfileIoError: {:?}", err), + } + } +} + +impl std::error::Error for RunfilesError {} + +/// A specialized [`std::result::Result`] type for +pub type Result = std::result::Result; + #[derive(Debug)] enum Mode { + /// Runfiles located in a directory indicated by the `RUNFILES_DIR` environment + /// variable or a neighboring `*.runfiles` directory to the executable. DirectoryBased(PathBuf), + + /// Runfiles represented as a mapping of `rlocationpath` to real paths indicated + /// by the `RUNFILES_MANIFEST_FILE` environment variable. ManifestBased(HashMap), } type RepoMappingKey = (String, String); type RepoMapping = HashMap; +/// An interface for accessing to [Bazel runfiles](https://bazel.build/extending/rules#runfiles). #[derive(Debug)] pub struct Runfiles { mode: Mode, @@ -68,31 +133,31 @@ impl Runfiles { /// Creates a manifest based Runfiles object when /// RUNFILES_MANIFEST_FILE environment variable is present, /// or a directory based Runfiles object otherwise. - pub fn create() -> io::Result { + pub fn create() -> Result { let mode = if let Some(manifest_file) = std::env::var_os(MANIFEST_FILE_ENV_VAR) { Self::create_manifest_based(Path::new(&manifest_file))? } else { Mode::DirectoryBased(find_runfiles_dir()?) }; - let repo_mapping = parse_repo_mapping(raw_rlocation(&mode, "_repo_mapping")) - .unwrap_or_else(|_| { - println!("No repo mapping found!"); - RepoMapping::new() - }); + let repo_mapping = raw_rlocation(&mode, "_repo_mapping") + .map(parse_repo_mapping) + .transpose()? + .unwrap_or_default(); Ok(Runfiles { mode, repo_mapping }) } - fn create_manifest_based(manifest_path: &Path) -> io::Result { - let manifest_content = std::fs::read_to_string(manifest_path)?; + fn create_manifest_based(manifest_path: &Path) -> Result { + let manifest_content = std::fs::read_to_string(manifest_path) + .map_err(RunfilesError::RunfilesManifestIoError)?; let path_mapping = manifest_content .lines() - .map(|line| { + .flat_map(|line| { let pair = line .split_once(' ') - .expect("manifest file contained unexpected content"); - (pair.0.into(), pair.1.into()) + .ok_or(RunfilesError::RunfilesManifestInvalidFormat)?; + Ok::<(PathBuf, PathBuf), RunfilesError>((pair.0.into(), pair.1.into())) }) .collect::>(); Ok(Mode::ManifestBased(path_mapping)) @@ -104,10 +169,10 @@ impl Runfiles { /// The returned path may not be valid. The caller should check the path's /// validity and that the path exists. /// @deprecated - this is not bzlmod-aware. Prefer the `rlocation!` macro or `rlocation_from` - pub fn rlocation(&self, path: impl AsRef) -> PathBuf { + pub fn rlocation(&self, path: impl AsRef) -> Option { let path = path.as_ref(); if path.is_absolute() { - return path.to_path_buf(); + return Some(path.to_path_buf()); } raw_rlocation(&self.mode, path) } @@ -119,10 +184,10 @@ impl Runfiles { /// validity and that the path exists. /// /// Typically this should be used via the `rlocation!` macro to properly set source_repo. - pub fn rlocation_from(&self, path: impl AsRef, source_repo: &str) -> PathBuf { + pub fn rlocation_from(&self, path: impl AsRef, source_repo: &str) -> Option { let path = path.as_ref(); if path.is_absolute() { - return path.to_path_buf(); + return Some(path.to_path_buf()); } let path_str = path.to_str().expect("Should be valid UTF8"); @@ -144,24 +209,24 @@ impl Runfiles { } } -fn raw_rlocation(mode: &Mode, path: impl AsRef) -> PathBuf { +fn raw_rlocation(mode: &Mode, path: impl AsRef) -> Option { let path = path.as_ref(); match mode { - Mode::DirectoryBased(runfiles_dir) => runfiles_dir.join(path), - Mode::ManifestBased(path_mapping) => path_mapping - .get(path) - .unwrap_or_else(|| panic!("Path {} not found among runfiles.", path.to_string_lossy())) - .clone(), + Mode::DirectoryBased(runfiles_dir) => Some(runfiles_dir.join(path)), + Mode::ManifestBased(path_mapping) => path_mapping.get(path).cloned(), } } -fn parse_repo_mapping(path: PathBuf) -> io::Result { +fn parse_repo_mapping(path: PathBuf) -> Result { let mut repo_mapping = RepoMapping::new(); - for line in std::fs::read_to_string(path)?.lines() { + for line in std::fs::read_to_string(path) + .map_err(RunfilesError::RepoMappingIoError)? + .lines() + { let parts: Vec<&str> = line.splitn(3, ',').collect(); if parts.len() < 3 { - return Err(make_io_error("Malformed repo_mapping file")); + return Err(RunfilesError::RepoMappingInvalidFormat); } repo_mapping.insert((parts[0].into(), parts[1].into()), parts[2].into()); } @@ -170,10 +235,14 @@ fn parse_repo_mapping(path: PathBuf) -> io::Result { } /// Returns the .runfiles directory for the currently executing binary. -pub fn find_runfiles_dir() -> io::Result { - assert!(std::env::var_os(MANIFEST_FILE_ENV_VAR).is_none()); - - // If bazel told us about the runfiles dir, use that without looking further. +pub fn find_runfiles_dir() -> Result { + assert!( + std::env::var_os(MANIFEST_FILE_ENV_VAR).is_none(), + "Unexpected call when {} exists", + MANIFEST_FILE_ENV_VAR + ); + + // If Bazel told us about the runfiles dir, use that without looking further. if let Some(runfiles_dir) = std::env::var_os(RUNFILES_DIR_ENV_VAR).map(PathBuf::from) { if runfiles_dir.is_dir() { return Ok(runfiles_dir); @@ -188,9 +257,12 @@ pub fn find_runfiles_dir() -> io::Result { // Consume the first argument (argv[0]) let exec_path = std::env::args().next().expect("arg 0 was not set"); + let current_dir = + env::current_dir().expect("The current working directory is always expected to be set."); + let mut binary_path = PathBuf::from(&exec_path); loop { - // Check for our neighboring $binary.runfiles directory. + // Check for our neighboring `${binary}.runfiles` directory. let mut runfiles_name = binary_path.file_name().unwrap().to_owned(); runfiles_name.push(".runfiles"); @@ -199,7 +271,7 @@ pub fn find_runfiles_dir() -> io::Result { return Ok(runfiles_path); } - // Check if we're already under a *.runfiles directory. + // Check if we're already under a `*.runfiles` directory. { // TODO: 1.28 adds Path::ancestors() which is a little simpler. let mut next = binary_path.parent(); @@ -214,33 +286,107 @@ pub fn find_runfiles_dir() -> io::Result { } } - if !fs::symlink_metadata(&binary_path)?.file_type().is_symlink() { + if !fs::symlink_metadata(&binary_path) + .map_err(RunfilesError::RunfilesDirIoError)? + .file_type() + .is_symlink() + { break; } // Follow symlinks and keep looking. - let link_target = binary_path.read_link()?; + let link_target = binary_path + .read_link() + .map_err(RunfilesError::RunfilesDirIoError)?; binary_path = if link_target.is_absolute() { link_target } else { let link_dir = binary_path.parent().unwrap(); - env::current_dir()?.join(link_dir).join(link_target) + current_dir.join(link_dir).join(link_target) } } - Err(make_io_error("failed to find .runfiles directory")) -} - -fn make_io_error(msg: &str) -> io::Error { - io::Error::new(io::ErrorKind::Other, msg) + Err(RunfilesError::RunfilesDirNotFound) } #[cfg(test)] mod test { use super::*; + use std::ffi::OsStr; use std::fs::File; use std::io::prelude::*; + /// Only `RUNFILES_DIR`` is set. + /// + /// This test is a part of `test_manifest_based_can_read_data_from_runfiles` as + /// it modifies environment variables. + fn test_env_only_runfiles_dir(test_srcdir: &OsStr, runfiles_manifest_file: &OsStr) { + env::remove_var(TEST_SRCDIR_ENV_VAR); + env::remove_var(MANIFEST_FILE_ENV_VAR); + let r = Runfiles::create().unwrap(); + + let d = rlocation!(r, "rules_rust").unwrap(); + let f = rlocation!(r, "rules_rust/tools/runfiles/data/sample.txt").unwrap(); + assert_eq!(d.join("tools/runfiles/data/sample.txt"), f); + + let mut f = File::open(f).unwrap(); + + let mut buffer = String::new(); + f.read_to_string(&mut buffer).unwrap(); + + assert_eq!("Example Text!", buffer); + env::set_var(TEST_SRCDIR_ENV_VAR, test_srcdir); + env::set_var(MANIFEST_FILE_ENV_VAR, runfiles_manifest_file); + } + + /// Only `TEST_SRCDIR` is set. + /// + /// This test is a part of `test_manifest_based_can_read_data_from_runfiles` as + /// it modifies environment variables. + fn test_env_only_test_srcdir(runfiles_dir: &OsStr, runfiles_manifest_file: &OsStr) { + env::remove_var(RUNFILES_DIR_ENV_VAR); + env::remove_var(MANIFEST_FILE_ENV_VAR); + let r = Runfiles::create().unwrap(); + + let mut f = File::open(rlocation!(r, "rules_rust/tools/runfiles/data/sample.txt").unwrap()) + .unwrap(); + + let mut buffer = String::new(); + f.read_to_string(&mut buffer).unwrap(); + + assert_eq!("Example Text!", buffer); + env::set_var(RUNFILES_DIR_ENV_VAR, runfiles_dir); + env::set_var(MANIFEST_FILE_ENV_VAR, runfiles_manifest_file); + } + + /// Neither `RUNFILES_DIR` or `TEST_SRCDIR` are set + /// + /// This test is a part of `test_manifest_based_can_read_data_from_runfiles` as + /// it modifies environment variables. + fn test_env_nothing_set( + test_srcdir: &OsStr, + runfiles_dir: &OsStr, + runfiles_manifest_file: &OsStr, + ) { + env::remove_var(RUNFILES_DIR_ENV_VAR); + env::remove_var(TEST_SRCDIR_ENV_VAR); + env::remove_var(MANIFEST_FILE_ENV_VAR); + + let r = Runfiles::create().unwrap(); + + let mut f = File::open(rlocation!(r, "rules_rust/tools/runfiles/data/sample.txt").unwrap()) + .unwrap(); + + let mut buffer = String::new(); + f.read_to_string(&mut buffer).unwrap(); + + assert_eq!("Example Text!", buffer); + + env::set_var(TEST_SRCDIR_ENV_VAR, test_srcdir); + env::set_var(RUNFILES_DIR_ENV_VAR, runfiles_dir); + env::set_var(MANIFEST_FILE_ENV_VAR, runfiles_manifest_file); + } + #[test] fn test_can_read_data_from_runfiles() { // We want to run multiple test cases with different environment variables set. Since @@ -254,66 +400,25 @@ mod test { env::var_os(RUNFILES_DIR_ENV_VAR).expect("bazel did not provide RUNFILES_DIR"); let runfiles_manifest_file = env::var_os(MANIFEST_FILE_ENV_VAR).unwrap_or("".into()); - // Test case 1: Only $RUNFILES_DIR is set. - { - env::remove_var(TEST_SRCDIR_ENV_VAR); - env::remove_var(MANIFEST_FILE_ENV_VAR); - let r = Runfiles::create().unwrap(); - - let d = rlocation!(r, "rules_rust"); - let f = rlocation!(r, "rules_rust/tools/runfiles/data/sample.txt"); - assert_eq!(d.join("tools/runfiles/data/sample.txt"), f); - - let mut f = File::open(f).unwrap(); - - let mut buffer = String::new(); - f.read_to_string(&mut buffer).unwrap(); - - assert_eq!("Example Text!", buffer); - env::set_var(TEST_SRCDIR_ENV_VAR, &test_srcdir); - env::set_var(MANIFEST_FILE_ENV_VAR, &runfiles_manifest_file); - } - // Test case 2: Only $TEST_SRCDIR is set. - { - env::remove_var(RUNFILES_DIR_ENV_VAR); - env::remove_var(MANIFEST_FILE_ENV_VAR); - let r = Runfiles::create().unwrap(); - - let mut f = - File::open(rlocation!(r, "rules_rust/tools/runfiles/data/sample.txt")).unwrap(); - - let mut buffer = String::new(); - f.read_to_string(&mut buffer).unwrap(); - - assert_eq!("Example Text!", buffer); - env::set_var(RUNFILES_DIR_ENV_VAR, &runfiles_dir); - env::set_var(MANIFEST_FILE_ENV_VAR, &runfiles_manifest_file); - } - - // Test case 3: Neither are set - { - env::remove_var(RUNFILES_DIR_ENV_VAR); - env::remove_var(TEST_SRCDIR_ENV_VAR); - env::remove_var(MANIFEST_FILE_ENV_VAR); - - let r = Runfiles::create().unwrap(); - - let mut f = - File::open(rlocation!(r, "rules_rust/tools/runfiles/data/sample.txt")).unwrap(); - - let mut buffer = String::new(); - f.read_to_string(&mut buffer).unwrap(); + test_env_only_runfiles_dir(&test_srcdir, &runfiles_manifest_file); + test_env_only_test_srcdir(&runfiles_dir, &runfiles_manifest_file); + test_env_nothing_set(&test_srcdir, &runfiles_dir, &runfiles_manifest_file); + } - assert_eq!("Example Text!", buffer); + #[test] + fn test_manifest_based_can_read_data_from_runfiles() { + let mut path_mapping = HashMap::new(); + path_mapping.insert("a/b".into(), "c/d".into()); + let r = Runfiles { + mode: Mode::ManifestBased(path_mapping), + repo_mapping: RepoMapping::new(), + }; - env::set_var(TEST_SRCDIR_ENV_VAR, &test_srcdir); - env::set_var(RUNFILES_DIR_ENV_VAR, &runfiles_dir); - env::set_var(MANIFEST_FILE_ENV_VAR, &runfiles_manifest_file); - } + assert_eq!(r.rlocation("a/b"), Some(PathBuf::from("c/d"))); } #[test] - fn test_manifest_based_can_read_data_from_runfiles() { + fn test_manifest_based_missing_file() { let mut path_mapping = HashMap::new(); path_mapping.insert("a/b".into(), "c/d".into()); let r = Runfiles { @@ -321,6 +426,6 @@ mod test { repo_mapping: RepoMapping::new(), }; - assert_eq!(r.rlocation("a/b"), PathBuf::from("c/d")); + assert_eq!(r.rlocation("does/not/exist"), None); } } diff --git a/tools/rust_analyzer/lib.rs b/tools/rust_analyzer/lib.rs index 874f85566d..55f1946071 100644 --- a/tools/rust_analyzer/lib.rs +++ b/tools/rust_analyzer/lib.rs @@ -62,7 +62,8 @@ pub fn write_rust_project( let path = runfiles::rlocation!( Runfiles::create()?, "rules_rust/rust/private/rust_analyzer_detect_sysroot.rust_analyzer_toolchain.json" - ); + ) + .unwrap(); let toolchain_info: HashMap = serde_json::from_str(&std::fs::read_to_string(path)?)?; diff --git a/tools/rustfmt/src/lib.rs b/tools/rustfmt/src/lib.rs index eb51bbd1bb..840aa39dfc 100644 --- a/tools/rustfmt/src/lib.rs +++ b/tools/rustfmt/src/lib.rs @@ -21,12 +21,12 @@ pub struct RustfmtConfig { pub fn parse_rustfmt_config() -> RustfmtConfig { let runfiles = runfiles::Runfiles::create().unwrap(); - let rustfmt = runfiles::rlocation!(runfiles, env!("RUSTFMT")); + let rustfmt = runfiles::rlocation!(runfiles, env!("RUSTFMT")).unwrap(); if !rustfmt.exists() { panic!("rustfmt does not exist at: {}", rustfmt.display()); } - let config = runfiles::rlocation!(runfiles, env!("RUSTFMT_CONFIG")); + let config = runfiles::rlocation!(runfiles, env!("RUSTFMT_CONFIG")).unwrap(); if !config.exists() { panic!( "rustfmt config file does not exist at: {}", @@ -71,7 +71,7 @@ pub fn parse_rustfmt_manifest(manifest: &Path) -> RustfmtManifest { edition, sources: lines .into_iter() - .map(|src| runfiles::rlocation!(runfiles, src)) + .map(|src| runfiles::rlocation!(runfiles, src).unwrap()) .collect(), } } @@ -90,7 +90,7 @@ pub fn find_manifests() -> Vec { std::env::var("RUSTFMT_MANIFESTS") .map(|var| { var.split(PATH_ENV_SEP) - .map(|path| runfiles::rlocation!(runfiles, path)) + .map(|path| runfiles::rlocation!(runfiles, path).unwrap()) .collect() }) .unwrap_or_default() diff --git a/tools/upstream_wrapper/src/main.rs b/tools/upstream_wrapper/src/main.rs index 6d6e167c26..d57492dff4 100644 --- a/tools/upstream_wrapper/src/main.rs +++ b/tools/upstream_wrapper/src/main.rs @@ -13,7 +13,7 @@ const PATH_SEPARATOR: &str = ";"; fn main() { let runfiles = runfiles::Runfiles::create().unwrap(); - let wrapped_tool_path = runfiles::rlocation!(runfiles, WRAPPED_TOOL_TARGET); + let wrapped_tool_path = runfiles::rlocation!(runfiles, WRAPPED_TOOL_TARGET).unwrap(); if !wrapped_tool_path.exists() { panic!( "{WRAPPED_TOOL_NAME} does not exist at: {}",