From 757510f048cd69f848179c7cca64b26db92caa3f Mon Sep 17 00:00:00 2001
From: Marcel Hlopko <hlopko@google.com>
Date: Tue, 2 Feb 2021 13:18:57 +0100
Subject: [PATCH] Do not pass native deps to rlib compilations

When creating an rlib artifact, rustc will open every
native static library and "copy" its contents into the rlib. This
results in a quadratic disk usage growth and that's not necessary for
rules_rust because we track native dependencies are we can safely pass
them to the final linking actions.

This fixes the build failure when a native dep is actually a linker
script. Rustc will try to open the linker script as if it was a static
archive, and then fail the build.

Creating shared library artifacts (dylib, cdylib) and binaries is not
affected, for those rustc uses the linker, and linker will consume the
linker script just fine. Rustc won't try to open/interpret the linker
script itself.

This also unblocks https://github.com/bazelbuild/rules_rust/pull/562.
---
 proto/proto.bzl                               |   1 +
 rust/private/clippy.bzl                       |   2 +
 rust/private/rust.bzl                         |  14 +-
 rust/private/rustc.bzl                        |  16 +-
 rust/rust.bzl                                 |   3 +-
 rust/rust_common.bzl                          |  32 +++
 test/unit/BUILD                               |   1 +
 test/unit/native_deps/BUILD                   |   4 +
 test/unit/native_deps/bin_using_native_dep.rs |   6 +
 test/unit/native_deps/lib_using_native_dep.rs |   6 +
 test/unit/native_deps/native_dep.cc           |   1 +
 test/unit/native_deps/native_deps_test.bzl    | 195 ++++++++++++++++++
 .../proc_macro_using_native_dep.rs            |  10 +
 13 files changed, 282 insertions(+), 9 deletions(-)
 create mode 100644 rust/rust_common.bzl
 create mode 100644 test/unit/BUILD
 create mode 100644 test/unit/native_deps/BUILD
 create mode 100644 test/unit/native_deps/bin_using_native_dep.rs
 create mode 100644 test/unit/native_deps/lib_using_native_dep.rs
 create mode 100644 test/unit/native_deps/native_dep.cc
 create mode 100644 test/unit/native_deps/native_deps_test.bzl
 create mode 100644 test/unit/native_deps/proc_macro_using_native_dep.rs

diff --git a/proto/proto.bzl b/proto/proto.bzl
index 01ca0242b7..ceeb0500d8 100644
--- a/proto/proto.bzl
+++ b/proto/proto.bzl
@@ -217,6 +217,7 @@ def _rust_proto_compile(protos, descriptor_sets, imports, crate_name, ctx, is_gr
     return rustc_compile_action(
         ctx = ctx,
         toolchain = find_toolchain(ctx),
+        crate_type = "rlib",
         crate_info = rust_common.crate_info(
             name = crate_name,
             type = "rlib",
diff --git a/rust/private/clippy.bzl b/rust/private/clippy.bzl
index 5fcc50ddcd..715996c347 100644
--- a/rust/private/clippy.bzl
+++ b/rust/private/clippy.bzl
@@ -49,6 +49,7 @@ def _clippy_aspect_impl(target, ctx):
 
     toolchain = find_toolchain(ctx)
     crate_info = target[rust_common.crate_info]
+    crate_type = crate_info.type
 
     if crate_info.is_test:
         root = crate_info.root
@@ -89,6 +90,7 @@ def _clippy_aspect_impl(target, ctx):
         toolchain.clippy_driver.path,
         cc_toolchain,
         feature_configuration,
+        crate_type,
         crate_info,
         dep_info,
         output_hash = determine_output_hash(root),
diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl
index f17a78f56e..27214214a2 100644
--- a/rust/private/rust.bzl
+++ b/rust/private/rust.bzl
@@ -155,9 +155,10 @@ def _rust_library_impl(ctx):
     output_hash = determine_output_hash(crate_root)
 
     crate_name = name_to_crate_name(ctx.label.name)
+    crate_type = getattr(ctx.attr, "crate_type")
     rust_lib_name = _determine_lib_name(
         crate_name,
-        ctx.attr.crate_type,
+        crate_type,
         toolchain,
         output_hash,
     )
@@ -166,9 +167,10 @@ def _rust_library_impl(ctx):
     return rustc_compile_action(
         ctx = ctx,
         toolchain = toolchain,
+        crate_type = crate_type,
         crate_info = rust_common.crate_info(
             name = crate_name,
-            type = ctx.attr.crate_type,
+            type = crate_type,
             root = crate_root,
             srcs = ctx.files.srcs,
             deps = ctx.attr.deps,
@@ -201,6 +203,7 @@ def _rust_binary_impl(ctx):
     return rustc_compile_action(
         ctx = ctx,
         toolchain = toolchain,
+        crate_type = crate_type,
         crate_info = rust_common.crate_info(
             name = crate_name,
             type = crate_type,
@@ -230,14 +233,14 @@ def _rust_test_common(ctx, toolchain, output):
     _assert_no_deprecated_attributes(ctx)
 
     crate_name = name_to_crate_name(ctx.label.name)
-
+    crate_type = "bin"
     if ctx.attr.crate:
         # Target is building the crate in `test` config
         # Build the test binary using the dependency's srcs.
         crate = ctx.attr.crate[rust_common.crate_info]
         target = rust_common.crate_info(
             name = crate_name,
-            type = crate.type,
+            type = crate_type,
             root = crate.root,
             srcs = crate.srcs + ctx.files.srcs,
             deps = crate.deps + ctx.attr.deps,
@@ -252,7 +255,7 @@ def _rust_test_common(ctx, toolchain, output):
         # Target is a standalone crate. Build the test binary as its own crate.
         target = rust_common.crate_info(
             name = crate_name,
-            type = "lib",
+            type = crate_type,
             root = crate_root_src(ctx.attr, ctx.files.srcs, "lib"),
             srcs = ctx.files.srcs,
             deps = ctx.attr.deps,
@@ -267,6 +270,7 @@ def _rust_test_common(ctx, toolchain, output):
     return rustc_compile_action(
         ctx = ctx,
         toolchain = toolchain,
+        crate_type = crate_type,
         crate_info = target,
         rust_flags = ["--test"],
     )
diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl
index a2917217c2..f8b5a7b25b 100644
--- a/rust/private/rustc.bzl
+++ b/rust/private/rustc.bzl
@@ -364,6 +364,7 @@ def construct_arguments(
         tool_path,
         cc_toolchain,
         feature_configuration,
+        crate_type,
         crate_info,
         dep_info,
         output_hash,
@@ -383,6 +384,7 @@ def construct_arguments(
         tool_path (str): Path to rustc
         cc_toolchain (CcToolchain): The CcToolchain for the current target.
         feature_configuration (FeatureConfiguration): Class used to construct command lines from CROSSTOOL features.
+        crate_type (str): Crate type of the current target.
         crate_info (CrateInfo): The CrateInfo provider of the target crate
         dep_info (DepInfo): The DepInfo provider of the target crate
         output_hash (str): The hashed path of the crate root
@@ -508,7 +510,7 @@ def construct_arguments(
             args.add("--codegen=linker=" + ld)
             args.add_joined("--codegen", link_args, join_with = " ", format_joined = "link-args=%s")
 
-        add_native_link_flags(args, dep_info)
+        add_native_link_flags(args, dep_info, crate_type)
 
     # These always need to be added, even if not linking this crate.
     add_crate_link_flags(args, dep_info)
@@ -541,6 +543,7 @@ def construct_arguments(
 def rustc_compile_action(
         ctx,
         toolchain,
+        crate_type,
         crate_info,
         output_hash = None,
         rust_flags = []):
@@ -549,6 +552,7 @@ def rustc_compile_action(
     Args:
         ctx (ctx): The rule's context object
         toolchain (rust_toolchain): The current `rust_toolchain`
+        crate_type (str): Crate type of the current target
         crate_info (CrateInfo): The CrateInfo provider for the current target.
         output_hash (str, optional): The hashed path of the crate root. Defaults to None.
         rust_flags (list, optional): Additional flags to pass to rustc. Defaults to [].
@@ -586,6 +590,7 @@ def rustc_compile_action(
         toolchain.rustc.path,
         cc_toolchain,
         feature_configuration,
+        crate_type,
         crate_info,
         dep_info,
         output_hash,
@@ -807,13 +812,18 @@ def _get_crate_dirname(crate):
     """
     return crate.output.dirname
 
-def add_native_link_flags(args, dep_info):
+def add_native_link_flags(args, dep_info, crate_type):
     """Adds linker flags for all dependencies of the current target.
 
     Args:
         args (Args): The Args struct for a ctx.action
-        dep_info (DepInfo): Dependeincy Info provider
+        dep_info (DepInfo): Dependency Info provider
+        crate_type: Crate type of the current target
+
     """
+    if crate_type in ["lib", "rlib"]:
+        return
+
     native_libs = depset(transitive = [dep_info.transitive_dylibs, dep_info.transitive_staticlibs])
     args.add_all(native_libs, map_each = _get_dirname, uniquify = True, format_each = "-Lnative=%s")
     args.add_all(dep_info.transitive_dylibs, map_each = get_lib_name, format_each = "-ldylib=%s")
diff --git a/rust/rust.bzl b/rust/rust.bzl
index 949a58b68a..faf8556d49 100644
--- a/rust/rust.bzl
+++ b/rust/rust.bzl
@@ -12,7 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# buildifier: disable=module-docstring
+"""Public entry point to all Rust rules and supported APIs."""
+
 load(
     "//rust/private:clippy.bzl",
     _rust_clippy = "rust_clippy",
diff --git a/rust/rust_common.bzl b/rust/rust_common.bzl
new file mode 100644
index 0000000000..d23e51356f
--- /dev/null
+++ b/rust/rust_common.bzl
@@ -0,0 +1,32 @@
+# Copyright 2015 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Module with Rust definitions required to write custom Rust rules."""
+
+CrateInfo = provider(
+    doc = "A provider containing general Crate information.",
+    fields = {
+        "aliases": "Dict[Label, String]: Renamed and aliased crates",
+        "deps": "List[Provider]: This crate's (rust or cc) dependencies' providers.",
+        "edition": "str: The edition of this crate.",
+        "is_test": "bool: If the crate is being compiled in a test context",
+        "name": "str: The name of this crate.",
+        "output": "File: The output File that will be produced, depends on crate type.",
+        "proc_macro_deps": "List[CrateInfo]: This crate's rust proc_macro dependencies' providers.",
+        "root": "File: The source File entrypoint to this crate, eg. lib.rs",
+        "rustc_env": "Dict[String, String]: Additional `\"key\": \"value\"` environment variables to set for rustc.",
+        "srcs": "List[File]: All source Files that are part of the crate.",
+        "type": "str: The type of this crate. eg. lib or bin",
+    },
+)
diff --git a/test/unit/BUILD b/test/unit/BUILD
new file mode 100644
index 0000000000..f47ba755c9
--- /dev/null
+++ b/test/unit/BUILD
@@ -0,0 +1 @@
+# BUILD file intentionally left empty
diff --git a/test/unit/native_deps/BUILD b/test/unit/native_deps/BUILD
new file mode 100644
index 0000000000..7864dfb0f2
--- /dev/null
+++ b/test/unit/native_deps/BUILD
@@ -0,0 +1,4 @@
+load(":native_deps_test.bzl", "native_deps_test_suite")
+
+############################ UNIT TESTS #############################
+native_deps_test_suite(name = "native_deps_test_suite")
diff --git a/test/unit/native_deps/bin_using_native_dep.rs b/test/unit/native_deps/bin_using_native_dep.rs
new file mode 100644
index 0000000000..6250ab356b
--- /dev/null
+++ b/test/unit/native_deps/bin_using_native_dep.rs
@@ -0,0 +1,6 @@
+extern "C" {
+    fn native_dep() -> isize;
+}
+fn main() {
+    println!("{}", unsafe { native_dep() })
+}
\ No newline at end of file
diff --git a/test/unit/native_deps/lib_using_native_dep.rs b/test/unit/native_deps/lib_using_native_dep.rs
new file mode 100644
index 0000000000..1c0a8bcdb0
--- /dev/null
+++ b/test/unit/native_deps/lib_using_native_dep.rs
@@ -0,0 +1,6 @@
+extern "C" {
+    fn native_dep() -> isize;
+}
+pub fn use_native_dep() {
+    println!("{}", unsafe { native_dep() })
+}
\ No newline at end of file
diff --git a/test/unit/native_deps/native_dep.cc b/test/unit/native_deps/native_dep.cc
new file mode 100644
index 0000000000..252780d75c
--- /dev/null
+++ b/test/unit/native_deps/native_dep.cc
@@ -0,0 +1 @@
+extern "C" int native_dep() { return 42; }
\ No newline at end of file
diff --git a/test/unit/native_deps/native_deps_test.bzl b/test/unit/native_deps/native_deps_test.bzl
new file mode 100644
index 0000000000..8bf318f9a3
--- /dev/null
+++ b/test/unit/native_deps/native_deps_test.bzl
@@ -0,0 +1,195 @@
+"""Unittests for rust rules."""
+
+load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
+load("@rules_cc//cc:defs.bzl", "cc_library")
+
+# buildifier: disable=bzl-visibility
+load("//rust/private:rust.bzl", "rust_binary", "rust_library")
+
+def _assert_argv_contains_not(env, action, flag):
+    asserts.true(
+        env,
+        flag not in action.argv,
+        "Expected {args} to not contain {flag}".format(args = action.argv, flag = flag),
+    )
+
+def _assert_argv_contains(env, action, flag):
+    asserts.true(
+        env,
+        flag in action.argv,
+        "Expected {args} to contain {flag}".format(args = action.argv, flag = flag),
+    )
+
+def _lib_has_no_native_libs_test_impl(ctx):
+    env = analysistest.begin(ctx)
+    tut = analysistest.target_under_test(env)
+    actions = analysistest.target_actions(env)
+    asserts.equals(env, 1, len(actions))
+    action = actions[0]
+    _assert_argv_contains_not(env, action, "-lstatic=native_dep")
+    _assert_argv_contains_not(env, action, "-ldylib=native_dep")
+    return analysistest.end(env)
+
+def _rlib_has_no_native_libs_test_impl(ctx):
+    env = analysistest.begin(ctx)
+    tut = analysistest.target_under_test(env)
+    actions = analysistest.target_actions(env)
+    asserts.equals(env, 1, len(actions))
+    action = actions[0]
+    _assert_argv_contains_not(env, action, "-lstatic=native_dep")
+    _assert_argv_contains_not(env, action, "-ldylib=native_dep")
+    return analysistest.end(env)
+
+def _dylib_has_native_libs_test_impl(ctx):
+    env = analysistest.begin(ctx)
+    tut = analysistest.target_under_test(env)
+    actions = analysistest.target_actions(env)
+    action = actions[0]
+    _assert_argv_contains(env, action, "-lstatic=native_dep")
+    return analysistest.end(env)
+
+def _cdylib_has_native_libs_test_impl(ctx):
+    env = analysistest.begin(ctx)
+    tut = analysistest.target_under_test(env)
+    actions = analysistest.target_actions(env)
+    action = actions[0]
+    _assert_argv_contains(env, action, "-lstatic=native_dep")
+    return analysistest.end(env)
+
+def _staticlib_has_native_libs_test_impl(ctx):
+    env = analysistest.begin(ctx)
+    tut = analysistest.target_under_test(env)
+    actions = analysistest.target_actions(env)
+    action = actions[0]
+    _assert_argv_contains(env, action, "-lstatic=native_dep")
+    return analysistest.end(env)
+
+def _proc_macro_has_native_libs_test_impl(ctx):
+    env = analysistest.begin(ctx)
+    tut = analysistest.target_under_test(env)
+    actions = analysistest.target_actions(env)
+    asserts.equals(env, 1, len(actions))
+    action = actions[0]
+    _assert_argv_contains(env, action, "-lstatic=native_dep")
+    return analysistest.end(env)
+
+def _bin_has_native_libs_test_impl(ctx):
+    env = analysistest.begin(ctx)
+    tut = analysistest.target_under_test(env)
+    actions = analysistest.target_actions(env)
+    action = actions[0]
+    _assert_argv_contains(env, action, "-lstatic=native_dep")
+    return analysistest.end(env)
+
+lib_has_no_native_libs_test = analysistest.make(_lib_has_no_native_libs_test_impl)
+rlib_has_no_native_libs_test = analysistest.make(_rlib_has_no_native_libs_test_impl)
+staticlib_has_native_libs_test = analysistest.make(_staticlib_has_native_libs_test_impl)
+dylib_has_native_libs_test = analysistest.make(_dylib_has_native_libs_test_impl)
+cdylib_has_native_libs_test = analysistest.make(_cdylib_has_native_libs_test_impl)
+proc_macro_has_native_libs_test = analysistest.make(_proc_macro_has_native_libs_test_impl)
+bin_has_native_libs_test = analysistest.make(_bin_has_native_libs_test_impl)
+
+def _native_dep_test():
+    rust_library(
+        name = "lib_has_no_native_dep",
+        srcs = ["lib_using_native_dep.rs"],
+        deps = [":native_dep"],
+        crate_type = "lib",
+    )
+
+    rust_library(
+        name = "rlib_has_no_native_dep",
+        srcs = ["lib_using_native_dep.rs"],
+        deps = [":native_dep"],
+        crate_type = "rlib",
+    )
+
+    rust_library(
+        name = "staticlib_has_native_dep",
+        srcs = ["lib_using_native_dep.rs"],
+        deps = [":native_dep"],
+        crate_type = "staticlib",
+    )
+
+    rust_library(
+        name = "dylib_has_native_dep",
+        srcs = ["lib_using_native_dep.rs"],
+        deps = [":native_dep"],
+        crate_type = "dylib",
+    )
+
+    rust_library(
+        name = "cdylib_has_native_dep",
+        srcs = ["lib_using_native_dep.rs"],
+        deps = [":native_dep"],
+        crate_type = "cdylib",
+    )
+
+    rust_library(
+        name = "proc_macro_has_native_dep",
+        srcs = ["proc_macro_using_native_dep.rs"],
+        deps = [":native_dep"],
+        crate_type = "proc-macro",
+        edition = "2018",
+    )
+
+    rust_binary(
+        name = "bin_has_native_dep",
+        srcs = ["bin_using_native_dep.rs"],
+        deps = [":native_dep"],
+    )
+
+    cc_library(
+        name = "native_dep",
+        srcs = ["native_dep.cc"],
+    )
+
+    lib_has_no_native_libs_test(
+        name = "lib_has_no_native_libs_test",
+        target_under_test = ":lib_has_no_native_dep",
+    )
+    rlib_has_no_native_libs_test(
+        name = "rlib_has_no_native_libs_test",
+        target_under_test = ":rlib_has_no_native_dep",
+    )
+    staticlib_has_native_libs_test(
+        name = "staticlib_has_native_libs_test",
+        target_under_test = ":staticlib_has_native_dep",
+    )
+    dylib_has_native_libs_test(
+        name = "dylib_has_native_libs_test",
+        target_under_test = ":dylib_has_native_dep",
+    )
+    cdylib_has_native_libs_test(
+        name = "cdylib_has_native_libs_test",
+        target_under_test = ":cdylib_has_native_dep",
+    )
+    proc_macro_has_native_libs_test(
+        name = "proc_macro_has_native_libs_test",
+        target_under_test = ":proc_macro_has_native_dep",
+    )
+    bin_has_native_libs_test(
+        name = "bin_has_native_libs_test",
+        target_under_test = ":bin_has_native_dep",
+    )
+
+def native_deps_test_suite(name):
+    """Entry-point macro called from the BUILD file.
+
+    Args:
+        name: Name of the macro.
+    """
+    _native_dep_test()
+
+    native.test_suite(
+        name = name,
+        tests = [
+            ":lib_has_no_native_libs_test",
+            ":rlib_has_no_native_libs_test",
+            ":staticlib_has_native_libs_test",
+            ":dylib_has_native_libs_test",
+            ":cdylib_has_native_libs_test",
+            ":proc_macro_has_native_libs_test",
+            ":bin_has_native_libs_test",
+        ],
+    )
diff --git a/test/unit/native_deps/proc_macro_using_native_dep.rs b/test/unit/native_deps/proc_macro_using_native_dep.rs
new file mode 100644
index 0000000000..edbce61f3e
--- /dev/null
+++ b/test/unit/native_deps/proc_macro_using_native_dep.rs
@@ -0,0 +1,10 @@
+use proc_macro::TokenStream;
+
+extern "C" {
+    fn native_dep() -> isize;
+}
+#[proc_macro_derive(UsingNativeDep)]
+pub fn use_native_dep(_input: TokenStream) -> TokenStream {
+    println!("{}", unsafe { native_dep() });
+    panic!("done")
+}
\ No newline at end of file