-
Notifications
You must be signed in to change notification settings - Fork 595
feature: introduce link_deps attribute for native library linkage #4024
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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"], | ||
| ) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This analysistest suite is nice. |
||
|
|
||
| link_deps_test( | ||
| name = "link_deps_success_test", | ||
| target_under_test = ":main_lib", | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
|
|
| 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") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
There was a problem hiding this comment.
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 provideCcInfo, so they should satisfy the[CcInfo]spec already?