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

[7.1.0] Fixes for experimental extend rule and subrule functionality #21237

Merged
merged 4 commits into from
Feb 8, 2024
Merged
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Make Google proto_library and extension of Bazel proto_library
PiperOrigin-RevId: 580146223
Change-Id: I13ac76086d56cd26ae0c661d1c36c45a2ee270d1
comius committed Feb 7, 2024
commit f9534d3a63083f3f28ba7894c9b879a8362f7537
Original file line number Diff line number Diff line change
@@ -516,7 +516,9 @@ public static StarlarkRuleFunction createRule(
}

// Verify the child against parent's allowlist
if (parent != null && parent.getExtendableAllowlist() != null) {
if (parent != null
&& parent.getExtendableAllowlist() != null
&& !bzlFile.getRepository().getNameWithAt().equals("@_builtins")) {
builder.addAllowlistChecker(EXTEND_RULE_ALLOWLIST_CHECKER);
Attribute.Builder<Label> allowlistAttr =
attr("$allowlist_extend_rule", LABEL)
Original file line number Diff line number Diff line change
@@ -465,7 +465,7 @@ public boolean isDefaultExecutableCreated() {
*/
public void close() {
// Check super was called
if (ruleClassUnderEvaluation.getStarlarkParent() != null && !superCalled) {
if (ruleClassUnderEvaluation.getStarlarkParent() != null && !superCalled && !isForAspect()) {
ruleContext.ruleError("'super' was not called.");
}

@@ -595,7 +595,7 @@ public Object callParent(StarlarkThread thread) throws EvalException, Interrupte
BuiltinRestriction.failIfCalledOutsideAllowlist(thread, ALLOWLIST_RULE_EXTENSION_API);
}
checkMutable("super()");
if (aspectDescriptor != null) {
if (isForAspect()) {
throw Starlark.errorf("Can't use 'super' call in an aspect.");
}
if (ruleClassUnderEvaluation.getStarlarkParent() == null) {

This file was deleted.

Original file line number Diff line number Diff line change
@@ -43,7 +43,7 @@ public final class ProtoConstants {
* it with the .proto file that violates strict proto deps.
*/
static final String STRICT_PROTO_DEPS_VIOLATION_MESSAGE =
"%%s is imported, but %1$s doesn't directly depend on a proto_library that 'srcs' it.";
"--direct_dependencies_violation_msg=%%s is imported, but %1$s doesn't directly depend on a proto_library that 'srcs' it.";

private ProtoConstants() {}
}
2 changes: 2 additions & 0 deletions src/main/starlark/builtins_bzl/bazel/exports.bzl
Original file line number Diff line number Diff line change
@@ -19,6 +19,7 @@ load("@_builtins//:common/java/java_import.bzl", "java_import")
load("@_builtins//:common/java/java_library.bzl", "JAVA_LIBRARY_ATTRS", "bazel_java_library_rule", "java_library", "make_sharded_java_library")
load("@_builtins//:common/java/java_plugin.bzl", "java_plugin")
load("@_builtins//:common/java/proto/java_proto_library.bzl", "java_proto_library")
load("@_builtins//:common/proto/proto_library.bzl", "proto_library")
load("@_builtins//:common/python/py_binary_macro.bzl", "py_binary")
load("@_builtins//:common/python/py_internal.bzl", "py_internal")
load("@_builtins//:common/python/py_library_macro.bzl", "py_library")
@@ -38,6 +39,7 @@ exported_toplevels = {
"py_internal": py_internal,
}
exported_rules = {
"proto_library": proto_library,
"java_library": java_library,
"java_plugin": java_plugin,
"java_import": java_import,
2 changes: 0 additions & 2 deletions src/main/starlark/builtins_bzl/common/exports.bzl
Original file line number Diff line number Diff line change
@@ -35,7 +35,6 @@ load("@_builtins//:common/objc/objc_library.bzl", "objc_library")
load("@_builtins//:common/proto/proto_common.bzl", "proto_common_do_not_use")
load("@_builtins//:common/proto/proto_info.bzl", "ProtoInfo")
load("@_builtins//:common/proto/proto_lang_toolchain.bzl", "proto_lang_toolchain")
load("@_builtins//:common/proto/proto_library.bzl", "proto_library")
load("@_builtins//:common/python/providers.bzl", "PyCcLinkParamsProvider", "PyInfo", "PyRuntimeInfo")
load("@_builtins//:common/python/py_runtime_macro.bzl", "py_runtime")
load(":common/java/java_common.bzl", "java_common")
@@ -75,7 +74,6 @@ exported_rules = {
"objc_import": objc_import,
"objc_library": objc_library,
"j2objc_library": j2objc_library,
"proto_library": proto_library,
"cc_shared_library": cc_shared_library,
"cc_binary": cc_binary,
"cc_test": cc_test,
10 changes: 4 additions & 6 deletions src/main/starlark/builtins_bzl/common/proto/proto_library.bzl
Original file line number Diff line number Diff line change
@@ -36,7 +36,7 @@ def _check_srcs_package(target_package, srcs):
def _get_import_prefix(ctx):
"""Gets and verifies import_prefix attribute if it is declared."""

import_prefix = ctx.attr.import_prefix if hasattr(ctx.attr, "import_prefix") else ""
import_prefix = ctx.attr.import_prefix

if not paths.is_normalized(import_prefix):
fail("should be normalized (without uplevel references or '.' path segments)", attr = "import_prefix")
@@ -61,8 +61,6 @@ def _get_strip_import_prefix(ctx):
return strip_import_prefix.removesuffix("/")

def _proto_library_impl(ctx):
semantics.preprocess(ctx)

# Verifies attributes.
_check_srcs_package(ctx.label.package, ctx.attr.srcs)
srcs = ctx.files.srcs
@@ -247,6 +245,7 @@ proto_library = rule(
providers = [ProtoInfo],
),
"strip_import_prefix": attr.string(default = "/"),
"import_prefix": attr.string(),
"allow_exports": attr.label(
cfg = "exec",
providers = [PackageSpecificationInfo],
@@ -263,9 +262,8 @@ proto_library = rule(
allow_files = True,
default = configuration_field("proto", "proto_compiler"),
),
}) | semantics.EXTRA_ATTRIBUTES,
fragments = ["proto"] + semantics.EXTRA_FRAGMENTS,
}),
fragments = ["proto"],
provides = [ProtoInfo],
exec_groups = semantics.EXEC_GROUPS,
toolchains = toolchains.use_toolchain(semantics.PROTO_TOOLCHAIN),
)
10 changes: 0 additions & 10 deletions src/main/starlark/builtins_bzl/common/proto/proto_semantics.bzl
Original file line number Diff line number Diff line change
@@ -16,17 +16,8 @@
Proto Semantics
"""

def _preprocess(ctx):
pass

semantics = struct(
PROTO_TOOLCHAIN = "@rules_proto//proto:toolchain_type",
PROTO_COMPILER_LABEL = "@bazel_tools//tools/proto:protoc",
EXTRA_ATTRIBUTES = {
"import_prefix": attr.string(),
},
EXTRA_FRAGMENTS = [],
preprocess = _preprocess,
# This constant is used in ProtoCompileActionBuilder to generate an error message that's
# displayed when a strict proto deps violation occurs.
#
@@ -37,6 +28,5 @@ semantics = struct(
"--direct_dependencies_violation_msg=" +
"%%s is imported, but %s doesn't directly depend on a proto_library that 'srcs' it."
),
EXEC_GROUPS = {},
allowlist_different_package = None,
)
Original file line number Diff line number Diff line change
@@ -225,7 +225,7 @@ public void testDescriptorSetOutput_strictDeps() throws Exception {
.containsAtLeast("--direct_dependencies", "x/nodeps.proto")
.inOrder();
assertThat(getGeneratingSpawnAction(getDescriptorOutput("//x:nodeps")).getRemainingArguments())
.contains(String.format(ProtoCompileActionBuilder.STRICT_DEPS_FLAG_TEMPLATE, "//x:nodeps"));
.contains(String.format(ProtoConstants.STRICT_PROTO_DEPS_VIOLATION_MESSAGE, "//x:nodeps"));

assertThat(
getGeneratingSpawnAction(getDescriptorOutput("//x:withdeps")).getRemainingArguments())
@@ -275,7 +275,7 @@ public void testDescriptorSetOutput_strictDeps_disabled() throws Exception {
assertThat(arg).doesNotContain("--direct_dependencies=");
assertThat(arg)
.doesNotContain(
String.format(ProtoCompileActionBuilder.STRICT_DEPS_FLAG_TEMPLATE, "//x:foo_proto"));
String.format(ProtoConstants.STRICT_PROTO_DEPS_VIOLATION_MESSAGE, "//x:foo_proto"));
}
}