Strict deps for proto_library.
In other words, all imported protos must be declared as BUILD dependencies.
RELNOTES: proto_library supports strict proto deps.
--
MOS_MIGRATED_REVID=140078632
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 6b107fe..b81e6e0 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
@@ -14,13 +14,14 @@
package com.google.devtools.build.lib.rules.proto;
+import static com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode.TARGET;
import static com.google.devtools.build.lib.collect.nestedset.Order.STABLE_ORDER;
+import static com.google.devtools.build.lib.syntax.Type.BOOLEAN;
import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
-import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.Runfiles;
@@ -37,7 +38,7 @@
public ConfiguredTarget create(RuleContext ruleContext)
throws InterruptedException, RuleErrorException {
ImmutableList<Artifact> protoSources =
- ruleContext.getPrerequisiteArtifacts("srcs", Mode.TARGET).list();
+ ruleContext.getPrerequisiteArtifacts("srcs", TARGET).list();
ImmutableList<Artifact> checkDepsProtoSources =
ProtoCommon.getCheckDepsProtoSources(ruleContext, protoSources);
ProtoCommon.checkSourceFilesAreInSamePackage(ruleContext);
@@ -50,11 +51,16 @@
ProtoSourcesProvider.create(
transitiveImports, transitiveImports, protoSources, checkDepsProtoSources);
+ NestedSet<Artifact> protosInDirectDeps =
+ ruleContext.attributes().get("strict_proto_deps", BOOLEAN)
+ ? ProtoCommon.computeProtosInDirectDeps(ruleContext)
+ : null;
+
final SupportData supportData =
SupportData.create(
Predicates.<TransitiveInfoCollection>alwaysTrue() /* nonWeakDepsPredicate */,
protoSources,
- null /* protosInDirectDeps */,
+ protosInDirectDeps,
transitiveImports,
!protoSources.isEmpty());
@@ -74,7 +80,7 @@
descriptorSetOutput.getExecPathString(),
checkDepsProtoSources,
transitiveImports,
- null /* protosInDirectDeps */,
+ protosInDirectDeps,
ImmutableList.of(descriptorSetOutput),
true /* allowServices */);
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 c005efe..ee9a23d 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
@@ -18,6 +18,7 @@
import static com.google.devtools.build.lib.packages.Attribute.attr;
import static com.google.devtools.build.lib.packages.BuildType.LABEL;
import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
+import static com.google.devtools.build.lib.syntax.Type.BOOLEAN;
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
import com.google.devtools.build.lib.analysis.RuleDefinition;
@@ -56,11 +57,7 @@
// but these are still 'experimental' according to the documentation.
.setUndocumented()
.setOutputToGenfiles()
- .add(
- attr(":proto_compiler", LABEL)
- .cfg(HOST)
- .exec()
- .value(PROTO_COMPILER))
+ .add(attr(":proto_compiler", LABEL).cfg(HOST).exec().value(PROTO_COMPILER))
/* <!-- #BLAZE_RULE(proto_library).ATTRIBUTE(deps) -->
The list of other <code>proto_library</code> rules that the target depends upon.
A <code>proto_library</code> may only depend on other
@@ -79,6 +76,7 @@
attr("srcs", LABEL_LIST)
.direct_compile_time_input()
.allowedFileTypes(FileType.of(".proto"), FileType.of(".protodevel")))
+ .add(attr("strict_proto_deps", BOOLEAN).value(true).undocumented("for migration only"))
.advertiseProvider(ProtoSourcesProvider.class)
.build();
}
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 41778d8..626d713 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
@@ -14,6 +14,8 @@
package com.google.devtools.build.lib.rules.proto;
+import static com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode.TARGET;
+
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Root;
@@ -27,6 +29,7 @@
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.PathFragment;
+import javax.annotation.Nullable;
/**
* Utility functions for proto_library and proto aspect implementations.
@@ -155,4 +158,26 @@
ImmutableList<Artifact> protoSources, String extension) {
return getGeneratedOutputs(ruleContext, protoSources, extension, false);
}
+
+ /**
+ * Returns the .proto files that are the direct srcs of the direct-dependencies of this rule. If
+ * the current rule is an alias proto_library (=no srcs), we use the direct srcs of the
+ * direct-dependencies of our direct-dependencies.
+ */
+ @Nullable
+ public static NestedSet<Artifact> computeProtosInDirectDeps(RuleContext ruleContext) {
+ NestedSetBuilder<Artifact> result = NestedSetBuilder.stableOrder();
+ if (ruleContext.getPrerequisiteArtifacts("srcs", TARGET).list().isEmpty()) {
+ for (ProtoSupportDataProvider provider :
+ ruleContext.getPrerequisites("deps", TARGET, ProtoSupportDataProvider.class)) {
+ result.addTransitive(provider.getSupportData().getProtosInDirectDeps());
+ }
+ } else {
+ for (ProtoSourcesProvider provider :
+ ruleContext.getPrerequisites("deps", TARGET, ProtoSourcesProvider.class)) {
+ result.addAll(provider.getCheckDepsProtoSources());
+ }
+ }
+ return result.build();
+ }
}
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 f672649..9fb22b9 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
@@ -74,4 +74,61 @@
assertThat(ActionsTestUtil.getFirstArtifactEndingWith(getFilesToBuild(target), ".proto.bin"))
.isNull();
}
+
+ @Test
+ public void testDescriptorSetOutput_strictDeps() throws Exception {
+ scratch.file(
+ "x/BUILD",
+ "proto_library(name='nodeps', srcs=['nodeps.proto'])",
+ "proto_library(name='withdeps', srcs=['withdeps.proto'], deps=[':dep1', ':dep2'])",
+ "proto_library(name='depends_on_alias', srcs=['depends_on_alias.proto'], deps=[':alias'])",
+ "proto_library(name='alias', deps=[':dep1', ':dep2'])",
+ "proto_library(name='dep1', srcs=['dep1.proto'])",
+ "proto_library(name='dep2', srcs=['dep2.proto'])");
+
+ assertThat(getGeneratingSpawnAction(getDescriptorOutput("//x:nodeps")).getRemainingArguments())
+ .contains("--direct_dependencies=");
+
+ assertThat(
+ getGeneratingSpawnAction(getDescriptorOutput("//x:withdeps")).getRemainingArguments())
+ .contains("--direct_dependencies=x/dep1.proto:x/dep2.proto");
+
+ assertThat(
+ getGeneratingSpawnAction(getDescriptorOutput("//x:depends_on_alias"))
+ .getRemainingArguments())
+ .contains("--direct_dependencies=x/dep1.proto:x/dep2.proto");
+ }
+
+ @Test
+ public void testDescriptorSetOutput_strictDeps_aliasLibrary() throws Exception {
+ scratch.file(
+ "x/BUILD",
+ "proto_library(name='alias', deps=[':dep1', ':subalias'])",
+ "proto_library(name='dep1', srcs=['dep1.proto'], deps = [':subdep1'])",
+ "proto_library(name='subdep1', srcs=['subdep1.proto'])",
+ "proto_library(name='subalias', deps = [':dep2'])",
+ "proto_library(name='dep2', srcs = ['dep2.proto'], deps = [':subdep2'])",
+ "proto_library(name='subdep2', srcs=['subdep2.proto'])");
+
+ assertThat(getGeneratingSpawnAction(getDescriptorOutput("//x:alias")).getRemainingArguments())
+ .containsAllOf(
+ "--direct_dependencies=x/subdep1.proto:x/subdep2.proto",
+ "x/dep1.proto",
+ "x/dep2.proto");
+ }
+
+ @Test
+ public void testDescriptorSetOutput_strictDeps_disabled() throws Exception {
+ scratch.file("x/BUILD", "proto_library(name='foo', srcs=['foo.proto'], strict_proto_deps=0)");
+
+ for (String arg :
+ getGeneratingSpawnAction(getDescriptorOutput("//x:foo")).getRemainingArguments()) {
+ assertThat(arg).doesNotContain("--direct_dependencies=");
+ }
+ }
+
+ private Artifact getDescriptorOutput(String label) throws Exception {
+ return ActionsTestUtil.getFirstArtifactEndingWith(
+ getFilesToBuild(getConfiguredTarget(label)), ".proto.bin");
+ }
}
