Introduce Target.getPackageMetadata and MacroInstance.getPackageMetad…

…ata and use them where possible ... and since some usages of Target.getPackage only needed the package for the source root, move the source root into Package.Metadata too (which means Package.Metadata now needs a builder). First step towards making lazy macro expansion possible. Working towards #23852 PiperOrigin-RevId: 730977089 Change-Id: I72bc38fb871eef43a81b2e38399e400759f92855
bazelbuild · Feb 25, 2025 · f7f9f96 · f7f9f96
1 parent 825159c
commit f7f9f96
Show file tree

Hide file tree

Showing 29 changed files with 256 additions and 150 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CommonPrerequisiteValidator.java b/src/main/java/com/google/devtools/build/lib/analysis/CommonPrerequisiteValidator.java
@@ -27,7 +27,6 @@
 import com.google.devtools.build.lib.packages.InputFile;
 import com.google.devtools.build.lib.packages.MacroInstance;
 import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
-import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.PackageSpecification.PackageGroupContents;
 import com.google.devtools.build.lib.packages.RawAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
@@ -173,13 +172,13 @@ private void validateDirectPrerequisiteVisibility(
   // MacroInstance.
   private boolean isVisibleToDeclaration(
       ConfiguredTargetAndData prerequisite, Object consumingDeclaration) {
-    Package pkg;
+    PackageIdentifier consumingDeclarationPkg;
     @Nullable MacroInstance declaringMacro;
     if (consumingDeclaration instanceof Rule target) {
-      pkg = target.getPackage();
+      consumingDeclarationPkg = target.getPackageMetadata().packageIdentifier();
       declaringMacro = target.getDeclaringMacro();
     } else if (consumingDeclaration instanceof MacroInstance macroInstance) {
-      pkg = macroInstance.getPackage();
+      consumingDeclarationPkg = macroInstance.getPackageMetadata().packageIdentifier();
       declaringMacro = macroInstance.getParent();
     } else {
       throw new IllegalArgumentException(
@@ -205,7 +204,7 @@ private boolean isVisibleToDeclaration(
             // Macro's location.
             ? declaringMacro.getMacroClass().getDefiningBzlLabel().getPackageIdentifier()
             // BUILD file's location.
-            : pkg.getPackageIdentifier();
+            : consumingDeclarationPkg;
 
     return isVisibleToLocation(prerequisite, ruleTargetLocation);
   }

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -274,7 +274,7 @@ public ConfiguredTarget createConfiguredTarget(
                   analysisEnvironment
                       .getStarlarkSemantics()
                       .getBool(BuildLanguageOptions.EXPERIMENTAL_SIBLING_REPOSITORY_LAYOUT)),
-              inputFile.getPackage().getSourceRoot().get(),
+              inputFile.getPackageMetadata().sourceRoot().get(),
               ConfiguredTargetKey.builder()
                   .setLabel(target.getLabel())
                   .setConfiguration(config)

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java b/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java
@@ -121,7 +121,7 @@ private LocationExpander(
             () -> LocationExpander.buildLocationMap(ruleContext, labelMap, allowData, true)),
         execPaths,
         ruleContext.getConfiguration().legacyExternalRunfiles(),
-        ruleContext.getRule().getPackage().getRepositoryMapping());
+        ruleContext.getRule().getPackageMetadata().repositoryMapping());
   }
 
   /**

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/LocationTemplateContext.java b/src/main/java/com/google/devtools/build/lib/analysis/LocationTemplateContext.java
@@ -85,7 +85,7 @@ public LocationTemplateContext(
             () -> LocationExpander.buildLocationMap(ruleContext, labelMap, allowData, collectSrcs)),
         execPaths,
         ruleContext.getConfiguration().legacyExternalRunfiles(),
-        ruleContext.getRule().getPackage().getRepositoryMapping(),
+        ruleContext.getRule().getPackageMetadata().repositoryMapping(),
         windowsPath,
         ruleContext.getWorkspaceName());
   }

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -2085,7 +2085,7 @@ private void validateExtraPrerequisites(
       // to unconditionally check visibility. See
       // https://github.com/bazelbuild/bazel/issues/12669.
       ConfigSettingVisibilityPolicy configSettingVisibilityPolicy =
-          target.getPackage().getConfigSettingVisibilityPolicy();
+          target.getPackageMetadata().configSettingVisibilityPolicy();
       if (configSettingVisibilityPolicy != ConfigSettingVisibilityPolicy.LEGACY_OFF) {
 
         // Validate config conditions.

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TargetContext.java b/src/main/java/com/google/devtools/build/lib/analysis/TargetContext.java
@@ -91,7 +91,7 @@ public Label getLabel() {
 
   public Label.PackageContext getPackageContext() {
     return Label.PackageContext.of(
-        getLabel().getPackageIdentifier(), target.getPackage().getRepositoryMapping());
+        getLabel().getPackageIdentifier(), target.getPackageMetadata().repositoryMapping());
   }
 
   /**

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java
@@ -991,8 +991,12 @@ public String getBuildFileRelativePath() throws EvalException {
     checkMutable("build_file_path");
     checkDeprecated("ctx.label.package + '/BUILD'", "ctx.build_file_path", getStarlarkSemantics());
 
-    Package pkg = ruleContext.getRule().getPackage();
-    return pkg.getSourceRoot().get().relativize(pkg.getBuildFile().getPath()).getPathString();
+    Package.Metadata pkgMetadata = ruleContext.getRule().getPackageMetadata();
+    return pkgMetadata
+        .sourceRoot()
+        .get()
+        .relativize(pkgMetadata.buildFilename().asPath())
+        .getPathString();
   }
 
   private static void checkDeprecated(String newApi, String oldApi, StarlarkSemantics semantics)

diff --git a/...a/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java b/...a/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java
@@ -213,7 +213,7 @@ private FetchResult fetchInternal(
       }
       mainRepoMapping = mainRepoMappingValue.repositoryMapping();
     } else {
-      mainRepoMapping = rule.getPackage().getRepositoryMapping();
+      mainRepoMapping = rule.getPackageMetadata().repositoryMapping();
     }
 
     IgnoredSubdirectoriesValue ignoredSubdirectories =

diff --git a/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java b/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java
@@ -266,6 +266,11 @@ public Package getPackage() {
     return containingPackage;
   }
 
+  @Override
+  public Package.Metadata getPackageMetadata() {
+    return containingPackage.getMetadata();
+  }
+
   @Override
   public String getTargetKind() {
     return targetKind();

diff --git a/src/main/java/com/google/devtools/build/lib/packages/FileTarget.java b/src/main/java/com/google/devtools/build/lib/packages/FileTarget.java
@@ -31,7 +31,10 @@ public abstract class FileTarget implements Target, FileType.HasFileType {
 
   /** Constructs a file with the given label, which must be in the given package. */
   FileTarget(Package pkg, Label label) {
-    Preconditions.checkArgument(label.getPackageFragment().equals(pkg.getNameFragment()));
+    Preconditions.checkArgument(
+        label
+            .getPackageFragment()
+            .equals(pkg.getMetadata().packageIdentifier().getPackageFragment()));
     this.label = label;
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/packages/InputFile.java b/src/main/java/com/google/devtools/build/lib/packages/InputFile.java
@@ -59,6 +59,11 @@ public Package getPackage() {
     return pkg;
   }
 
+  @Override
+  public Package.Metadata getPackageMetadata() {
+    return pkg.getMetadata();
+  }
+
   public boolean isVisibilitySpecified() {
     return false;
   }
@@ -91,7 +96,7 @@ public License getLicense() {
    * <p>Prefer {@link #getExecPath} if possible.
    */
   public Path getPath() {
-    return pkg.getPackageDirectory().getRelative(label.getName());
+    return pkg.getMetadata().getPackageDirectory().getRelative(label.getName());
   }
 
   /**

diff --git a/src/main/java/com/google/devtools/build/lib/packages/MacroClass.java b/src/main/java/com/google/devtools/build/lib/packages/MacroClass.java
@@ -392,7 +392,7 @@ public static void executeMacroImplementation(
               /* contextDescription= */ "",
               SymbolGenerator.create(
                   MacroInstance.UniqueId.create(
-                      macro.getPackage().getPackageIdentifier(), macro.getId())));
+                      macro.getPackageMetadata().packageIdentifier(), macro.getId())));
       thread.setPrintHandler(Event.makeDebugPrintHandler(builder.getLocalEventHandler()));
 
       // TODO: #19922 - Technically the embedded SymbolGenerator field should use a different key
@@ -477,7 +477,7 @@ private static String getRecursionErrorMessage(MacroInstance macro) {
       ancestor = ancestor.getParent();
     }
     for (MacroInstance item : Lists.reverse(allAncestors)) {
-      String pkg = item.getPackage().getPackageIdentifier().getCanonicalForm();
+      String pkg = item.getPackageMetadata().packageIdentifier().getCanonicalForm();
       String type =
           item.getMacroClass().getDefiningBzlLabel().getCanonicalForm()
               + "%"

diff --git a/src/main/java/com/google/devtools/build/lib/packages/MacroInstance.java b/src/main/java/com/google/devtools/build/lib/packages/MacroInstance.java
@@ -44,7 +44,7 @@ public final class MacroInstance {
 
   // TODO: #19922 - If we want to save the cost of a field here, we can merge pkg and parent into a
   // single field of type Object, and walk up the parent hierarchy to answer getPackage() queries.
-  private final Package pkg;
+  private final Package.Metadata packageMetadata;
 
   @Nullable private final MacroInstance parent;
 
@@ -78,13 +78,13 @@ public final class MacroInstance {
   // storing internal-typed values (the way Rules do) instead of Starlark-typed values, or else just
   // making the constructor private and moving instantiateMacro() to this class.
   public MacroInstance(
-      Package pkg,
+      Package.Metadata packageMetadata,
       @Nullable MacroInstance parent,
       MacroClass macroClass,
       Map<String, Object> attrValues,
       int sameNameDepth)
       throws EvalException {
-    this.pkg = pkg;
+    this.packageMetadata = packageMetadata;
     this.parent = parent;
     this.macroClass = macroClass;
     this.attrValues = ImmutableMap.copyOf(attrValues);
@@ -93,9 +93,9 @@ public MacroInstance(
     Preconditions.checkArgument(macroClass.getAttributes().keySet().equals(attrValues.keySet()));
   }
 
-  /** Returns the package this instance was created in. */
-  public Package getPackage() {
-    return pkg;
+  /** Returns the Package.Metadata of the package in which this instance was created. */
+  public Package.Metadata getPackageMetadata() {
+    return packageMetadata;
   }
 
   /**

diff --git a/src/main/java/com/google/devtools/build/lib/packages/OutputFile.java b/src/main/java/com/google/devtools/build/lib/packages/OutputFile.java
@@ -94,6 +94,11 @@ public final Package getPackage() {
     return generatingRule.getPackage();
   }
 
+  @Override
+  public Package.Metadata getPackageMetadata() {
+    return generatingRule.getPackageMetadata();
+  }
+
   /**
    * A kind of output file.
    *