Skip to content

Propagate CcInfo for native dependencies #257

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Propagate CcInfo for native dependencies #257

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
79bf77c
Propagate `CcInfo` for native dependencies
Yannic Jan 2, 2025
9987e8d
cache cc info
Yannic Jan 7, 2025
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
11 changes: 8 additions & 3 deletions .bazelci/presubmit.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ build_targets_bazel6: &build_targets_bazel6
build_targets_integration: &build_targets_integration
- "//..."
- "//:bin_deploy.jar"
- "-//:native_test"

test_targets: &test_targets
- "//test/..."
Expand All @@ -29,6 +30,10 @@ test_targets_bazel6: &test_targets_bazel6

test_target_integration: &test_target_integration
- "//:MyTest"
- "//:native_test"

test_target_integration_pre_bazel_8: &test_target_integration_pre_bazel_8
- "//:MyTest"

flags_workspace_integration: &flags_workspace_integration
- "--noenable_bzlmod"
Expand All @@ -52,7 +57,7 @@ tasks:
shell_commands:
- sh setup.sh
build_targets: *build_targets_integration
test_targets: *test_target_integration
test_targets: *test_target_integration_pre_bazel_8
ubuntu2004_integration_workspace:
name: "Bazel 7.x Integration (WORKSPACE)"
bazel: "7.4.0"
Expand All @@ -62,7 +67,7 @@ tasks:
- sh setup.sh
build_targets: *build_targets_integration
build_flags: *flags_workspace_integration
test_targets: *test_target_integration
test_targets: *test_target_integration_pre_bazel_8
test_flags: *flags_workspace_integration
macos:
name: "Bazel 7.x"
Expand Down Expand Up @@ -125,7 +130,7 @@ tasks:
shell_commands:
- sh setup.sh
build_targets: *build_targets_integration
test_targets: *test_target_integration
test_targets: *test_target_integration_pre_bazel_8
macos_bazel6:
name: "Bazel 6.x"
bazel: 6.4.0
Expand Down
17 changes: 16 additions & 1 deletion java/common/rules/impl/basic_java_library_impl.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
Common code for reuse across java_* rules
"""

load("@rules_cc//cc/common:cc_common.bzl", "cc_common")
load("@rules_cc//cc/common:cc_info.bzl", "CcInfo")
load("//java/common/rules:android_lint.bzl", "android_lint_subrule")
load("//java/private:boot_class_path_info.bzl", "BootClassPathInfo")
Expand Down Expand Up @@ -124,6 +125,7 @@ def basic_java_library(
resources = list(resources)
resources.extend(properties)

native_libraries = _collect_native_libraries(deps, runtime_deps, exports)
java_info, compilation_info = compile_action(
ctx,
ctx.outputs.classjar,
Expand All @@ -138,7 +140,7 @@ def basic_java_library(
resources,
resource_jars,
classpath_resources,
_collect_native_libraries(deps, runtime_deps, exports),
native_libraries,
javacopts,
neverlink,
ctx.fragments.java.strict_java_deps,
Expand All @@ -150,6 +152,19 @@ def basic_java_library(
)
target = {"JavaInfo": java_info}

if native_libraries:
dependencies_cc_info = cc_common.merge_cc_infos(cc_infos = native_libraries)

# Native dependencies have the same semantics as
# `cc_library#implementation_deps`. We only want to propagate
# `Cc{Debug,Linking}Context` to libraries depending on this.
target["CcInfo"] = CcInfo(
linking_context = dependencies_cc_info.linking_context,
debug_context = dependencies_cc_info.debug_context(),
)
else:
target["CcInfo"] = CcInfo()
Copy link
Contributor

Choose a reason for hiding this comment

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

As this is a common case, you can reduce retained heap by sharing a single empty instance.

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'm not sure how much of a difference this makes in practice: java_library will output CcInfo, so every target with deps will take the if case and produce a different, non-equal CcInfo with probably no native deps.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I see. I don't know whether CcInfo merging attempts to preserve empty instances, which would help here.

That said, I now realize that I don't quite understand why each java_library now needs to advertise CcInfo. What information is missing from transitive_native_libraries? Why can't this be a private provider distinct from CcInfo up to the java_binary level?


output_groups = dict(
compilation_outputs = compilation_info.files_to_build,
_source_jars = java_info.transitive_source_jars,
Expand Down
21 changes: 21 additions & 0 deletions test/repo/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,30 @@
load("@rules_cc//cc:defs.bzl", "cc_library", "cc_test")
load("@rules_java//java:defs.bzl", "java_binary", "java_library", "java_test") # copybara-use-repo-external-label
load("@rules_java//toolchains:default_java_toolchain.bzl", "default_java_toolchain") # copybara-use-repo-external-label

cc_library(
name = "native_dep",
srcs = [
"src/native_dep.cc",
],
)

java_library(
name = "lib",
srcs = ["src/Main.java"],
deps = [
":native_dep",
],
)

cc_test(
name = "native_test",
srcs = [
"src/native_test.cc",
],
deps = [
":lib",
],
)

java_binary(
Expand Down
1 change: 1 addition & 0 deletions test/repo/MODULE.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
module(name = "integration_test_repo")

bazel_dep(name = "rules_cc", version = "0.0.15")
bazel_dep(name = "rules_java", version = "7.5.0")
archive_override(
module_name = "rules_java",
Expand Down
3 changes: 3 additions & 0 deletions test/repo/src/native_dep.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
int my_number() {
return 42;
}
12 changes: 12 additions & 0 deletions test/repo/src/native_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#include <iostream>

int my_number();

int main() {
int actual = my_number();
if (actual != 42) {
std::cerr << "Expected my_number() to return 42, got " << actual << std::endl;
return 1;
}
return 0;
}