Skip to content

Don't emit CrateInfo from rust_static_library and rust_shared_library #1298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
3 changes: 2 additions & 1 deletion rust/private/common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ which exports the `rust_common` struct.
In the Bazel lingo, `rust_common` gives the access to the Rust Sandwich API.
"""

load(":providers.bzl", "CrateInfo", "DepInfo", "StdLibInfo")
load(":providers.bzl", "CrateInfo", "DepInfo", "StdLibInfo", "TestCrateInfo")

# This constant only represents the default value for attributes and macros
# defined in `rules_rust`. Like any attribute public attribute, it can be
Expand Down Expand Up @@ -54,5 +54,6 @@ rust_common = struct(
crate_info = CrateInfo,
dep_info = DepInfo,
stdlib_info = StdLibInfo,
test_crate_info = TestCrateInfo,
default_version = DEFAULT_RUST_VERSION,
)
12 changes: 12 additions & 0 deletions rust/private/providers.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,15 @@ ClippyInfo = provider(
"output": "File with the clippy output.",
},
)

TestCrateInfo = provider(
doc = "A wrapper around a CrateInfo. " +
"Certain rule types, like rust_static_library and rust_shared_library " +
"are not intended for consumption by other Rust targets, and should not " +
"provide a CrateInfo. However, one should still be able to write a rust_test " +
"for them. Thus, we create a CrateInfo, but do not advertise it as such, " +
"but rather through this provider, that rust_test understands.",
fields = {
"crate": "CrateInfo: The underlying CrateInfo of the dependency",
},
)
4 changes: 1 addition & 3 deletions rust/private/rust.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ def _rust_test_common(ctx, toolchain, output):

if ctx.attr.crate:
# Target is building the crate in `test` config
crate = ctx.attr.crate[rust_common.crate_info]
crate = ctx.attr.crate[rust_common.crate_info] if rust_common.crate_info in ctx.attr.crate else ctx.attr.crate[rust_common.test_crate_info].crate

# Optionally join compile data
if crate.compile_data:
Expand Down Expand Up @@ -745,7 +745,6 @@ rust_library = rule(

rust_static_library = rule(
implementation = _rust_static_library_impl,
provides = _common_providers,
attrs = dict(_common_attrs.items()),
fragments = ["cpp"],
host_fragments = ["cpp"],
Expand All @@ -769,7 +768,6 @@ rust_static_library = rule(

rust_shared_library = rule(
implementation = _rust_shared_library_impl,
provides = _common_providers,
attrs = dict(_common_attrs.items()),
fragments = ["cpp"],
host_fragments = ["cpp"],
Expand Down
12 changes: 10 additions & 2 deletions rust/private/rustc.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -986,15 +986,23 @@ def rustc_compile_action(
out_binary = getattr(attr, "out_binary", False)

providers = [
crate_info,
dep_info,
DefaultInfo(
# nb. This field is required for cc_library to depend on our output.
files = depset(outputs),
runfiles = runfiles,
executable = crate_info.output if crate_info.type == "bin" or crate_info.is_test or out_binary else None,
),
]

if crate_info.type in ["staticlib", "cdylib"]:
Copy link
Contributor

@PiotrSikora PiotrSikora May 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This confuses rule type with crate_info.type, and breaks rust_binary(crate_type="cdylib") which we use when building WebAssembly targets (due to a missing first-class support for rust_wasm_binary()):

[...]
ERROR: /tmp/proxy-wasm-cpp-host/test/test_data/BUILD:45:17: in rust_binary rule //test/test_data:_wasm_env_wasm: 
.../external/rules_rust/rust/private/rust.bzl:295:5: rule advertised the 'CrateInfo' provider, but this provider was not among those returned
ERROR: /tmp/proxy-wasm-cpp-host/test/test_data/BUILD:45:17: Analysis of target '//test/test_data:_wasm_env_wasm' failed
ERROR: Analysis of target '//test/test_data:env.wasm' failed; build aborted: 
[...]

Copy link
Collaborator

@UebelAndre UebelAndre May 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PiotrSikora Sorry about the breakage 😞

can you create a separate issue for this? It'd then be good if you could provide a local repro case or maybe just open a PR if you've already got a solution in mind.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a fix (#1315), but local repro is tricky due to a use of custom rule and transition to force build for Wasm.

# These rules are not supposed to be depended on by other rust targets, and
# as such they shouldn't provide a CrateInfo. However, one may still want to
# write a rust_test for them, so we provide the CrateInfo wrapped in a provider
# that rust_test understands.
providers.extend([rust_common.test_crate_info(crate = crate_info), dep_info])
else:
providers.extend([crate_info, dep_info])

if toolchain.target_arch != "wasm32":
providers += establish_cc_info(ctx, attr, crate_info, toolchain, cc_toolchain, feature_configuration, interface_library)
if pdb_file:
Expand Down
4 changes: 4 additions & 0 deletions test/unit/crate_info/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
load(":crate_info_test.bzl", "crate_info_test_suite")

############################ UNIT TESTS #############################
crate_info_test_suite(name = "crate_info_test_suite")
102 changes: 102 additions & 0 deletions test/unit/crate_info/crate_info_test.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
"""Unittests for rust rules."""

load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
load(
"//rust:defs.bzl",
"rust_common",
"rust_library",
"rust_proc_macro",
"rust_shared_library",
"rust_static_library",
)

def _rule_provides_crate_info_test_impl(ctx):
env = analysistest.begin(ctx)
tut = analysistest.target_under_test(env)
asserts.true(
env,
rust_common.crate_info in tut,
"{} should provide CrateInfo".format(tut.label.name),
)
return analysistest.end(env)

def _rule_does_not_provide_crate_info_test_impl(ctx):
env = analysistest.begin(ctx)
tut = analysistest.target_under_test(env)
asserts.false(
env,
rust_common.crate_info in tut,
"{} should not provide CrateInfo".format(tut.label.name),
)
asserts.true(
env,
rust_common.test_crate_info in tut,
"{} should provide a TestCrateInfo".format(tut.label.name),
)
return analysistest.end(env)

rule_provides_crate_info_test = analysistest.make(_rule_provides_crate_info_test_impl)
rule_does_not_provide_crate_info_test = analysistest.make(_rule_does_not_provide_crate_info_test_impl)

def _crate_info_test():
rust_library(
name = "rlib",
srcs = ["lib.rs"],
edition = "2018",
)

rust_proc_macro(
name = "proc_macro",
srcs = ["lib.rs"],
edition = "2018",
)

rust_static_library(
name = "staticlib",
srcs = ["lib.rs"],
edition = "2018",
)

rust_shared_library(
name = "cdylib",
srcs = ["lib.rs"],
edition = "2018",
)

rule_provides_crate_info_test(
name = "rlib_provides_crate_info_test",
target_under_test = ":rlib",
)

rule_provides_crate_info_test(
name = "proc_macro_provides_crate_info_test",
target_under_test = ":proc_macro",
)

rule_does_not_provide_crate_info_test(
name = "cdylib_does_not_provide_crate_info_test",
target_under_test = ":cdylib",
)

rule_does_not_provide_crate_info_test(
name = "staticlib_does_not_provide_crate_info_test",
target_under_test = ":staticlib",
)

def crate_info_test_suite(name):
"""Entry-point macro called from the BUILD file.

Args:
name: Name of the macro.
"""
_crate_info_test()

native.test_suite(
name = name,
tests = [
":rlib_provides_crate_info_test",
":proc_macro_provides_crate_info_test",
":cdylib_does_not_provide_crate_info_test",
":staticlib_does_not_provide_crate_info_test",
],
)
1 change: 1 addition & 0 deletions test/unit/crate_info/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@