Skip to content

Commit

Permalink
Introduce aliases in @bazel_tools for protobuf
Browse files Browse the repository at this point in the history
Certain flags have default label values pointing to within @com_google_protobuf. Since the built-in module bazel_tools already has a dependency on @com_google_protobuf, we can simply create aliases for those targets in @bazel_tools, which removes the need for protobuf to be a well-known module.

Related: #14279
PiperOrigin-RevId: 459066103
Change-Id: I583f28ace7afe2bc91997293bb9870f836e393eb
  • Loading branch information
Wyverald authored and copybara-github committed Jul 5, 2022
1 parent 0062f89 commit abdb1d6
Show file tree
Hide file tree
Showing 12 changed files with 67 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,11 @@
public abstract class ModuleKey {

/**
* A mapping from module name to repository name.
* A mapping from module name to repository name for certain special "well-known" modules.
*
* <p>For some well known modules, their repository names are referenced in default label values
* of some native rules' attributes and command line flags, which don't go through repo mappings.
* Therefore, we have to keep its canonical repository name the same as its well known repository
* name. Eg. "@com_google_protobuf//:protoc" is used for --proto_compiler flag.
* <p>The repository name of certain modules are required to be exact strings (instead of the
* normal format seen in {@link #getCanonicalRepoName()}) due to backwards compatibility reasons.
* For example, bazel_tools must be known as "@bazel_tools" for WORKSPACE repos to work correctly.
*
* <p>NOTE(wyv): We don't prepend an '@' to the repo names of well-known modules. This is because
* we still need the repo name to be 'bazel_tools' (not '@bazel_tools') since the command line
Expand All @@ -38,16 +37,15 @@ public abstract class ModuleKey {
* Bzlmod is not enabled. On the other hand, this means we cannot write '@@bazel_tools//:thing' to
* bypass repo mapping at all, which can be awkward.
*
* <p>TODO(wyv): After we get rid of usage of com_google_protobuf in flag defaults, and make all
* flag values go through repo mapping, we can remove the concept of well-known modules
* altogether.
* <p>TODO(wyv): After we make all flag values go through repo mapping, we can remove the concept
* of well-known modules altogether.
*/
private static final ImmutableMap<String, RepositoryName> WELL_KNOWN_MODULES =
ImmutableMap.of(
"com_google_protobuf", RepositoryName.createUnvalidated("com_google_protobuf"),
"protobuf", RepositoryName.createUnvalidated("com_google_protobuf"),
"bazel_tools", RepositoryName.BAZEL_TOOLS,
"local_config_platform", RepositoryName.createUnvalidated("local_config_platform"));
"bazel_tools",
RepositoryName.BAZEL_TOOLS,
"local_config_platform",
RepositoryName.createUnvalidated("local_config_platform"));

public static final ModuleKey ROOT = create("", Version.EMPTY);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
public class BazelJavaLiteProtoAspect extends JavaLiteProtoAspect {

public static final String DEFAULT_PROTO_TOOLCHAIN_LABEL =
"@com_google_protobuf//:javalite_toolchain";
"@bazel_tools//tools/proto:javalite_toolchain";

public BazelJavaLiteProtoAspect(RuleDefinitionEnvironment env) {
super(BazelJavaSemantics.INSTANCE, DEFAULT_PROTO_TOOLCHAIN_LABEL, env);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public BazelJavaProtoAspect(RuleDefinitionEnvironment env) {
super(
BazelJavaSemantics.INSTANCE,
new NoopRpcSupport(),
"@com_google_protobuf//:java_toolchain",
"@bazel_tools//tools/proto:java_toolchain",
env);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public abstract class CcProtoAspect extends NativeAspectClass implements Configu
private static final LabelLateBoundDefault<?> PROTO_TOOLCHAIN_LABEL =
LabelLateBoundDefault.fromTargetConfiguration(
ProtoConfiguration.class,
Label.parseAbsoluteUnchecked("@com_google_protobuf//:cc_toolchain"),
Label.parseAbsoluteUnchecked("@bazel_tools//tools/proto:cc_toolchain"),
(rule, attributes, protoConfig) -> protoConfig.protoToolchainForCc());

private final CppSemantics cppSemantics;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public static class Options extends FragmentOptions {

@Option(
name = "proto_toolchain_for_javalite",
defaultValue = "@com_google_protobuf//:javalite_toolchain",
defaultValue = "@bazel_tools//tools/proto:javalite_toolchain",
converter = CoreOptionConverters.LabelConverter.class,
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.AFFECTS_OUTPUTS, OptionEffectTag.LOADING_AND_ANALYSIS},
Expand All @@ -103,7 +103,7 @@ public static class Options extends FragmentOptions {

@Option(
name = "proto_toolchain_for_java",
defaultValue = "@com_google_protobuf//:java_toolchain",
defaultValue = "@bazel_tools//tools/proto:java_toolchain",
converter = CoreOptionConverters.EmptyToNullLabelConverter.class,
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.AFFECTS_OUTPUTS, OptionEffectTag.LOADING_AND_ANALYSIS},
Expand All @@ -122,7 +122,7 @@ public static class Options extends FragmentOptions {

@Option(
name = "proto_toolchain_for_cc",
defaultValue = "@com_google_protobuf//:cc_toolchain",
defaultValue = "@bazel_tools//tools/proto:cc_toolchain",
converter = CoreOptionConverters.EmptyToNullLabelConverter.class,
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.AFFECTS_OUTPUTS, OptionEffectTag.LOADING_AND_ANALYSIS},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@
/** Constants used in Proto rules. */
public final class ProtoConstants {
/** Default label for proto compiler. */
static final String DEFAULT_PROTOC_LABEL = "@com_google_protobuf//:protoc";
static final String DEFAULT_PROTOC_LABEL = "@bazel_tools//tools/proto:protoc";

/** Default label for java proto toolchains. */
static final String DEFAULT_JAVA_PROTO_LABEL = "@com_google_protobuf//:java_toolchain";
static final String DEFAULT_JAVA_PROTO_LABEL = "@bazel_tools//tools/proto:java_toolchain";

/** Default label for java lite proto toolchains. */
static final String DEFAULT_JAVA_LITE_PROTO_LABEL = "@com_google_protobuf//:javalite_toolchain";
static final String DEFAULT_JAVA_LITE_PROTO_LABEL =
"@bazel_tools//tools/proto:javalite_toolchain";

/**
* This constant is used in ProtoCompileActionBuilder to generate an error message that's
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def _preprocess(ctx):
pass

semantics = struct(
PROTO_COMPILER_LABEL = "@com_google_protobuf//:protoc",
PROTO_COMPILER_LABEL = "@bazel_tools//tools/proto:protoc",
EXTRA_ATTRIBUTES = {
"import_prefix": attr.string(),
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,14 @@ public void setupMockClient(MockToolsConfig config, List<String> workspaceConten
"package_group(name='config_feature_flag', packages=['//...'])",
"package_group(name='config_feature_flag_Setter', packages=['//...'])");

config.create(
"embedded_tools/tools/proto/BUILD",
"package(default_visibility=['//visibility:public'])",
"alias(name='protoc',actual='@com_google_protobuf//:protoc')",
"alias(name='javalite_toolchain',actual='@com_google_protobuf//:javalite_toolchain')",
"alias(name='java_toolchain',actual='@com_google_protobuf//:java_toolchain')",
"alias(name='cc_toolchain',actual='@com_google_protobuf//:cc_toolchain')");

config.create(
"embedded_tools/tools/zip/BUILD",
"package(default_visibility=['//visibility:public'])",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ java_test(
deps = [
"//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_collection",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/rules/proto",
"//src/test/java/com/google/devtools/build/lib/analysis/util",
"//src/test/java/com/google/devtools/build/lib/packages:testutil",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.packages.Provider;
import com.google.devtools.build.lib.packages.StarlarkProvider;
import com.google.devtools.build.lib.packages.util.MockProtoSupport;
import com.google.devtools.build.lib.testutil.TestConstants;
import org.junit.Before;
Expand All @@ -42,12 +39,6 @@ public void setUp() throws Exception {
invalidatePackages();
}

Provider.Key getStarlarkProtoLangToolchainInfoKey() throws LabelSyntaxException {
return new StarlarkProvider.Key(
Label.parseCanonical("@_builtins//:common/proto/proto_common.bzl"),
"ProtoLangToolchainInfo");
}

private void validateProtoLangToolchain(ProtoLangToolchainProvider toolchain) throws Exception {
assertThat(toolchain.outReplacementFormatFlag()).isEqualTo("cmd-line:%s");
assertThat(toolchain.pluginFormatFlag()).isEqualTo("--plugin=%s");
Expand All @@ -63,9 +54,11 @@ private void validateProtoLangToolchain(ProtoLangToolchainProvider toolchain) th
assertThat(toolchain.mnemonic()).isEqualTo("MyMnemonic");
}

private void validateProtoCompiler(ProtoLangToolchainProvider toolchain, Label protoCompiler) {
private void validateProtoCompiler(ProtoLangToolchainProvider toolchain, String protocLabel)
throws Exception {
Label actualProtocLabel = getConfiguredTarget(protocLabel).getActual().getLabel();
assertThat(toolchain.protoc().getExecutable().prettyPrint())
.isEqualTo(protoCompiler.toPathFragment().getPathString());
.isEqualTo(actualProtocLabel.toPathFragment().getPathString());
}

@Test
Expand Down Expand Up @@ -96,10 +89,9 @@ public void protoToolchain() throws Exception {
update(ImmutableList.of("//foo:toolchain"), false, 1, true, new EventBus());
ProtoLangToolchainProvider toolchain =
ProtoLangToolchainProvider.get(getConfiguredTarget("//foo:toolchain"));
Label protoc = Label.parseAbsoluteUnchecked(ProtoConstants.DEFAULT_PROTOC_LABEL);

validateProtoLangToolchain(toolchain);
validateProtoCompiler(toolchain, protoc);
validateProtoCompiler(toolchain, ProtoConstants.DEFAULT_PROTOC_LABEL);
}

@Test
Expand Down Expand Up @@ -131,10 +123,9 @@ public void protoToolchain_setProtoCompiler() throws Exception {

ProtoLangToolchainProvider toolchain =
ProtoLangToolchainProvider.get(getConfiguredTarget("//foo:toolchain"));
Label protoc = Label.parseAbsoluteUnchecked("//third_party/x:compiler");

validateProtoLangToolchain(toolchain);
validateProtoCompiler(toolchain, protoc);
validateProtoCompiler(toolchain, "//third_party/x:compiler");
}

@Test
Expand Down Expand Up @@ -164,10 +155,9 @@ public void protoToolchainBlacklistProtoLibraries() throws Exception {
update(ImmutableList.of("//foo:toolchain"), false, 1, true, new EventBus());
ProtoLangToolchainProvider toolchain =
ProtoLangToolchainProvider.get(getConfiguredTarget("//foo:toolchain"));
Label protoc = Label.parseAbsoluteUnchecked(ProtoConstants.DEFAULT_PROTOC_LABEL);

validateProtoLangToolchain(toolchain);
validateProtoCompiler(toolchain, protoc);
validateProtoCompiler(toolchain, ProtoConstants.DEFAULT_PROTOC_LABEL);
}

@Test
Expand Down Expand Up @@ -197,10 +187,9 @@ public void protoToolchainBlacklistTransitiveProtos() throws Exception {
update(ImmutableList.of("//foo:toolchain"), false, 1, true, new EventBus());
ProtoLangToolchainProvider toolchain =
ProtoLangToolchainProvider.get(getConfiguredTarget("//foo:toolchain"));
Label protoc = Label.parseAbsoluteUnchecked(ProtoConstants.DEFAULT_PROTOC_LABEL);

validateProtoLangToolchain(toolchain);
validateProtoCompiler(toolchain, protoc);
validateProtoCompiler(toolchain, ProtoConstants.DEFAULT_PROTOC_LABEL);
}

@Test
Expand Down
2 changes: 2 additions & 0 deletions tools/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ filegroup(
"//tools/objc:srcs",
"//tools/osx:srcs",
"//tools/osx/crosstool:srcs",
"//tools/proto:srcs",
"//tools/windows:srcs",
"//tools/test:srcs",
"//tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:srcs",
Expand Down Expand Up @@ -60,6 +61,7 @@ filegroup(
"//tools/def_parser:srcs",
"//tools/windows:srcs",
"//tools/platforms:srcs",
"//tools/proto:srcs",
"//tools/objc:srcs",
"//tools/python:embedded_tools",
"//tools/runfiles:embedded_tools",
Expand Down
28 changes: 28 additions & 0 deletions tools/proto/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package(default_visibility = ["//visibility:public"])

filegroup(
name = "srcs",
srcs = glob(["**"] + ["**/*"]),
)

exports_files(["BUILD"])

alias(
name = "protoc",
actual = "@com_google_protobuf//:protoc",
)

alias(
name = "javalite_toolchain",
actual = "@com_google_protobuf//:javalite_toolchain",
)

alias(
name = "java_toolchain",
actual = "@com_google_protobuf//:java_toolchain",
)

alias(
name = "cc_toolchain",
actual = "@com_google_protobuf//:cc_toolchain",
)

0 comments on commit abdb1d6

Please sign in to comment.