Skip to content

Commit

Permalink
Stop exporting java rules / providers from @_builtins
Browse files Browse the repository at this point in the history
`java_common` needs to remain (minimally exported) for private APIs

PiperOrigin-RevId: 698412708
Change-Id: I58708761e7abbc63013f75276b1cd1f98c11430f
  • Loading branch information
hvadehra authored and copybara-github committed Nov 20, 2024
1 parent 8a56158 commit ae3262f
Show file tree
Hide file tree
Showing 10 changed files with 25 additions and 23 deletions.
5 changes: 3 additions & 2 deletions src/main/java/com/google/devtools/build/lib/bazel/rules/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,14 @@ gen_workspace_stanza(
name = "rules_suffix",
out = "rules_suffix.WORKSPACE",
postamble = """
load("@rules_java//java:repositories.bzl", "rules_java_dependencies", "rules_java_toolchains")
load("@rules_java//java:rules_java_deps.bzl", "rules_java_dependencies")
rules_java_dependencies()
rules_java_toolchains()
load("@rules_python//python:repositories.bzl", "py_repositories")
py_repositories()
load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")
protobuf_deps()
load("@rules_java//java:repositories.bzl", "rules_java_toolchains")
rules_java_toolchains()
""",
preamble = """
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

package com.google.devtools.build.lib.rules.java;

import static com.google.devtools.build.lib.skyframe.BzlLoadValue.keyForBuiltins;
import static com.google.devtools.build.lib.skyframe.BzlLoadValue.keyForBuild;

import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact;
Expand Down Expand Up @@ -42,8 +42,10 @@ public final class MessageBundleInfo {
private static class Provider extends StarlarkProviderWrapper<MessageBundleInfo> {
private Provider() {
super(
keyForBuiltins(
Label.parseCanonicalUnchecked("@_builtins//:common/java/message_bundle_info.bzl")),
keyForBuild(
Label.parseCanonicalUnchecked(
JavaSemantics.RULES_JAVA_PROVIDER_LABELS_PREFIX
+ "java/private:message_bundle_info.bzl")),
STARLARK_NAME);
}

Expand Down
9 changes: 1 addition & 8 deletions src/main/starlark/builtins_bzl/bazel/exports.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,10 @@
"""Exported builtins symbols that are specific to OSS Bazel."""

load("@_builtins//:common/python/py_internal.bzl", "py_internal")
load(":common/java/java_package_configuration.bzl", "java_package_configuration")
load(":common/java/java_runtime.bzl", "java_runtime")
load(":common/java/java_toolchain.bzl", "java_toolchain")

exported_toplevels = {
"py_internal": py_internal,
"proto_common_do_not_use": struct(INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION = _builtins.toplevel.proto_common_do_not_use.incompatible_enable_proto_toolchain_resolution()),
}
exported_rules = {
"java_package_configuration": java_package_configuration,
"java_runtime": java_runtime,
"java_toolchain": java_toolchain,
}
exported_rules = {}
exported_to_java = {}
5 changes: 1 addition & 4 deletions src/main/starlark/builtins_bzl/common/exports.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ load(":common/cc/fdo/fdo_profile.bzl", "fdo_profile")
load(":common/cc/fdo/memprof_profile.bzl", "memprof_profile")
load(":common/cc/fdo/propeller_optimize.bzl", "propeller_optimize")
load(":common/java/java_common.bzl", "java_common")
load(":common/java/java_info.bzl", "JavaInfo", "JavaPluginInfo")
load(":common/objc/apple_common.bzl", "apple_common")
load(":common/objc/objc_common.bzl", "objc_common")

Expand All @@ -49,9 +48,7 @@ exported_toplevels = {
"CcSharedLibraryInfo": CcSharedLibraryInfo,
"CcSharedLibraryHintInfo": CcSharedLibraryHintInfo,
"cc_common": cc_common,
"+JavaPluginInfo": JavaPluginInfo,
"+JavaInfo": JavaInfo,
"java_common": java_common,
"java_common": struct(internal_DO_NOT_USE = java_common.internal_DO_NOT_USE),

This comment has been minimized.

Copy link
@fmeum

fmeum Nov 21, 2024

Collaborator

@hvadehra This causes failures when a ruleset uses java_common without a load and autoload doesn't seem to prevent that. Is that an intentional breaking change?

This comment has been minimized.

Copy link
@hvadehra

hvadehra Nov 21, 2024

Author Member

cc @comius

This comment has been minimized.

Copy link
@comius

comius Nov 21, 2024

Contributor

Is there a repro? Which rules repository is causing it?

This comment has been minimized.

Copy link
@fmeum

fmeum Nov 21, 2024

Collaborator

The test module of rules_jni (https://github.com/fmeum/rules_jni/tree/main/tests) is failing with it. You can reproduce with bazel build --config=bzlmod //....

This comment has been minimized.

Copy link
@hvadehra

hvadehra Nov 21, 2024

Author Member

Okay, looks like autoloads for java_common were missed. Can you try building with --incompatible_autoload_externally with the default + +java_common

This comment has been minimized.

Copy link
@fmeum

fmeum Nov 21, 2024

Collaborator

Yes, that fixes the failure.

This comment has been minimized.

Copy link
@hvadehra

hvadehra Nov 21, 2024

Author Member

Good to know, thanks, fix incoming.

This comment has been minimized.

Copy link
@hvadehra

hvadehra Nov 21, 2024

Author Member
"apple_common": apple_common,
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1277,6 +1277,7 @@ public void nativeAndStarlarkJavaOutputsCanBeAddedToADepset() throws Exception {
scratch.file(
"foo/extension.bzl",
"""
load("@rules_java//java/common:java_info.bzl", "JavaInfo")
def _impl(ctx):
f = ctx.actions.declare_file(ctx.label.name + ".jar")
ctx.actions.write(f, "")
Expand Down Expand Up @@ -1313,6 +1314,7 @@ public void testNeverlinkIsStoredAsABoolean() throws Exception {
scratch.file(
"foo/extension.bzl",
"""
load("@rules_java//java/common:java_info.bzl", "JavaInfo")
def _impl(ctx):
f = ctx.actions.declare_file(ctx.label.name + ".jar")
ctx.actions.write(f, "")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1573,6 +1573,7 @@ public void testLacksAdvertisedBuiltinProvider() throws Exception {
scratch.file(
"test/foo.bzl",
"""
load("@rules_java//java/common:java_info.bzl", "JavaInfo")
FooInfo = provider()
def _impl(ctx):
MyFooInfo = FooInfo()
Expand Down
4 changes: 3 additions & 1 deletion src/test/shell/bazel/bazel_rules_java_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,11 @@ EOF
filegroup(name = 'yolo')
EOF
touch override/java/BUILD || fail "couldn't touch override/java/BUILD"
cat > override/java/repositories.bzl <<EOF
cat > override/java/rules_java_deps.bzl <<EOF
def rules_java_dependencies():
pass
EOF
cat > override/java/repositories.bzl <<EOF
def rules_java_toolchains():
pass
EOF
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,11 @@ function mock_rules_java() {
rules_java_workspace="${TEST_TMPDIR}/rules_java_workspace"
mkdir -p "${rules_java_workspace}/java"
touch "${rules_java_workspace}/java/BUILD"
cat > "${rules_java_workspace}/java/repositories.bzl" <<EOF
cat > "${rules_java_workspace}/java/rules_java_deps.bzl" <<EOF
def rules_java_dependencies():
pass
EOF
cat > "${rules_java_workspace}/java/repositories.bzl" <<EOF
def rules_java_toolchains():
pass
EOF
Expand Down
4 changes: 3 additions & 1 deletion src/test/shell/testenv.sh
Original file line number Diff line number Diff line change
Expand Up @@ -851,9 +851,11 @@ EOF
JDK_BUILD_TEMPLATE = ''
EOF
touch "${rules_java_workspace}/java/BUILD"
cat > "${rules_java_workspace}/java/repositories.bzl" <<EOF
cat > "${rules_java_workspace}/java/rules_java_deps.bzl" <<EOF
def rules_java_dependencies():
pass
EOF
cat > "${rules_java_workspace}/java/repositories.bzl" <<EOF
def rules_java_toolchains():
pass
EOF
Expand Down
6 changes: 3 additions & 3 deletions workspace_deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ WORKSPACE_REPOS = {
"strip_prefix": "protobuf-29.0-rc3",
},
"rules_java": {
"archive": "rules_java-8.3.1.tar.gz",
"sha256": "ee786b943e00da4fea7c233e70e5f5b8a01cc69b9341b3f49169f174fe0df1c5",
"urls": ["https://github.com/bazelbuild/rules_java/releases/download/8.3.1/rules_java-8.3.1.tar.gz"],
"archive": "rules_java-8.5.0.tar.gz",
"sha256": "5c215757b9a6c3dd5312a3cdc4896cef3f0c5b31db31baa8da0d988685d42ae4",
"urls": ["https://github.com/bazelbuild/rules_java/releases/download/8.5.0/rules_java-8.5.0.tar.gz"],
},
"bazel_skylib": {
"archive": "bazel-skylib-1.6.1.tar.gz",
Expand Down

0 comments on commit ae3262f

Please sign in to comment.