Introduce --experimental_lazy_macro_expansion_packages and allow PackageFunction to assemble packages from package pieces

At present, lazy symbolic macro expansion is incompatible with any package that
calls native.existing_rule() or native.existing_rules() outside of a finalizer
symbolic macro, and so cannot be enabled globally.

Working towards #23852

PiperOrigin-RevId: 792253787
Change-Id: Ifb1fc9f0259b188e2bf33f58cd0a2ec610362db2

Author: tetromino

src/main/java/com/google/devtools/build/lib/packages/BUILD

@@ -93,6 +93,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/io:file_symlink_exception",
         "//src/main/java/com/google/devtools/build/lib/packages/semantics",
+        "//src/main/java/com/google/devtools/build/lib/pkgcache:package_options",
         "//src/main/java/com/google/devtools/build/lib/profiler",
         "//src/main/java/com/google/devtools/build/lib/skyframe:bzl_load_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:detailed_exceptions",

src/main/java/com/google/devtools/build/lib/packages/OutputFile.java

@@ -48,7 +48,7 @@ static OutputFile createExplicit(Label label, Rule generatingRule, String attrNa
   private final String outputKey;
 
   private OutputFile(Label label, Rule generatingRule, String outputKey) {
-    super(generatingRule.getPackage(), label);
+    super(generatingRule.getPackageoid(), label);
     this.generatingRule = generatingRule;
     this.outputKey = outputKey;
   }

src/main/java/com/google/devtools/build/lib/packages/Package.java

@@ -191,8 +191,8 @@ public enum ConfigSettingVisibilityPolicy {
    */
   // TODO(#19922): Better separate fields that must be known a priori from those determined through
   // BUILD evaluation.
-  private Package(Metadata metadata) {
-    super(metadata, new Declarations.Builder());
+  private Package(Metadata metadata, Declarations declarations) {
+    super(metadata, declarations);
   }
 
   // ==== General package metadata accessors ====
@@ -597,6 +597,7 @@ public static Builder newPackageBuilder(
             .configSettingVisibilityPolicy(configSettingVisibilityPolicy)
             .succinctTargetNotFoundErrors(packageSettings.succinctTargetNotFoundErrors())
             .build(),
+        new Declarations.Builder(),
         SymbolGenerator.create(id),
         packageSettings.precomputeTransitiveLoads(),
         noImplicitFileExport,
@@ -611,6 +612,45 @@ public static Builder newPackageBuilder(
         packageLimits);
   }
 
+  /** Returns a new {@link Builder} suitable for constructing a package from package pieces. */
+  public static Builder newPackageFromPackagePiecesBuilder(
+      PackageSettings packageSettings,
+      Metadata metadata,
+      Declarations declarations,
+      boolean noImplicitFileExport,
+      boolean simplifyUnconditionalSelectsInRuleAttrs,
+      RepositoryMapping mainRepositoryMapping,
+      @Nullable Semaphore cpuBoundSemaphore,
+      PackageOverheadEstimator packageOverheadEstimator,
+      @Nullable ImmutableMap<Location, String> generatorMap,
+      @Nullable Globber globber,
+      boolean enableNameConflictChecking,
+      boolean trackFullMacroInformation,
+      PackageLimits packageLimits,
+      InputFile buildFile) {
+    Builder builder =
+        new Builder(
+            metadata,
+            declarations.checkImmutable(),
+            SymbolGenerator.create(metadata.packageIdentifier()),
+            packageSettings.precomputeTransitiveLoads(),
+            noImplicitFileExport,
+            simplifyUnconditionalSelectsInRuleAttrs,
+            mainRepositoryMapping,
+            cpuBoundSemaphore,
+            packageOverheadEstimator,
+            generatorMap,
+            globber,
+            enableNameConflictChecking,
+            trackFullMacroInformation,
+            packageLimits);
+    checkArgument(
+        buildFile.getPackageMetadata().packageIdentifier().equals(metadata.packageIdentifier()));
+    builder.recorder.replaceInputFileUnchecked(buildFile);
+    builder.setBuildFile(buildFile);
+    return builder;
+  }
+
   // ==== Non-trivial nested classes ====
 
   /**
@@ -962,7 +1002,9 @@ protected void packageoidInitializationHook() {
         mergePackageArgsFrom(PackageArgs.builder().setDefaultTestOnly(true));
       }
       // Add target for the BUILD file itself.
-      // (This may be overridden by an exports_file declaration.)
+      // (This may be overridden by an exports_file declaration; or, for a package from package
+      // pieces, by the PackagePiece.ForBuildFile's BUILD file target set in
+      // newPackageFromPackagePiecesBuilder().)
       recorder.addInputFileUnchecked(
           new InputFile(pkg, metadata.buildFileLabel(), metadata.getBuildFileLocation()));
     }
@@ -975,8 +1017,8 @@ protected void packageoidInitializationHook() {
   public static class Builder extends AbstractBuilder {
 
     /**
-     * A bundle of options affecting package construction, that is not specific to any particular
-     * package.
+     * A bundle of statically-defined options affecting package construction, that is not specific
+     * to any particular package and does not change for the lifetime of the server.
      */
     public interface PackageSettings {
       /**
@@ -1063,6 +1105,7 @@ default long maxStarlarkComputationStepsPerPackage() {
 
     private Builder(
         Metadata metadata,
+        Declarations declarations,
         SymbolGenerator<?> symbolGenerator,
         boolean precomputeTransitiveLoads,
         boolean noImplicitFileExport,
@@ -1077,7 +1120,7 @@ private Builder(
         PackageLimits packageLimits) {
       super(
           metadata,
-          new Package(metadata),
+          new Package(metadata, declarations),
           symbolGenerator,
           precomputeTransitiveLoads,
           noImplicitFileExport,
@@ -1134,6 +1177,15 @@ public void addRuleUnchecked(Rule rule) {
       recorder.addRuleUnchecked(rule);
     }
 
+    /** Adds all targets, macros, and Starlark computation steps from a given package piece. */
+    public void addAllFromPackagePiece(PackagePiece packagePiece) throws NameConflictException {
+      // We add the BUILD file in newPackageFromPackagePiecesBuilder(), not here. (We want to ensure
+      // that the package always has a BUILD file target, even if addAllFromPackagePiece would throw
+      // a name conflict.)
+      this.recorder.addAllFromPackagePiece(packagePiece, /* skipBuildFile= */ true);
+      this.computationSteps += packagePiece.getComputationSteps();
+    }
+
     @Override
     public boolean eagerlyExpandMacros() {
       return true;

src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java

@@ -34,6 +34,7 @@
 import com.google.devtools.build.lib.packages.PackageValidator.InvalidPackageException;
 import com.google.devtools.build.lib.packages.PackageValidator.InvalidPackagePieceException;
 import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
+import com.google.devtools.build.lib.pkgcache.PackageOptions.LazyMacroExpansionPackages;
 import com.google.devtools.build.lib.profiler.Profiler;
 import com.google.devtools.build.lib.profiler.ProfilerTask;
 import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
@@ -243,6 +244,37 @@ public Package.Builder newPackageBuilder(
         packageValidator.getPackageLimits());
   }
 
+  // This function is public only for the benefit of skyframe.PackageFunction,
+  // which is morally part of lib.packages, so that it can create empty packages
+  // in case of error before BUILD execution. Do not call it from anywhere else.
+  public Package.Builder newPackageFromPackagePiecesBuilder(
+      Package.Metadata metadata,
+      Package.Declarations declarations,
+      StarlarkSemantics starlarkSemantics,
+      RepositoryMapping mainRepositoryMapping,
+      @Nullable Semaphore cpuBoundSemaphore,
+      @Nullable ImmutableMap<Location, String> generatorMap,
+      @Nullable ConfigSettingVisibilityPolicy configSettingVisibilityPolicy,
+      @Nullable Globber globber,
+      InputFile buildFile) {
+    return Package.newPackageFromPackagePiecesBuilder(
+        packageSettings,
+        metadata,
+        declarations,
+        starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT),
+        starlarkSemantics.getBool(
+            BuildLanguageOptions.INCOMPATIBLE_SIMPLIFY_UNCONDITIONAL_SELECTS_IN_RULE_ATTRS),
+        mainRepositoryMapping,
+        cpuBoundSemaphore,
+        packageOverheadEstimator,
+        generatorMap,
+        globber,
+        /* enableNameConflictChecking= */ true,
+        /* trackFullMacroInformation= */ true,
+        packageValidator.getPackageLimits(),
+        buildFile);
+  }
+
   // This function is public only for the benefit of skyframe.PackageFunction, which is morally part
   // of lib.packages, so that it can create empty package pieces in case of error before BUILD
   // execution. Do not call it from anywhere else.
@@ -337,6 +369,7 @@ public NonSkyframeGlobber createNonSkyframeGlobber(
   public void afterDoneLoadingPackage(
       Package pkg,
       StarlarkSemantics starlarkSemantics,
+      LazyMacroExpansionPackages lazyMacroExpansionPackages,
       Metrics metrics,
       ExtendedEventHandler eventHandler)
       throws InvalidPackageException {
@@ -364,7 +397,8 @@ public void afterDoneLoadingPackage(
                   .build()));
     }
 
-    packageLoadingListener.onLoadingCompleteAndSuccessful(pkg, starlarkSemantics, metrics);
+    packageLoadingListener.onLoadingCompleteAndSuccessful(
+        pkg, starlarkSemantics, lazyMacroExpansionPackages, metrics);
   }
 
   /**
@@ -373,6 +407,9 @@ public void afterDoneLoadingPackage(
    *
    * @throws InvalidPackagePieceException if the package is determined to be invalid
    */
+  // TODO(https://github.com/bazelbuild/bazel/issues/23852): merge with afterDoneLoadingPackagePiece
+  // and perhaps move it all to PackageFunction (combining with existing PackageFunction.compute()
+  // boilerplate such as finishBuild() and event replay). Requires package piece validation.
   public void afterDoneLoadingPackagePiece(
       PackagePiece pkgPiece,
       StarlarkSemantics starlarkSemantics,

src/main/java/com/google/devtools/build/lib/packages/PackageLoadingListener.java

@@ -14,23 +14,26 @@
 
 package com.google.devtools.build.lib.packages;
 
+import com.google.devtools.build.lib.pkgcache.PackageOptions.LazyMacroExpansionPackages;
 import java.util.List;
 import net.starlark.java.eval.StarlarkSemantics;
 
 /** Listener for package-loading events. */
 public interface PackageLoadingListener {
 
-  PackageLoadingListener NOOP_LISTENER = (pkg, semantics, metrics) -> {};
+  PackageLoadingListener NOOP_LISTENER =
+      (pkg, semantics, lazyMacroExpansionPackages, metrics) -> {};
 
   /** Returns a {@link PackageLoadingListener} from a composed of the input listeners. */
   static PackageLoadingListener create(List<PackageLoadingListener> listeners) {
     return switch (listeners.size()) {
       case 0 -> NOOP_LISTENER;
       case 1 -> listeners.get(0);
       default ->
-          (pkg, semantics, metrics) -> {
+          (pkg, semantics, lazyMacroExpansionPackages, metrics) -> {
             for (PackageLoadingListener listener : listeners) {
-              listener.onLoadingCompleteAndSuccessful(pkg, semantics, metrics);
+              listener.onLoadingCompleteAndSuccessful(
+                  pkg, semantics, lazyMacroExpansionPackages, metrics);
             }
           };
     };
@@ -57,7 +60,12 @@ record Metrics(long loadTimeNanos, long globFilesystemOperationCost) {}
    *
    * @param pkg the loaded {@link Package}
    * @param starlarkSemantics are the semantics used to load the package
+   * @param lazyMacroExpansionPackages determines which packages are loaded with lazy symbolic macro
+   *     expansion enabled
    */
   void onLoadingCompleteAndSuccessful(
-      Package pkg, StarlarkSemantics starlarkSemantics, Metrics metrics);
+      Package pkg,
+      StarlarkSemantics starlarkSemantics,
+      LazyMacroExpansionPackages lazyMacroExpansionPackages,
+      Metrics metrics);
 }

src/main/java/com/google/devtools/build/lib/packages/TargetDefinitionContext.java

@@ -156,7 +156,7 @@ public abstract class TargetDefinitionContext extends StarlarkThreadContext {
 
   protected boolean alreadyBuilt = false;
 
-  private long computationSteps = 0;
+  protected long computationSteps = 0;
 
   /** Retrieves this object from a Starlark thread. Returns null if not present. */
   @Nullable
@@ -772,7 +772,7 @@ void setFailureDetailOverride(FailureDetail failureDetail) {
   }
 
   @Nullable
-  FailureDetail getFailureDetail() {
+  public FailureDetail getFailureDetail() {
     if (failureDetailOverride != null) {
       return failureDetailOverride;
     }

src/main/java/com/google/devtools/build/lib/packages/TargetRecorder.java

@@ -454,7 +454,8 @@ public void addMacro(MacroInstance macro) throws NameConflictException {
    * Adds all rules and macros from a given package piece. Intended only for use by skyframe
    * machinery.
    */
-  public void addAllFromPackagePiece(PackagePiece packagePiece) throws NameConflictException {
+  public void addAllFromPackagePiece(PackagePiece packagePiece, boolean skipBuildFile)
+      throws NameConflictException {
     MacroFrame prev;
     if (packagePiece instanceof PackagePiece.ForMacro forMacro) {
       prev = setCurrentMacroFrame(new MacroFrame(forMacro.getEvaluatedMacro()));
@@ -464,7 +465,19 @@ public void addAllFromPackagePiece(PackagePiece packagePiece) throws NameConflic
     for (MacroInstance macro : packagePiece.getMacros()) {
       addMacro(macro);
     }
+    @Nullable
+    InputFile buildFile =
+        packagePiece instanceof PackagePiece.ForBuildFile forBuildFile
+            ? forBuildFile.getBuildFile()
+            : null;
     for (Target target : packagePiece.getTargets().values()) {
+      if (skipBuildFile && target == buildFile) {
+        continue;
+      }
+      if (target instanceof OutputFile) {
+        // Rule output files are recorded as a side effect of addTarget(rule)
+        continue;
+      }
       addTarget(target);
     }
     var unused = setCurrentMacroFrame(prev);

src/main/java/com/google/devtools/build/lib/packages/metrics/BUILD

@@ -25,6 +25,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/collect:extrema",
         "//src/main/java/com/google/devtools/build/lib/packages",
+        "//src/main/java/com/google/devtools/build/lib/pkgcache:package_options",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/common/options",
         "//src/main/java/net/starlark/java/eval",

src/main/java/com/google/devtools/build/lib/packages/metrics/PackageMetricsPackageLoadingListener.java

@@ -15,6 +15,7 @@
 
 import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.PackageLoadingListener;
+import com.google.devtools.build.lib.pkgcache.PackageOptions.LazyMacroExpansionPackages;
 import com.google.protobuf.util.Durations;
 import javax.annotation.concurrent.GuardedBy;
 import net.starlark.java.eval.StarlarkSemantics;
@@ -43,7 +44,10 @@ private PackageMetricsPackageLoadingListener() {
 
   @Override
   public synchronized void onLoadingCompleteAndSuccessful(
-      Package pkg, StarlarkSemantics starlarkSemantics, Metrics metrics) {
+      Package pkg,
+      StarlarkSemantics starlarkSemantics,
+      LazyMacroExpansionPackages lazyMacroExpansionPackages,
+      Metrics metrics) {
     if (recorder == null) {
       // Micro-optimization - no need to track.
       return;