Implement proto_common.write_descriptor_set in Starlark.

The semantics of empty descriptor set is changed, as it doesn't depend on other descriptor sets any longer. This kind of dependencies would be hard to observe by integrations and testing showed there are no such integrations.

PiperOrigin-RevId: 410501409

src/main/java/com/google/devtools/build/lib/rules/proto/BUILD

@@ -22,6 +22,7 @@ java_library(
         ["*.java"],
     ),
     deps = [
+        "//src/main/java/com/google/devtools/build/docgen/annot",
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
         "//src/main/java/com/google/devtools/build/lib/actions:commandline_item",

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

@@ -17,7 +17,6 @@
 import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleContext;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.BazelModuleContext;
-import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.Services;
 import com.google.devtools.build.lib.starlarkbuildapi.proto.ProtoCommonApi;
 import javax.annotation.Nullable;
 import net.starlark.java.annot.Param;
@@ -56,26 +55,4 @@ public ProtoInfo createProtoInfo(StarlarkRuleContext ruleContext, StarlarkThread
             .getFragment(ProtoConfiguration.class)
             .generatedProtosInVirtualImports());
   }
-
-  @StarlarkMethod(
-      name = "write_descriptor_set",
-      documented = false,
-      parameters = {
-        @Param(name = "ctx", doc = "The rule context"),
-        @Param(name = "proto_info", doc = "The ProtoInfo")
-      },
-      useStarlarkThread = true)
-  public void writeDescriptorSet(
-      StarlarkRuleContext ruleContext, ProtoInfo protoInfo, StarlarkThread thread)
-      throws EvalException {
-    Label label =
-        ((BazelModuleContext) Module.ofInnermostEnclosingStarlarkFunction(thread).getClientData())
-            .label();
-    if (!label.getPackageIdentifier().getRepository().toString().equals("@_builtins")) {
-      throw Starlark.errorf("Rule in '%s' cannot use private API", label.getPackageName());
-    }
-
-    ProtoCompileActionBuilder.writeDescriptorSet(
-        ruleContext.getRuleContext(), protoInfo, Services.ALLOW);
-  }
 }

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

@@ -31,11 +31,16 @@
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.concurrent.BlazeInterners;
+import com.google.devtools.build.lib.packages.BazelModuleContext;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import javax.annotation.Nullable;
+import net.starlark.java.eval.EvalException;
+import net.starlark.java.eval.Module;
+import net.starlark.java.eval.Starlark;
+import net.starlark.java.eval.StarlarkThread;
 
 /**
  * Utility functions for proto_library and proto aspect implementations.
@@ -519,4 +524,14 @@ public static boolean areDepsStrict(RuleContext ruleContext) {
     StrictDepsMode getBool = ruleContext.getFragment(ProtoConfiguration.class).strictProtoDeps();
     return getBool != StrictDepsMode.OFF && getBool != StrictDepsMode.DEFAULT;
   }
+
+  public static void checkPrivateStarlarkificationAllowlist(StarlarkThread thread)
+      throws EvalException {
+    Label label =
+        ((BazelModuleContext) Module.ofInnermostEnclosingStarlarkFunction(thread).getClientData())
+            .label();
+    if (!label.getPackageIdentifier().getRepository().toString().equals("@_builtins")) {
+      throw Starlark.errorf("Rule in '%s' cannot use private API", label.getPackageName());
+    }
+  }
 }

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

@@ -31,6 +31,9 @@
 import com.google.devtools.common.options.OptionEffectTag;
 import com.google.devtools.common.options.OptionMetadataTag;
 import java.util.List;
+import net.starlark.java.annot.StarlarkMethod;
+import net.starlark.java.eval.EvalException;
+import net.starlark.java.eval.StarlarkThread;
 
 /** Configuration for Protocol Buffer Libraries. */
 @Immutable
@@ -203,10 +206,26 @@ public ProtoConfiguration(BuildOptions buildOptions) {
     this.options = options;
   }
 
+  @StarlarkMethod(name = "protoc_opts", useStarlarkThread = true, documented = false)
+  public ImmutableList<String> protocOptsForStarlark(StarlarkThread thread) throws EvalException {
+    ProtoCommon.checkPrivateStarlarkificationAllowlist(thread);
+    return protocOpts();
+  }
+
   public ImmutableList<String> protocOpts() {
     return protocOpts;
   }
 
+  @StarlarkMethod(
+      name = "experimental_proto_descriptorsets_include_source_info",
+      useStarlarkThread = true,
+      documented = false)
+  public boolean experimentalProtoDescriptorSetsIncludeSourceInfoForStarlark(StarlarkThread thread)
+      throws EvalException {
+    ProtoCommon.checkPrivateStarlarkificationAllowlist(thread);
+    return experimentalProtoDescriptorSetsIncludeSourceInfo();
+  }
+
   public boolean experimentalProtoDescriptorSetsIncludeSourceInfo() {
     return options.experimentalProtoDescriptorSetsIncludeSourceInfo;
   }
@@ -244,6 +263,12 @@ public Label protoToolchainForCc() {
     return options.protoToolchainForCc;
   }
 
+  @StarlarkMethod(name = "strict_proto_deps", useStarlarkThread = true, documented = false)
+  public String strictProtoDepsForStarlark(StarlarkThread thread) throws EvalException {
+    ProtoCommon.checkPrivateStarlarkificationAllowlist(thread);
+    return strictProtoDeps().toString();
+  }
+
   public StrictDepsMode strictProtoDeps() {
     return options.strictProtoDeps;
   }

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

@@ -24,6 +24,8 @@
 import com.google.devtools.build.lib.starlarkbuildapi.ProtoInfoApi;
 import com.google.devtools.build.lib.starlarkbuildapi.proto.ProtoBootstrap;
 import com.google.devtools.build.lib.vfs.PathFragment;
+import net.starlark.java.eval.EvalException;
+import net.starlark.java.eval.StarlarkThread;
 
 /**
  * Configured target classes that implement this class can contribute .proto files to the
@@ -128,6 +130,12 @@ public String getDirectProtoSourceRoot() {
     return Depset.of(Artifact.TYPE, getTransitiveProtoSources());
   }
 
+  @Override
+  public Depset getTransitiveSourcesForStarlark(StarlarkThread thread) throws EvalException {
+    ProtoCommon.checkPrivateStarlarkificationAllowlist(thread);
+    return Depset.of(ProtoSource.TYPE, transitiveSources);
+  }
+
   /**
    * The {@code .proto} source files in this {@code proto_library}'s {@code srcs} and all of its
    * transitive dependencies.
@@ -217,6 +225,12 @@ public NestedSet<ProtoSource> getStrictImportableSources() {
     return strictImportableSources;
   }
 
+  @Override
+  public Depset getStrictImportableSourcesForStarlark(StarlarkThread thread) throws EvalException {
+    ProtoCommon.checkPrivateStarlarkificationAllowlist(thread);
+    return Depset.of(ProtoSource.TYPE, strictImportableSources);
+  }
+
   /**
    * Returns a set of {@code .proto} sources that may be re-exported by this {@code proto_library}'s
    * direct sources.

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

@@ -14,15 +14,25 @@
 
 package com.google.devtools.build.lib.rules.proto;
 
+import com.google.devtools.build.docgen.annot.DocCategory;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.collect.nestedset.Depset;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.vfs.PathFragment;
+import net.starlark.java.annot.StarlarkBuiltin;
+import net.starlark.java.annot.StarlarkMethod;
+import net.starlark.java.eval.EvalException;
+import net.starlark.java.eval.StarlarkThread;
+import net.starlark.java.eval.StarlarkValue;
 
 /** Represents a single {@code .proto} source file. */
 @Immutable
 @AutoCodec
-class ProtoSource {
+@StarlarkBuiltin(name = "ProtoSource", category = DocCategory.BUILTIN, documented = false)
+class ProtoSource implements StarlarkValue {
+  public static final Depset.ElementType TYPE = Depset.ElementType.of(ProtoSource.class);
+
   private final Artifact sourceFile;
   private final Artifact originalSourceFile;
   private final PathFragment sourceRoot;
@@ -42,6 +52,12 @@ public Artifact getSourceFile() {
     return sourceFile;
   }
 
+  @StarlarkMethod(name = "source_file", documented = false, useStarlarkThread = true)
+  public Artifact getSourceFileForStarlark(StarlarkThread thread) throws EvalException {
+    ProtoCommon.checkPrivateStarlarkificationAllowlist(thread);
+    return sourceFile;
+  }
+
   /** Returns the original source file. Only for forbidding protos! */
   @Deprecated
   Artifact getOriginalSourceFile() {
@@ -52,6 +68,12 @@ public PathFragment getSourceRoot() {
     return sourceRoot;
   }
 
+  @StarlarkMethod(name = "import_path", documented = false, useStarlarkThread = true)
+  public String getImportPathForStarlark(StarlarkThread thread) throws EvalException {
+    ProtoCommon.checkPrivateStarlarkificationAllowlist(thread);
+    return getImportPath().getPathString();
+  }
+
   public PathFragment getImportPath() {
     return sourceFile.getExecPath().relativeTo(sourceRoot);
   }

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

@@ -21,6 +21,8 @@
 import com.google.devtools.build.lib.starlarkbuildapi.core.StructApi;
 import net.starlark.java.annot.StarlarkBuiltin;
 import net.starlark.java.annot.StarlarkMethod;
+import net.starlark.java.eval.EvalException;
+import net.starlark.java.eval.StarlarkThread;
 
 /** Info object propagating information about protocol buffer sources. */
 @StarlarkBuiltin(
@@ -105,4 +107,10 @@ interface ProtoInfoProviderApi extends ProviderApi {
               + " as a source, that source file would be imported as 'import c/d.proto'",
       structField = true)
   String getDirectProtoSourceRoot();
+
+  @StarlarkMethod(name = "strict_importable_sources", documented = false, useStarlarkThread = true)
+  Depset getStrictImportableSourcesForStarlark(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_common.bzl

@@ -0,0 +1,87 @@
+# Copyright 2021 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Definition of proto_common module.
+"""
+
+load(":common/proto/proto_semantics.bzl", "semantics")
+
+ProtoInfo = _builtins.toplevel.ProtoInfo
+
+def _write_descriptor_set(ctx, proto_info):
+    output = proto_info.direct_descriptor_set
+    proto_deps = [dep[ProtoInfo] for dep in ctx.attr.deps]
+    dependencies_descriptor_sets = depset(transitive = [dep.transitive_descriptor_sets for dep in proto_deps])
+
+    if proto_info.direct_sources == []:
+        ctx.actions.write(output, "")
+        return
+
+    args = ctx.actions.args()
+    args.use_param_file(param_file_arg = "@%s")
+    args.set_param_file_format("multiline")
+    args.add_all(proto_info.transitive_proto_path, map_each = _EXPAND_TRANSITIVE_PROTO_PATH_FLAGS)
+    args.add(output, format = "--descriptor_set_out=%s")
+
+    if ctx.fragments.proto.experimental_proto_descriptorsets_include_source_info():
+        args.add("--include_source_info")
+    args.add_all(ctx.fragments.proto.protoc_opts())
+
+    strict_deps_mode = ctx.fragments.proto.strict_proto_deps()
+    are_deps_strict = strict_deps_mode != "OFF" and strict_deps_mode != "DEFAULT"
+
+    # Include maps
+
+    # For each import, include both the import as well as the import relativized against its
+    # protoSourceRoot. This ensures that protos can reference either the full path or the short
+    # path when including other protos.
+    args.add_all(proto_info.transitive_proto_sources(), map_each = _ExpandImportArgsFn)
+    if are_deps_strict:
+        strict_importable_sources = proto_info.strict_importable_sources()
+        if strict_importable_sources:
+            args.add_joined("--direct_dependencies", strict_importable_sources, map_each = _EXPAND_TO_IMPORT_PATHS, join_with = ":")
+        else:
+            # The proto compiler requires an empty list to turn on strict deps checking
+            args.add("--direct_dependencies=")
+
+        args.add(ctx.label, format = semantics.STRICT_DEPS_FLAG_TEMPLATE)
+
+    # use exports
+
+    args.add_all(proto_info.direct_sources)
+
+    ctx.actions.run(
+        mnemonic = "GenProtoDescriptorSet",
+        progress_message = "Generating Descriptor Set proto_library %{label}",
+        executable = ctx.executable._proto_compiler,
+        arguments = [args],
+        inputs = depset(transitive = [dependencies_descriptor_sets, proto_info.transitive_sources]),
+        outputs = [output],
+    )
+
+def _EXPAND_TRANSITIVE_PROTO_PATH_FLAGS(flag):
+    if flag == ".":
+        return None
+    return "--proto_path=" + flag
+
+def _EXPAND_TO_IMPORT_PATHS(proto_source):
+    return proto_source.import_path()
+
+def _ExpandImportArgsFn(proto_source):
+    return "-I%s=%s" % (proto_source.import_path(), proto_source.source_file().path)
+
+proto_common = struct(
+    write_descriptor_set = _write_descriptor_set,
+)

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

@@ -17,9 +17,10 @@ Definition of proto_library rule.
 """
 
 load(":common/proto/proto_semantics.bzl", "semantics")
+load(":common/proto/proto_common.bzl", "proto_common")
 
 ProtoInfo = _builtins.toplevel.ProtoInfo
-proto_common = _builtins.toplevel.proto_common
+native_proto_common = _builtins.toplevel.proto_common
 
 def _check_srcs_package(target_package, srcs):
     """Makes sure the given srcs live in the given package."""
@@ -32,7 +33,7 @@ def _proto_library_impl(ctx):
 
     _check_srcs_package(ctx.label.package, ctx.attr.srcs)
 
-    proto_info = proto_common.create_proto_info(ctx)
+    proto_info = native_proto_common.create_proto_info(ctx)
 
     proto_common.write_descriptor_set(ctx, proto_info)
 

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

@@ -26,4 +26,14 @@ semantics = struct(
    },
    EXTRA_FRAGMENTS = [],
    preprocess = _preprocess,
+    # This constant is used in ProtoCompileActionBuilder to generate an error message that's
+    # displayed when a strict proto deps violation occurs.
+    #
+    # %s is replaced with the label of the proto_library rule that's currently being built.
+    # %%s is replaced with the literal "%s", which is passed to the proto-compiler, which replaces
+    # it with the .proto file that violates strict proto deps.
+    STRICT_DEPS_FLAG_TEMPLATE = (
+        "--direct_dependencies_violation_msg=" +
+        "%%s is imported, but %s doesn't directly depend on a proto_library that 'srcs' it."
+    ),
  )

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

@@ -30,6 +30,7 @@
 import com.google.devtools.build.lib.testutil.TestConstants;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -102,6 +103,7 @@ public void descriptorSets_ruleWithoutSrcsWritesEmptyFile() throws Exception {
    * deps, would break.
    */
   @Test
+  @Ignore("b/204266604 Remove if the testing shows it's not needed.")
   public void descriptorSetsDependOnChildren() throws Exception {
     scratch.file(
         "x/BUILD",