Remove strictImportableSources and publicImportSources from ProtoInfo.

PiperOrigin-RevId: 456471967
Change-Id: I2f4429bd4c6c009d00949b6eb04462b497603983

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

@@ -59,8 +59,6 @@ public ProtoSource protoSource(
         @Param(name = "direct_descriptor_set", doc = "Direct descriptor set."),
         @Param(name = "transitive_descriptor_set", doc = "Transitive descriptor sets."),
         @Param(name = "exported_sources", doc = "Exported sources"),
-        @Param(name = "strict_importable_sources", doc = "Strict importable sources."),
-        @Param(name = "public_import_protos", doc = "Public import protos."),
       },
       useStarlarkThread = true)
   @SuppressWarnings("unchecked")
@@ -74,8 +72,6 @@ public ProtoInfo protoInfo(
       Artifact directDescriptorSet,
       Depset transitiveDescriptorSets,
       Depset exportedSources,
-      Depset strictImportableSources,
-      Depset publicImportSources,
       StarlarkThread thread)
       throws EvalException {
     ProtoCommon.checkPrivateStarlarkificationAllowlist(thread);
@@ -89,8 +85,6 @@ public ProtoInfo protoInfo(
             strictImportableProtoSourcesForDependents, Artifact.class, "check_deps_sources"),
         directDescriptorSet,
         Depset.cast(transitiveDescriptorSets, Artifact.class, "transitive_descriptor_set"),
-        Depset.cast(exportedSources, ProtoSource.class, "exported_sources"),
-        Depset.cast(strictImportableSources, ProtoSource.class, "strict_importable_sources"),
-        Depset.cast(publicImportSources, ProtoSource.class, "public_import_protos"));
+        Depset.cast(exportedSources, ProtoSource.class, "exported_sources"));
   }
 }

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

@@ -60,13 +60,7 @@ private static ImmutableList<Artifact> extractProtoSources(ImmutableList<ProtoSo
   private final NestedSet<Artifact> strictImportableProtoSourcesForDependents;
   private final Artifact directDescriptorSet;
   private final NestedSet<Artifact> transitiveDescriptorSets;
-
-  // Layering checks.
-  // TODO(yannic): Consider removing some of these. It should be sufficient to do
-  // layering checks when creating the descriptor-set.
   private final NestedSet<ProtoSource> exportedSources;
-  private final NestedSet<ProtoSource> strictImportableSources;
-  private final NestedSet<ProtoSource> publicImportSources;
 
   public ProtoInfo(
       ImmutableList<ProtoSource> directSources,
@@ -78,9 +72,7 @@ public ProtoInfo(
       Artifact directDescriptorSet,
       NestedSet<Artifact> transitiveDescriptorSets,
       // Layering checks.
-      NestedSet<ProtoSource> exportedSources,
-      NestedSet<ProtoSource> strictImportableSources,
-      NestedSet<ProtoSource> publicImportSources) {
+      NestedSet<ProtoSource> exportedSources) {
     this.directSources = directSources;
     this.directProtoSources = extractProtoSources(directSources);
     this.directProtoSourceRoot = ProtoCommon.memoryEfficientProtoSourceRoot(directProtoSourceRoot);
@@ -93,8 +85,6 @@ public ProtoInfo(
 
     // Layering checks.
     this.exportedSources = exportedSources;
-    this.strictImportableSources = strictImportableSources;
-    this.publicImportSources = publicImportSources;
   }
 
   /** The {@code .proto} source files in this {@code proto_library}'s {@code srcs}. */
@@ -229,32 +219,4 @@ public Depset getExportedSourcesForStarlark(StarlarkThread thread) throws EvalEx
   public NestedSet<ProtoSource> getExportedSources() {
     return exportedSources;
   }
-
-  /**
-   * Returns a set of {@code .proto} sources that may be imported by this {@code proto_library}
-   * target.
-   */
-  public NestedSet<ProtoSource> getStrictImportableSources() {
-    return strictImportableSources;
-  }
-
-  @Override
-  public Depset getStrictImportableSourcesForStarlark(StarlarkThread thread) throws EvalException {
-    ProtoCommon.checkPrivateStarlarkificationAllowlist(thread);
-    return Depset.of(ProtoSource.TYPE, strictImportableSources);
-  }
-
-  @Override
-  public Depset getPublicImportSourcesForStarlark(StarlarkThread thread) throws EvalException {
-    ProtoCommon.checkPrivateStarlarkificationAllowlist(thread);
-    return Depset.of(ProtoSource.TYPE, publicImportSources);
-  }
-
-  /**
-   * Returns a set of {@code .proto} sources that may be re-exported by this {@code proto_library}'s
-   * direct sources.
-   */
-  NestedSet<ProtoSource> getPublicImportSources() {
-    return publicImportSources;
-  }
 }

src/main/java/com/google/devtools/build/lib/starlarkbuildapi/ProtoInfoApi.java

@@ -111,12 +111,6 @@ interface ProtoInfoProviderApi extends ProviderApi {
       structField = true)
   String getDirectProtoSourceRoot();
 
-  @StarlarkMethod(name = "strict_importable_sources", documented = false, useStarlarkThread = true)
-  Depset getStrictImportableSourcesForStarlark(StarlarkThread thread) throws EvalException;
-
-  @StarlarkMethod(name = "public_import_sources", documented = false, useStarlarkThread = true)
-  Depset getPublicImportSourcesForStarlark(StarlarkThread thread) throws EvalException;
-
   @StarlarkMethod(name = "transitive_proto_sources", documented = false, useStarlarkThread = true)
   Depset getTransitiveSourcesForStarlark(StarlarkThread thread) throws EvalException;
 

src/main/starlark/builtins_bzl/common/proto/proto_library.bzl

@@ -76,7 +76,7 @@ def _proto_library_impl(ctx):
     proto_path, direct_sources = _create_proto_sources(ctx, srcs, import_prefix, strip_import_prefix)
     descriptor_set = ctx.actions.declare_file(ctx.label.name + "-descriptor-set.proto.bin")
     proto_info = _create_proto_info(ctx, direct_sources, deps, exports, proto_path, descriptor_set)
-    _write_descriptor_set(ctx, deps, proto_info, descriptor_set)
+    _write_descriptor_set(ctx, direct_sources, deps, exports, proto_info, descriptor_set)
 
     # We assume that the proto sources will not have conflicting artifacts
     # with the same root relative path
@@ -193,14 +193,8 @@ def _create_proto_info(ctx, direct_sources, deps, exports, proto_path, descripto
     # Layering checks.
     if direct_sources:
         exported_sources = depset(direct = direct_sources)
-        strict_importable_sources = depset(
-            direct = direct_sources,
-            transitive = [dep.exported_sources() for dep in deps],
-        )
     else:
         exported_sources = depset(transitive = [dep.exported_sources() for dep in deps])
-        strict_importable_sources = depset()
-    public_import_protos = depset(transitive = [export.exported_sources() for export in exports])
 
     return native_proto_common.ProtoInfo(
         direct_sources,
@@ -212,14 +206,12 @@ def _create_proto_info(ctx, direct_sources, deps, exports, proto_path, descripto
         descriptor_set,
         transitive_descriptor_sets,
         exported_sources,
-        strict_importable_sources,
-        public_import_protos,
     )
 
 def _get_import_path(proto_source):
     return proto_source.import_path()
 
-def _write_descriptor_set(ctx, deps, proto_info, descriptor_set):
+def _write_descriptor_set(ctx, direct_sources, deps, exports, proto_info, descriptor_set):
     """Writes descriptor set."""
     if proto_info.direct_sources == []:
         ctx.actions.write(descriptor_set, "")
@@ -234,7 +226,13 @@ def _write_descriptor_set(ctx, deps, proto_info, descriptor_set):
     strict_deps_mode = ctx.fragments.proto.strict_proto_deps()
     strict_deps = strict_deps_mode != "OFF" and strict_deps_mode != "DEFAULT"
     if strict_deps:
-        strict_importable_sources = proto_info.strict_importable_sources()
+        if direct_sources:
+            strict_importable_sources = depset(
+                direct = direct_sources,
+                transitive = [dep.exported_sources() for dep in deps],
+            )
+        else:
+            strict_importable_sources = None
         if strict_importable_sources:
             args.add_joined("--direct_dependencies", strict_importable_sources, map_each = _get_import_path, join_with = ":")
             # Example: `--direct_dependencies a.proto:b.proto`
@@ -249,11 +247,12 @@ def _write_descriptor_set(ctx, deps, proto_info, descriptor_set):
     strict_public_imports_mode = ctx.fragments.proto.strict_public_imports()
     strict_imports = strict_public_imports_mode != "OFF" and strict_public_imports_mode != "DEFAULT"
     if strict_imports:
-        if not proto_info.public_import_sources():
+        public_import_protos = depset(transitive = [export.exported_sources() for export in exports])
+        if not public_import_protos:
             # This line is necessary to trigger the check.
             args.add("--allowed_public_imports=")
         else:
-            args.add_joined("--allowed_public_imports", proto_info.public_import_sources(), map_each = _get_import_path, join_with = ":")
+            args.add_joined("--allowed_public_imports", public_import_protos, map_each = _get_import_path, join_with = ":")
     proto_lang_toolchain_info = proto_common.ProtoLangToolchainInfo(
         out_replacement_format_flag = "--descriptor_set_out=%s",
         mnemonic = "GenProtoDescriptorSet",

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

@@ -532,19 +532,19 @@ public void testExportedStrippedImportPrefixes() throws Exception {
         "    exports=['//a:a'],",
         "    deps=['//b:b'])");
 
-    ConfiguredTarget c = getConfiguredTarget("//c:c");
+    ConfiguredTarget a = getConfiguredTarget("//a:a");
     // exported proto source roots should be the source root of the rule and the direct source roots
     // of its exports and nothing else (not the exports of its exports or the deps of its exports
     // or the exports of its deps)
     String genfiles = getTargetConfiguration().getGenfilesFragment(RepositoryName.MAIN).toString();
     assertThat(
             Iterables.transform(
-                c.get(ProtoInfo.PROVIDER).getPublicImportSources().toList(),
+                a.get(ProtoInfo.PROVIDER).getExportedSources().toList(),
                 s -> s.getSourceRoot().getSafePathString()))
         .containsExactly(genfiles + "/a/_virtual_imports/a");
     assertThat(
             Iterables.transform(
-                c.get(ProtoInfo.PROVIDER).getPublicImportSources().toList(),
+                a.get(ProtoInfo.PROVIDER).getExportedSources().toList(),
                 s -> s.getImportPath().getSafePathString()))
         .containsExactly("a.proto");
   }
@@ -574,21 +574,10 @@ private void testImportPrefixInExternalRepo(boolean siblingRepoLayout) throws Ex
         "  visibility = ['//visibility:public'],",
         ")");
 
-    scratch.file(
-        "main.proto", "syntax = 'proto3'';", "import 'bazel.build/yolo/yolo_pkg/yolo.proto';");
-    scratch.file(
-        "BUILD",
-        TestConstants.LOAD_PROTO_LIBRARY,
-        "proto_library(",
-        "  name = 'main_proto',",
-        "  srcs = ['main.proto'],",
-        "  deps = ['@yolo_repo//yolo_pkg:yolo_proto'],",
-        ")");
-
-    ConfiguredTarget target = getConfiguredTarget("//:main_proto");
+    ConfiguredTarget target = getConfiguredTarget("@yolo_repo//yolo_pkg:yolo_proto");
     assertThat(
             Iterables.transform(
-                target.get(ProtoInfo.PROVIDER).getStrictImportableSources().toList(),
+                target.get(ProtoInfo.PROVIDER).getExportedSources().toList(),
                 s -> s.getImportPath().getPathString()))
         .contains("bazel.build/yolo/yolo_pkg/yolo.proto");
   }
@@ -629,21 +618,11 @@ private void testImportPrefixAndStripInExternalRepo(boolean siblingRepoLayout) t
         "  visibility = ['//visibility:public'],",
         ")");
 
-    scratch.file(
-        "main.proto", "syntax = 'proto3'';", "import 'bazel.build/yolo/yolo_pkg/yolo.proto';");
-    scratch.file(
-        "BUILD",
-        TestConstants.LOAD_PROTO_LIBRARY,
-        "proto_library(",
-        "  name = 'main_proto',",
-        "  srcs = ['main.proto'],",
-        "  deps = ['@yolo_repo//yolo_pkg_to_be_stripped/yolo_pkg:yolo_proto'],",
-        ")");
-
-    ConfiguredTarget target = getConfiguredTarget("//:main_proto");
+    ConfiguredTarget target =
+        getConfiguredTarget("@yolo_repo//yolo_pkg_to_be_stripped/yolo_pkg:yolo_proto");
     assertThat(
             Iterables.transform(
-                target.get(ProtoInfo.PROVIDER).getStrictImportableSources().toList(),
+                target.get(ProtoInfo.PROVIDER).getExportedSources().toList(),
                 s -> s.getImportPath().getPathString()))
         .contains("bazel.build/yolo/yolo_pkg/yolo.proto");
   }
@@ -683,20 +662,11 @@ private void testStripImportPrefixInExternalRepo(boolean siblingRepoLayout) thro
         "  visibility = ['//visibility:public'],",
         ")");
 
-    scratch.file("main.proto", "syntax = 'proto3'';", "import 'yolo_pkg/yolo.proto';");
-    scratch.file(
-        "BUILD",
-        TestConstants.LOAD_PROTO_LIBRARY,
-        "proto_library(",
-        "  name = 'main_proto',",
-        "  srcs = ['main.proto'],",
-        "  deps = ['@yolo_repo//yolo_pkg_to_be_stripped/yolo_pkg:yolo_proto'],",
-        ")");
-
-    ConfiguredTarget target = getConfiguredTarget("//:main_proto");
+    ConfiguredTarget target =
+        getConfiguredTarget("@yolo_repo//yolo_pkg_to_be_stripped/yolo_pkg:yolo_proto");
     assertThat(
             Iterables.transform(
-                target.get(ProtoInfo.PROVIDER).getStrictImportableSources().toList(),
+                target.get(ProtoInfo.PROVIDER).getExportedSources().toList(),
                 s -> s.getImportPath().getPathString()))
         .contains("yolo_pkg/yolo.proto");
   }
@@ -737,20 +707,10 @@ private void testRelativeStripImportPrefixInExternalRepo(boolean siblingRepoLayo
         "  visibility = ['//visibility:public'],",
         ")");
 
-    scratch.file("main.proto", "syntax = 'proto3'';", "import 'yolo_pkg/yolo.proto';");
-    scratch.file(
-        "BUILD",
-        TestConstants.LOAD_PROTO_LIBRARY,
-        "proto_library(",
-        "  name = 'main_proto',",
-        "  srcs = ['main.proto'],",
-        "  deps = ['@yolo_repo//:yolo_proto'],",
-        ")");
-
-    ConfiguredTarget target = getConfiguredTarget("//:main_proto");
+    ConfiguredTarget target = getConfiguredTarget("@yolo_repo//:yolo_proto");
     assertThat(
             Iterables.transform(
-                target.get(ProtoInfo.PROVIDER).getStrictImportableSources().toList(),
+                target.get(ProtoInfo.PROVIDER).getExportedSources().toList(),
                 s -> s.getImportPath().getPathString()))
         .contains("yolo_pkg/yolo.proto");
   }