Rollforward of commit 4162cc54ba0b128b616c0bd05b65bf9ad5e66f2e.

src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java

@@ -127,6 +127,19 @@ private static NestedSet<Artifact> computeTransitiveProtoSources(
     return result.build();
   }
 
+  private static NestedSet<Artifact> computeTransitiveOriginalProtoSources(
+      RuleContext ruleContext, ImmutableList<Artifact> originalProtoSources) {
+    NestedSetBuilder<Artifact> result = NestedSetBuilder.naiveLinkOrder();
+
+    result.addAll(originalProtoSources);
+
+    for (ProtoInfo dep : ruleContext.getPrerequisites("deps", Mode.TARGET, ProtoInfo.PROVIDER)) {
+      result.addTransitive(dep.getOriginalTransitiveProtoSources());
+    }
+
+    return result.build();
+  }
+
   static NestedSet<Artifact> computeDependenciesDescriptorSets(RuleContext ruleContext) {
     NestedSetBuilder<Artifact> result = NestedSetBuilder.stableOrder();
 
@@ -462,6 +475,8 @@ public static ProtoInfo createProtoInfo(
 
     NestedSet<Artifact> transitiveProtoSources =
         computeTransitiveProtoSources(ruleContext, library.getSources());
+    NestedSet<Artifact> transitiveOriginalProtoSources =
+        computeTransitiveOriginalProtoSources(ruleContext, directProtoSources);
     NestedSet<String> transitiveProtoSourceRoots =
         computeTransitiveProtoSourceRoots(ruleContext, library.getSourceRoot());
 
@@ -494,6 +509,7 @@ public static ProtoInfo createProtoInfo(
             directProtoSources,
             library.getSourceRoot(),
             transitiveProtoSources,
+            transitiveOriginalProtoSources,
             transitiveProtoSourceRoots,
             strictImportableProtosForDependents,
             strictImportableProtos,

src/main/java/com/google/devtools/build/lib/rules/proto/ProtoInfo.java

@@ -57,6 +57,7 @@ public Provider() {
   private final ImmutableList<Artifact> originalDirectProtoSources;
   private final String directProtoSourceRoot;
   private final NestedSet<Artifact> transitiveProtoSources;
+  private final NestedSet<Artifact> originalTransitiveProtoSources;
   private final NestedSet<String> transitiveProtoSourceRoots;
   private final NestedSet<Artifact> strictImportableProtoSourcesForDependents;
   private final NestedSet<Pair<Artifact, String>> strictImportableProtoSourcesImportPaths;
@@ -74,6 +75,7 @@ public ProtoInfo(
       ImmutableList<Artifact> originalDirectProtoSources,
       String directProtoSourceRoot,
       NestedSet<Artifact> transitiveProtoSources,
+      NestedSet<Artifact> originalTransitiveProtoSources,
       NestedSet<String> transitiveProtoSourceRoots,
       NestedSet<Artifact> strictImportableProtoSourcesForDependents,
       NestedSet<Pair<Artifact, String>> strictImportableProtoSourcesImportPaths,
@@ -89,6 +91,7 @@ public ProtoInfo(
     this.originalDirectProtoSources = originalDirectProtoSources;
     this.directProtoSourceRoot = directProtoSourceRoot;
     this.transitiveProtoSources = transitiveProtoSources;
+    this.originalTransitiveProtoSources = originalTransitiveProtoSources;
     this.transitiveProtoSourceRoots = transitiveProtoSourceRoots;
     this.strictImportableProtoSourcesForDependents = strictImportableProtoSourcesForDependents;
     this.strictImportableProtoSourcesImportPaths = strictImportableProtoSourcesImportPaths;
@@ -103,15 +106,18 @@ public ProtoInfo(
 
   /**
    * The proto source files that are used in compiling this {@code proto_library}.
-   *
-   * <p>Different from {@link #getOriginalDirectProtoSources()} when a virtual import root is used.
    */
   @Override
   public ImmutableList<Artifact> getDirectProtoSources() {
     return directProtoSources;
   }
 
-  /** The proto sources of the {@code proto_library} declaring this provider. */
+  /**
+   * The non-virtual proto sources of the {@code proto_library} declaring this provider.
+   *
+   * <p> Different from {@link #getDirectProtoSources()} if a transitive dependency has
+   * {@code import_prefix} or the like.
+   */
   public ImmutableList<Artifact> getOriginalDirectProtoSources() {
     return originalDirectProtoSources;
   }
@@ -132,6 +138,17 @@ public NestedSet<Artifact> getTransitiveProtoSources() {
     return transitiveProtoSources;
   }
 
+  /**
+   * The non-virtual transitive proto source files.
+   *
+   * <p> Different from {@link #getTransitiveProtoSources()} if a transitive dependency has
+   * {@code import_prefix} or the like.
+   */
+  public NestedSet<Artifact> getOriginalTransitiveProtoSources() {
+    return originalTransitiveProtoSources;
+  }
+
+
   /**
    * The proto source roots of the transitive closure of this rule. These flags will be passed to
    * {@code protoc} in the specified order, via the {@code --proto_path} flag.

src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchain.java

@@ -44,8 +44,10 @@ public ConfiguredTarget create(RuleContext ruleContext)
       ProtoInfo protoInfo = protos.get(ProtoInfo.PROVIDER);
       // TODO(cushon): it would be nice to make this mandatory and stop adding files to build too
       if (protoInfo != null) {
-        blacklistedProtos.addTransitive(protoInfo.getTransitiveProtoSources());
+        blacklistedProtos.addTransitive(protoInfo.getOriginalTransitiveProtoSources());
       } else {
+        // Only add files from FileProvider if |protos| is not a proto_library to avoid adding
+        // the descriptor_set of proto_library to the list of blacklisted files.
         blacklistedProtos.addTransitive(protos.getProvider(FileProvider.class).getFilesToBuild());
       }
     }

src/test/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryTest.java

@@ -52,12 +52,17 @@ public void setUp() throws Exception {
     scratch.overwriteFile(
         "protobuf/BUILD",
         TestConstants.LOAD_PROTO_LANG_TOOLCHAIN,
+        TestConstants.LOAD_PROTO_LIBRARY,
         "package(default_visibility=['//visibility:public'])",
         "exports_files(['protoc'])",
+        "proto_library(",
+        "    name = 'any_proto',",
+        "    srcs = ['any.proto'],",
+        ")",
         "proto_lang_toolchain(",
         "    name = 'cc_toolchain',",
         "    command_line = '--cpp_out=$(OUT)',",
-        "    blacklisted_protos = [],",
+        "    blacklisted_protos = [':any_proto'],",
         ")");
     scratch.appendFile(
         "WORKSPACE",
@@ -140,6 +145,36 @@ public void aliasProtos() throws Exception {
         .containsExactly("x/foo.pb.h");
   }
 
+  @Test
+  public void blacklistedProtos() throws Exception {
+    scratch.file(
+        "x/BUILD",
+        TestConstants.LOAD_PROTO_LIBRARY,
+        "cc_proto_library(name = 'any_cc_proto', deps = ['@com_google_protobuf//:any_proto'])");
+
+    CcCompilationContext ccCompilationContext =
+        getConfiguredTarget("//x:any_cc_proto").get(CcInfo.PROVIDER).getCcCompilationContext();
+    assertThat(prettyArtifactNames(ccCompilationContext.getDeclaredIncludeSrcs())).isEmpty();
+  }
+
+  @Test
+  public void blacklistedProtosInTransitiveDeps() throws Exception {
+    scratch.file(
+        "x/BUILD",
+        TestConstants.LOAD_PROTO_LIBRARY,
+        "cc_proto_library(name = 'foo_cc_proto', deps = ['foo_proto'])",
+        "proto_library(",
+        "    name = 'foo_proto',",
+        "    srcs = ['foo.proto'],",
+        "    deps = ['@com_google_protobuf//:any_proto'],",
+        ")");
+
+    CcCompilationContext ccCompilationContext =
+        getConfiguredTarget("//x:foo_cc_proto").get(CcInfo.PROVIDER).getCcCompilationContext();
+    assertThat(prettyArtifactNames(ccCompilationContext.getDeclaredIncludeSrcs()))
+        .containsExactly("x/foo.pb.h");
+  }
+
   @Test
   public void ccCompilationContext() throws Exception {
     scratch.file(

src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java

@@ -66,7 +66,8 @@ private ProtoInfo protoInfo(
     return new ProtoInfo(
         directProtos,
         directProtos,
-        "",
+        /* directProtoSourceRoot= */ "",
+        transitiveProtos,
         transitiveProtos,
         transitiveProtoSourceRoots,
         /* strictImportableProtosForDependents */ NestedSetBuilder.emptySet(Order.STABLE_ORDER),

src/test/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainTest.java

@@ -39,10 +39,27 @@ public void setUp() throws Exception {
     invalidatePackages();
   }
 
+  private void validateProtoLangToolchain(ProtoLangToolchainProvider toolchain) throws Exception {
+    assertThat(toolchain.commandLine()).isEqualTo("cmd-line");
+    assertThat(toolchain.pluginExecutable().getExecutable().getRootRelativePathString())
+        .isEqualTo("third_party/x/plugin");
+
+    TransitiveInfoCollection runtimes = toolchain.runtime();
+    assertThat(runtimes.getLabel())
+        .isEqualTo(Label.parseAbsolute("//third_party/x:runtime", ImmutableMap.of()));
+
+    assertThat(prettyArtifactNames(toolchain.blacklistedProtos()))
+        .containsExactly(
+            "third_party/x/metadata.proto",
+            "third_party/x/descriptor.proto",
+            "third_party/x/any.proto");
+  }
+
   @Test
   public void protoToolchain() throws Exception {
     scratch.file(
-        "x/BUILD",
+        "third_party/x/BUILD",
+        "licenses(['unencumbered'])",
         "cc_binary(name = 'plugin', srcs = ['plugin.cc'])",
         "cc_library(name = 'runtime', srcs = ['runtime.cc'])",
         "filegroup(name = 'descriptors', srcs = ['metadata.proto', 'descriptor.proto'])",
@@ -52,29 +69,82 @@ public void protoToolchain() throws Exception {
     scratch.file(
         "foo/BUILD",
         TestConstants.LOAD_PROTO_LANG_TOOLCHAIN,
+        "licenses(['unencumbered'])",
         "proto_lang_toolchain(",
         "    name = 'toolchain',",
         "    command_line = 'cmd-line',",
-        "    plugin = '//x:plugin',",
-        "    runtime = '//x:runtime',",
-        "    blacklisted_protos = ['//x:blacklist']",
+        "    plugin = '//third_party/x:plugin',",
+        "    runtime = '//third_party/x:runtime',",
+        "    blacklisted_protos = ['//third_party/x:blacklist']",
         ")");
 
     update(ImmutableList.of("//foo:toolchain"), false, 1, true, new EventBus());
 
-    ProtoLangToolchainProvider toolchain =
-        getConfiguredTarget("//foo:toolchain").getProvider(ProtoLangToolchainProvider.class);
+    validateProtoLangToolchain(
+        getConfiguredTarget("//foo:toolchain").getProvider(ProtoLangToolchainProvider.class));
+  }
 
-    assertThat(toolchain.commandLine()).isEqualTo("cmd-line");
-    assertThat(toolchain.pluginExecutable().getExecutable().getRootRelativePathString())
-        .isEqualTo("x/plugin");
+  @Test
+  public void protoToolchainBlacklistProtoLibraries() throws Exception {
+    scratch.file(
+        "third_party/x/BUILD",
+        TestConstants.LOAD_PROTO_LIBRARY,
+        "licenses(['unencumbered'])",
+        "cc_binary(name = 'plugin', srcs = ['plugin.cc'])",
+        "cc_library(name = 'runtime', srcs = ['runtime.cc'])",
+        "proto_library(name = 'descriptors', srcs = ['metadata.proto', 'descriptor.proto'])",
+        "proto_library(name = 'any', srcs = ['any.proto'], strip_import_prefix = '/third_party')");
 
-    TransitiveInfoCollection runtimes = toolchain.runtime();
-    assertThat(runtimes.getLabel())
-        .isEqualTo(Label.parseAbsolute("//x:runtime", ImmutableMap.of()));
+    scratch.file(
+        "foo/BUILD",
+        TestConstants.LOAD_PROTO_LANG_TOOLCHAIN,
+        "proto_lang_toolchain(",
+        "    name = 'toolchain',",
+        "    command_line = 'cmd-line',",
+        "    plugin = '//third_party/x:plugin',",
+        "    runtime = '//third_party/x:runtime',",
+        "    blacklisted_protos = ['//third_party/x:descriptors', '//third_party/x:any']",
+        ")");
 
-    assertThat(prettyArtifactNames(toolchain.blacklistedProtos()))
-        .containsExactly("x/metadata.proto", "x/descriptor.proto", "x/any.proto");
+    update(ImmutableList.of("//foo:toolchain"), false, 1, true, new EventBus());
+
+    validateProtoLangToolchain(
+        getConfiguredTarget("//foo:toolchain").getProvider(ProtoLangToolchainProvider.class));
+  }
+
+  @Test
+  public void protoToolchainMixedBlacklist() throws Exception {
+    scratch.file(
+        "third_party/x/BUILD",
+        TestConstants.LOAD_PROTO_LIBRARY,
+        "licenses(['unencumbered'])",
+        "cc_binary(name = 'plugin', srcs = ['plugin.cc'])",
+        "cc_library(name = 'runtime', srcs = ['runtime.cc'])",
+        "proto_library(name = 'metadata', srcs = ['metadata.proto'])",
+        "proto_library(",
+        "    name = 'descriptor',",
+        "    srcs = ['descriptor.proto'],",
+        "    strip_import_prefix = '/third_party')",
+        "filegroup(name = 'any', srcs = ['any.proto'])");
+
+    scratch.file(
+        "foo/BUILD",
+        TestConstants.LOAD_PROTO_LANG_TOOLCHAIN,
+        "proto_lang_toolchain(",
+        "    name = 'toolchain',",
+        "    command_line = 'cmd-line',",
+        "    plugin = '//third_party/x:plugin',",
+        "    runtime = '//third_party/x:runtime',",
+        "    blacklisted_protos = [",
+        "        '//third_party/x:metadata',",
+        "        '//third_party/x:descriptor',",
+        "        '//third_party/x:any']",
+        ")");
+
+    update(ImmutableList.of("//foo:toolchain"), false, 1, true, new EventBus());
+
+    validateProtoLangToolchain(
+        getConfiguredTarget("//foo:toolchain").getProvider(ProtoLangToolchainProvider.class));
   }
 
   @Test