Move the descriptorset output to the ProtoSourcesProvider and expose it to skylark.
This does mean that the type name 'ProtoSourcesProvider' is a little inaccurate, since descriptorSet() is an output. Alternatively we could expose the DescriptorSetProvider to skylark.
RELNOTES: expose proto_library descriptor set to skylark via <dep>.proto.descriptor_set
--
PiperOrigin-RevId: 142680666
MOS_MIGRATED_REVID=142680666
diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java
index 3557c5f..c524829 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java
@@ -46,11 +46,6 @@
NestedSet<Artifact> transitiveImports =
ProtoCommon.collectTransitiveImports(ruleContext, protoSources);
- // TODO(bazel-team): this second constructor argument is superfluous and should be removed.
- ProtoSourcesProvider sourcesProvider =
- ProtoSourcesProvider.create(
- transitiveImports, transitiveImports, protoSources, checkDepsProtoSources);
-
NestedSet<Artifact> protosInDirectDeps =
areDepsStrict(ruleContext) ? ProtoCommon.computeProtosInDirectDeps(ruleContext) : null;
@@ -67,10 +62,11 @@
RuleConfiguredTargetBuilder result = new RuleConfiguredTargetBuilder(ruleContext);
+ Artifact descriptorSetOutput = null;
if (checkDepsProtoSources.isEmpty() || !outputDescriptorSetFlagEnabled(ruleContext)) {
result.setFilesToBuild(NestedSetBuilder.<Artifact>create(STABLE_ORDER));
} else {
- Artifact descriptorSetOutput =
+ descriptorSetOutput =
ruleContext.getGenfilesArtifact(
ruleContext.getLabel().getName() + "-descriptor-set.proto.bin");
ProtoCompileActionBuilder.writeDescriptorSet(
@@ -85,9 +81,17 @@
dataRunfiles.addArtifact(descriptorSetOutput);
result.setFilesToBuild(NestedSetBuilder.create(STABLE_ORDER, descriptorSetOutput));
- result.addProvider(DescriptorSetProvider.create(descriptorSetOutput));
}
+ // TODO(bazel-team): this second constructor argument is superfluous and should be removed.
+ ProtoSourcesProvider sourcesProvider =
+ ProtoSourcesProvider.create(
+ transitiveImports,
+ transitiveImports,
+ protoSources,
+ checkDepsProtoSources,
+ descriptorSetOutput);
+
return result
.addProvider(RunfilesProvider.withData(Runfiles.EMPTY, dataRunfiles.build()))
.addProvider(ProtoSourcesProvider.class, sourcesProvider)
diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/DescriptorSetProvider.java b/src/main/java/com/google/devtools/build/lib/rules/proto/DescriptorSetProvider.java
deleted file mode 100644
index c1d323b..0000000
--- a/src/main/java/com/google/devtools/build/lib/rules/proto/DescriptorSetProvider.java
+++ /dev/null
@@ -1,39 +0,0 @@
-// Copyright 2016 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.auto.value.AutoValue;
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
-
-/**
- * A provider that exposes a compiled descriptor set from a proto_library.
- *
- * <p>Produced by passing --descriptor_set_out to the proto-compiler. See
- * https://developers.google.com/protocol-buffers/docs/techniques#self-description
- */
-@AutoValue
-public abstract class DescriptorSetProvider implements TransitiveInfoProvider {
-
- /**
- * Be careful while using this artifact - it is the parsing of the transitive set of .proto files.
- * It's possible to cause a O(n^2) behavior, where n is the length of a proto chain-graph.
- */
- public abstract Artifact descriptorSet();
-
- public static DescriptorSetProvider create(Artifact descriptorSet) {
- return new AutoValue_DescriptorSetProvider(descriptorSet);
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSourcesProvider.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSourcesProvider.java
index d8624c4..5220e4c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSourcesProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSourcesProvider.java
@@ -22,7 +22,9 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
+import javax.annotation.Nullable;
+// TODO(carmi): Rename the class to ProtoInfoProvider.
/**
* Configured target classes that implement this class can contribute .proto files to the
* compilation of proto_library rules.
@@ -38,9 +40,14 @@
NestedSet<Artifact> transitiveImports,
NestedSet<Artifact> transitiveProtoSources,
ImmutableList<Artifact> protoSources,
- ImmutableList<Artifact> checkDepsProtoSources) {
+ ImmutableList<Artifact> checkDepsProtoSources,
+ @Nullable Artifact descriptorSet) {
return new AutoValue_ProtoSourcesProvider(
- transitiveImports, transitiveProtoSources, protoSources, checkDepsProtoSources);
+ transitiveImports,
+ transitiveProtoSources,
+ protoSources,
+ checkDepsProtoSources,
+ descriptorSet);
}
/**
@@ -95,5 +102,20 @@
)
public abstract ImmutableList<Artifact> getCheckDepsProtoSources();
+ /**
+ * Be careful while using this artifact - it is the parsing of the transitive set of .proto files.
+ * It's possible to cause a O(n^2) behavior, where n is the length of a proto chain-graph.
+ */
+ @SkylarkCallable(
+ name = "descriptor_set",
+ doc =
+ "The FileDescriptorSet of all transitive sources. Returns None if "
+ + "--output_descriptor_set isn't enabled or if there are no sources",
+ structField = true,
+ allowReturnNones = true
+ )
+ @Nullable
+ public abstract Artifact descriptorSet();
+
ProtoSourcesProvider() {}
}
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index 6761357..40c40b9 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -1220,6 +1220,7 @@
":actions_testutil",
":analysis_testutil",
"//src/main/java/com/google/devtools/build/lib:build-base",
+ "//src/main/java/com/google/devtools/build/lib:proto-rules",
"//src/main/java/com/google/devtools/build/lib/actions",
"//third_party:junit4",
"//third_party:truth",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java
index d0a1211..504e07c 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java
@@ -40,6 +40,7 @@
Artifact file =
ActionsTestUtil.getFirstArtifactEndingWith(getFilesToBuild(target), ".proto.bin");
assertThat(file.getRootRelativePathString()).isEqualTo("x/foo-descriptor-set.proto.bin");
+ assertThat(target.getProvider(ProtoSourcesProvider.class).descriptorSet()).isEqualTo(file);
assertThat(getGeneratingSpawnAction(file).getRemainingArguments())
.containsAllOf(
@@ -73,6 +74,38 @@
ConfiguredTarget target = scratchConfiguredTarget("x", "foo", "proto_library(name='foo')");
assertThat(ActionsTestUtil.getFirstArtifactEndingWith(getFilesToBuild(target), ".proto.bin"))
.isNull();
+
+ assertThat(target.getProvider(ProtoSourcesProvider.class).descriptorSet()).isNull();
+ }
+
+ @Test
+ public void testDescriptorSetOutput_noSrcs_transitive() throws Exception {
+ ConfiguredTarget target =
+ scratchConfiguredTarget(
+ "x",
+ "foo",
+ "proto_library(name='foo', deps = [':dep1'])",
+ "proto_library(name='dep1', deps = [':dep2'])",
+ "proto_library(name='dep2')");
+ assertThat(ActionsTestUtil.getFirstArtifactEndingWith(getFilesToBuild(target), ".proto.bin"))
+ .isNull();
+
+ assertThat(target.getProvider(ProtoSourcesProvider.class).descriptorSet()).isNull();
+ }
+
+ @Test
+ public void testDescriptorSetOutput_srcs_transitive() throws Exception {
+ ConfiguredTarget target =
+ scratchConfiguredTarget(
+ "x",
+ "foo",
+ "proto_library(name='foo', deps = [':dep1'])",
+ "proto_library(name='dep1', deps = [':dep2'])",
+ "proto_library(name='dep2', srcs=['foo.proto'])");
+ Artifact file =
+ ActionsTestUtil.getFirstArtifactEndingWith(getFilesToBuild(target), ".proto.bin");
+ assertThat(file.getRootRelativePathString()).isEqualTo("x/foo-descriptor-set.proto.bin");
+ assertThat(target.getProvider(ProtoSourcesProvider.class).descriptorSet()).isEqualTo(file);
}
@Test
