From c4eba38eacc73c7908eb7fc59261bfc4ce8d821a Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 26 Sep 2024 16:12:42 -0700 Subject: [PATCH] Rename TargetRegistrationEnvironment -> TargetRecorder And rename Package.Builder#regEnv -> Package.Builder#recorder. See discussion in https://github.com/bazelbuild/bazel/commit/11c090f896b25ba13633a7d80d9fd96e5b70116c for (some of) the bikeshedding background. Work toward #19922. PiperOrigin-RevId: 679329986 Change-Id: Idcf5c6f24ab243d496b5ea826d1f00806877898e --- .../lib/analysis/ConfiguredTargetFactory.java | 2 +- .../starlark/StarlarkRuleClassFunctions.java | 2 +- .../bazel/bzlmod/BzlmodRepoRuleCreator.java | 2 +- .../starlark/StarlarkRepositoryModule.java | 2 +- .../build/lib/packages/BuildGlobals.java | 2 +- .../build/lib/packages/MacroClass.java | 4 +- .../devtools/build/lib/packages/Package.java | 102 +++++++++--------- .../build/lib/packages/RuleFactory.java | 2 +- .../lib/packages/StarlarkNativeModule.java | 2 +- ...onEnvironment.java => TargetRecorder.java} | 2 +- .../build/lib/packages/WorkspaceFactory.java | 2 +- .../lib/packages/WorkspaceFactoryHelper.java | 2 +- .../build/lib/packages/WorkspaceGlobals.java | 2 +- .../LocalRepositoryLookupFunction.java | 2 +- .../lib/skyframe/WorkspaceFileFunction.java | 2 +- 15 files changed, 65 insertions(+), 67 deletions(-) rename src/main/java/com/google/devtools/build/lib/packages/{TargetRegistrationEnvironment.java => TargetRecorder.java} (99%) 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 index 5f1911a7d6f2a5..96431febb41845 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java @@ -68,7 +68,7 @@ import com.google.devtools.build.lib.packages.RuleVisibility; import com.google.devtools.build.lib.packages.StarlarkProviderIdentifier; import com.google.devtools.build.lib.packages.Target; -import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.MacroNamespaceViolationException; +import com.google.devtools.build.lib.packages.TargetRecorder.MacroNamespaceViolationException; import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.profiler.memory.CurrentRuleTracker; import com.google.devtools.build.lib.server.FailureDetails.FailAction.Code; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java index ffc1a33e903e85..8869f747316755 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java @@ -100,7 +100,7 @@ import com.google.devtools.build.lib.packages.StarlarkExportable; import com.google.devtools.build.lib.packages.StarlarkProvider; import com.google.devtools.build.lib.packages.StarlarkProviderIdentifier; -import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.NameConflictException; +import com.google.devtools.build.lib.packages.TargetRecorder.NameConflictException; import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.packages.Type; import com.google.devtools.build.lib.packages.Type.LabelClass; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java index 7d91c19e3d0e23..d4023f3c1c936e 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java @@ -27,7 +27,7 @@ import com.google.devtools.build.lib.packages.RuleFactory; import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap; import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException; -import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.NameConflictException; +import com.google.devtools.build.lib.packages.TargetRecorder.NameConflictException; import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.RootedPath; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java index 5d83bda6ea95f2..f759d43aae92dd 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java @@ -40,7 +40,7 @@ import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException; import com.google.devtools.build.lib.packages.RuleFunction; import com.google.devtools.build.lib.packages.StarlarkExportable; -import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.NameConflictException; +import com.google.devtools.build.lib.packages.TargetRecorder.NameConflictException; import com.google.devtools.build.lib.packages.WorkspaceFactoryHelper; import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.starlarkbuildapi.repository.RepositoryModuleApi; diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuildGlobals.java b/src/main/java/com/google/devtools/build/lib/packages/BuildGlobals.java index f72b90fe5322da..82e19241178baf 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/BuildGlobals.java +++ b/src/main/java/com/google/devtools/build/lib/packages/BuildGlobals.java @@ -17,7 +17,7 @@ import com.google.devtools.build.docgen.annot.GlobalMethods.Environment; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; -import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.NameConflictException; +import com.google.devtools.build.lib.packages.TargetRecorder.NameConflictException; import com.google.devtools.build.lib.packages.Type.ConversionException; import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code; import java.util.List; 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 index 29bff03f80be0a..1e1eeea51058b6 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/MacroClass.java +++ b/src/main/java/com/google/devtools/build/lib/packages/MacroClass.java @@ -23,8 +23,8 @@ import com.google.common.collect.Lists; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.Event; -import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.MacroFrame; -import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.NameConflictException; +import com.google.devtools.build.lib.packages.TargetRecorder.MacroFrame; +import com.google.devtools.build.lib.packages.TargetRecorder.NameConflictException; import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.util.ArrayList; 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 e8b87acf562cf8..c312fe5b56c850 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 @@ -43,9 +43,9 @@ import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.packages.Package.Builder.PackageSettings; -import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.MacroFrame; -import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.MacroNamespaceViolationException; -import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.NameConflictException; +import com.google.devtools.build.lib.packages.TargetRecorder.MacroFrame; +import com.google.devtools.build.lib.packages.TargetRecorder.MacroNamespaceViolationException; +import com.google.devtools.build.lib.packages.TargetRecorder.NameConflictException; import com.google.devtools.build.lib.packages.WorkspaceFileValue.WorkspaceFileKey; import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; @@ -587,9 +587,7 @@ public void checkMacroNamespaceCompliance(Target target) throws MacroNamespaceVi String.format( "Target %s declared in symbolic macro '%s' violates macro naming rules and cannot be" + " built. %s", - target.getLabel(), - macroNamespaceViolated, - TargetRegistrationEnvironment.MACRO_NAMING_RULES)); + target.getLabel(), macroNamespaceViolated, TargetRecorder.MACRO_NAMING_RULES)); } } @@ -742,12 +740,12 @@ private void finishInit(Builder builder) { this.workspaceName = builder.workspaceName; this.makeEnv = ImmutableMap.copyOf(builder.makeEnv); - this.targets = ImmutableSortedMap.copyOf(builder.regEnv.getTargetMap()); - this.macros = ImmutableSortedMap.copyOf(builder.regEnv.getMacroMap()); + this.targets = ImmutableSortedMap.copyOf(builder.recorder.getTargetMap()); + this.macros = ImmutableSortedMap.copyOf(builder.recorder.getMacroMap()); this.macroNamespaceViolatingTargets = - ImmutableMap.copyOf(builder.regEnv.getMacroNamespaceViolatingTargets()); + ImmutableMap.copyOf(builder.recorder.getMacroNamespaceViolatingTargets()); this.targetsToDeclaringMacros = - ImmutableSortedMap.copyOf(builder.regEnv.getTargetsToDeclaringMacros()); + ImmutableSortedMap.copyOf(builder.recorder.getTargetsToDeclaringMacros()); this.failureDetail = builder.getFailureDetail(); this.registeredExecutionPlatforms = ImmutableList.copyOf(builder.registeredExecutionPlatforms); this.registeredToolchains = ImmutableList.copyOf(builder.registeredToolchains); @@ -1049,7 +1047,7 @@ default boolean precomputeTransitiveLoads() { // The container object on which targets and macro instances are added and conflicts are // detected. - private final TargetRegistrationEnvironment regEnv = new TargetRegistrationEnvironment(); + private final TargetRecorder recorder = new TargetRecorder(); // Initialized from outside but also potentially set by `workspace()` function in WORKSPACE // file. @@ -1229,7 +1227,7 @@ private Builder( // Add target for the BUILD file itself. // (This may be overridden by an exports_file declaration.) - regEnv.addInputFileUnchecked( + recorder.addInputFileUnchecked( new InputFile( pkg, buildFileLabel, @@ -1284,8 +1282,8 @@ public static Builder fromOrFail( boolean bad = false; if (ctx instanceof Builder builder) { bad |= !allowBuild && !builder.isRepoRulePackage(); - bad |= !allowFinalizers && builder.regEnv.currentlyInFinalizer(); - bad |= !allowNonFinalizerSymbolicMacros && builder.regEnv.currentlyInNonFinalizerMacro(); + bad |= !allowFinalizers && builder.recorder.currentlyInFinalizer(); + bad |= !allowNonFinalizerSymbolicMacros && builder.recorder.currentlyInNonFinalizerMacro(); bad |= !allowWorkspace && builder.isRepoRulePackage(); if (!bad) { return builder; @@ -1551,7 +1549,7 @@ void setComputationSteps(long n) { } public boolean containsErrors() { - return regEnv.containsErrors(); + return recorder.containsErrors(); } /** @@ -1561,7 +1559,7 @@ public boolean containsErrors() { * the {@link Package.Builder}. The event should include a {@link FailureDetail}. */ public void setContainsErrors() { - regEnv.setContainsErrors(); + recorder.setContainsErrors(); } void setIOException(IOException e, String message, DetailedExitCode detailedExitCode) { @@ -1653,7 +1651,7 @@ public boolean simplifyUnconditionalSelectsInRuleAttrs() { */ @Nullable public MacroInstance currentMacro() { - MacroFrame frame = regEnv.getCurrentMacroFrame(); + MacroFrame frame = recorder.getCurrentMacroFrame(); return frame == null ? null : frame.macroInstance; } @@ -1693,25 +1691,25 @@ MacroInstance createMacro( @Nullable public MacroFrame getCurrentMacroFrame() { - return regEnv.getCurrentMacroFrame(); + return recorder.getCurrentMacroFrame(); } @Nullable public MacroFrame setCurrentMacroFrame(@Nullable MacroFrame frame) { - return regEnv.setCurrentMacroFrame(frame); + return recorder.setCurrentMacroFrame(frame); } public boolean currentlyInNonFinalizerMacro() { - return regEnv.currentlyInNonFinalizerMacro(); + return recorder.currentlyInNonFinalizerMacro(); } @Nullable public Target getTarget(String name) { - return regEnv.getTarget(name); + return recorder.getTarget(name); } public Set getTargets() { - return regEnv.getTargets(); + return recorder.getTargets(); } void replaceTarget(Target newTarget) { @@ -1720,7 +1718,7 @@ void replaceTarget(Target newTarget) { "Replacement target belongs to package '%s', expected '%s'", newTarget.getPackage(), pkg); - regEnv.replaceTarget(newTarget); + recorder.replaceTarget(newTarget); } /** @@ -1734,9 +1732,9 @@ void replaceTarget(Target newTarget) { Map getRulesSnapshotView() { if (rulesSnapshotViewForFinalizers != null) { return rulesSnapshotViewForFinalizers; - } else if (regEnv.getTargetMap() instanceof SnapshottableBiMap) { + } else if (recorder.getTargetMap() instanceof SnapshottableBiMap) { return Maps.transformValues( - ((SnapshottableBiMap) regEnv.getTargetMap()).getTrackedSnapshot(), + ((SnapshottableBiMap) recorder.getTargetMap()).getTrackedSnapshot(), target -> (Rule) target); } else { throw new IllegalStateException( @@ -1757,7 +1755,7 @@ Rule getNonFinalizerInstantiatedRule(String name) { if (rulesSnapshotViewForFinalizers != null) { return rulesSnapshotViewForFinalizers.get(name); } else { - Target target = regEnv.getTargetMap().get(name); + Target target = recorder.getTargetMap().get(name); return target instanceof Rule ? (Rule) target : null; } } @@ -1776,7 +1774,7 @@ Rule getNonFinalizerInstantiatedRule(String name) { * @throws IllegalArgumentException if the name is not a valid label */ InputFile createInputFile(String targetName, Location location) throws NameConflictException { - Target existing = regEnv.getTargetMap().get(targetName); + Target existing = recorder.getTargetMap().get(targetName); if (existing instanceof InputFile) { return (InputFile) existing; // idempotent @@ -1790,7 +1788,7 @@ InputFile createInputFile(String targetName, Location location) throws NameConfl "FileTarget in package " + metadata.getName() + " has illegal name: " + targetName, e); } - regEnv.addTarget(inputFile); + recorder.addTarget(inputFile); return inputFile; } @@ -1806,7 +1804,7 @@ InputFile createInputFile(String targetName, Location location) throws NameConfl // visibility of :BUILD inside a symbolic macro. void setVisibilityAndLicense(InputFile inputFile, RuleVisibility visibility, License license) { String filename = inputFile.getName(); - Target cacheInstance = regEnv.getTargetMap().get(filename); + Target cacheInstance = recorder.getTargetMap().get(filename); if (!(cacheInstance instanceof InputFile)) { throw new IllegalArgumentException( "Can't set visibility for nonexistent FileTarget " @@ -1818,7 +1816,7 @@ void setVisibilityAndLicense(InputFile inputFile, RuleVisibility visibility, Lic if (!((InputFile) cacheInstance).isVisibilitySpecified() || cacheInstance.getVisibility() != visibility || !Objects.equals(cacheInstance.getLicense(), license)) { - regEnv.replaceInputFileUnchecked( + recorder.replaceInputFileUnchecked( new VisibilityLicenseSpecifiedInputFile( pkg, cacheInstance.getLabel(), cacheInstance.getLocation(), visibility, license)); } @@ -1853,7 +1851,7 @@ void addPackageGroup( repoRootMeansCurrentRepo, eventHandler, location); - regEnv.addTarget(group); + recorder.addTarget(group); if (group.containsErrors()) { setContainsErrors(); @@ -1903,7 +1901,7 @@ void addEnvironmentGroup( EnvironmentGroup group = new EnvironmentGroup(createLabel(name), pkg, environments, defaults, location); - regEnv.addTarget(group); + recorder.addTarget(group); // Invariant: once group is inserted into targets, it must also: // (a) be inserted into environmentGroups, or @@ -1938,29 +1936,29 @@ void addEnvironmentGroup( // For Package deserialization. void disableNameConflictChecking() { - regEnv.disableNameConflictChecking(); + recorder.disableNameConflictChecking(); } public void addRuleUnchecked(Rule rule) { Preconditions.checkArgument(rule.getPackage() == pkg); - regEnv.addRuleUnchecked(rule); + recorder.addRuleUnchecked(rule); } public void addRule(Rule rule) throws NameConflictException { Preconditions.checkArgument(rule.getPackage() == pkg); - regEnv.addRule(rule); + recorder.addRule(rule); } public void addMacro(MacroInstance macro) throws NameConflictException { Preconditions.checkState( !isRepoRulePackage(), "Cannot instantiate symbolic macros in this context"); - regEnv.addMacro(macro); + recorder.addMacro(macro); unexpandedMacros.add(macro.getId()); } // For Package deserialization. void putAllMacroNamespaceViolatingTargets(Map macroNamespaceViolatingTargets) { - regEnv.putAllMacroNamespaceViolatingTargets(macroNamespaceViolatingTargets); + recorder.putAllMacroNamespaceViolatingTargets(macroNamespaceViolatingTargets); } /** @@ -2015,7 +2013,7 @@ public void expandAllRemainingMacros(StarlarkSemantics semantics) throws Interru if (!unexpandedMacros.isEmpty()) { Preconditions.checkState( unexpandedMacros.stream() - .allMatch(id -> regEnv.getMacroMap().get(id).getMacroClass().isFinalizer()), + .allMatch(id -> recorder.getMacroMap().get(id).getMacroClass().isFinalizer()), "At the beginning of finalizer expansion, unexpandedMacros must contain only" + " finalizers"); @@ -2024,14 +2022,14 @@ public void expandAllRemainingMacros(StarlarkSemantics semantics) throws Interru // include any rule instantiated by a finalizer or macro called from a finalizer. if (rulesSnapshotViewForFinalizers == null) { Preconditions.checkState( - regEnv.getTargetMap() instanceof SnapshottableBiMap, + recorder.getTargetMap() instanceof SnapshottableBiMap, "Cannot call expandAllRemainingMacros() after beforeBuild() has been called"); rulesSnapshotViewForFinalizers = getRulesSnapshotView(); } while (!unexpandedMacros.isEmpty()) { // NB: collection mutated by body String id = unexpandedMacros.iterator().next(); - MacroInstance macro = regEnv.getMacroMap().get(id); + MacroInstance macro = recorder.getMacroMap().get(id); MacroClass.executeMacroImplementation(macro, this, semantics); } } @@ -2064,8 +2062,8 @@ private Builder beforeBuild(boolean discoverAssumedInputFiles) throws NoSuchPack // map to the SnapshottableBiMap's underlying bimap and thus stop tracking insertion order. // After this point, snapshots of targets should no longer be used, and any further // getRulesSnapshotView calls will throw. - if (regEnv.getTargetMap() instanceof SnapshottableBiMap) { - regEnv.unwrapSnapshottableBiMap(); + if (recorder.getTargetMap() instanceof SnapshottableBiMap) { + recorder.unwrapSnapshottableBiMap(); rulesSnapshotViewForFinalizers = null; } @@ -2074,7 +2072,7 @@ private Builder beforeBuild(boolean discoverAssumedInputFiles) throws NoSuchPack // until now to obtain the current instance from the targets map. pkg.buildFile = Preconditions.checkNotNull( - (InputFile) regEnv.getTargetMap().get(buildFileLabel.getName())); + (InputFile) recorder.getTargetMap().get(buildFileLabel.getName())); // TODO(bazel-team): We run testSuiteImplicitTestsAccumulator here in beforeBuild(), but what // if one of the accumulated tests is later removed in PackageFunction, between the call to @@ -2087,7 +2085,7 @@ private Builder beforeBuild(boolean discoverAssumedInputFiles) throws NoSuchPack testSuiteImplicitTestsAccumulator.clearAccumulatedTests(); Map newInputFiles = new HashMap<>(); - for (Rule rule : regEnv.getRules()) { + for (Rule rule : recorder.getRules()) { if (discoverAssumedInputFiles) { // Labels mentioned by a rule that refer to an unknown target in the current package are // assumed to be InputFiles, unless they overlap a namespace owned by a macro. Create @@ -2095,25 +2093,25 @@ private Builder beforeBuild(boolean discoverAssumedInputFiles) throws NoSuchPack // since we don't want the evaluation of the macro to affect the semantics of whether or // not this target was created (i.e. all implicitly created files are knowable without // necessarily evaluating symbolic macros). - if (regEnv.isRuleCreatedInMacro(rule)) { + if (recorder.isRuleCreatedInMacro(rule)) { continue; } // We use a temporary map, newInputFiles, to avoid concurrent modification to this.targets // while iterating (via getRules() above). - List