Skip to content
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

Don't emit CrateInfo from rust_static_library and rust_shared_library #1298

Prev Previous commit
Next Next commit
Have stdlib and cdylib emit TestCrateInfo
scentini committed Apr 27, 2022
commit 80e048c766f49d95443ca6c11bca4911c30d9b1e
2 changes: 1 addition & 1 deletion examples/ffi/c_calling_rust/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -8,7 +8,7 @@ rust_static_library(

rust_test(
name = "rusty_test",
srcs = ["lib.rs"],
crate = ":rusty",
)

cc_test(
3 changes: 2 additions & 1 deletion rust/private/common.bzl
Original file line number Diff line number Diff line change
@@ -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
@@ -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
@@ -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
@@ -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:
@@ -745,7 +745,6 @@ rust_library = rule(

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

rust_shared_library = rule(
implementation = _rust_shared_library_impl,
provides = [CcInfo],
attrs = dict(_common_attrs.items()),
fragments = ["cpp"],
host_fragments = ["cpp"],
8 changes: 7 additions & 1 deletion rust/private/rustc.bzl
Original file line number Diff line number Diff line change
@@ -994,7 +994,13 @@ def rustc_compile_action(
),
]

if crate_info.type not in ["staticlib", "cdylib"]:
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":