From acc05b0a806bf29bd1706c98ffb1f6196d6363cc Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Tue, 10 Mar 2020 20:04:45 +0100 Subject: [PATCH 01/14] Create proto_toolchain rule --- .../java/com/google/devtools/build/lib/BUILD | 1 + .../bazel/rules/BazelRuleClassProvider.java | 2 + .../build/lib/rules/proto/ProtoToolchain.java | 39 ++++++++++++++++++ .../lib/rules/proto/ProtoToolchainInfo.java | 33 +++++++++++++++ .../lib/rules/proto/ProtoToolchainRule.java | 40 +++++++++++++++++++ .../proto/ProtoToolchainInfoApi.java | 26 ++++++++++++ 6 files changed, 141 insertions(+) create mode 100644 src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchain.java create mode 100644 src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchainInfo.java create mode 100644 src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchainRule.java create mode 100644 src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoToolchainInfoApi.java diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD index 1d5251137a5556..68faf6f2cf8981 100644 --- a/src/main/java/com/google/devtools/build/lib/BUILD +++ b/src/main/java/com/google/devtools/build/lib/BUILD @@ -903,6 +903,7 @@ java_library( ":util", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:commandline_item", + "//src/main/java/com/google/devtools/build/lib/analysis/platform", "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/concurrent", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java index c4cbffac6a57d0..a3410162753d91 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java @@ -89,6 +89,7 @@ import com.google.devtools.build.lib.rules.proto.ProtoConfiguration; import com.google.devtools.build.lib.rules.proto.ProtoInfo; import com.google.devtools.build.lib.rules.proto.ProtoLangToolchainRule; +import com.google.devtools.build.lib.rules.proto.ProtoToolchainRule; import com.google.devtools.build.lib.rules.python.PyInfo; import com.google.devtools.build.lib.rules.python.PyRuleClasses.PySymlink; import com.google.devtools.build.lib.rules.python.PyRuntimeInfo; @@ -254,6 +255,7 @@ public void init(ConfiguredRuleClassProvider.Builder builder) { builder.addConfigurationFragment(new ProtoConfiguration.Loader()); builder.addRuleDefinition(new BazelProtoLibraryRule()); builder.addRuleDefinition(new ProtoLangToolchainRule()); + builder.addRuleDefinition(new ProtoToolchainRule()); ProtoBootstrap bootstrap = new ProtoBootstrap( diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchain.java new file mode 100644 index 00000000000000..1d7f4d0340284d --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchain.java @@ -0,0 +1,39 @@ +// Copyright 2020 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.rules.proto; + +import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; +import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; +import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory; +import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.Runfiles; +import com.google.devtools.build.lib.analysis.RunfilesProvider; + +/** The implementation of the proto_toolchain rule. */ +public class ProtoToolchain implements RuleConfiguredTargetFactory { + @Override + public ConfiguredTarget create(RuleContext ruleContext) + throws ActionConflictException, RuleErrorException, InterruptedException { + ProtoCommon.checkRuleHasValidMigrationTag(ruleContext); + + RuleConfiguredTargetBuilder builder = + new RuleConfiguredTargetBuilder(ruleContext) + .addNativeDeclaredProvider(new ProtoToolchainInfo()) + .addProvider(RunfilesProvider.class, RunfilesProvider.simple(Runfiles.EMPTY)); + + return builder.build(); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchainInfo.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchainInfo.java new file mode 100644 index 00000000000000..77a5b10db9e0e9 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchainInfo.java @@ -0,0 +1,33 @@ +// Copyright 2020 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.rules.proto; + +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.analysis.platform.ToolchainInfo; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; +import com.google.devtools.build.lib.events.Location; +import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; +import com.google.devtools.build.lib.skylarkbuildapi.proto.ProtoToolchainInfoApi; + +/** + * Information about the tools used by the proto_* and LANG_proto_* rules. + */ +@Immutable +@AutoCodec +public class ProtoToolchainInfo extends ToolchainInfo implements ProtoToolchainInfoApi { + public ProtoToolchainInfo() { + super(ImmutableMap.of(), Location.BUILTIN); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchainRule.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchainRule.java new file mode 100644 index 00000000000000..e1b4e412320cab --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchainRule.java @@ -0,0 +1,40 @@ +// Copyright 2020 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.rules.proto; + +import com.google.devtools.build.lib.analysis.BaseRuleClasses; +import com.google.devtools.build.lib.analysis.RuleDefinition; +import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; +import com.google.devtools.build.lib.packages.RuleClass; + +/** Rule definition of the proto_toolchain rule. */ +public class ProtoToolchainRule implements RuleDefinition { + @Override + public RuleClass build(RuleClass.Builder builder, final RuleDefinitionEnvironment env) { + return builder + .requiresConfigurationFragments(ProtoConfiguration.class) + .advertiseProvider(ProtoToolchainInfo.class) + .build(); + } + + @Override + public Metadata getMetadata() { + return RuleDefinition.Metadata.builder() + .name("proto_toolchain") + .ancestors(BaseRuleClasses.BaseRule.class) + .factoryClass(ProtoToolchain.class) + .build(); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoToolchainInfoApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoToolchainInfoApi.java new file mode 100644 index 00000000000000..02a3ac2807c2d3 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoToolchainInfoApi.java @@ -0,0 +1,26 @@ +// Copyright 2020 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.skylarkbuildapi.proto; + +import com.google.devtools.build.lib.skylarkbuildapi.core.StructApi; +import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; +import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; + +/** Provides access to information about the Protobuf toolchain rule. */ +@SkylarkModule( + name = "ProtoToolchainInfo", + doc = "Provides access to information about the Protobuf toolchain rule.", + category = SkylarkModuleCategory.PROVIDER) +public interface ProtoToolchainInfoApi extends StructApi {} From 85fd28f4c032c99cf97e44d83d27d006dbeafff8 Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Sat, 28 Mar 2020 21:16:11 +0100 Subject: [PATCH 02/14] Make ProtoToolchainInfoApi extend ToolchainInfoApi --- .../com/google/devtools/build/lib/skylarkbuildapi/proto/BUILD | 1 + .../lib/skylarkbuildapi/proto/ProtoToolchainInfoApi.java | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/BUILD b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/BUILD index 845e718de241e8..627a35b889d739 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/BUILD @@ -23,6 +23,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib:syntax", "//src/main/java/com/google/devtools/build/lib/skylarkbuildapi", "//src/main/java/com/google/devtools/build/lib/skylarkbuildapi/core", + "//src/main/java/com/google/devtools/build/lib/skylarkbuildapi/platform", "//src/main/java/com/google/devtools/build/lib/skylarkinterface", "//third_party:guava", ], diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoToolchainInfoApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoToolchainInfoApi.java index 02a3ac2807c2d3..b86129c811a849 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoToolchainInfoApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoToolchainInfoApi.java @@ -14,7 +14,7 @@ package com.google.devtools.build.lib.skylarkbuildapi.proto; -import com.google.devtools.build.lib.skylarkbuildapi.core.StructApi; +import com.google.devtools.build.lib.skylarkbuildapi.platform.ToolchainInfoApi; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; @@ -23,4 +23,4 @@ name = "ProtoToolchainInfo", doc = "Provides access to information about the Protobuf toolchain rule.", category = SkylarkModuleCategory.PROVIDER) -public interface ProtoToolchainInfoApi extends StructApi {} +public interface ProtoToolchainInfoApi extends ToolchainInfoApi {} From 6789eeb3ee74cf9b3b211c84b9be50ba5119173e Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Sun, 29 Mar 2020 00:53:25 +0100 Subject: [PATCH 03/14] Register toolchain and add it to *_proto_library --- .../lib/bazel/rules/BazelRulesModule.java | 4 +- .../build/lib/bazel/rules/proto.WORKSPACE | 1 + .../lib/rules/cpp/proto/CcProtoAspect.java | 4 +- .../rules/java/proto/JavaLiteProtoAspect.java | 4 ++ .../lib/rules/java/proto/JavaProtoAspect.java | 4 ++ .../rules/proto/BazelProtoLibraryRule.java | 1 + .../build/lib/rules/proto/ProtoCommon.java | 7 ++++ tools/BUILD | 3 ++ tools/proto/BUILD | 33 +++++++++++++++ tools/proto/BUILD.tools | 25 +++++++++++ tools/proto/toolchain.bzl | 41 +++++++++++++++++++ 11 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/bazel/rules/proto.WORKSPACE create mode 100644 tools/proto/BUILD create mode 100644 tools/proto/BUILD.tools create mode 100644 tools/proto/toolchain.bzl diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java index e3cd129d0884e8..5756f709cda6e3 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java @@ -353,7 +353,7 @@ public static class GraveyardOptions extends OptionsBase { metadataTags = {OptionMetadataTag.DEPRECATED}, help = "Deprecated no-op.") public String glibc; - + @Deprecated @Option( name = "experimental_shortened_obj_file_path", @@ -416,6 +416,8 @@ public void initializeRuleClasses(ConfiguredRuleClassProvider.Builder builder) { try { // Load auto-configuration files, it is made outside of the rule class provider so that it // will not be loaded for our Java tests. + builder.addWorkspaceFileSuffix( + ResourceFileLoader.loadResource(BazelRulesModule.class, "proto.WORKSPACE")); builder.addWorkspaceFileSuffix( ResourceFileLoader.loadResource(BazelRulesModule.class, "xcode_configure.WORKSPACE")); builder.addWorkspaceFileSuffix( diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/proto.WORKSPACE b/src/main/java/com/google/devtools/build/lib/bazel/rules/proto.WORKSPACE new file mode 100644 index 00000000000000..f0ae8f86a08196 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/proto.WORKSPACE @@ -0,0 +1 @@ +register_toolchains("@bazel_tools//tools/proto:toolchain") diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java index f36609570655a9..f70d93d8c705e0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java @@ -91,11 +91,13 @@ public abstract class CcProtoAspect extends NativeAspectClass implements Configu private final CppSemantics cppSemantics; private final LabelLateBoundDefault ccToolchainAttrValue; private final Label ccToolchainType; + private final Label protoToolchainType; protected CcProtoAspect(AspectLegalCppSemantics cppSemantics, RuleDefinitionEnvironment env) { this.cppSemantics = cppSemantics; this.ccToolchainAttrValue = CppRuleClasses.ccToolchainAttribute(env); this.ccToolchainType = CppRuleClasses.ccToolchainTypeAttribute(env); + this.protoToolchainType = ProtoCommon.protoToolchainTypeAttribute(env); } @Override @@ -124,7 +126,7 @@ public AspectDefinition getDefinition(AspectParameters aspectParameters) { .propagateAlongAttribute("deps") .requiresConfigurationFragments(CppConfiguration.class, ProtoConfiguration.class) .requireSkylarkProviders(ProtoInfo.PROVIDER.id()) - .addRequiredToolchains(ccToolchainType) + .addRequiredToolchains(ccToolchainType, protoToolchainType) .add( attr(PROTO_TOOLCHAIN_ATTR, LABEL) .mandatoryNativeProviders(ImmutableList.of(ProtoLangToolchainProvider.class)) diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java index 8f917c3fe72691..999b1beb7461f7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java @@ -48,6 +48,7 @@ import com.google.devtools.build.lib.rules.java.JavaSemantics; import com.google.devtools.build.lib.rules.java.JavaSkylarkApiProvider; import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider; +import com.google.devtools.build.lib.rules.proto.ProtoCommon; import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder; import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.Exports; import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.Services; @@ -71,6 +72,7 @@ public static LabelLateBoundDefault getProtoToolchainLabel(String defaultValu private final String defaultProtoToolchainLabel; private final Label hostJdkAttribute; private final Label javaToolchainAttribute; + private final Label protoToolchainType; public JavaLiteProtoAspect( JavaSemantics javaSemantics, @@ -80,6 +82,7 @@ public JavaLiteProtoAspect( this.defaultProtoToolchainLabel = defaultProtoToolchainLabel; this.hostJdkAttribute = JavaSemantics.hostJdkAttribute(env); this.javaToolchainAttribute = JavaSemantics.javaToolchainAttribute(env); + this.protoToolchainType = ProtoCommon.protoToolchainTypeAttribute(env); } @Override @@ -110,6 +113,7 @@ public AspectDefinition getDefinition(AspectParameters aspectParameters) { .requiresConfigurationFragments( JavaConfiguration.class, ProtoConfiguration.class, PlatformConfiguration.class) .requireSkylarkProviders(ProtoInfo.PROVIDER.id()) + .addRequiredToolchains(protoToolchainType) .advertiseProvider(JavaProtoLibraryAspectProvider.class) .advertiseProvider( ImmutableList.of(SkylarkProviderIdentifier.forKey(JavaInfo.PROVIDER.getKey()))) diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java index 2077149c56ad95..a2ebafc9c0495a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java @@ -48,6 +48,7 @@ import com.google.devtools.build.lib.rules.java.JavaSemantics; import com.google.devtools.build.lib.rules.java.JavaSkylarkApiProvider; import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider; +import com.google.devtools.build.lib.rules.proto.ProtoCommon; import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder; import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.Exports; import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.Services; @@ -62,6 +63,7 @@ public class JavaProtoAspect extends NativeAspectClass implements ConfiguredAspe private final Label hostJdkAttribute; private final Label javaToolchainAttribute; + private final Label protoToolchainType; private static LabelLateBoundDefault getSpeedProtoToolchainLabel(String defaultValue) { return LabelLateBoundDefault.fromTargetConfiguration( @@ -86,6 +88,7 @@ protected JavaProtoAspect( Preconditions.checkNotNull(defaultSpeedProtoToolchainLabel); this.hostJdkAttribute = JavaSemantics.hostJdkAttribute(env); this.javaToolchainAttribute = JavaSemantics.javaToolchainAttribute(env); + this.protoToolchainType = ProtoCommon.protoToolchainTypeAttribute(env); } @Override @@ -119,6 +122,7 @@ public AspectDefinition getDefinition(AspectParameters aspectParameters) { .requiresConfigurationFragments( JavaConfiguration.class, ProtoConfiguration.class, PlatformConfiguration.class) .requireSkylarkProviders(ProtoInfo.PROVIDER.id()) + .addRequiredToolchains(protoToolchainType) .advertiseProvider(JavaProtoLibraryAspectProvider.class) .advertiseProvider( ImmutableList.of(SkylarkProviderIdentifier.forKey(JavaInfo.PROVIDER.getKey()))) diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryRule.java b/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryRule.java index 3da4c6570c73df..a93abc54a3cf6e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryRule.java @@ -49,6 +49,7 @@ public RuleClass build(RuleClass.Builder builder, final RuleDefinitionEnvironmen return builder .requiresConfigurationFragments(ProtoConfiguration.class) + .addRequiredToolchains(ProtoCommon.protoToolchainTypeAttribute(env)) .setOutputToGenfiles() .add( attr(":proto_compiler", LABEL) diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java index 81415b18f7385e..5af29472ec8083 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java @@ -34,6 +34,7 @@ import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; +import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.packages.Type; import com.google.devtools.build.lib.syntax.StarlarkSemantics; import com.google.devtools.build.lib.util.Pair; @@ -55,6 +56,12 @@ private ProtoCommon() { public static final String PROTO_RULES_MIGRATION_LABEL = "__PROTO_RULES_MIGRATION_DO_NOT_USE_WILL_BREAK__"; + private static final String TOOLCHAIN_TYPE_LABEL = "//tools/proto:toolchain_type"; + + public static Label protoToolchainTypeAttribute(RuleDefinitionEnvironment env) { + return env.getToolsLabel(TOOLCHAIN_TYPE_LABEL); + } + /** * Gets the direct sources of a proto library. If protoSources is not empty, the value is just * protoSources. Otherwise, it's the combined sources of all direct dependencies of the given diff --git a/tools/BUILD b/tools/BUILD index 15c409cfabd764..534e7a965f3df5 100644 --- a/tools/BUILD +++ b/tools/BUILD @@ -29,6 +29,7 @@ filegroup( "//tools/test:srcs", "//tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:srcs", "//tools/test/CoverageOutputGenerator/javatests/com/google/devtools/coverageoutputgenerator:srcs", + "//tools/proto:srcs", "//tools/python:srcs", "//tools/runfiles:srcs", "//tools/sh:srcs", @@ -58,6 +59,7 @@ filegroup( "//tools/def_parser:srcs", "//tools/platforms:srcs", "//tools/objc:srcs", + "//tools/proto:embedded_tools", "//tools/python:embedded_tools", "//tools/runfiles:embedded_tools", "//tools/test:embedded_tools", @@ -81,6 +83,7 @@ filegroup( "//tools/cpp:bzl_srcs", "//tools/jdk:bzl_srcs", "//tools/osx:bzl_srcs", + "//tools/proto:bzl_srcs", "//tools/python:bzl_srcs", "//tools/sh:bzl_srcs", ], diff --git a/tools/proto/BUILD b/tools/proto/BUILD new file mode 100644 index 00000000000000..c282792d841898 --- /dev/null +++ b/tools/proto/BUILD @@ -0,0 +1,33 @@ +# Copyright 2020 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +package(default_visibility = ["//visibility:public"]) + +licenses(["notice"]) # Apache 2.0 + +filegroup( + name = "srcs", + srcs = glob(["**"]), +) + +filegroup( + name = "embedded_tools", + srcs = glob(["**"]), +) + +filegroup( + name = "bzl_srcs", + srcs = glob(["*.bzl"]), + visibility = ["//tools:__pkg__"], +) diff --git a/tools/proto/BUILD.tools b/tools/proto/BUILD.tools new file mode 100644 index 00000000000000..090846c270f66d --- /dev/null +++ b/tools/proto/BUILD.tools @@ -0,0 +1,25 @@ +# Copyright 2020 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +load(":toolchain.bzl", "maybe_register_proto_toolchain") + +licenses(["notice"]) # Apache 2.0 + +# The toolchain type used to distinguish proto toolchains. +toolchain_type( + name = "toolchain_type", + visibility = ["//visibility:public"], +) + +maybe_register_proto_toolchain() diff --git a/tools/proto/toolchain.bzl b/tools/proto/toolchain.bzl new file mode 100644 index 00000000000000..53029bd36f7bbd --- /dev/null +++ b/tools/proto/toolchain.bzl @@ -0,0 +1,41 @@ +# Copyright 2020 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# buildifier: disable=native-proto +_proto_common = proto_common + +_use_proto_toolchain_from_rules_proto = "use_proto_toolchain_from_rules_proto_do_not_use_or_we_will_break_you_without_mercy" + +def maybe_register_proto_toolchain(): + if getattr(_proto_common, _use_proto_toolchain_from_rules_proto, False): + # --incompatible_use_proto_toolchain_from_rules_proto has been flipped, + # nothing to do. + return + + # buildifier: disable=native-proto + native.proto_toolchain( + name = "default_toolchain", + # Must be earlier than the first version of rules_proto that defined + # a proto toolchain (0.1.0). + rules_proto_version = "0.0.1", + tags = [ + "__PROTO_RULES_MIGRATION_DO_NOT_USE_WILL_BREAK__", + ], + ) + + native.toolchain( + name = "toolchain", + toolchain = ":default_toolchain", + toolchain_type = "@bazel_tools//tools/proto:toolchain_type", + ) From 4af486393c797650faff412a55e535b9ceb9bf86 Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Sun, 29 Mar 2020 01:02:06 +0100 Subject: [PATCH 04/14] Remove unused attr --- tools/proto/toolchain.bzl | 3 --- 1 file changed, 3 deletions(-) diff --git a/tools/proto/toolchain.bzl b/tools/proto/toolchain.bzl index 53029bd36f7bbd..6e71cd1f280ec5 100644 --- a/tools/proto/toolchain.bzl +++ b/tools/proto/toolchain.bzl @@ -26,9 +26,6 @@ def maybe_register_proto_toolchain(): # buildifier: disable=native-proto native.proto_toolchain( name = "default_toolchain", - # Must be earlier than the first version of rules_proto that defined - # a proto toolchain (0.1.0). - rules_proto_version = "0.0.1", tags = [ "__PROTO_RULES_MIGRATION_DO_NOT_USE_WILL_BREAK__", ], From c663062f477bca7167d23eb8cd850be8395f9563 Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Sun, 29 Mar 2020 01:48:14 +0100 Subject: [PATCH 05/14] Add flag --- .../build/lib/bazel/rules/proto.WORKSPACE | 4 +++- .../lib/packages/StarlarkSemanticsOptions.java | 16 ++++++++++++++++ .../skylarkbuildapi/proto/ProtoModuleApi.java | 15 ++++++++++++++- .../build/lib/syntax/StarlarkSemantics.java | 9 +++++++++ tools/proto/BUILD.tools | 10 ++++++++-- tools/proto/toolchain.bzl | 16 ++++++++++++---- 6 files changed, 62 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/proto.WORKSPACE b/src/main/java/com/google/devtools/build/lib/bazel/rules/proto.WORKSPACE index f0ae8f86a08196..f80e17f2b2c4ed 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/proto.WORKSPACE +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/proto.WORKSPACE @@ -1 +1,3 @@ -register_toolchains("@bazel_tools//tools/proto:toolchain") +load("@bazel_tools//tools/proto:toolchain.bzl", "maybe_register_proto_toolchain") + +maybe_register_proto_toolchain() diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java index 1c7b062ea1cc1a..49126813c4460f 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java @@ -629,6 +629,20 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl + " modest cost in memory. The stack is visible in some forms of query output.") public boolean recordRuleInstantiationCallstack; + @Option( + name = "incompatible_provide_proto_toolchain_in_tools_workspace", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, + effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, + metadataTags = { + OptionMetadataTag.INCOMPATIBLE_CHANGE, + OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES + }, + help = + "If set to true, the default proto_toolchain is no longer provided by Bazel and should " + + "be loaded from https://github.com/bazelbuild/rules_proto instead.") + public boolean incompatibleProvideProtoToolchainInToolsWorkspace; + /** * An interner to reduce the number of StarlarkSemantics instances. A single Blaze instance should * never accumulate a large number of these and being able to shortcut on object identity makes a @@ -688,6 +702,8 @@ public StarlarkSemantics toSkylarkSemantics() { .incompatibleLinkoptsToLinkLibs(incompatibleLinkoptsToLinkLibs) .maxComputationSteps(maxComputationSteps) .recordRuleInstantiationCallstack(recordRuleInstantiationCallstack) + .incompatibleProvideProtoToolchainInToolsWorkspace( + incompatibleProvideProtoToolchainInToolsWorkspace) .build(); return INTERNER.intern(semantics); } diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoModuleApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoModuleApi.java index b608994d91d6e2..192e60c1811ddf 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoModuleApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoModuleApi.java @@ -14,7 +14,9 @@ package com.google.devtools.build.lib.skylarkbuildapi.proto; +import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; +import com.google.devtools.build.lib.syntax.StarlarkSemantics.FlagIdentifier; import com.google.devtools.build.lib.syntax.StarlarkValue; /** @@ -31,4 +33,15 @@ + "to load this symbol from " + "rules_proto" + "

") -public interface ProtoModuleApi extends StarlarkValue {} +public interface ProtoModuleApi extends StarlarkValue { + @Deprecated + @SkylarkCallable( + name = + "provide_proto_toolchain_in_tools_workspace" + + "_do_not_use_or_we_will_break_you_without_mercy", + doc = "Indicates whether `@bazel_tools//tools/proto:toolchain` exists.", + documented = false, + structField = true, + disableWithFlag = FlagIdentifier.INCOMPATIBLE_PROVIDE_PROTO_TOOLCHAIN_IN_TOOLS_WORKSPACE) + default void provideProtoToolchainInToolsWorkspace() {}; +} diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java index 60ec41a2e6ae7f..c368ac125a3c0b 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java @@ -110,6 +110,8 @@ private FlagIdentifier() {} // uninstantiable "incompatible_linkopts_to_linklibs"; public static final String RECORD_RULE_INSTANTIATION_CALLSTACK = "record_rule_instantiation_callstack"; + public static final String INCOMPATIBLE_PROVIDE_PROTO_TOOLCHAIN_IN_TOOLS_WORKSPACE = + "incompatible_provide_proto_toolchain_in_tools_workspace"; } // TODO(adonovan): replace the fields of StarlarkSemantics @@ -164,6 +166,8 @@ public boolean flagValue(String flag) { return incompatibleLinkoptsToLinkLibs(); case FlagIdentifier.RECORD_RULE_INSTANTIATION_CALLSTACK: return recordRuleInstantiationCallstack(); + case FlagIdentifier.INCOMPATIBLE_PROVIDE_PROTO_TOOLCHAIN_IN_TOOLS_WORKSPACE: + return incompatibleProvideProtoToolchainInToolsWorkspace(); default: throw new IllegalArgumentException(flag); } @@ -291,6 +295,8 @@ boolean isFeatureEnabledBasedOnTogglingFlags(String enablingFlag, String disabli public abstract boolean recordRuleInstantiationCallstack(); + public abstract boolean incompatibleProvideProtoToolchainInToolsWorkspace(); + @Memoized @Override public abstract int hashCode(); @@ -367,6 +373,7 @@ public static Builder builderWithDefaults() { .incompatibleLinkoptsToLinkLibs(false) .maxComputationSteps(0) .recordRuleInstantiationCallstack(false) + .incompatibleProvideProtoToolchainInToolsWorkspace(false) .build(); /** Builder for {@link StarlarkSemantics}. All fields are mandatory. */ @@ -460,6 +467,8 @@ public abstract static class Builder { public abstract Builder recordRuleInstantiationCallstack(boolean value); + public abstract Builder incompatibleProvideProtoToolchainInToolsWorkspace(boolean value); + public abstract StarlarkSemantics build(); } } diff --git a/tools/proto/BUILD.tools b/tools/proto/BUILD.tools index 090846c270f66d..a26f891e87d4d4 100644 --- a/tools/proto/BUILD.tools +++ b/tools/proto/BUILD.tools @@ -12,14 +12,20 @@ # See the License for the specific language governing permissions and # limitations under the License. -load(":toolchain.bzl", "maybe_register_proto_toolchain") +load(":toolchain.bzl", "maybe_create_proto_toolchain_targets") licenses(["notice"]) # Apache 2.0 +filegroup( + name = "bzl_srcs", + srcs = glob(["*.bzl"]), + visibility = ["//tools:__pkg__"], +) + # The toolchain type used to distinguish proto toolchains. toolchain_type( name = "toolchain_type", visibility = ["//visibility:public"], ) -maybe_register_proto_toolchain() +maybe_create_proto_toolchain_targets() diff --git a/tools/proto/toolchain.bzl b/tools/proto/toolchain.bzl index 6e71cd1f280ec5..ff48ce02c6d5b0 100644 --- a/tools/proto/toolchain.bzl +++ b/tools/proto/toolchain.bzl @@ -15,12 +15,20 @@ # buildifier: disable=native-proto _proto_common = proto_common -_use_proto_toolchain_from_rules_proto = "use_proto_toolchain_from_rules_proto_do_not_use_or_we_will_break_you_without_mercy" +_provide_proto_toolchain_in_tools_workspace = "provide_proto_toolchain_in_tools_workspace_do_not_use_or_we_will_break_you_without_mercy" def maybe_register_proto_toolchain(): - if getattr(_proto_common, _use_proto_toolchain_from_rules_proto, False): - # --incompatible_use_proto_toolchain_from_rules_proto has been flipped, - # nothing to do. + if not hasattr(_proto_common, _provide_proto_toolchain_in_tools_workspace): + # --incompatible_provide_proto_toolchain_in_tools_workspace has been + # flipped, nothing to do. + return + + native.register_toolchains("@bazel_tools//tools/proto:toolchain") + +def maybe_create_proto_toolchain_targets(): + if not hasattr(_proto_common, _provide_proto_toolchain_in_tools_workspace): + # --incompatible_provide_proto_toolchain_in_tools_workspace has been + # flipped, nothing to do. return # buildifier: disable=native-proto From 699afaa7a74b036c3551ff93d8804af163138941 Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Sun, 29 Mar 2020 14:43:01 +0200 Subject: [PATCH 06/14] Increase embedded_tools_size_test --- src/BUILD | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/BUILD b/src/BUILD index 1873fd184c5162..9f93bb4159b3e4 100644 --- a/src/BUILD +++ b/src/BUILD @@ -161,8 +161,8 @@ rule_size_test( # WARNING: Only adjust the number in `expect` if you are intentionally # adding or removing embedded tools. Know that the more embedded tools there # are in Bazel, the bigger the binary becomes and the slower Bazel starts. - expect = 470, - margin = 5, # percentage + expect = 500, + margin = 2, # percentage ) filegroup( From 74e41523f5ffa7867735d055560678b0624729e0 Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Sun, 29 Mar 2020 15:25:38 +0200 Subject: [PATCH 07/14] Fix some tests --- .../lib/analysis/mock/BazelAnalysisMock.java | 7 ++++++ .../lib/packages/util/MockObjcSupport.java | 2 ++ .../lib/packages/util/MockProtoSupport.java | 23 +++++++++++++++++-- .../shell/bazel/bazel_proto_library_test.sh | 21 +++++++++++++++++ 4 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java index 1f3b22521fa177..c481fa3763359a 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java @@ -282,6 +282,13 @@ public void setupMockClient(MockToolsConfig config, List workspaceConten config.create("bazel_tools_workspace/objcproto/empty.cc"); config.create("bazel_tools_workspace/objcproto/well_known_type.proto"); + config.create( + "bazel_tools_workspace/tools/proto/BUILD", + "toolchain_type(", + " name = 'toolchain_type',", + " visibility = ['//visibility:public'],", + ")"); + config.create("rules_java_workspace/WORKSPACE", "workspace(name = 'rules_java')"); config.create("rules_java_workspace/java/BUILD"); config.create( diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockObjcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockObjcSupport.java index d450d33bc5d728..8e60ce0a9115c6 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/MockObjcSupport.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockObjcSupport.java @@ -17,6 +17,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.packages.util.Crosstool.CcToolchainConfig; +import com.google.devtools.build.lib.packages.util.MockProtoSupport; import com.google.devtools.build.lib.testutil.Scratch; import com.google.devtools.build.lib.testutil.TestConstants; import java.io.IOException; @@ -566,6 +567,7 @@ public static String readCcToolchainConfigFile() throws IOException { /** Creates a mock objc_proto_library rule in the current main workspace. */ public static void setupObjcProtoLibrary(Scratch scratch) throws Exception { + MockProtoSupport.setupWorkspace(scratch); // Append file instead of creating one in case it already exists. String toolsRepo = TestConstants.TOOLS_REPOSITORY; scratch.file("objc_proto_library/BUILD", ""); diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockProtoSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockProtoSupport.java index c4daec89bde857..2914f6eb8e26fa 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/MockProtoSupport.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockProtoSupport.java @@ -244,9 +244,28 @@ public static void setupWorkspace(Scratch scratch) throws Exception { "local_repository(", " name = 'rules_proto',", " path = 'third_party/rules_proto',", - ")"); + ")", + "register_toolchains('@rules_proto//proto:toolchain')"); scratch.file("third_party/rules_proto/WORKSPACE"); - scratch.file("third_party/rules_proto/proto/BUILD", "licenses(['notice'])"); + scratch.file( + "third_party/rules_proto/proto/BUILD", + "licenses(['notice'])", + "alias(", + " name = 'toolchain_type',", + " actual = '@bazel_tools//tools/proto:toolchain_type',", + " visibility = ['//visibility:public'],", + ")", + "proto_toolchain(", + " name = 'default_toolchain',", + " tags = [", + " '__PROTO_RULES_MIGRATION_DO_NOT_USE_WILL_BREAK__',", + " ],", + ")", + "toolchain(", + " name = 'toolchain',", + " toolchain = ':default_toolchain',", + " toolchain_type = ':toolchain_type',", + ")"); scratch.file( "third_party/rules_proto/proto/defs.bzl", "def _add_tags(kargs):", diff --git a/src/test/shell/bazel/bazel_proto_library_test.sh b/src/test/shell/bazel/bazel_proto_library_test.sh index 6ff835ba84f5da..aeda221d966062 100755 --- a/src/test/shell/bazel/bazel_proto_library_test.sh +++ b/src/test/shell/bazel/bazel_proto_library_test.sh @@ -681,4 +681,25 @@ EOF bazel build //h || fail "build failed" } +function test_disable_proto_toolchain_in_tools_workspace() { + write_workspace "" + + cat > foo/BUILD < foo/foo.proto < Date: Sun, 29 Mar 2020 15:59:58 +0200 Subject: [PATCH 08/14] Fix more tests --- .../packages/SkylarkSemanticsConsistencyTest.java | 4 +++- .../build/lib/packages/util/MockObjcSupport.java | 2 -- .../ActionGraphProtoOutputFormatterCallbackTest.java | 12 +++++++----- .../google/devtools/build/lib/query2/aquery/BUILD | 1 + 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java index 84266d20fdacff..82ebb1dde1a473 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java @@ -164,7 +164,8 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E "--incompatible_use_cc_configure_from_rules_cc=" + rand.nextBoolean(), "--internal_skylark_flag_test_canary=" + rand.nextBoolean(), "--max_computation_steps=" + rand.nextLong(), - "--record_rule_instantiation_callstack=" + rand.nextBoolean()); + "--record_rule_instantiation_callstack=" + rand.nextBoolean(), + "--incompatible_provide_proto_toolchain_in_tools_workspace=" + rand.nextBoolean()); } /** @@ -218,6 +219,7 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) { .internalSkylarkFlagTestCanary(rand.nextBoolean()) .maxComputationSteps(rand.nextLong()) .recordRuleInstantiationCallstack(rand.nextBoolean()) + .incompatibleProvideProtoToolchainInToolsWorkspace(rand.nextBoolean()) .build(); } diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockObjcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockObjcSupport.java index 8e60ce0a9115c6..d450d33bc5d728 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/MockObjcSupport.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockObjcSupport.java @@ -17,7 +17,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.packages.util.Crosstool.CcToolchainConfig; -import com.google.devtools.build.lib.packages.util.MockProtoSupport; import com.google.devtools.build.lib.testutil.Scratch; import com.google.devtools.build.lib.testutil.TestConstants; import java.io.IOException; @@ -567,7 +566,6 @@ public static String readCcToolchainConfigFile() throws IOException { /** Creates a mock objc_proto_library rule in the current main workspace. */ public static void setupObjcProtoLibrary(Scratch scratch) throws Exception { - MockProtoSupport.setupWorkspace(scratch); // Append file instead of creating one in case it already exists. String toolsRepo = TestConstants.TOOLS_REPOSITORY; scratch.file("objc_proto_library/BUILD", ""); diff --git a/src/test/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoOutputFormatterCallbackTest.java b/src/test/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoOutputFormatterCallbackTest.java index 42cda4f86c04b0..e3a845f99fc107 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoOutputFormatterCallbackTest.java +++ b/src/test/java/com/google/devtools/build/lib/query2/aquery/ActionGraphProtoOutputFormatterCallbackTest.java @@ -39,6 +39,7 @@ import com.google.devtools.build.lib.query2.engine.QueryParser; import com.google.devtools.build.lib.query2.query.aspectresolvers.AspectResolver.Mode; import com.google.devtools.build.lib.skyframe.ConfiguredTargetValue; +import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.util.OS; import java.util.ArrayList; import java.util.HashSet; @@ -58,11 +59,12 @@ public class ActionGraphProtoOutputFormatterCallbackTest extends ActionGraphQuer private final List events = new ArrayList<>(); @Before - public final void setUpAqueryOptions() { + public final void setUpAqueryOptions() throws Exception { this.options = new AqueryOptions(); options.aspectDeps = Mode.OFF; options.includeArtifacts = true; this.reporter = new Reporter(new EventBus(), events::add); + MockProtoSupport.setupWorkspace(getHelper().getScratch()); } @Test @@ -587,11 +589,11 @@ public void testIncludeAspects_AspectOnAspect() throws Exception { writeFile( "test/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, "load(':rule.bzl', 'my_rule', 'my_jpl_rule')", "proto_library(", " name = 'x',", " srcs = [':x.proto'],", - MockProtoSupport.MIGRATION_TAG, ")", "my_jpl_rule(", " name = 'my_java_proto',", @@ -678,11 +680,11 @@ public void testIncludeAspects_SingleAspect() throws Exception { writeFile( "test/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, "load(':rule.bzl', 'my_jpl_rule')", "proto_library(", " name = 'x',", " srcs = [':x.proto'],", - MockProtoSupport.MIGRATION_TAG, ")", "my_jpl_rule(", " name = 'my_java_proto',", @@ -778,11 +780,11 @@ public void testIncludeAspects_twoAspectsOneTarget_separateAspectDescriptors() t writeFile( "test/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, "load(':rule.bzl', 'my_jpl_rule', 'my_random_rule')", "proto_library(", " name = 'x',", " srcs = [':x.proto'],", - MockProtoSupport.MIGRATION_TAG, ")", "my_jpl_rule(", " name = 'target_1',", @@ -874,11 +876,11 @@ public void testIncludeAspects_flagDisabled_NoAspect() throws Exception { writeFile( "test/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, "load(':rule.bzl', 'my_jpl_rule')", "proto_library(", " name = 'x',", " srcs = [':x.proto'],", - MockProtoSupport.MIGRATION_TAG, ")", "my_jpl_rule(", " name = 'my_java_proto',", diff --git a/src/test/java/com/google/devtools/build/lib/query2/aquery/BUILD b/src/test/java/com/google/devtools/build/lib/query2/aquery/BUILD index be5178f41592ae..ddd825ebde6cf7 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/aquery/BUILD +++ b/src/test/java/com/google/devtools/build/lib/query2/aquery/BUILD @@ -25,6 +25,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/query2/query/aspectresolvers", "//src/main/protobuf:analysis_java_proto", "//src/test/java/com/google/devtools/build/lib/packages:testutil", + "//src/test/java/com/google/devtools/build/lib/testutil:TestConstants", "//third_party:guava", "//third_party:junit4", "//third_party:truth", From c9ec911e2a824182b19e594ebce96c7f77178423 Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Sun, 29 Mar 2020 16:14:50 +0200 Subject: [PATCH 09/14] Fix SkylarkTests and bazel_proto_library_test --- .../devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java | 1 + src/test/shell/bazel/bazel_proto_library_test.sh | 1 + 2 files changed, 2 insertions(+) diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java index 069659ec202798..1af5a4e2e8f66a 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java @@ -68,6 +68,7 @@ public final void initializeToolsConfigMock() throws Exception { MockObjcSupport.setup(mockToolsConfig); // Required for tests including the proto_library rule. MockProtoSupport.setup(mockToolsConfig); + MockProtoSupport.setupWorkspace(scratch); } @Test diff --git a/src/test/shell/bazel/bazel_proto_library_test.sh b/src/test/shell/bazel/bazel_proto_library_test.sh index aeda221d966062..2292f17a51e41e 100755 --- a/src/test/shell/bazel/bazel_proto_library_test.sh +++ b/src/test/shell/bazel/bazel_proto_library_test.sh @@ -684,6 +684,7 @@ EOF function test_disable_proto_toolchain_in_tools_workspace() { write_workspace "" + mkdir -p foo cat > foo/BUILD < Date: Sun, 29 Mar 2020 16:23:10 +0200 Subject: [PATCH 10/14] Fix scripts/bootstrap/compile.sh --- scripts/bootstrap/compile.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/bootstrap/compile.sh b/scripts/bootstrap/compile.sh index 035aa2ac6615bb..c12eddafe14393 100755 --- a/scripts/bootstrap/compile.sh +++ b/scripts/bootstrap/compile.sh @@ -285,6 +285,10 @@ EOF link_file "${PWD}/tools/java/runfiles/Util.java" "${BAZEL_TOOLS_REPO}/tools/java/runfiles/Util.java" link_file "${PWD}/tools/java/runfiles/BUILD.tools" "${BAZEL_TOOLS_REPO}/tools/java/runfiles/BUILD" + # Create @bazel_tools/tools/proto/BUILD + mkdir -p ${BAZEL_TOOLS_REPO}/tools/proto + link_file "${PWD}/tools/proto/BUILD.tools" "${BAZEL_TOOLS_REPO}/tools/proto/BUILD" + # Create @bazel_tools/tools/python/BUILD mkdir -p ${BAZEL_TOOLS_REPO}/tools/python link_file "${PWD}/tools/python/BUILD.tools" "${BAZEL_TOOLS_REPO}/tools/python/BUILD" @@ -292,7 +296,6 @@ EOF # Create the rest of @bazel_tools//tools/... link_children "${PWD}" tools/cpp "${BAZEL_TOOLS_REPO}" mv -f ${BAZEL_TOOLS_REPO}/tools/cpp/BUILD.tools ${BAZEL_TOOLS_REPO}/tools/cpp/BUILD - link_children "${PWD}" tools/python "${BAZEL_TOOLS_REPO}" link_children "${PWD}" tools "${BAZEL_TOOLS_REPO}" # Set up @bazel_tools//platforms properly From 2c1db065b224678a6850f5bf73b7f907ee5a8893 Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Sun, 29 Mar 2020 17:08:50 +0200 Subject: [PATCH 11/14] Increase packages for discard_graph_edges_test --- src/test/shell/integration/discard_graph_edges_test.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/shell/integration/discard_graph_edges_test.sh b/src/test/shell/integration/discard_graph_edges_test.sh index 35538bb1161340..05d41a146bb77c 100755 --- a/src/test/shell/integration/discard_graph_edges_test.sh +++ b/src/test/shell/integration/discard_graph_edges_test.sh @@ -271,8 +271,8 @@ function test_packages_cleared() { package_count="$(extract_histogram_count "$histo_file" \ 'devtools\.build\.lib\..*\.Package$')" # A few packages aren't cleared. - [[ "$package_count" -le 25 ]] \ - || fail "package count $package_count too high. Expected <= 25" + [[ "$package_count" -le 26 ]] \ + || fail "package count $package_count too high. Expected <= 26" glob_count="$(extract_histogram_count "$histo_file" "GlobValue$")" [[ "$glob_count" -le 1 ]] \ || fail "glob count $glob_count too high" From fbaf337aedeaf94fdf2f142602f6c725668998be Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Sun, 29 Mar 2020 17:24:32 +0200 Subject: [PATCH 12/14] Fix BazelPackageLoaderTest --- .../build/lib/skyframe/packages/BazelPackageLoaderTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoaderTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoaderTest.java index 6bf36812faddeb..5fa211e4b39859 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoaderTest.java @@ -62,6 +62,7 @@ private void mockEmbeddedTools(Path embeddedBinaries) throws IOException { Path tools = embeddedBinaries.getRelative("embedded_tools"); tools.getRelative("tools/cpp").createDirectoryAndParents(); tools.getRelative("tools/osx").createDirectoryAndParents(); + tools.getRelative("tools/proto").createDirectoryAndParents(); FileSystemUtils.writeIsoLatin1(tools.getRelative("WORKSPACE"), ""); FileSystemUtils.writeIsoLatin1(tools.getRelative("tools/cpp/BUILD"), ""); FileSystemUtils.writeIsoLatin1( @@ -91,6 +92,11 @@ private void mockEmbeddedTools(Path embeddedBinaries) throws IOException { "def maybe(repo_rule, name, **kwargs):", " if name not in native.existing_rules():", " repo_rule(name = name, **kwargs)"); + FileSystemUtils.writeIsoLatin1(tools.getRelative("tools/proto/BUILD")); + FileSystemUtils.writeIsoLatin1( + tools.getRelative("tools/proto/toolchain.bzl"), + "def maybe_register_proto_toolchain():", + " pass"); } private void fetchExternalRepo(RepositoryName externalRepo) { From d511051abb89fc5046f5d80834e75885f2ee2ec4 Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Mon, 30 Mar 2020 16:52:22 +0200 Subject: [PATCH 13/14] Fix bootstrap test --- scripts/bootstrap/compile.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/bootstrap/compile.sh b/scripts/bootstrap/compile.sh index c12eddafe14393..e36dadc9492cba 100755 --- a/scripts/bootstrap/compile.sh +++ b/scripts/bootstrap/compile.sh @@ -296,6 +296,8 @@ EOF # Create the rest of @bazel_tools//tools/... link_children "${PWD}" tools/cpp "${BAZEL_TOOLS_REPO}" mv -f ${BAZEL_TOOLS_REPO}/tools/cpp/BUILD.tools ${BAZEL_TOOLS_REPO}/tools/cpp/BUILD + link_children "${PWD}" tools/proto "${BAZEL_TOOLS_REPO}" + link_children "${PWD}" tools/python "${BAZEL_TOOLS_REPO}" link_children "${PWD}" tools "${BAZEL_TOOLS_REPO}" # Set up @bazel_tools//platforms properly From 9e6a34f606f64134eaddcdcffafbcd97a9841f74 Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Sun, 12 Apr 2020 17:40:57 +0200 Subject: [PATCH 14/14] Rebase --- src/main/java/com/google/devtools/build/lib/rules/proto/BUILD | 1 + .../com/google/devtools/build/lib/rules/proto/ProtoCommon.java | 2 +- .../google/devtools/build/lib/rules/proto/ProtoToolchain.java | 3 ++- .../devtools/build/lib/rules/proto/ProtoToolchainInfo.java | 3 ++- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/BUILD b/src/main/java/com/google/devtools/build/lib/rules/proto/BUILD index f2046f2426a255..600c1195012a3b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/BUILD @@ -25,6 +25,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib:build-base", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:commandline_item", + "//src/main/java/com/google/devtools/build/lib/analysis/platform", "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/concurrent", diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java index 2aeedab33b46fd..eb4a333fdd2a57 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java @@ -22,6 +22,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.analysis.Runfiles; import com.google.devtools.build.lib.analysis.actions.SymlinkAction; import com.google.devtools.build.lib.analysis.config.CoreOptionConverters.StrictDepsMode; @@ -31,7 +32,6 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; -import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.packages.Type; import com.google.devtools.build.lib.syntax.Location; import com.google.devtools.build.lib.syntax.StarlarkSemantics; diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchain.java index 1d7f4d0340284d..759f65b814cfed 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchain.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchain.java @@ -29,9 +29,10 @@ public ConfiguredTarget create(RuleContext ruleContext) throws ActionConflictException, RuleErrorException, InterruptedException { ProtoCommon.checkRuleHasValidMigrationTag(ruleContext); + ProtoToolchainInfo toolchain = new ProtoToolchainInfo(); RuleConfiguredTargetBuilder builder = new RuleConfiguredTargetBuilder(ruleContext) - .addNativeDeclaredProvider(new ProtoToolchainInfo()) + .addNativeDeclaredProvider(toolchain) .addProvider(RunfilesProvider.class, RunfilesProvider.simple(Runfiles.EMPTY)); return builder.build(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchainInfo.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchainInfo.java index 77a5b10db9e0e9..63bafab7383080 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchainInfo.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoToolchainInfo.java @@ -17,9 +17,9 @@ import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.platform.ToolchainInfo; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; -import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.skylarkbuildapi.proto.ProtoToolchainInfoApi; +import com.google.devtools.build.lib.syntax.Location; /** * Information about the tools used by the proto_* and LANG_proto_* rules. @@ -27,6 +27,7 @@ @Immutable @AutoCodec public class ProtoToolchainInfo extends ToolchainInfo implements ProtoToolchainInfoApi { + @AutoCodec.Instantiator public ProtoToolchainInfo() { super(ImmutableMap.of(), Location.BUILTIN); }