Add out_dir support in cargo_dep_env (#1571)

* Add `out_dir` support in `cargo_dep_env`

This is handy for things like passing a `PROTOC` value to `prost-build`.

Also fix the other tests for `cargo_dep_env`, looks like I forgot to
actually run them...

* Fix buildifier and add directory checking

* Add some comments

* Fix buildifier

Co-authored-by: UebelAndre <[email protected]>

Author: bsilver8192

cargo/cargo_build_script.bzl

@@ -450,8 +450,36 @@ def _cargo_dep_env_implementation(ctx):
         outputs = [empty_dir],
         executable = "true",
     )
+
+    build_infos = []
+    out_dir = ctx.file.out_dir
+    if out_dir:
+        if not out_dir.is_directory:
+            fail("out_dir must be a directory artifact")
+
+        # BuildInfos in this list are collected up for all transitive cargo_build_script
+        # dependencies. This is important for any flags set in `dep_env` which reference this
+        # `out_dir`.
+        #
+        # TLDR: This BuildInfo propagates up build script dependencies.
+        build_infos.append(BuildInfo(
+            dep_env = empty_file,
+            flags = empty_file,
+            link_flags = empty_file,
+            link_search_paths = empty_file,
+            out_dir = out_dir,
+            rustc_env = empty_file,
+        ))
     return [
         DefaultInfo(files = depset(ctx.files.src)),
+        # Parts of this BuildInfo is used when building all transitive dependencies
+        # (cargo_build_script and otherwise), alongside the DepInfo. This is how other rules
+        # identify this one as a valid dependency, but we don't otherwise have a use for it.
+        #
+        # TLDR: This BuildInfo propagates up normal (non build script) depenencies.
+        #
+        # In the future, we could consider setting rustc_env here, and also propagating dep_dir
+        # so files in it can be referenced there.
         BuildInfo(
             dep_env = empty_file,
             flags = empty_file,
@@ -460,11 +488,14 @@ def _cargo_dep_env_implementation(ctx):
             out_dir = empty_dir,
             rustc_env = empty_file,
         ),
+        # Information here is used directly by dependencies, and it is an error to have more than
+        # one dependency which sets this. This is the main way to specify information from build
+        # scripts, which is what we're looking to do.
         _DepInfo(
             dep_env = ctx.file.src,
             direct_crates = depset(),
             link_search_path_files = depset(),
-            transitive_build_infos = depset(),
+            transitive_build_infos = depset(direct = build_infos),
             transitive_crate_outputs = depset(),
             transitive_crates = depset(),
             transitive_noncrates = depset(),
@@ -480,6 +511,16 @@ cargo_dep_env = rule(
         "for build scripts which depend on this crate."
     ),
     attrs = {
+        "out_dir": attr.label(
+            doc = dedent("""\
+                Folder containing additional inputs when building all direct dependencies.
+
+                This has the same effect as a `cargo_build_script` which prints
+                puts files into `$OUT_DIR`, but without requiring a build script.
+            """),
+            allow_single_file = True,
+            mandatory = False,
+        ),
         "src": attr.label(
             doc = dedent("""\
                 File containing additional environment variables to set for build scripts of direct dependencies.

test/dep_env/BUILD.bazel

@@ -1,7 +1,8 @@
 """Tests for passing configuration to cargo_build_script rules"""
 
 load("//cargo:cargo_build_script.bzl", "cargo_build_script", "cargo_dep_env")
-load("//rust:defs.bzl", "rust_binary", "rust_library")
+load("//rust:defs.bzl", "rust_library", "rust_test")
+load(":dep_env.bzl", "create_dep_dir")
 
 cargo_build_script(
     name = "set_a_build",
@@ -36,16 +37,47 @@ cargo_build_script(
     deps = [":set_b"],
 )
 
-rust_binary(
+cargo_build_script(
+    name = "read_dep_dir",
+    srcs = ["read_dep_dir.rs"],
+    edition = "2018",
+    deps = [":set_dep_dir"],
+)
+
+rust_test(
     name = "build_read_a",
-    srcs = ["empty_main.rs"],
+    srcs = ["read_a.rs"],
     edition = "2018",
     deps = [":read_a"],
 )
 
-rust_binary(
+rust_test(
     name = "build_read_b",
-    srcs = ["empty_main.rs"],
+    srcs = ["read_b.rs"],
     edition = "2018",
     deps = [":read_b"],
 )
+
+rust_test(
+    name = "build_read_dep_dir",
+    srcs = ["read_dep_dir.rs"],
+    edition = "2018",
+    deps = [":read_dep_dir"],
+)
+
+create_dep_dir(
+    name = "dep_dir",
+)
+
+cargo_dep_env(
+    name = "set_dep_dir",
+    src = "set_dep_dir.env",
+    out_dir = "dep_dir",
+)
+
+genrule(
+    name = "gen_set_dep_dir",
+    srcs = ["dep_dir"],
+    outs = ["set_dep_dir.env"],
+    cmd = "echo DEP_DIR=\\$${pwd}/$(execpath dep_dir) > $@",
+)

test/dep_env/dep_env.bzl

@@ -0,0 +1,14 @@
+"""Tests for passing configuration to cargo_build_script rules"""
+
+def _create_dep_dir(ctx):
+    out = ctx.actions.declare_directory("dep_dir")
+    ctx.actions.run_shell(
+        outputs = [out],
+        arguments = [out.path],
+        command = 'echo contents > "$@/a_file"',
+    )
+    return [DefaultInfo(files = depset(direct = [out]))]
+
+create_dep_dir = rule(
+    implementation = _create_dep_dir,
+)

test/dep_env/empty_main.rs

@@ -0,0 +1,21 @@
+use std::{env::var, fs, io::Result};
+
+fn main() {
+    let dep_dir = &var("DEP_DIR").expect("DEP_DIR should be set");
+    let entries = fs::read_dir(dep_dir)
+        .expect("Failed to open DEP_DIR directory")
+        .collect::<Result<Vec<_>>>()
+        .expect("Failed to read DEP_DIR directory entries");
+    let entries = entries
+        .iter()
+        .map(|entry| {
+            entry
+                .path()
+                .file_name()
+                .unwrap()
+                .to_string_lossy()
+                .to_string()
+        })
+        .collect::<Vec<_>>();
+    assert_eq!(entries, vec!["a_file".to_string()]);
+}