Do not crash when a strict proto_library depends on a non-strict one.

(strictness in the sense of strict proto deps)

The reason for the crash was that a non-strict proto_library would put 'null' in its SupportData.protosInDirectDeps, and then a strict proto_library that consumes it would choke on the null. This rearranges things so that protosInDirectDeps will never be null.

--
PiperOrigin-RevId: 146210040
MOS_MIGRATED_REVID=146210040

3 files changed, 10 insertions, 11 deletions

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 c97a746b06..562ee8703a 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
@@ -16,7 +16,6 @@ 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.rules.proto.ProtoCommon.areDepsStrict;
 
 import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
@@ -46,8 +45,7 @@ public class BazelProtoLibrary implements RuleConfiguredTargetFactory {
     NestedSet<Artifact> transitiveImports =
         ProtoCommon.collectTransitiveImports(ruleContext, protoSources);
 
-    NestedSet<Artifact> protosInDirectDeps =
-        areDepsStrict(ruleContext) ? ProtoCommon.computeProtosInDirectDeps(ruleContext) : null;
+    NestedSet<Artifact> protosInDirectDeps = ProtoCommon.computeProtosInDirectDeps(ruleContext);
 
     final SupportData supportData =
         SupportData.create(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java
index 7cae194729..f6b515f912 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java
@@ -17,6 +17,7 @@ package com.google.devtools.build.lib.rules.proto;
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.collect.Iterables.isEmpty;
 import static com.google.devtools.build.lib.collect.nestedset.Order.STABLE_ORDER;
+import static com.google.devtools.build.lib.rules.proto.ProtoCommon.areDepsStrict;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
@@ -287,12 +288,15 @@ public class ProtoCompileActionBuilder {
 
     result.add(ruleContext.getFragment(ProtoConfiguration.class).protocOpts());
 
+    boolean areDepsStrict = areDepsStrict(ruleContext);
+
     // Add include maps
     result.add(
         new ProtoCommandLineArgv(
-            supportData.getProtosInDirectDeps(), supportData.getTransitiveImports()));
+            areDepsStrict ? supportData.getProtosInDirectDeps() : null,
+            supportData.getTransitiveImports()));
 
-    if (supportData.getProtosInDirectDeps() != null) {
+    if (areDepsStrict) {
       // Note: the %s in the line below is used by proto-compiler. That is, the string we create
       // here should have a literal %s in it.
       result.add(
@@ -412,7 +416,7 @@ public class ProtoCompileActionBuilder {
       List<ToolchainInvocation> toolchainInvocations,
       Iterable<Artifact> protosToCompile,
       NestedSet<Artifact> transitiveSources,
-      @Nullable NestedSet<Artifact> protosInDirectDeps,
+      NestedSet<Artifact> protosInDirectDeps,
       String ruleLabel,
       Iterable<Artifact> outputs,
       String flavorName,
@@ -447,7 +451,7 @@ public class ProtoCompileActionBuilder {
                 toolchainInvocations,
                 protosToCompile,
                 transitiveSources,
-                protosInDirectDeps,
+                areDepsStrict(ruleContext) ? protosInDirectDeps : null,
                 ruleLabel,
                 allowServices,
                 ruleContext.getFragment(ProtoConfiguration.class).protocOpts()))
diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/SupportData.java b/src/main/java/com/google/devtools/build/lib/rules/proto/SupportData.java
index 4da9f2acfe..da35d13c34 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/proto/SupportData.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/proto/SupportData.java
@@ -21,7 +21,6 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-import javax.annotation.Nullable;
 
 /**
  * A helper class for the *Support classes containing some data from ProtoLibrary.
@@ -32,7 +31,7 @@ public abstract class SupportData {
   public static SupportData create(
       Predicate<TransitiveInfoCollection> nonWeakDepsPredicate,
       ImmutableList<Artifact> protoSources,
-      @Nullable NestedSet<Artifact> protosInDirectDeps,
+      NestedSet<Artifact> protosInDirectDeps,
       NestedSet<Artifact> transitiveImports,
       boolean hasProtoSources) {
     return new AutoValue_SupportData(
@@ -47,9 +46,7 @@ public abstract class SupportData {
 
   /**
    * .proto files in the direct dependencies of this proto_library. Used for strict deps checking.
-   * <code>null</code> means "strict deps checking is off".
    */
-  @Nullable
   public abstract NestedSet<Artifact> getProtosInDirectDeps();
 
   public abstract boolean hasProtoSources();