ProtoCompileActionBuilder can create strict-deps-checking command lines.

No behavior changes, for now.

--
MOS_MIGRATED_REVID=139614509

4 files changed, 115 insertions, 7 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 f98de01ef9..7819f13d22 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
@@ -54,6 +54,7 @@ public class BazelProtoLibrary implements RuleConfiguredTargetFactory {
         SupportData.create(
             Predicates.<TransitiveInfoCollection>alwaysTrue() /* nonWeakDepsPredicate */,
             protoSources,
+            null /* protosInDirectDeps */,
             transitiveImports,
             !protoSources.isEmpty());
 
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 951ba048b6..51d86fe7d9 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
@@ -37,6 +37,7 @@ import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.util.LazyString;
+import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -285,7 +286,9 @@ public class ProtoCompileActionBuilder {
     result.add(ruleContext.getFragment(ProtoConfiguration.class).protocOpts());
 
     // Add include maps
-    result.add(new ProtoCommandLineArgv(supportData.getTransitiveImports()));
+    result.add(
+        new ProtoCommandLineArgv(
+            null /* protosInDirectDependencies */, supportData.getTransitiveImports()));
 
     for (Artifact src : supportData.getDirectProtoSources()) {
       result.addPath(src.getRootRelativePath());
@@ -306,10 +309,15 @@ public class ProtoCompileActionBuilder {
    * Static inner class since these objects live into the execution phase and so they must not keep
    * alive references to the surrounding analysis-phase objects.
    */
-  private static class ProtoCommandLineArgv extends CustomCommandLine.CustomMultiArgv {
+  @VisibleForTesting
+  static class ProtoCommandLineArgv extends CustomCommandLine.CustomMultiArgv {
+    @Nullable private final Iterable<Artifact> protosInDirectDependencies;
     private final Iterable<Artifact> transitiveImports;
 
-    ProtoCommandLineArgv(Iterable<Artifact> transitiveImports) {
+    ProtoCommandLineArgv(
+        @Nullable Iterable<Artifact> protosInDirectDependencies,
+        Iterable<Artifact> transitiveImports) {
+      this.protosInDirectDependencies = protosInDirectDependencies;
       this.transitiveImports = transitiveImports;
     }
 
@@ -320,6 +328,13 @@ public class ProtoCompileActionBuilder {
         builder.add(
             "-I" + artifact.getRootRelativePathString() + "=" + artifact.getExecPathString());
       }
+      if (protosInDirectDependencies != null) {
+        ArrayList<String> rootRelativePaths = new ArrayList<>();
+        for (Artifact directDependency : protosInDirectDependencies) {
+          rootRelativePaths.add(directDependency.getRootRelativePathString());
+        }
+        builder.add("--direct_dependencies=" + Joiner.on(":").join(rootRelativePaths));
+      }
       return builder.build();
     }
   }
@@ -425,6 +440,8 @@ public class ProtoCompileActionBuilder {
    * Note {@code toolchainInvocations} is ordered, and affects the order in which plugins are
    * called. As some plugins rely on output from other plugins, their order matters.
    *
+   * @param toolchainInvocations See {@link #createCommandLineFromToolchains}.
+   * @param allowServices If false, the compilation will break if any .proto file has service
    * @return a command-line to pass to proto-compiler.
    */
   @VisibleForTesting
@@ -470,7 +487,9 @@ public class ProtoCompileActionBuilder {
     cmdLine.add(protocOpts);
 
     // Add include maps
-    cmdLine.add(new ProtoCommandLineArgv(supportData.getTransitiveImports()));
+    cmdLine.add(
+        new ProtoCommandLineArgv(
+            supportData.getProtosInDirectDeps(), supportData.getTransitiveImports()));
 
     for (Artifact src : supportData.getDirectProtoSources()) {
       cmdLine.addPath(src.getRootRelativePath());
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 762af4f7a3..4da9f2acfe 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,7 @@ 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,10 +32,11 @@ public abstract class SupportData {
   public static SupportData create(
       Predicate<TransitiveInfoCollection> nonWeakDepsPredicate,
       ImmutableList<Artifact> protoSources,
+      @Nullable NestedSet<Artifact> protosInDirectDeps,
       NestedSet<Artifact> transitiveImports,
       boolean hasProtoSources) {
     return new AutoValue_SupportData(
-        nonWeakDepsPredicate, protoSources, transitiveImports, hasProtoSources);
+        nonWeakDepsPredicate, protoSources, transitiveImports, protosInDirectDeps, hasProtoSources);
   }
 
   public abstract Predicate<TransitiveInfoCollection> getNonWeakDepsPredicate();
@@ -44,6 +45,13 @@ public abstract class SupportData {
 
   public abstract NestedSet<Artifact> getTransitiveImports();
 
+  /**
+   * .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();
 
   SupportData() {}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java
index a8a7d693fe..abbdac5d2c 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java
@@ -28,6 +28,7 @@ import com.google.devtools.build.lib.analysis.FilesToRunProvider;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.ProtoCommandLineArgv;
 import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.ToolchainInvocation;
 import com.google.devtools.build.lib.util.LazyString;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -39,7 +40,10 @@ import org.junit.runners.JUnit4;
 @RunWith(JUnit4.class)
 public class ProtoCompileActionBuilderTest {
 
-  private final Root root = Root.asSourceRoot(new InMemoryFileSystem().getPath("/"));
+  private static final InMemoryFileSystem FILE_SYSTEM = new InMemoryFileSystem();
+  private final Root root = Root.asSourceRoot(FILE_SYSTEM.getPath("/"));
+  private final Root derivedRoot =
+      Root.asDerivedRoot(FILE_SYSTEM.getPath("/"), FILE_SYSTEM.getPath("/out"));
 
   @Test
   public void commandLine_basic() throws Exception {
@@ -67,6 +71,7 @@ public class ProtoCompileActionBuilderTest {
         SupportData.create(
             Predicates.<TransitiveInfoCollection>alwaysFalse(),
             ImmutableList.of(artifact("source_file.proto")),
+            null /* protosInDirectDeps */,
             NestedSetBuilder.create(
                 STABLE_ORDER, artifact("import1.proto"), artifact("import2.proto")),
             true /* hasProtoSources */);
@@ -93,11 +98,47 @@ public class ProtoCompileActionBuilderTest {
   }
 
   @Test
+  public void commandLine_strictDeps() throws Exception {
+    ProtoLangToolchainProvider toolchain =
+        ProtoLangToolchainProvider.create(
+            "--java_out=param1,param2:$(OUT)",
+            null /* pluginExecutable */,
+            mock(TransitiveInfoCollection.class) /* runtime */,
+            NestedSetBuilder.<Artifact>emptySet(STABLE_ORDER) /* blacklistedProtos */);
+
+    SupportData supportData =
+        SupportData.create(
+            Predicates.<TransitiveInfoCollection>alwaysFalse(),
+            ImmutableList.of(artifact("source_file.proto")),
+            NestedSetBuilder.create(STABLE_ORDER, artifact("import1.proto")),
+            NestedSetBuilder.create(
+                STABLE_ORDER, artifact("import1.proto"), artifact("import2.proto")),
+            true /* hasProtoSources */);
+
+    CustomCommandLine cmdLine =
+        createCommandLineFromToolchains(
+            ImmutableList.of(new ToolchainInvocation("dontcare", toolchain, "foo.srcjar")),
+            supportData,
+            true /* allowServices */,
+            ImmutableList.<String>of() /* protocOpts */);
+
+    assertThat(cmdLine.arguments())
+        .containsExactly(
+            "--java_out=param1,param2:foo.srcjar",
+            "-Iimport1.proto=import1.proto",
+            "-Iimport2.proto=import2.proto",
+            "--direct_dependencies=import1.proto",
+            "source_file.proto")
+        .inOrder();
+  }
+
+  @Test
   public void otherParameters() throws Exception {
     SupportData supportData =
         SupportData.create(
             Predicates.<TransitiveInfoCollection>alwaysFalse(),
             ImmutableList.<Artifact>of(),
+            null /* protosInDirectDeps */,
             NestedSetBuilder.<Artifact>emptySet(STABLE_ORDER),
             true /* hasProtoSources */);
 
@@ -136,6 +177,7 @@ public class ProtoCompileActionBuilderTest {
         SupportData.create(
             Predicates.<TransitiveInfoCollection>alwaysFalse(),
             ImmutableList.<Artifact>of(),
+            null /* protosInDirectDeps */,
             NestedSetBuilder.<Artifact>emptySet(STABLE_ORDER),
             true /* hasProtoSources */);
 
@@ -161,6 +203,7 @@ public class ProtoCompileActionBuilderTest {
         SupportData.create(
             Predicates.<TransitiveInfoCollection>alwaysFalse(),
             ImmutableList.<Artifact>of(),
+            null /* protosInDirectDeps */,
             NestedSetBuilder.<Artifact>emptySet(STABLE_ORDER),
             true /* hasProtoSources */);
 
@@ -195,7 +238,44 @@ public class ProtoCompileActionBuilderTest {
     }
   }
 
+  @Test
+  public void testProtoCommandLineArgv() throws Exception {
+    assertThat(
+            new ProtoCommandLineArgv(
+                    null /* directDependencies */, ImmutableList.of(derivedArtifact("foo.proto")))
+                .argv())
+        .containsExactly("-Ifoo.proto=out/foo.proto");
+
+    assertThat(
+            new ProtoCommandLineArgv(
+                    ImmutableList.<Artifact>of() /* directDependencies */,
+                    ImmutableList.of(derivedArtifact("foo.proto")))
+                .argv())
+        .containsExactly("-Ifoo.proto=out/foo.proto", "--direct_dependencies=");
+
+    assertThat(
+            new ProtoCommandLineArgv(
+                    ImmutableList.of(derivedArtifact("foo.proto")) /* directDependencies */,
+                    ImmutableList.of(derivedArtifact("foo.proto")))
+                .argv())
+        .containsExactly("-Ifoo.proto=out/foo.proto", "--direct_dependencies=foo.proto");
+
+    assertThat(
+            new ProtoCommandLineArgv(
+                    ImmutableList.of(
+                        derivedArtifact("foo.proto"),
+                        derivedArtifact("bar.proto")) /* directDependencies */,
+                    ImmutableList.of(derivedArtifact("foo.proto")))
+                .argv())
+        .containsExactly("-Ifoo.proto=out/foo.proto", "--direct_dependencies=foo.proto:bar.proto");
+  }
+
   private Artifact artifact(String path) {
     return new Artifact(new PathFragment(path), root);
   }
+
+  /** Creates a dummy artifact with the given path, that actually resides in /out/<path>. */
+  private Artifact derivedArtifact(String path) {
+    return new Artifact(new PathFragment(path), derivedRoot);
+  }
 }