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

Advertise CcInfo provider on rules #2126

Merged
merged 6 commits into from
Jan 5, 2024
Merged
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
4 changes: 4 additions & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ coverage --combined_report=lcov
# https://bazel.build/reference/command-line-reference#flag--experimental_fetch_all_coverage_outputs
coverage --experimental_fetch_all_coverage_outputs

# Required for some of the tests
# https://bazel.build/reference/command-line-reference#flag--experimental_cc_shared_library
common --experimental_cc_shared_library

###############################################################################
## Unique configuration groups
###############################################################################
Expand Down
2 changes: 2 additions & 0 deletions rust/private/rust.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,7 @@ rust_static_library = rule(
str(Label("//rust:toolchain_type")),
"@bazel_tools//tools/cpp:toolchain_type",
],
provides = [CcInfo],
doc = dedent("""\
Builds a Rust static library.

Expand Down Expand Up @@ -948,6 +949,7 @@ rust_shared_library = rule(
str(Label("//rust:toolchain_type")),
"@bazel_tools//tools/cpp:toolchain_type",
],
provides = [CcInfo],
doc = dedent("""\
Builds a Rust shared library.

Expand Down
30 changes: 30 additions & 0 deletions test/cc_shared_library/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
load("@rules_cc//cc:defs.bzl", "cc_library", "cc_test")
load("@rules_rust//rust:defs.bzl", "rust_static_library")

rust_static_library(
name = "rust_lib",
srcs = ["lib.rs"],
edition = "2021",
)

cc_library(
name = "c_lib",
srcs = ["lib.c"],
hdrs = ["lib.h"],
deps = [":rust_lib"],
)

# Tests that cc_shared_library correctly traverses into
# `rust_static_library` when linking.
cc_shared_library(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will this lead to the same issue as #2359 if experimental_cc_shared_library is not set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean if people are querying it as an external repository, where the root workspace does not have this set?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose it will, although in 7.0.0 this is enabled by default so it shouldn't be an issue with the latest version.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@illicitonion given that the issue was referring to a cquery within the rules_rust repo, do you think this is an acceptable change? I do like the procides changes and love the test. I just don't wanna immediately regress something here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I sent out #2398 which add tests for what I think we should be aiming to support (i.e. bazel query ... and bazel cquery ... from inside the rules_rust repo) - if we have more things we want to support (e.g. bazel query @rules_rust//... from a repo that depends on rules_rust) , we should add tests to CI for them.

This PR passes the tests I sent out, so I'm happy :)

name = "shared",
deps = [":c_lib"],
)

cc_test(
name = "test",
srcs = ["main.c"],
dynamic_deps = [":shared"],
linkstatic = True,
deps = [":c_lib"],
)
5 changes: 5 additions & 0 deletions test/cc_shared_library/lib.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#include "lib.h"

extern int32_t bar();

int32_t foo() { return bar(); }
9 changes: 9 additions & 0 deletions test/cc_shared_library/lib.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#include <stdint.h>

#ifdef _WIN32
#define DllExport __declspec(dllexport)
#else
#define DllExport
#endif

DllExport int32_t foo();
4 changes: 4 additions & 0 deletions test/cc_shared_library/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#[no_mangle]
pub extern "C" fn bar() -> i32 {
4
}
9 changes: 9 additions & 0 deletions test/cc_shared_library/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#include <assert.h>
#include <stdint.h>

#include "lib.h"

int main(int argc, char** argv) {
assert(foo() == 4);
return 0;
}
Loading