Delete //cargo/settings:incompatible_runfiles_cargo_manifest_dir (#3881)

Closes #2898

Author: UebelAndre

cargo/private/cargo_build_script.bzl

@@ -372,28 +372,18 @@ def _cargo_build_script_impl(ctx):
     if not workspace_name:
         workspace_name = ctx.workspace_name
 
-    extra_args = []
-    extra_inputs = []
     extra_output = []
 
     # Relying on runfiles directories is unreliable when passing data to
     # dependent actions. Instead, an explicit directory should be created
     # until more reliable functionality is implemented in Bazel:
     # https://github.com/bazelbuild/bazel/issues/15486
-    incompatible_runfiles_cargo_manifest_dir = ctx.attr._incompatible_runfiles_cargo_manifest_dir[BuildSettingInfo].value
-    if not incompatible_runfiles_cargo_manifest_dir:
-        script_data.append(ctx.attr.script[DefaultInfo].default_runfiles.files)
-        manifest_dir = "{}.runfiles/{}/{}".format(script.path, workspace_name, ctx.label.package)
-    else:
-        runfiles_dir, runfiles_inputs, runfiles_args = _create_runfiles_dir(
-            ctx = ctx,
-            script = ctx.attr.script,
-            retain_list = ctx.attr._cargo_manifest_dir_filename_suffixes_to_retain[BuildSettingInfo].value,
-        )
-        manifest_dir = "{}/{}/{}".format(runfiles_dir.path, workspace_name, ctx.label.package)
-        extra_args.append(runfiles_args)
-        extra_inputs.append(runfiles_inputs)
-        extra_output = [runfiles_dir]
+    runfiles_dir, runfiles_inputs, runfiles_args = _create_runfiles_dir(
+        ctx = ctx,
+        script = ctx.attr.script,
+        retain_list = ctx.attr._cargo_manifest_dir_filename_suffixes_to_retain[BuildSettingInfo].value,
+    )
+    manifest_dir = "{}/{}/{}".format(runfiles_dir.path, workspace_name, ctx.label.package)
 
     pkg_name = ctx.attr.pkg_name
     if pkg_name == "":
@@ -635,17 +625,18 @@ def _cargo_build_script_impl(ctx):
 
     ctx.actions.run(
         executable = ctx.executable._cargo_build_script_runner,
-        arguments = [args] + extra_args,
+        arguments = [args, runfiles_args],
         outputs = [
             out_dir,
             env_out,
             flags_out,
             link_flags,
             link_search_paths,
             dep_env_out,
+            runfiles_dir,
         ] + extra_output,
         tools = tools,
-        inputs = depset(build_script_inputs, transitive = extra_inputs),
+        inputs = depset(build_script_inputs, transitive = [runfiles_inputs]),
         mnemonic = "CargoBuildScriptRun",
         progress_message = "Running Cargo build script {}".format(pkg_name),
         env = env,
@@ -665,7 +656,7 @@ def _cargo_build_script_impl(ctx):
             flags = flags_out,
             linker_flags = link_flags,
             link_search_paths = link_search_paths,
-            compile_data = depset(extra_output, transitive = script_data),
+            compile_data = depset([runfiles_dir] + extra_output, transitive = script_data),
         ),
         OutputGroupInfo(
             **output_groups
@@ -805,9 +796,6 @@ cargo_build_script = rule(
             allow_files = True,
             default = Label("//cargo/private:no_cxx"),
         ),
-        "_incompatible_runfiles_cargo_manifest_dir": attr.label(
-            default = Label("//cargo/settings:incompatible_runfiles_cargo_manifest_dir"),
-        ),
     },
     fragments = ["cpp"],
     toolchains = [

cargo/private/cargo_build_script_runner/bin.rs

@@ -51,9 +51,7 @@ fn run_buildrs() -> Result<(), String> {
         cargo_manifest_maker,
     } = Args::parse();
 
-    if let Some(cargo_manifest_maker) = &cargo_manifest_maker {
-        cargo_manifest_maker.create_runfiles_dir().unwrap()
-    }
+    cargo_manifest_maker.create_runfiles_dir().unwrap();
 
     let out_dir_abs = exec_root.join(out_dir);
     // For some reason Google's RBE does not create the output directory, force create it.
@@ -232,11 +230,9 @@ fn run_buildrs() -> Result<(), String> {
     }
 
     // Delete any runfiles that do not need to be propagated to down stream dependents.
-    if let Some(cargo_manifest_maker) = cargo_manifest_maker {
-        cargo_manifest_maker
-            .drain_runfiles_dir(&out_dir_abs)
-            .unwrap();
-    }
+    cargo_manifest_maker
+        .drain_runfiles_dir(&out_dir_abs)
+        .unwrap();
 
     // If out_dir is empty add an empty file to the directory to avoid an upstream Bazel bug
     // https://github.com/bazelbuild/bazel/issues/28286
@@ -312,7 +308,7 @@ struct Args {
     stderr_path: Option<String>,
     rundir: String,
     input_dep_env_paths: Vec<String>,
-    cargo_manifest_maker: Option<RunfilesMaker>,
+    cargo_manifest_maker: RunfilesMaker,
 }
 
 impl Args {
@@ -336,7 +332,8 @@ impl Args {
         let mut stderr_path = None;
         let mut rundir: Result<String, String> = Err("Argument `rundir` not provided".to_owned());
         let mut input_dep_env_paths = Vec::new();
-        let mut cargo_manifest_maker = None;
+        let mut cargo_manifest_maker: Result<RunfilesMaker, String> =
+            Err("Argument `cargo_manifest_args` not provided".to_owned());
 
         for mut arg in env::args().skip(1) {
             if arg.starts_with("--script=") {
@@ -364,7 +361,7 @@ impl Args {
             } else if arg.starts_with("--input_dep_env_path=") {
                 input_dep_env_paths.push(arg.split_off("--input_dep_env_path=".len()));
             } else if arg.starts_with("--cargo_manifest_args=") {
-                cargo_manifest_maker = Some(RunfilesMaker::from_param_file(
+                cargo_manifest_maker = Ok(RunfilesMaker::from_param_file(
                     &arg.split_off("--cargo_manifest_args=".len()),
                 ));
             }
@@ -383,7 +380,7 @@ impl Args {
             stderr_path,
             rundir: rundir.unwrap(),
             input_dep_env_paths,
-            cargo_manifest_maker,
+            cargo_manifest_maker: cargo_manifest_maker.unwrap(),
         }
     }
 }

cargo/settings/BUILD.bazel

@@ -4,7 +4,6 @@ load(
     "cargo_manifest_dir_filename_suffixes_to_retain",
     "debug_std_streams_output_group",
     "experimental_symlink_execroot",
-    "incompatible_runfiles_cargo_manifest_dir",
     "use_default_shell_env",
 )
 
@@ -27,6 +26,4 @@ debug_std_streams_output_group()
 
 experimental_symlink_execroot()
 
-incompatible_runfiles_cargo_manifest_dir()
-
 use_default_shell_env()

cargo/settings/settings.bzl

@@ -14,18 +14,6 @@ def experimental_symlink_execroot():
         build_setting_default = False,
     )
 
-def incompatible_runfiles_cargo_manifest_dir():
-    """A flag which causes `cargo_build_script` to write an explicit `CARGO_MANIFEST_DIR` \
-    directory from an action instead of using runfiles directories which cannot be \
-    passed to downstream actions.
-
-    https://github.com/bazelbuild/bazel/issues/15486
-    """
-    bool_flag(
-        name = "incompatible_runfiles_cargo_manifest_dir",
-        build_setting_default = True,
-    )
-
 def cargo_manifest_dir_filename_suffixes_to_retain():
     """A flag which determines what files are retained in `CARGO_MANIFEST_DIR` directories \
     that are created in `CargoBuildScriptRun` actions.