Remove runfiles.current_repository()

bazelbuild · Mar 21, 2024 · 30159c1 · 30159c1
1 parent 5c621aa
commit 30159c1
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 134 deletions.
diff --git a/tools/runfiles/BUILD.bazel b/tools/runfiles/BUILD.bazel
@@ -4,19 +4,11 @@ load(
     "rust_library",
     "rust_test",
 )
-load("//tools/runfiles/private:runfiles_utils.bzl", "workspace_name")
-
-workspace_name(
-    name = "workspace_name.env",
-)
 
 rust_library(
     name = "runfiles",
     srcs = ["runfiles.rs"],
     edition = "2018",
-    rustc_env_files = [
-        ":workspace_name.env",
-    ],
     visibility = ["//visibility:public"],
 )
 

diff --git a/tools/runfiles/private/BUILD.bazel b/tools/runfiles/private/BUILD.bazel
diff --git a/tools/runfiles/private/runfiles_utils.bzl b/tools/runfiles/private/runfiles_utils.bzl
diff --git a/tools/runfiles/runfiles.rs b/tools/runfiles/runfiles.rs
@@ -19,33 +19,31 @@
 //!     use runfiles::Runfiles;
 //!     ```
 //!
-//! 3.  Create a Runfiles object and use Rlocation! to look up runfile paths:
+//! 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");
+//!     let path = rlocation!(r, "my_workspace/path/to/my/data.txt");
 //!
 //!     let f = File::open(path).unwrap();
 //!     // ...
 //!     ```
 
 use std::collections::HashMap;
 use std::env;
-use std::ffi::OsString;
 use std::fs;
 use std::io;
 use std::path::Path;
 use std::path::PathBuf;
 
 const RUNFILES_DIR_ENV_VAR: &str = "RUNFILES_DIR";
 const MANIFEST_FILE_ENV_VAR: &str = "RUNFILES_MANIFEST_FILE";
-const MANIFEST_ONLY_ENV_VAR: &str = "RUNFILES_MANIFEST_ONLY";
 const TEST_SRCDIR_ENV_VAR: &str = "TEST_SRCDIR";
 
 #[macro_export]
-macro_rules! Rlocation {
+macro_rules! rlocation {
     ($r:ident, $path:expr) => {
         $r.rlocation_from($path, env!("REPOSITORY_NAME"))
     };
@@ -71,23 +69,22 @@ impl Runfiles {
     /// RUNFILES_MANIFEST_ONLY environment variable is present,
     /// or a directory based Runfiles object otherwise.
     pub fn create() -> io::Result<Self> {
-        let mode = if is_manifest_only() {
-            Self::create_manifest_based()
+        let mode = if let Ok(manifest_file) = std::env::var(MANIFEST_FILE_ENV_VAR) {
+            Self::create_manifest_based(&PathBuf::from(manifest_file))?
         } else {
-            Self::create_directory_based()
-        }?;
+            Mode::DirectoryBased(find_runfiles_dir()?)
+        };
 
-        let repo_mapping = parse_repo_mapping(raw_rlocation(&mode, "_repo_mapping"))?;
+        let repo_mapping = parse_repo_mapping(raw_rlocation(&mode, "_repo_mapping"))
+            .unwrap_or_else(|_| {
+                println!("No repo mapping found!");
+                RepoMapping::new()
+            });
 
         Ok(Runfiles { mode, repo_mapping })
     }
 
-    fn create_directory_based() -> io::Result<Mode> {
-        Ok(Mode::DirectoryBased(find_runfiles_dir()?))
-    }
-
-    fn create_manifest_based() -> io::Result<Mode> {
-        let manifest_path = find_manifest_path()?;
+    fn create_manifest_based(manifest_path: &Path) -> io::Result<Mode> {
         let manifest_content = std::fs::read_to_string(manifest_path)?;
         let path_mapping = manifest_content
             .lines()
@@ -121,29 +118,29 @@ impl Runfiles {
     /// The returned path may not be valid. The caller should check the path's
     /// validity and that the path exists.
     ///
-    /// Typically this should be used via the `RLocation!` macro to properly set source_repo.
+    /// Typically this should be used via the `rlocation!` macro to properly set source_repo.
     pub fn rlocation_from(&self, path: impl AsRef<Path>, source_repo: &str) -> PathBuf {
         let path = path.as_ref();
         if path.is_absolute() {
             return path.to_path_buf();
         }
 
-        let parts: Vec<&str> = path.to_str().expect("Should be valid UTF8").splitn(2, '/').collect();
+        let parts: Vec<&str> = path
+            .to_str()
+            .expect("Should be valid UTF8")
+            .splitn(2, '/')
+            .collect();
         if parts.len() == 2 {
             let key: (String, String) = (source_repo.into(), parts[0].into());
             if let Some(target_repo_directory) = self.repo_mapping.get(&key) {
-                return raw_rlocation(&self.mode, target_repo_directory.to_owned() + "/" + parts[1]);
+                return raw_rlocation(
+                    &self.mode,
+                    target_repo_directory.to_owned() + "/" + parts[1],
+                );
             };
         }
         raw_rlocation(&self.mode, path)
     }
-
-    /// Returns the canonical name of the caller's Bazel repository.
-    pub fn current_repository(&self) -> &str {
-        // This value must match the value of `_RULES_RUST_RUNFILES_WORKSPACE_NAME`
-        // which can be found in `@rules_rust//tools/runfiles/private:workspace_name.bzl`
-        env!("RULES_RUST_RUNFILES_WORKSPACE_NAME")
-    }
 }
 
 fn raw_rlocation(mode: &Mode, path: impl AsRef<Path>) -> PathBuf {
@@ -152,29 +149,24 @@ fn raw_rlocation(mode: &Mode, path: impl AsRef<Path>) -> PathBuf {
         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())
-            })
+            .unwrap_or_else(|| panic!("Path {} not found among runfiles.", path.to_string_lossy()))
             .clone(),
     }
 }
 
 fn parse_repo_mapping(path: PathBuf) -> io::Result<RepoMapping> {
     Ok(std::fs::read_to_string(path)?
-            .lines()
-            .map(|line| {
-                let parts: Vec<String> = line.splitn(3, ',').map(String::from).collect();
-                ((parts[0].clone(), parts[1].clone()), parts[2].clone())
-            })
-            .collect::<RepoMapping>())
+        .lines()
+        .map(|line| {
+            let parts: Vec<String> = line.splitn(3, ',').map(String::from).collect();
+            ((parts[0].clone(), parts[1].clone()), parts[2].clone())
+        })
+        .collect::<RepoMapping>())
 }
 
 /// Returns the .runfiles directory for the currently executing binary.
 pub fn find_runfiles_dir() -> io::Result<PathBuf> {
-    assert_ne!(
-        std::env::var_os(MANIFEST_ONLY_ENV_VAR).unwrap_or_else(|| OsString::from("0")),
-        "1"
-    );
+    assert!(std::env::var_os(MANIFEST_FILE_ENV_VAR).is_none());
 
     // 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) {
@@ -237,26 +229,6 @@ fn make_io_error(msg: &str) -> io::Error {
     io::Error::new(io::ErrorKind::Other, msg)
 }
 
-fn is_manifest_only() -> bool {
-    match std::env::var(MANIFEST_ONLY_ENV_VAR) {
-        Ok(val) => val == "1",
-        Err(_) => false,
-    }
-}
-
-fn find_manifest_path() -> io::Result<PathBuf> {
-    assert_eq!(
-        std::env::var_os(MANIFEST_ONLY_ENV_VAR).expect("RUNFILES_MANIFEST_ONLY was not set"),
-        OsString::from("1")
-    );
-    match std::env::var_os(MANIFEST_FILE_ENV_VAR) {
-        Some(path) => Ok(path.into()),
-        None => Err(
-            make_io_error(
-                "RUNFILES_MANIFEST_ONLY was set to '1', but RUNFILES_MANIFEST_FILE was not set. Did Bazel change?"))
-    }
-}
-
 #[cfg(test)]
 mod test {
     use super::*;
@@ -267,18 +239,20 @@ mod test {
     #[test]
     fn test_can_read_data_from_runfiles() {
         // We want to run multiple test cases with different environment variables set. Since
-        // environment variables are global state, we need to ensure the two test cases do not run
+        // environment variables are global state, we need to ensure the test cases do not run
         // concurrently. Rust runs tests in parallel and does not provide an easy way to synchronise
         // them, so we run all test cases in the same #[test] function.
 
         let test_srcdir =
             env::var_os(TEST_SRCDIR_ENV_VAR).expect("bazel did not provide TEST_SRCDIR");
         let runfiles_dir =
             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 mut f =
@@ -288,11 +262,13 @@ mod test {
             f.read_to_string(&mut buffer).unwrap();
 
             assert_eq!("Example Text!", buffer);
-            env::set_var(TEST_SRCDIR_ENV_VAR, &test_srcdir)
+            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 =
@@ -302,13 +278,15 @@ mod test {
             f.read_to_string(&mut buffer).unwrap();
 
             assert_eq!("Example Text!", buffer);
-            env::set_var(RUNFILES_DIR_ENV_VAR, &runfiles_dir)
+            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();
 
@@ -322,6 +300,7 @@ mod test {
 
             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);
         }
     }
 
@@ -336,13 +315,4 @@ mod test {
 
         assert_eq!(r.rlocation("a/b"), PathBuf::from("c/d"));
     }
-
-    #[test]
-    fn test_current_repository() {
-        let r = Runfiles::create().unwrap();
-
-        // This check is unique to the rules_rust repository. The name
-        // here is expected to be different in consumers of this library
-        assert_eq!(r.current_repository(), "rules_rust")
-    }
 }
diff --git a/tools/rustfmt/BUILD.bazel b/tools/rustfmt/BUILD.bazel
@@ -24,8 +24,8 @@ rust_library(
     ],
     edition = "2018",
     rustc_env = {
-        "RUSTFMT": "$(rootpath //rust/toolchain:current_rustfmt_toolchain)",
-        "RUSTFMT_CONFIG": "$(rootpath //:rustfmt.toml)",
+        "RUSTFMT": "$(rlocationpath //rust/toolchain:current_rustfmt_toolchain)",
+        "RUSTFMT_CONFIG": "$(rlocationpath //:rustfmt.toml)",
     },
     deps = [
         "//tools/runfiles",

diff --git a/tools/rustfmt/src/lib.rs b/tools/rustfmt/src/lib.rs
@@ -21,20 +21,12 @@ pub struct RustfmtConfig {
 pub fn parse_rustfmt_config() -> RustfmtConfig {
     let runfiles = runfiles::Runfiles::create().unwrap();
 
-    let rustfmt = runfiles.rlocation(format!(
-        "{}/{}",
-        runfiles.current_repository(),
-        env!("RUSTFMT")
-    ));
+    let rustfmt = runfiles::rlocation!(runfiles, env!("RUSTFMT"));
     if !rustfmt.exists() {
         panic!("rustfmt does not exist at: {}", rustfmt.display());
     }
 
-    let config = runfiles.rlocation(format!(
-        "{}/{}",
-        runfiles.current_repository(),
-        env!("RUSTFMT_CONFIG")
-    ));
+    let config = runfiles::rlocation!(runfiles, env!("RUSTFMT_CONFIG"));
     if !config.exists() {
         panic!(
             "rustfmt config file does not exist at: {}",
@@ -79,7 +71,7 @@ pub fn parse_rustfmt_manifest(manifest: &Path) -> RustfmtManifest {
         edition,
         sources: lines
             .into_iter()
-            .map(|src| runfiles.rlocation(format!("{}/{}", runfiles.current_repository(), src)))
+            .map(|src| runfiles::rlocation!(runfiles, format!("rules_rust/{}", src)))
             .collect(),
     }
 }
@@ -100,11 +92,10 @@ pub fn find_manifests() -> Vec<PathBuf> {
             var.split(PATH_ENV_SEP)
                 .filter_map(|path| match path.is_empty() {
                     true => None,
-                    false => Some(runfiles.rlocation(format!(
-                        "{}/{}",
-                        runfiles.current_repository(),
-                        path
-                    ))),
+                    false => Some(runfiles::rlocation!(
+                        runfiles,
+                        format!("rules_rust/{}", path)
+                    )),
                 })
                 .collect()
         })