Add support for experimental_output_paths (#4011)

For details on path mapping see
bazelbuild/bazel#22658

Changes:
- The process wrapper now explicitly handles `out_dir` to avoid location
expanded environment variables.
- `construct_arguments` is updated to support `rustc_flags` as an `Args`
object. `List` is still supported though

Author: UebelAndre

.bazelci/presubmit.yml

@@ -377,43 +377,100 @@ tasks:
     platform: ubuntu2204
     run_targets:
       - "//test/rustfmt:rustfmt_failure_tester"
-  rust_analyzer_tests_linux:
-    name: Rust-Analyzer Linux Tests
+  ide_integration_tests_linux:
+    name: IDE VSCode Tests
     platform: ubuntu2204
     run_targets:
+      - "//tools/vscode:gen_launch_json"
+      - "//test/vscode:vscode_test"
       - "//tools/rust_analyzer:gen_rust_project"
       - "//tools/rust_analyzer:discover_bazel_rust_project"
       - "//test/rust_analyzer:rust_analyzer_test"
-  rust_analyzer_tests_macos:
-    name: Rust-Analyzer Macos Tests
+  ide_integration_tests_macos:
+    name: IDE VSCode Tests
     platform: macos_arm64
     run_targets:
+      - "//tools/vscode:gen_launch_json"
+      - "//test/vscode:vscode_test"
       - "//tools/rust_analyzer:gen_rust_project"
       - "//tools/rust_analyzer:discover_bazel_rust_project"
       - "//test/rust_analyzer:rust_analyzer_test"
-  rust_analyzer_tests_windows:
-    name: Rust-Analyzer Windows Tests
+  ide_integration_tests_windows:
+    name: IDE VSCode Tests
     platform: windows
     run_targets:
+      - "//tools/vscode:gen_launch_json"
       - "//tools/rust_analyzer:gen_rust_project"
       - "//tools/rust_analyzer:discover_bazel_rust_project"
-  ide_integration_vscode_tests_linux:
-    name: IDE VSCode Tests
+      # TODO: Enable rust-analyzer tests on windows.
+      # - "//test/rust_analyzer:rust_analyzer_test"
+
+  path_mapping_ubuntu2204:
+    name: Path Mapping Linux
     platform: ubuntu2204
-    run_targets:
-      - "//tools/vscode:gen_launch_json"
-      - "//test/vscode:vscode_test"
-  ide_integration_vscode_tests_macos:
-    name: IDE VSCode Tests
+    build_flags:
+      - --config=clippy
+      - --config=rustfmt
+      - --experimental_output_paths=strip
+    test_flags:
+      - --config=clippy
+      - --config=rustfmt
+      - --experimental_output_paths=strip
+    build_targets: *default_linux_targets
+    test_targets: *default_linux_targets
+    coverage_targets: *default_linux_targets
+    post_shell_commands: *coverage_validation_post_shell_commands
+  path_mapping_rbe_ubuntu2204:
+    name: Path Mapping RBE
+    platform: rbe_ubuntu2204
+    shell_commands:
+      - sed -i 's/^# load("@bazel_ci_rules/load("@bazel_ci_rules/' WORKSPACE.bazel
+      - sed -i 's/^# rbe_preconfig/rbe_preconfig/' WORKSPACE.bazel
+    build_flags:
+      - --config=clippy
+      - --config=rustfmt
+      - --experimental_output_paths=strip
+    test_flags:
+      - --config=clippy
+      - --config=rustfmt
+      - --experimental_output_paths=strip
+    build_targets: *default_rbe_targets
+    test_targets: *default_rbe_targets
+    coverage_targets: *default_rbe_targets
+    coverage_flags: *rbe_coverage_flags
+    post_shell_commands: *coverage_validation_post_shell_commands
+  path_mapping_macos:
+    name: Path Mapping MacOS
     platform: macos_arm64
-    run_targets:
-      - "//tools/vscode:gen_launch_json"
-      - "//test/vscode:vscode_test"
-  ide_integration_vscode_tests_windows:
-    name: IDE VSCode Tests
-    platform: windows
-    run_targets:
-      - "//tools/vscode:gen_launch_json"
+    build_flags:
+      - --config=clippy
+      - --config=rustfmt
+      - --experimental_output_paths=strip
+    test_flags:
+      - --config=clippy
+      - --config=rustfmt
+      - --experimental_output_paths=strip
+    build_targets: *default_macos_targets
+    test_targets: *default_macos_targets
+    coverage_targets: *default_macos_targets
+    post_shell_commands: *coverage_validation_post_shell_commands
+  # TODO: path_mapping requires sandboxing
+  # https://github.com/bazelbuild/bazel/issues/7480
+  #
+  # path_mapping_windows:
+  #   name: Path Mapping Windows
+  #   platform: windows
+  #   build_flags:
+  #     - --config=clippy
+  #     - --config=rustfmt
+  #     - --experimental_output_paths=strip
+  #   test_flags:
+  #     - --config=clippy
+  #     - --config=rustfmt
+  #     - --experimental_output_paths=strip
+  #   build_targets: *default_windows_targets
+  #   test_targets: *default_windows_targets
+
   ###########################################################################
   # C R A T E   U N I V E R S E   I N T E G R A T I O N   T E S T S
   ###########################################################################

cargo/private/cargo_build_script_runner/bin.rs

@@ -53,7 +53,7 @@ fn run_buildrs() -> Result<(), String> {
 
     cargo_manifest_maker.create_runfiles_dir().unwrap();
 
-    let out_dir_abs = exec_root.join(out_dir);
+    let out_dir_abs = exec_root.join(&out_dir);
     // For some reason Google's RBE does not create the output directory, force create it.
     create_dir_all(&out_dir_abs)
         .unwrap_or_else(|_| panic!("Failed to make output directory: {:?}", out_dir_abs));
@@ -176,7 +176,7 @@ fn run_buildrs() -> Result<(), String> {
 
     write(
         &env_file,
-        BuildScriptOutput::outputs_to_env(&buildrs_outputs, &exec_root.to_string_lossy())
+        BuildScriptOutput::outputs_to_env(&buildrs_outputs, &exec_root.to_string_lossy(), &out_dir)
             .as_bytes(),
     )
     .unwrap_or_else(|e| panic!("Unable to write file {:?}: {:#?}", env_file, e));
@@ -186,6 +186,7 @@ fn run_buildrs() -> Result<(), String> {
             &buildrs_outputs,
             &crate_links,
             &exec_root.to_string_lossy(),
+            &out_dir,
         )
         .as_bytes(),
     )
@@ -204,7 +205,11 @@ fn run_buildrs() -> Result<(), String> {
         compile_flags,
         link_flags,
         link_search_paths,
-    } = BuildScriptOutput::outputs_to_flags(&buildrs_outputs, &exec_root.to_string_lossy());
+    } = BuildScriptOutput::outputs_to_flags(
+        &buildrs_outputs,
+        &exec_root.to_string_lossy(),
+        &out_dir,
+    );
 
     write(&compile_flags_file, compile_flags.as_bytes())
         .unwrap_or_else(|e| panic!("Unable to write file {:?}: {:#?}", compile_flags_file, e));

cargo/private/cargo_build_script_runner/lib.rs

@@ -158,13 +158,13 @@ impl BuildScriptOutput {
     }
 
     /// Convert a vector of [BuildScriptOutput] into a list of environment variables.
-    pub fn outputs_to_env(outputs: &[BuildScriptOutput], exec_root: &str) -> String {
+    pub fn outputs_to_env(outputs: &[BuildScriptOutput], exec_root: &str, out_dir: &str) -> String {
         outputs
             .iter()
             .filter_map(|x| {
                 if let BuildScriptOutput::Env(env) = x {
-                    Some(Self::escape_for_serializing(Self::redact_exec_root(
-                        env, exec_root,
+                    Some(Self::escape_for_serializing(Self::redact_paths(
+                        env, exec_root, out_dir,
                     )))
                 } else {
                     None
@@ -179,6 +179,7 @@ impl BuildScriptOutput {
         outputs: &[BuildScriptOutput],
         crate_links: &str,
         exec_root: &str,
+        out_dir: &str,
     ) -> String {
         let prefix = format!("DEP_{}_", crate_links.replace('-', "_").to_uppercase());
         outputs
@@ -188,7 +189,7 @@ impl BuildScriptOutput {
                     Some(format!(
                         "{}{}",
                         prefix,
-                        Self::escape_for_serializing(Self::redact_exec_root(env, exec_root))
+                        Self::escape_for_serializing(Self::redact_paths(env, exec_root, out_dir))
                     ))
                 } else {
                     None
@@ -199,7 +200,11 @@ impl BuildScriptOutput {
     }
 
     /// Convert a vector of [BuildScriptOutput] into a flagfile.
-    pub fn outputs_to_flags(outputs: &[BuildScriptOutput], exec_root: &str) -> CompileAndLinkFlags {
+    pub fn outputs_to_flags(
+        outputs: &[BuildScriptOutput],
+        exec_root: &str,
+        out_dir: &str,
+    ) -> CompileAndLinkFlags {
         let mut compile_flags = Vec::new();
         let mut link_flags = Vec::new();
         let mut link_search_paths = Vec::new();
@@ -217,13 +222,38 @@ impl BuildScriptOutput {
 
         CompileAndLinkFlags {
             compile_flags: compile_flags.join("\n"),
-            link_flags: Self::redact_exec_root(&link_flags.join("\n"), exec_root),
-            link_search_paths: Self::redact_exec_root(&link_search_paths.join("\n"), exec_root),
+            link_flags: Self::redact_paths(&link_flags.join("\n"), exec_root, out_dir),
+            link_search_paths: Self::redact_paths(
+                &link_search_paths.join("\n"),
+                exec_root,
+                out_dir,
+            ),
         }
     }
 
-    fn redact_exec_root(value: &str, exec_root: &str) -> String {
-        value.replace(exec_root, "${pwd}")
+    /// Replace the absolute exec-root with `${pwd}` and the relative
+    /// configuration-dependent `out_dir` path (e.g.
+    /// `bazel-out/<config>/bin/.../_bs.out_dir`) with `${out_dir}`.
+    ///
+    /// Both tokens are substituted by `process_wrapper` at action
+    /// execution time. Routing the `out_dir` portion through
+    /// `${out_dir}` lets Bazel's path mapping
+    /// (`--experimental_output_paths=strip`) rewrite it: the consumer
+    /// `Rustc` action passes the directory to `process_wrapper` via
+    /// `--out-dir <File>` from a `File`-typed `Args` entry, so the value
+    /// is the mapped `bazel-out/cfg/bin/...` path under path mapping and
+    /// the un-mapped path otherwise. Without this redaction,
+    /// build-script-emitted env vars (e.g. `cargo::rustc-env=FOO=$OUT_DIR/bar`)
+    /// would carry the un-mapped path through the `_bs.env` file and
+    /// cause the path-mapped Rustc action to look in the wrong location
+    /// at runtime.
+    fn redact_paths(value: &str, exec_root: &str, out_dir: &str) -> String {
+        let with_pwd = value.replace(exec_root, "${pwd}");
+        if out_dir.is_empty() {
+            with_pwd
+        } else {
+            with_pwd.replace(out_dir, "${out_dir}")
+        }
     }
 
     // The process-wrapper treats trailing backslashes as escapes for following newlines.
@@ -283,15 +313,15 @@ mod tests {
             BuildScriptOutput::Env("no_trailing_newline=true".to_owned())
         );
         assert_eq!(
-            BuildScriptOutput::outputs_to_dep_env(&result, "ssh2", "/some/absolute/path"),
+            BuildScriptOutput::outputs_to_dep_env(&result, "ssh2", "/some/absolute/path", ""),
             "DEP_SSH2_VERSION=123\nDEP_SSH2_VERSION_NUMBER=1010107f\nDEP_SSH2_INCLUDE_PATH=${pwd}/include".to_owned()
         );
         assert_eq!(
-            BuildScriptOutput::outputs_to_env(&result, "/some/absolute/path"),
+            BuildScriptOutput::outputs_to_env(&result, "/some/absolute/path", ""),
             "FOO=BAR\nBAR=FOO\nSOME_PATH=${pwd}/beep\nno_trailing_newline=true".to_owned()
         );
         assert_eq!(
-            BuildScriptOutput::outputs_to_flags(&result, "/some/absolute/path"),
+            BuildScriptOutput::outputs_to_flags(&result, "/some/absolute/path", ""),
             CompileAndLinkFlags {
                 // -Lblah was output as a rustc-flags, so even though it probably _should_ be a link
                 // flag, we don't treat it like one.
@@ -366,7 +396,7 @@ cargo::rustc-env=valid2=2
         let result = BuildScriptOutput::outputs_from_reader(reader);
         assert_eq!(result.len(), 2);
         assert_eq!(
-            &BuildScriptOutput::outputs_to_env(&result, "/some/absolute/path"),
+            &BuildScriptOutput::outputs_to_env(&result, "/some/absolute/path", ""),
             "valid1=1\nvalid2=2"
         );
     }
@@ -385,7 +415,7 @@ cargo:rustc-env=valid2=2
         let result = BuildScriptOutput::outputs_from_reader(reader);
         assert_eq!(result.len(), 2);
         assert_eq!(
-            &BuildScriptOutput::outputs_to_env(&result, "/some/absolute/path"),
+            &BuildScriptOutput::outputs_to_env(&result, "/some/absolute/path", ""),
             "valid1=1\nvalid2=2"
         );
     }
@@ -399,4 +429,32 @@ cargo:rustc-env=valid2=2
             vec![BuildScriptOutput::DepEnv("VERSION_1_10_0=1".to_owned())]
         );
     }
+
+    /// Verify that a build-script-emitted env var that references the
+    /// `out_dir` (e.g. `cargo::rustc-env=FOO=$OUT_DIR/bar`) is rewritten
+    /// to use the `${out_dir}` substitution token. `process_wrapper`
+    /// substitutes the token at action execution time using the value
+    /// from its `--out-dir` arg, which is sourced from a `File`-typed
+    /// `Args` entry on the rules_rust side and therefore picks up Bazel
+    /// path mapping (`--experimental_output_paths=strip`) when the
+    /// consumer Rustc action advertises `supports-path-mapping`.
+    #[test]
+    fn out_dir_in_env_value_is_redacted_to_substitution_token() {
+        let buff = Cursor::new(
+            "
+cargo::rustc-env=FOO=/abs/exec_root/bazel-out/cfg/bin/_bs.out_dir/op.rs
+cargo::rustc-env=BAR=/abs/exec_root/elsewhere/file.rs
+",
+        );
+        let reader = BufReader::new(buff);
+        let result = BuildScriptOutput::outputs_from_reader(reader);
+        assert_eq!(
+            BuildScriptOutput::outputs_to_env(
+                &result,
+                "/abs/exec_root",
+                "bazel-out/cfg/bin/_bs.out_dir",
+            ),
+            "FOO=${pwd}/${out_dir}/op.rs\nBAR=${pwd}/elsewhere/file.rs"
+        );
+    }
 }