Skip to content

Commit

Permalink
Make Google proto_library and extension of Bazel proto_library
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 580146223
Change-Id: I13ac76086d56cd26ae0c661d1c36c45a2ee270d1
  • Loading branch information
comius authored and copybara-github committed Nov 7, 2023
1 parent 41a1fbc commit c519916
Show file tree
Hide file tree
Showing 9 changed files with 14 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,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.");
}

Expand Down Expand Up @@ -585,7 +585,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) {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -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")
Expand All @@ -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,
Expand Down
2 changes: 0 additions & 2 deletions src/main/starlark/builtins_bzl/common/exports.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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_binary_deploy_jar.bzl", get_java_build_info = "get_build_info")
Expand Down Expand Up @@ -76,7 +75,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,
Expand Down
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
Expand Up @@ -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")
Expand All @@ -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
Expand Down Expand Up @@ -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],
Expand All @@ -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
Expand Up @@ -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.
#
Expand All @@ -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
Expand Up @@ -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())
Expand Down Expand Up @@ -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"));
}
}

Expand Down

0 comments on commit c519916

Please sign in to comment.