From 405d1a3a072bfe1acd3e779bc9a139baf8545796 Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 20 Dec 2023 10:49:19 -0800 Subject: [PATCH] Move more fields into Metadata Each of the three affected fields is initialized when the builder is constructed, so they may as well go alongside the other fields that are determined prior to BUILD evaluation. The affected fields are also conceivably useful to symbolic macros as well, so they probably belong on Metadata for that reason too. Work toward #19922. PiperOrigin-RevId: 592602808 Change-Id: Ia171bb67ff6d386cc9687ad46e05e8c6aeade1f8 --- .../devtools/build/lib/packages/Package.java | 66 ++++++++----------- 1 file changed, 29 insertions(+), 37 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java index 664a9c31c2561e..33afa49bb0c431 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Package.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java @@ -124,25 +124,6 @@ private NameConflictException(String message) { } } - /** Governs the error message behavior of {@link Package#getTarget}. */ - // TODO(bazel-team): Arguably, this could be replaced by a boolean param to getTarget(), or some - // separate action taken by the caller. But there's a lot of call sites that would need updating. - private final boolean succinctTargetNotFoundErrors; - - /** - * The name of the Bzlmod module associated with the repo this package is in. If this package is - * not from a Bzlmod repo, this is empty. For repos generated by module extensions, this is the - * name of the module hosting the extension. - */ - private final Optional associatedModuleName; - - /** - * The version of the Bzlmod module associated with the repo this package is in. If this package - * is not from a Bzlmod repo, this is empty. For repos generated by module extensions, this is the - * version of the module hosting the extension. - */ - private final Optional associatedModuleVersion; - /** The collection of all targets defined in this package, indexed by name. */ private ImmutableSortedMap targets; @@ -229,16 +210,8 @@ public long getComputationSteps() { */ // TODO(#19922): Better separate fields that must be known a priori from those determined through // BUILD evaluation. - private Package( - Metadata metadata, - Optional associatedModuleName, - Optional associatedModuleVersion, - boolean succinctTargetNotFoundErrors) { + private Package(Metadata metadata) { this.metadata = metadata; - // TODO(#19922): move these three fields into Metadata - this.associatedModuleName = associatedModuleName; - this.associatedModuleVersion = associatedModuleVersion; - this.succinctTargetNotFoundErrors = succinctTargetNotFoundErrors; } /** Returns this package's identifier. */ @@ -630,7 +603,7 @@ public Target getTarget(String targetName) throws NoSuchTargetException { throw new IllegalArgumentException(targetName, e); } - if (succinctTargetNotFoundErrors) { + if (metadata.succinctTargetNotFoundErrors) { throw new NoSuchTargetException( label, String.format("target '%s' not declared in package '%s'", targetName, getName())); } else { @@ -1011,12 +984,11 @@ public T intern(T sample) { metadata.packageIdentifier = Preconditions.checkNotNull(id); metadata.workspaceName = Preconditions.checkNotNull(workspaceName); metadata.repositoryMapping = Preconditions.checkNotNull(repositoryMapping); - this.pkg = - new Package( - metadata, - associatedModuleName, - associatedModuleVersion, - packageSettings.succinctTargetNotFoundErrors()); + metadata.associatedModuleName = Preconditions.checkNotNull(associatedModuleName); + metadata.associatedModuleVersion = Preconditions.checkNotNull(associatedModuleVersion); + metadata.succinctTargetNotFoundErrors = packageSettings.succinctTargetNotFoundErrors(); + + this.pkg = new Package(metadata); this.precomputeTransitiveLoads = packageSettings.precomputeTransitiveLoads(); this.noImplicitFileExport = noImplicitFileExport; this.labelConverter = new LabelConverter(id, repositoryMapping); @@ -1049,7 +1021,7 @@ String getPackageWorkspaceName() { * this is the name of the module hosting the extension. */ Optional getAssociatedModuleName() { - return pkg.associatedModuleName; + return pkg.metadata.associatedModuleName; } /** @@ -1058,7 +1030,7 @@ Optional getAssociatedModuleName() { * this is the version of the module hosting the extension. */ Optional getAssociatedModuleVersion() { - return pkg.associatedModuleVersion; + return pkg.metadata.associatedModuleVersion; } /** @@ -2029,6 +2001,20 @@ public RepositoryMapping getRepositoryMapping() { return repositoryMapping; } + /** + * The name of the Bzlmod module associated with the repo this package is in. If this package is + * not from a Bzlmod repo, this is empty. For repos generated by module extensions, this is the + * name of the module hosting the extension. + */ + private Optional associatedModuleName; + + /** + * The version of the Bzlmod module associated with the repo this package is in. If this package + * is not from a Bzlmod repo, this is empty. For repos generated by module extensions, this is + * the version of the module hosting the extension. + */ + private Optional associatedModuleVersion; + private ConfigSettingVisibilityPolicy configSettingVisibilityPolicy; /** Returns the visibility policy. */ @@ -2041,6 +2027,12 @@ public ConfigSettingVisibilityPolicy getConfigSettingVisibilityPolicy() { @Nullable private ImmutableList directLoads; @Nullable private ImmutableList