From 3ff19d6904e35da3d3a27579291239a7ace52724 Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 18 Jul 2022 02:14:54 -0700 Subject: [PATCH] Fix deserialization/wrapping of ProtoLangToolchainProvider I used the ProtoLangToolchainProvider.wrapStarlarkProviderWithNativeProvider for Bazel 5.3 to support non-starlarkified rules. There were problems with some code paths. Those code paths are not used in Bazel@HEAD (because we already removed native rules). PiperOrigin-RevId: 461564393 Change-Id: I6f6d5ef5bfdfd7535f940d7f98ec09923e7677c5 --- .../proto/ProtoLangToolchainProvider.java | 31 ++++------- .../devtools/build/lib/rules/proto/BUILD | 3 ++ .../rules/proto/ProtoLangToolchainTest.java | 53 +++++++++++++++++++ 3 files changed, 66 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainProvider.java index d34e2a6242725c..df343477339b05 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainProvider.java @@ -15,14 +15,12 @@ package com.google.devtools.build.lib.rules.proto; import com.google.auto.value.AutoValue; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.collect.nestedset.Depset; -import com.google.devtools.build.lib.collect.nestedset.Depset.ElementType; -import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.packages.StarlarkInfo; import com.google.devtools.build.lib.packages.StarlarkProvider; import com.google.devtools.build.lib.packages.StarlarkProviderIdentifier; @@ -98,31 +96,20 @@ public static StarlarkInfo create( ImmutableList protocOpts, String progressMessage, String mnemonic) { - - NestedSetBuilder providedProtoSourcesSet = NestedSetBuilder.stableOrder(); - providedProtoSources.forEach(providedProtoSourcesSet::add); - NestedSetBuilder protocOptsSet = NestedSetBuilder.stableOrder(); - protocOpts.forEach(protocOptsSet::add); - Map m = new LinkedHashMap<>(); m.put("plugin", pluginExecutable == null ? Starlark.NONE : pluginExecutable); - m.put("plugin_format_flag", pluginFormatFlag); + m.put("plugin_format_flag", pluginFormatFlag == null ? Starlark.NONE : pluginFormatFlag); m.put("proto_compiler", protoc == null ? Starlark.NONE : protoc); - m.put( - "provided_proto_sources", - Depset.of(ElementType.of(ProtoSource.class), providedProtoSourcesSet.build())); - m.put("protoc_opts", Depset.of(ElementType.of(ProtoSource.class), protocOptsSet.build())); + m.put("provided_proto_sources", StarlarkList.immutableCopyOf(providedProtoSources)); + m.put("protoc_opts", StarlarkList.immutableCopyOf(protocOpts)); m.put("out_replacement_format_flag", outReplacementFormatFlag); m.put("progress_message", progressMessage); m.put("mnemonic", mnemonic); m.put("plugin", pluginExecutable == null ? Starlark.NONE : pluginExecutable); m.put("runtime", runtime == null ? Starlark.NONE : runtime); - StarlarkProvider.Builder builder = - StarlarkProvider.builder( - Location.fromFileLineColumn(protoc.getExecutable().getFilename(), 0, 0)); + StarlarkProvider.Builder builder = StarlarkProvider.builder(Location.BUILTIN); builder.setExported(starlarkProtoLangToolchainKey); - return StarlarkInfo.create(builder.build(), m, Location.BUILTIN); } @@ -165,13 +152,15 @@ public static StarlarkInfo getStarlarkProvider(TransitiveInfoCollection prerequi @Nullable @SuppressWarnings("unchecked") - private static ProtoLangToolchainProvider wrapStarlarkProviderWithNativeProvider( - StarlarkInfo provider) { + @VisibleForTesting + static ProtoLangToolchainProvider wrapStarlarkProviderWithNativeProvider(StarlarkInfo provider) { if (provider != null) { try { return new AutoValue_ProtoLangToolchainProvider( provider.getValue("out_replacement_format_flag", String.class), - provider.getValue("plugin_format_flag", String.class), + provider.getValue("plugin_format_flag") instanceof NoneType + ? null + : provider.getValue("plugin_format_flag", String.class), provider.getValue("plugin") instanceof NoneType ? null : provider.getValue("plugin", FilesToRunProvider.class), diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/BUILD b/src/test/java/com/google/devtools/build/lib/rules/proto/BUILD index c1c71abb5c6aec..9f3925c874deb9 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/proto/BUILD +++ b/src/test/java/com/google/devtools/build/lib/rules/proto/BUILD @@ -37,9 +37,12 @@ java_test( name = "ProtoLangToolchainTest", srcs = ["ProtoLangToolchainTest.java"], deps = [ + "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", "//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/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/test/java/com/google/devtools/build/lib/analysis/util", "//src/test/java/com/google/devtools/build/lib/packages:testutil", "//src/test/java/com/google/devtools/build/lib/testutil:TestConstants", diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainTest.java index 70af5b21b5938e..23daa19494d566 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainTest.java @@ -18,11 +18,14 @@ import com.google.common.collect.ImmutableList; import com.google.common.eventbus.EventBus; +import com.google.devtools.build.lib.analysis.FilesToRunProvider; 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.packages.StarlarkInfo; import com.google.devtools.build.lib.packages.util.MockProtoSupport; import com.google.devtools.build.lib.testutil.TestConstants; +import com.google.devtools.build.lib.vfs.PathFragment; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -210,4 +213,54 @@ public void optionalFieldsAreEmpty() throws Exception { assertThat(toolchain.runtime()).isNull(); assertThat(toolchain.mnemonic()).isEqualTo("GenProto"); } + + @Test + public void protoLangToolchainProvider_deserialization() throws Exception { + scratch.file( + "foo/BUILD", + "cc_binary(", + " name = 'plugin',", + " srcs = ['plugin.cc'],", + ")", + "cc_binary(", + " name = 'runtime',", + " srcs = ['runtime.cc'],", + ")"); + FilesToRunProvider plugin = + getConfiguredTarget("//foo:plugin").getProvider(FilesToRunProvider.class); + TransitiveInfoCollection runtime = getConfiguredTarget("//foo:runtime"); + FilesToRunProvider protoCompiler = + getConfiguredTarget("//net/proto2/compiler/public:protocol_compiler") + .getProvider(FilesToRunProvider.class); + ImmutableList providedProtoSources = + ImmutableList.of( + new ProtoSource( + getSourceArtifact("a.proto"), + getSourceArtifact("_virtual_imports/b/a.proto"), + PathFragment.create("b"))); + StarlarkInfo starlarkProvider = + ProtoLangToolchainProvider.create( + /* outReplacementFormatFlag= */ "outReplacementFormatFlag", + /* pluginFormatFlag= */ null, + /* pluginExecutable= */ plugin, + /* runtime= */ runtime, + /* providedProtoSources= */ providedProtoSources, + protoCompiler, + ImmutableList.of("--a", "--b"), + "Generating C++ proto_library %{label}", + "GenProto"); + + ProtoLangToolchainProvider nativeProvider = + ProtoLangToolchainProvider.wrapStarlarkProviderWithNativeProvider(starlarkProvider); + + assertThat(nativeProvider.outReplacementFormatFlag()).isEqualTo("outReplacementFormatFlag"); + assertThat(nativeProvider.pluginFormatFlag()).isNull(); + assertThat(nativeProvider.pluginExecutable()).isEqualTo(plugin); + assertThat(nativeProvider.runtime()).isEqualTo(runtime); + assertThat(nativeProvider.providedProtoSources()).isEqualTo(providedProtoSources); + assertThat(nativeProvider.protoc()).isEqualTo(protoCompiler); + assertThat(nativeProvider.protocOpts()).containsExactly("--a", "--b"); + assertThat(nativeProvider.progressMessage()).isEqualTo("Generating C++ proto_library %{label}"); + assertThat(nativeProvider.mnemonic()).isEqualTo("GenProto"); + } }