Support the RUNFILES_DIR environment variable.

[matts1]

RUNFILES_DIR is the preferred way to get runfiles directories in bazel.

Also refactor strings into constants for the environment variables while I'm at it.

[UebelAndre]

RUNFILES_DIR is the preferred way to get runfiles directories in bazel.

Out of curiosity, Is this documented somewhere?

[matts1]

Runfiles is a standardized concept for bazel which is supposed to have the same implementation across all languages. I remember seeing a while back a document detailing exactly what resolution steps are supposed to be done, but I can't seem to find it again.

In the meantime, you can see that bash runfiles and python runfiles both use RUNFILES_DIR as their first step to resolve what the runfiles directory is.

In fact, strictly speaking, we should remove TEST_SRCDIR, as that isn't supposed to be used for runfiles dir resolution (unfortunately that would break backwards compatibility though, which is why I'm not doing that).

[matts1]

Ah, I found it!
https://docs.google.com/document/d/e/2PACX-1vSDIrFnFvEYhKsCMdGdD40wZRBX3m3aZ5HhVj4CtHPmiXKDCxioTUbYsDydjKtFDAzER5eg7OjJWs3V/pub

That being said, "preferred" may be a bit of a misnomer. More accurately, it's supposed to be the first step in runfiles discovery.

[UebelAndre]

Thanks, seems good to me! Looks like you just gotta run rustfmt and the change should be good:

(00:40:59) ERROR: /Users/buildkite/builds/bk-imacpro-17/bazel/rules-rust-rustlang/tools/runfiles/BUILD.bazel:13:13: Rustfmt tools/runfiles/runfiles.rustfmt.ok failed: (Exit 1): process_wrapper failed: error executing command (from target //tools/runfiles:runfiles)
  (cd /private/var/tmp/_bazel_buildkite/da7d54490da9a8b7b2b3fbc7041273ea/sandbox/darwin-sandbox/998/execroot/rules_rust && \
  exec env - \
  bazel-out/darwin-opt-exec-2B5CBBC6/bin/util/process_wrapper/process_wrapper --touch-file bazel-out/darwin-fastbuild/bin/tools/runfiles/runfiles.rustfmt.ok -- bazel-out/darwin-fastbuild/bin/external/rust_darwin_x86_64__x86_64-apple-darwin__stable_tools/rust_toolchain/bin/rustfmt --config-path tools/rustfmt/rustfmt.toml --edition 2018 --check tools/runfiles/runfiles.rs)
# Configuration: 190f4be5a18d18e85f1d2018e915d8ba4089f75a5e745f15f7a00572971ca6bf
# Execution platform: @local_config_platform//:host
 
Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
Diff in /Users/buildkite/builds/bk-imacpro-17/bazel/rules-rust-rustlang/tools/runfiles/runfiles.rs at line 221:
         // 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 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");
 
         // Test case 1: Only $RUNFILES_DIR is set.
         {

[liningpan]

Should the rust runfiles library also set the RUNFILES_DIR? This is also mentioned in the document linked above.

otherwise (C++) the runfiles library's Create() method implements the discovery logic, and also sets the RUNFILES_MANIFEST_FILE or RUNFILES_DIR envvars (for the benefit of data-dependency binaries)

However, Rust std::env::set_var has some know thread-safety issues interacting with libc (rust-lang/rust#27970). Maybe it could be an opt-in feature.