Skip to content

Commit

Permalink
Replace uses of pkg with metadata in builder where possible
Browse files Browse the repository at this point in the history
This is a little more consistent and makes it easier to factor metadata-related accessors in a follow-up.

Added a `Metadata#getName` convenience accessor, like `Package#getName`. Added a `Metadata` field to `Package.Builder`.

Work toward #19922.

PiperOrigin-RevId: 676525169
Change-Id: I9f9451695ed0a6e621d6288b25932412e48a538c
  • Loading branch information
brandjon authored and iancha1992 committed Sep 20, 2024
1 parent 2ec92f3 commit 02170aa
Showing 1 changed file with 20 additions and 12 deletions.
32 changes: 20 additions & 12 deletions src/main/java/com/google/devtools/build/lib/packages/Package.java
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ public PackageIdentifier getPackageIdentifier() {
* may not be unique!
*/
public String getName() {
return metadata.packageIdentifier().getPackageFragment().getPathString();
return metadata.getName();
}

/** Like {@link #getName}, but has type {@code PathFragment}. */
Expand Down Expand Up @@ -1029,6 +1029,9 @@ default boolean precomputeTransitiveLoads() {

private final SymbolGenerator<?> symbolGenerator;

// Same as pkg.metadata.
private final Metadata metadata;

/**
* The output instance for this builder. Needs to be instantiated and available with name info
* throughout initialization. All other settings are applied during {@link #build}. See {@link
Expand Down Expand Up @@ -1310,7 +1313,7 @@ private Builder(

this.workspaceName = Preconditions.checkNotNull(workspaceName);

Metadata metadata =
this.metadata =
new Metadata(
/* packageIdentifier= */ id,
/* buildFilename= */ filename,
Expand All @@ -1335,7 +1338,7 @@ private Builder(
this.precomputeTransitiveLoads = packageSettings.precomputeTransitiveLoads();
this.noImplicitFileExport = noImplicitFileExport;
this.labelConverter = new LabelConverter(id, repositoryMapping);
if (pkg.getName().startsWith("javatests/")) {
if (metadata.getName().startsWith("javatests/")) {
mergePackageArgsFrom(PackageArgs.builder().setDefaultTestOnly(true));
}
this.cpuBoundSemaphore = cpuBoundSemaphore;
Expand Down Expand Up @@ -1507,15 +1510,15 @@ public static Builder fromOrFailDisallowWorkspace(StarlarkThread thread, String
}

PackageIdentifier getPackageIdentifier() {
return pkg.getPackageIdentifier();
return metadata.packageIdentifier();
}

/**
* Determine whether this package should contain build rules (returns {@code false}) or repo
* rules (returns {@code true}).
*/
public boolean isRepoRulePackage() {
return pkg.isRepoRulePackage();
return metadata.isRepoRulePackage();
}

/**
Expand All @@ -1534,7 +1537,7 @@ String getWorkspaceName() {
* this is the name of the module hosting the extension.
*/
Optional<String> getAssociatedModuleName() {
return pkg.metadata.associatedModuleName();
return metadata.associatedModuleName();
}

/**
Expand All @@ -1543,7 +1546,7 @@ Optional<String> getAssociatedModuleName() {
* this is the version of the module hosting the extension.
*/
Optional<String> getAssociatedModuleVersion() {
return pkg.metadata.associatedModuleVersion();
return metadata.associatedModuleVersion();
}

/**
Expand Down Expand Up @@ -1611,7 +1614,7 @@ RepositoryMapping getRepositoryMappingFor(RepositoryName name) {
}

RootedPath getFilename() {
return pkg.metadata.buildFilename();
return metadata.buildFilename();
}

/** Returns the {@link StoredEventHandler} associated with this builder. */
Expand Down Expand Up @@ -1905,7 +1908,7 @@ InputFile createInputFile(String targetName, Location location) throws NameConfl
inputFile = new InputFile(pkg, createLabel(targetName), location);
} catch (LabelSyntaxException e) {
throw new IllegalArgumentException(
"FileTarget in package " + pkg.getName() + " has illegal name: " + targetName, e);
"FileTarget in package " + metadata.getName() + " has illegal name: " + targetName, e);
}

checkTargetName(inputFile);
Expand All @@ -1931,7 +1934,7 @@ void setVisibilityAndLicense(InputFile inputFile, RuleVisibility visibility, Lic
"Can't set visibility for nonexistent FileTarget "
+ filename
+ " in package "
+ pkg.getName()
+ metadata.getName()
+ ".");
}
if (!((InputFile) cacheInstance).isVisibilitySpecified()
Expand All @@ -1949,7 +1952,7 @@ void setVisibilityAndLicense(InputFile inputFile, RuleVisibility visibility, Lic
* @throws LabelSyntaxException if the {@code targetName} is invalid
*/
Label createLabel(String targetName) throws LabelSyntaxException {
return Label.create(pkg.getPackageIdentifier(), targetName);
return Label.create(metadata.packageIdentifier(), targetName);
}

/** Adds a package group to the package. */
Expand Down Expand Up @@ -2353,7 +2356,7 @@ private Builder beforeBuild(boolean discoverAssumedInputFiles) throws NoSuchPack
List<Label> labels = (ruleLabels != null) ? ruleLabels.get(rule) : rule.getLabels();
for (Label label : labels) {
String name = label.getName();
if (label.getPackageIdentifier().equals(pkg.getPackageIdentifier())
if (label.getPackageIdentifier().equals(metadata.packageIdentifier())
&& !targets.containsKey(name)
&& !newInputFiles.containsKey(name)) {
// Check for collision with a macro namespace. Currently this is a linear loop over
Expand Down Expand Up @@ -2767,6 +2770,11 @@ public record Metadata(
Preconditions.checkArgument(isRepoRulePackage == (isWorkspaceFile || isModuleDotBazelFile));
}

/** Returns the name of this package (sans repository), e.g. "foo/bar". */
public String getName() {
return packageIdentifier.getPackageFragment().getPathString();
}

/**
* Returns the directory in which this package's BUILD file resides.
*
Expand Down

0 comments on commit 02170aa

Please sign in to comment.