Link flags are passed to transitive rdeps (#346)

And absolute paths are redacted into $EXEC_ROOT-relative paths

This mirrors Cargo's behaviour of accumulating link flags and passing
them to the linker - otherwise there's not much point in sys libraries
outputting them.

This currently over-adds out_dirs and link flags to all actions, rather
than just those that will invoke the linker - we can reduce that if we
have a clear indication of what targets will invoke a linker, but I
wanted to go with a safe-but-over-invalidating solution rather than
picking a bad heuristic.

Author: illicitonion

WORKSPACE

@@ -99,3 +99,6 @@ rbe_autoconfig(
 
 load("@io_bazel_rules_rust//:workspace.bzl", "bazel_version")
 bazel_version(name = "bazel_version")
+
+load("@examples//hello_sys:workspace.bzl", "remote_deps")
+remote_deps()

cargo/cargo_build_script.bzl

@@ -9,6 +9,7 @@ def _cargo_build_script_run(ctx, script):
     env_out = ctx.actions.declare_file(ctx.label.name + ".env")
     dep_env_out = ctx.actions.declare_file(ctx.label.name + ".depenv")
     flags_out = ctx.actions.declare_file(ctx.label.name + ".flags")
+    link_flags = ctx.actions.declare_file(ctx.label.name + ".linkflags")
     manifest_dir = "%s.runfiles/%s" % (script.path, ctx.label.workspace_name or ctx.workspace_name)
     compilation_mode_opt_level = get_compilation_mode_opts(ctx, toolchain).opt_level
 
@@ -33,10 +34,10 @@ def _cargo_build_script_run(ctx, script):
         "CARGO_MANIFEST_DIR": manifest_dir,
         "HOST": toolchain.exec_triple,
         "OPT_LEVEL": compilation_mode_opt_level,
-        "OUT_DIR": out_dir.path,
         "RUSTC": toolchain.rustc.path,
         "RUST_BACKTRACE": "full",
         "TARGET": toolchain.target_triple,
+        # OUT_DIR is set by the runner itself, rather than on the action.
     }
 
     # Pull in env vars which may be required for the cc_toolchain to work (e.g. on OSX, the SDK version).
@@ -77,8 +78,8 @@ def _cargo_build_script_run(ctx, script):
 
     ctx.actions.run_shell(
         command = cmd,
-        arguments = [ctx.executable._cargo_build_script_runner.path, script.path, crate_name, env_out.path, flags_out.path, dep_env_out.path],
-        outputs = [out_dir, env_out, flags_out, dep_env_out],
+        arguments = [ctx.executable._cargo_build_script_runner.path, script.path, crate_name, out_dir.path, env_out.path, flags_out.path, link_flags.path, dep_env_out.path],
+        outputs = [out_dir, env_out, flags_out, link_flags, dep_env_out],
         tools = tools,
         inputs = dep_env_files,
         mnemonic = "CargoBuildScriptRun",
@@ -91,6 +92,7 @@ def _cargo_build_script_run(ctx, script):
             rustc_env = env_out,
             dep_env = dep_env_out,
             flags = flags_out,
+            link_flags = link_flags,
         ),
     ]
 

cargo/cargo_build_script_runner/bin.rs

@@ -16,10 +16,9 @@
 // by rust_library/rust_binary.
 extern crate cargo_build_script_output_parser;
 
-use cargo_build_script_output_parser::BuildScriptOutput;
+use cargo_build_script_output_parser::{BuildScriptOutput, CompileAndLinkFlags};
 use std::env;
-use std::fs::{File, create_dir_all};
-use std::io::Write;
+use std::fs::{create_dir_all, write};
 use std::path::Path;
 use std::process::{exit, Command};
 
@@ -33,13 +32,8 @@ fn main() {
 
     let mut args = env::args().skip(1);
     let manifest_dir_env = env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR was not set");
-    let out_dir_env = env::var("OUT_DIR").expect("OUT_DIR was not set");
-    // For some reason RBE does not creat the output directory, force create it
-    create_dir_all(out_dir_env.clone()).expect(&format!("Failed to create OUT_DIR: {}", out_dir_env));
     let rustc_env = env::var("RUSTC").expect("RUSTC was not set");
-    // Because of the Bazel's sandbox, bazel cannot provide full path, convert all relative path to correct path.
     let manifest_dir = exec_root.join(&manifest_dir_env);
-    let out_dir = exec_root.join(&out_dir_env);
     let rustc = exec_root.join(&rustc_env);
 
     let cc = env::var_os("CC").map(|env_var| {
@@ -51,13 +45,17 @@ fn main() {
         }
     });
 
-    match (args.next(), args.next(), args.next(), args.next(), args.next()) {
-        (Some(progname), Some(crate_name), Some(envfile), Some(flagfile), Some(depenvfile)) => {
+    match (args.next(), args.next(), args.next(), args.next(), args.next(), args.next(), args.next()) {
+        (Some(progname), Some(crate_name), Some(out_dir), Some(envfile), Some(flagfile), Some(linkflags), Some(depenvfile)) => {
+            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).expect(&format!("Failed to make output directory: {:?}", out_dir_abs));
+
             let mut command = Command::new(exec_root.join(&progname));
             command
                 .args(args)
                 .current_dir(manifest_dir.clone())
-                .env("OUT_DIR", out_dir)
+                .env("OUT_DIR", out_dir_abs)
                 .env("CARGO_MANIFEST_DIR", manifest_dir)
                 .env("RUSTC", rustc);
 
@@ -66,21 +64,20 @@ fn main() {
             }
 
             let output = BuildScriptOutput::from_command(&mut command);
-            let mut f =
-                File::create(&envfile).expect(&format!("Unable to create file {}", envfile));
-            f.write_all(BuildScriptOutput::to_env(&output).as_bytes())
-                .expect(&format!("Unable to write file {}", envfile));
-            let mut f =
-                File::create(&depenvfile).expect(&format!("Unable to create file {}", depenvfile));
-            f.write_all(BuildScriptOutput::to_dep_env(&output, &crate_name).as_bytes())
-                .expect(&format!("Unable to write file {}", depenvfile));
-            let mut f =
-                File::create(&flagfile).expect(&format!("Unable to create file {}", flagfile));
-            f.write_all(BuildScriptOutput::to_flags(&output).as_bytes())
-                .expect(&format!("Unable to write file {}", flagfile));
+            write(&envfile, BuildScriptOutput::to_env(&output).as_bytes())
+                .expect(&format!("Unable to write file {:?}", envfile));
+            write(&depenvfile, BuildScriptOutput::to_dep_env(&output, &crate_name).as_bytes())
+                .expect(&format!("Unable to write file {:?}", depenvfile));
+
+            let CompileAndLinkFlags { compile_flags, link_flags } = BuildScriptOutput::to_flags(&output, &exec_root.to_string_lossy());
+
+            write(&flagfile, compile_flags.as_bytes())
+                .expect(&format!("Unable to write file {:?}", flagfile));
+            write(&linkflags, link_flags.as_bytes())
+                .expect(&format!("Unable to write file {:?}", linkflags));
         }
         _ => {
-            eprintln!("Usage: $0 progname crate_name envfile flagfile depenvfile [arg1...argn]");
+            eprintln!("Usage: $0 progname crate_name out_dir envfile flagfile linkflagfile depenvfile [arg1...argn]");
             exit(1);
         }
     }

cargo/cargo_build_script_runner/lib.rs

@@ -17,6 +17,12 @@
 use std::io::{BufRead, BufReader, Read};
 use std::process::{Command, Stdio};
 
+#[derive(Debug, PartialEq, Eq)]
+pub struct CompileAndLinkFlags {
+    pub compile_flags: String,
+    pub link_flags: String,
+}
+
 /// Enum containing all the considered return value from the script
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum BuildScriptOutput {
@@ -141,17 +147,23 @@ impl BuildScriptOutput {
     }
 
     /// Convert a vector of [BuildScriptOutput] into a flagfile.
-    pub fn to_flags(v: &Vec<BuildScriptOutput>) -> String {
-        v.iter()
-            .filter_map(|x| match x {
-                BuildScriptOutput::Cfg(e) => Some(format!("--cfg={}", e)),
-                BuildScriptOutput::Flags(e) => Some(e.to_owned()),
-                BuildScriptOutput::LinkLib(e) => Some(format!("-l{}", e)),
-                BuildScriptOutput::LinkSearch(e) => Some(format!("-L{}", e)),
-                _ => None,
-            })
-            .collect::<Vec<_>>()
-            .join(" ")
+    pub fn to_flags(v: &Vec<BuildScriptOutput>, exec_root: &str) -> CompileAndLinkFlags {
+        let mut compile_flags = Vec::new();
+        let mut link_flags = Vec::new();
+
+        for flag in v {
+            match flag {
+                BuildScriptOutput::Cfg(e) => compile_flags.push(format!("--cfg={}", e)),
+                BuildScriptOutput::Flags(e) => compile_flags.push(e.to_owned()),
+                BuildScriptOutput::LinkLib(e) => link_flags.push(format!("-l{}", e)),
+                BuildScriptOutput::LinkSearch(e) => link_flags.push(format!("-L{}", e)),
+                _ => { },
+            }
+        }
+        CompileAndLinkFlags {
+            compile_flags: compile_flags.join(" "),
+            link_flags: link_flags.join(" ").replace(exec_root, "${EXEC_ROOT}"),
+        }
     }
 }
 
@@ -166,7 +178,7 @@ mod tests {
             "
 cargo:rustc-link-lib=sdfsdf
 cargo:rustc-env=FOO=BAR
-cargo:rustc-link-search=bleh
+cargo:rustc-link-search=/some/absolute/path/bleh
 cargo:rustc-env=BAR=FOO
 cargo:rustc-flags=-Lblah
 cargo:rerun-if-changed=ignored
@@ -180,7 +192,7 @@ cargo:version_number=1010107f
         assert_eq!(result.len(), 8);
         assert_eq!(result[0], BuildScriptOutput::LinkLib("sdfsdf".to_owned()));
         assert_eq!(result[1], BuildScriptOutput::Env("FOO=BAR".to_owned()));
-        assert_eq!(result[2], BuildScriptOutput::LinkSearch("bleh".to_owned()));
+        assert_eq!(result[2], BuildScriptOutput::LinkSearch("/some/absolute/path/bleh".to_owned()));
         assert_eq!(result[3], BuildScriptOutput::Env("BAR=FOO".to_owned()));
         assert_eq!(result[4], BuildScriptOutput::Flags("-Lblah".to_owned()));
         assert_eq!(
@@ -199,8 +211,13 @@ cargo:version_number=1010107f
             "FOO=BAR BAR=FOO".to_owned()
         );
         assert_eq!(
-            BuildScriptOutput::to_flags(&result),
-            "-lsdfsdf -Lbleh -Lblah --cfg=feature=awesome".to_owned()
+            BuildScriptOutput::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.
+                compile_flags: "-Lblah --cfg=feature=awesome".to_owned(),
+                link_flags: "-lsdfsdf -L${EXEC_ROOT}/bleh".to_owned(),
+            }
         );
     }
 

examples/WORKSPACE

@@ -69,3 +69,7 @@ rust_wasm_bindgen_repositories()
 load("@io_bazel_rules_rust//:workspace.bzl", "bazel_version")
 
 bazel_version(name = "bazel_version")
+
+load("@examples//hello_sys:workspace.bzl", "remote_deps")
+
+remote_deps()

examples/hello_sys/BUILD

@@ -0,0 +1,20 @@
+package(default_visibility = ["//visibility:public"])
+
+load(
+    "@io_bazel_rules_rust//rust:rust.bzl",
+    "rust_binary",
+)
+
+rust_binary(
+    name = "hello_sys",
+    srcs = ["src/main.rs"],
+    edition = "2018",
+    deps = ["@bzip2"],
+)
+
+sh_test(
+    name = "test",
+    srcs = ["test.sh"],
+    args = ["$(location :hello_sys)"],
+    data = [":hello_sys"],
+)

examples/hello_sys/bzip2-0.3.3.BUILD

@@ -0,0 +1,21 @@
+load(
+    "@io_bazel_rules_rust//rust:rust.bzl",
+    "rust_library",
+)
+
+rust_library(
+    name = "bzip2",
+    srcs = glob(["**/*.rs"]),
+    crate_root = "src/lib.rs",
+    crate_type = "lib",
+    edition = "2015",
+    rustc_flags = [
+        "--cap-lints=allow",
+    ],
+    version = "0.3.3",
+    visibility = ["//visibility:public"],
+    deps = [
+        "@bzip2_sys",
+        "@libc",
+    ],
+)

examples/hello_sys/bzip2-sys-0.1.9+1.0.8.BUILD

@@ -0,0 +1,41 @@
+load(
+    "@io_bazel_rules_rust//rust:rust.bzl",
+    "rust_library",
+)
+load(
+    "@io_bazel_rules_rust//cargo:cargo_build_script.bzl",
+    "cargo_build_script",
+)
+
+cargo_build_script(
+    name = "bzip2_sys_build_script",
+    srcs = glob(["**/*.rs"]),
+    crate_root = "build.rs",
+    data = glob(["**"]),
+    edition = "2015",
+    rustc_flags = [
+        "--cap-lints=allow",
+    ],
+    version = "0.1.9+1.0.8",
+    deps = [
+        "@cc",
+        "@pkg_config",
+    ],
+)
+
+rust_library(
+    name = "bzip2_sys",
+    srcs = glob(["**/*.rs"]),
+    crate_root = "lib.rs",
+    crate_type = "lib",
+    edition = "2015",
+    rustc_flags = [
+        "--cap-lints=allow",
+    ],
+    version = "0.1.9+1.0.8",
+    visibility = ["//visibility:public"],
+    deps = [
+        ":bzip2_sys_build_script",
+        "@libc",
+    ],
+)

examples/hello_sys/cc-1.0.54.BUILD

@@ -0,0 +1,17 @@
+load(
+    "@io_bazel_rules_rust//rust:rust.bzl",
+    "rust_library",
+)
+
+rust_library(
+    name = "cc",
+    srcs = glob(["**/*.rs"]),
+    crate_root = "src/lib.rs",
+    crate_type = "lib",
+    edition = "2018",
+    rustc_flags = [
+        "--cap-lints=allow",
+    ],
+    version = "1.0.54",
+    visibility = ["//visibility:public"],
+)

examples/hello_sys/pkg-config-0.3.17.BUILD

@@ -0,0 +1,17 @@
+load(
+    "@io_bazel_rules_rust//rust:rust.bzl",
+    "rust_library",
+)
+
+rust_library(
+    name = "pkg_config",
+    srcs = glob(["**/*.rs"]),
+    crate_root = "src/lib.rs",
+    crate_type = "lib",
+    edition = "2015",
+    rustc_flags = [
+        "--cap-lints=allow",
+    ],
+    version = "0.3.17",
+    visibility = ["//visibility:public"],
+)