Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 41 additions & 7 deletions rust/private/rust.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ load(
"get_edition",
"get_import_macro_deps",
"transform_deps",
"transform_link_deps",
"transform_sources",
)

Expand All @@ -66,20 +67,40 @@ def _assert_no_deprecated_attributes(_ctx):
pass

def _assert_correct_dep_mapping(ctx):
"""Forces a failure if proc_macro_deps and deps are mixed inappropriately
"""Ensures dependencies are correctly mapped between 'deps', 'proc_macro_deps', and 'link_deps'.

This function validates that procedural macros and native libraries are listed in
their appropriate attributes to maintain the rules_rust dependency model.

Args:
ctx (ctx): The current rule's context object
"""
for dep in ctx.attr.deps:
if rust_common.crate_info in dep:
if dep[rust_common.crate_info].type == "proc-macro":
# Identify if this is a Rust-related target using any known Rust provider.
is_rust_target = (
rust_common.crate_info in dep or
rust_common.crate_group_info in dep or
rust_common.test_crate_info in dep or
rust_common.dep_info in dep or
BuildInfo in dep
)

if is_rust_target:
if rust_common.crate_info in dep and dep[rust_common.crate_info].type == "proc-macro":
fail(
"{} listed {} in its deps, but it is a proc-macro. It should instead be in the bazel property proc_macro_deps.".format(
ctx.label,
dep.label,
),
)

continue

# If it's not a known Rust target but provides CcInfo, it's a native library
# that should ideally be in 'link_deps'.
if CcInfo in dep:
pass

for dep in ctx.attr.proc_macro_deps:
if CrateInfo in dep:
types = [dep[CrateInfo].type]
Expand Down Expand Up @@ -217,6 +238,8 @@ def _rust_library_common(ctx, crate_type):
)

deps = transform_deps(ctx.attr.deps)
if hasattr(ctx.attr, "link_deps"):
deps += transform_link_deps(ctx.attr.link_deps)
proc_macro_deps = transform_deps(ctx.attr.proc_macro_deps + get_import_macro_deps(ctx))

return rustc_compile_action(
Expand Down Expand Up @@ -269,6 +292,8 @@ def _rust_binary_impl(ctx):
output = ctx.actions.declare_file(output_filename + toolchain.binary_ext)

deps = transform_deps(ctx.attr.deps)
if hasattr(ctx.attr, "link_deps"):
deps += transform_link_deps(ctx.attr.link_deps)
proc_macro_deps = transform_deps(ctx.attr.proc_macro_deps + get_import_macro_deps(ctx))

crate_root = getattr(ctx.file, "crate_root", None)
Expand Down Expand Up @@ -356,6 +381,8 @@ def _rust_test_impl(ctx):

crate_type = "bin"
deps = transform_deps(ctx.attr.deps)
if hasattr(ctx.attr, "link_deps"):
deps += transform_link_deps(ctx.attr.link_deps)
proc_macro_deps = transform_deps(ctx.attr.proc_macro_deps + get_import_macro_deps(ctx))

if ctx.attr.crate and ctx.attr.srcs:
Expand Down Expand Up @@ -713,15 +740,22 @@ _COMMON_ATTRS = {
),
"deps": attr.label_list(
doc = dedent("""\
List of other libraries to be linked to this library target.
List of other Rust libraries to be linked to this library target.

These can be either other `rust_library` targets or `cc_library` targets if
linking a native library.
These must be targets that provide `CrateInfo`, such as `rust_library`.
"""),
),
"edition": attr.string(
doc = "The rust edition to use for this crate. Defaults to the edition specified in the rust_toolchain.",
),
"link_deps": attr.label_list(
doc = dedent("""\
List of other native libraries to be linked to this library target.

These are typically `cc_library` targets.
"""),
providers = [[CcInfo], [rust_common.crate_info]],
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why the [rust_common.crate_info] part here? Naively, while we should allow for rust_library dependencies here, they by themselves already provide CcInfo, so they should satisfy the [CcInfo] spec already?

),
"lint_config": attr.label(
doc = "Set of lints to apply when building this crate.",
providers = [LintsInfo],
Expand Down Expand Up @@ -1128,7 +1162,7 @@ rust_proc_macro = rule(
# need to declare `_allowlist_function_transition`, see
# https://docs.bazel.build/versions/main/skylark/config.html#user-defined-transitions.
attrs = dict(
_COMMON_ATTRS.items(),
{name: value for name, value in _COMMON_ATTRS.items() if name != "link_deps"}.items(),
_allowlist_function_transition = attr.label(
default = Label("//tools/allowlists/function_transition_allowlist"),
),
Expand Down
17 changes: 17 additions & 0 deletions rust/private/utils.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,23 @@ def transform_deps(deps):
crate_group_info = dep[CrateGroupInfo] if CrateGroupInfo in dep else None,
) for dep in deps]

def transform_link_deps(link_deps):
"""Transforms a [Target] into [DepVariantInfo] for native symbol linkage.

Args:
link_deps (list of Targets): Dependencies coming from ctx.attr.link_deps

Returns:
list of DepVariantInfos with only CcInfo populated.
"""
return [DepVariantInfo(
crate_info = None,
dep_info = None,
build_info = None,
cc_info = dep[CcInfo] if CcInfo in dep else None,
crate_group_info = None,
) for dep in link_deps]

def get_import_macro_deps(ctx):
"""Returns a list of targets to be added to proc_macro_deps.

Expand Down
21 changes: 21 additions & 0 deletions test/unit/link_deps/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
load("//rust:defs.bzl", "rust_library", "rust_proc_macro")
load(":link_deps_test.bzl", "link_deps_test", "link_deps_failure_test")

# A dummy library to be used as a dependency
rust_library(
name = "leaf_lib",
srcs = ["lib.rs"],
)

# Test 1: rust_library supports link_deps
rust_library(
name = "main_lib",
srcs = ["lib.rs"],
link_deps = [":leaf_lib"],
tags = ["manual"],
)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This analysistest suite is nice.
Could you also add a minimal non-analysistest build-test example: something like a cc_library that defines an extern "C" function and a rust_binary that refers to it via link_deps and uses the symbol (so the symbol must be available at link-time, but only there), and a build test that ensures that the rust_binary links successfully.


link_deps_test(
name = "link_deps_success_test",
target_under_test = ":main_lib",
)
1 change: 1 addition & 0 deletions test/unit/link_deps/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

21 changes: 21 additions & 0 deletions test/unit/link_deps/link_deps_test.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
load("//rust:rust_common.bzl", "CrateInfo")
load("@rules_cc//cc/common:cc_info.bzl", "CcInfo")
load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")

def _link_deps_test_impl(ctx):
env = analysistest.begin(ctx)
target = analysistest.target_under_test(env)

# Verify that CcInfo (symbols) is present
asserts.true(env, CcInfo in target, "Target should provide CcInfo from link_deps")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This doesn't seem to test much with link_deps -- it asserts that the (client) target_under_test (in the BUILD file main_lib) produces a CcInfo. Instead, you probably want to have the test code examine the client providers of the target under test, locate the information about the link_deps dependency and examine that. Look at the DepsInfo provider, and its direct_crates and transitive_noncrates fields.


return analysistest.end(env)

link_deps_test = analysistest.make(_link_deps_test_impl)

def _link_deps_failure_test_impl(ctx):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hey, we don't seem to actually be using this (instantiating) in tests?

env = analysistest.begin(ctx)
asserts.expect_failure(env, "no such attribute 'link_deps' in 'rust_proc_macro' rule")
return analysistest.end(env)

link_deps_failure_test = analysistest.make(_link_deps_failure_test_impl, expect_failure = True)