Skip to content

Commit

Permalink
Do not pass native deps to rlib compilations
Browse files Browse the repository at this point in the history
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 bazelbuild#562.
  • Loading branch information
hlopko committed Feb 3, 2021
1 parent 454860a commit 4584d8d
Show file tree
Hide file tree
Showing 18 changed files with 252 additions and 17 deletions.
1 change: 0 additions & 1 deletion bindgen/bindgen.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

# buildifier: disable=module-docstring
load("//rust:rust.bzl", "rust_library")
load("//rust/private:utils.bzl", "find_toolchain", "get_libs_for_static_executable")

# buildifier: disable=bzl-visibility
load("//rust/private:utils.bzl", "find_toolchain", "get_libs_for_static_executable")
Expand Down
3 changes: 0 additions & 3 deletions cargo/cargo_build_script.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@
load("@bazel_tools//tools/build_defs/cc:action_names.bzl", "C_COMPILE_ACTION_NAME")
load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain")
load("//rust:rust.bzl", "rust_binary")
load("//rust/private:rust.bzl", "name_to_crate_name")
load("//rust/private:rustc.bzl", "BuildInfo", "DepInfo", "get_cc_toolchain", "get_compilation_mode_opts", "get_linker_and_args")
load("//rust/private:utils.bzl", "expand_locations", "find_toolchain")

# buildifier: disable=bzl-visibility
load("//rust/private:rust.bzl", "name_to_crate_name")
Expand Down
1 change: 1 addition & 0 deletions proto/proto.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions rust/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ toolchain_type(

bzl_library(
name = "rules",
srcs = glob(["**/*.bzl"]),
deps = [
"//rust/platform:rules",
"//rust/private:rules",
Expand Down
2 changes: 1 addition & 1 deletion rust/private/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ bzl_library(
deps = [
"//rust/platform:rules",
],
)
)
2 changes: 2 additions & 0 deletions rust/private/clippy.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
Expand Down
15 changes: 10 additions & 5 deletions rust/private/rust.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -230,14 +233,15 @@ def _rust_test_common(ctx, toolchain, output):
_assert_no_deprecated_attributes(ctx)

crate_name = name_to_crate_name(ctx.label.name)

crate_type = "lib"
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]
crate_type = crate.type
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,
Expand All @@ -252,7 +256,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,
Expand All @@ -267,6 +271,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"],
)
Expand Down
16 changes: 13 additions & 3 deletions rust/private/rustc.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ def construct_arguments(
tool_path,
cc_toolchain,
feature_configuration,
crate_type,
crate_info,
dep_info,
output_hash,
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -541,6 +543,7 @@ def construct_arguments(
def rustc_compile_action(
ctx,
toolchain,
crate_type,
crate_info,
output_hash = None,
rust_flags = []):
Expand All @@ -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 [].
Expand Down Expand Up @@ -586,6 +590,7 @@ def rustc_compile_action(
toolchain.rustc.path,
cc_toolchain,
feature_configuration,
crate_type,
crate_info,
dep_info,
output_hash,
Expand Down Expand Up @@ -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")
Expand Down
2 changes: 0 additions & 2 deletions rust/private/rustdoc.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.

load("//rust:rust_common.bzl", "CrateInfo")

# buildifier: disable=module-docstring
load("//rust/private:common.bzl", "rust_common")
load("//rust/private:rustc.bzl", "DepInfo", "add_crate_link_flags", "add_edition_flags")
Expand Down
2 changes: 0 additions & 2 deletions rust/private/rustdoc_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.

load("//rust:rust_common.bzl", "CrateInfo")

# buildifier: disable=module-docstring
load("//rust/private:common.bzl", "rust_common")
load("//rust/private:rustc.bzl", "DepInfo")
Expand Down
1 change: 1 addition & 0 deletions rust/rust.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# limitations under the License.

# buildifier: disable=module-docstring

load(
"//rust/private:clippy.bzl",
_rust_clippy = "rust_clippy",
Expand Down
1 change: 1 addition & 0 deletions test/unit/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# BUILD file intentionally left empty
4 changes: 4 additions & 0 deletions test/unit/native_deps/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
load(":native_deps_test.bzl", "native_deps_test_suite")

############################ UNIT TESTS #############################
native_deps_test_suite(name = "native_deps_test_suite")
6 changes: 6 additions & 0 deletions test/unit/native_deps/bin_using_native_dep.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
extern "C" {
fn native_dep() -> isize;
}
fn main() {
println!("{}", unsafe { native_dep() })
}
6 changes: 6 additions & 0 deletions test/unit/native_deps/lib_using_native_dep.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
extern "C" {
fn native_dep() -> isize;
}
pub fn use_native_dep() {
println!("{}", unsafe { native_dep() })
}
1 change: 1 addition & 0 deletions test/unit/native_deps/native_dep.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
extern "C" int native_dep() { return 42; }
Loading

0 comments on commit 4584d8d

Please sign in to comment.