From 33a2bb79c6d1b03adb3b9c46c658fe18438cc5d1 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 24 Sep 2024 15:02:34 -0700 Subject: [PATCH] Merge TargetDefinitionContext into TargetRegistrationEnvironment The only thing in TargetDefinitionContext is NameConflictException and its subclass. Moving it into TargetRegistrationEnvironment matches how the name conflict checking is now performed there anyway. A subsequent CL may reintroduce TargetDefinitionContext as a branched file from Package.java, factoring with it a number of methods and fields from Package.Builder. Work toward #19922. PiperOrigin-RevId: 678410292 Change-Id: I8e21076a5736a10d8d7053bf39e650e5c7b86a6e --- .../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 | 2 +- .../devtools/build/lib/packages/Package.java | 2 +- .../build/lib/packages/RuleFactory.java | 2 +- .../lib/packages/StarlarkNativeModule.java | 2 +- .../lib/packages/TargetDefinitionContext.java | 62 ------------------- .../TargetRegistrationEnvironment.java | 34 +++++++++- .../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 +- 16 files changed, 45 insertions(+), 79 deletions(-) delete mode 100644 src/main/java/com/google/devtools/build/lib/packages/TargetDefinitionContext.java 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 d77e0fb51861b7..5f1911a7d6f2a5 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.TargetDefinitionContext.MacroNamespaceViolationException; +import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.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 3a2392a8ced02c..ffc1a33e903e85 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.TargetDefinitionContext.NameConflictException; +import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.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 77b6f0bf1ac35b..b737f85588943b 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.TargetDefinitionContext.NameConflictException; +import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.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 844f1f09cac985..5d83bda6ea95f2 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.TargetDefinitionContext.NameConflictException; +import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.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 03561df8b23470..f72b90fe5322da 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.TargetDefinitionContext.NameConflictException; +import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.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 d61ce28086af41..29bff03f80be0a 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.TargetDefinitionContext.NameConflictException; import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.MacroFrame; +import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.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 bb17fb193f7b77..1877897d36894a 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,7 +43,7 @@ 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.TargetDefinitionContext.MacroNamespaceViolationException; +import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.MacroNamespaceViolationException; 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; diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java b/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java index dca0fb68a8e52a..64f3fd3f5ee2bf 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java @@ -21,7 +21,7 @@ import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.packages.Attribute.StarlarkComputedDefaultTemplate.CannotPrecomputeDefaultsException; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; -import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException; +import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.NameConflictException; import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.util.List; diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java index 2aa52486c1510d..d0f253e9f1bc5c 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java @@ -31,7 +31,7 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.io.FileSymlinkException; import com.google.devtools.build.lib.packages.Globber.BadGlobException; -import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException; +import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.NameConflictException; import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code; import com.google.devtools.build.lib.starlarkbuildapi.StarlarkNativeModuleApi; diff --git a/src/main/java/com/google/devtools/build/lib/packages/TargetDefinitionContext.java b/src/main/java/com/google/devtools/build/lib/packages/TargetDefinitionContext.java deleted file mode 100644 index 697bf29ce96f33..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/packages/TargetDefinitionContext.java +++ /dev/null @@ -1,62 +0,0 @@ -// Copyright 2024 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.packages; - -import com.google.devtools.build.lib.cmdline.RepositoryMapping; -import com.google.devtools.build.lib.cmdline.StarlarkThreadContext; - -/** - * A context object, usually stored in a {@link net.starlark.java.eval.StarlarkThread}, upon which - * rules and symbolic macros can be instantiated. - */ -// TODO(#19922): This class isn't really needed until we implement lazy macro evaluation. At that -// point, we'll need to split the concept of a Package.Builder into a separate PackagePiece.Builder -// that represents the object produced by evaluating a macro implementation. Then we can factor the -// accessors and mutations that are common to BUILD files / lazy macros and to symbolic macros into -// this common parent class, while Package.Builder retains the stuff that's prohibited inside -// symbolic macros. -public abstract class TargetDefinitionContext extends StarlarkThreadContext { - - /** - * An exception used when the name of a target or symbolic macro clashes with another entity - * defined in the package. - * - *

Common examples of conflicts include two targets or symbolic macros sharing the same name, - * and one output file being a prefix of another. See {@link Package.Builder#checkForExistingName} - * and {@link Package.Builder#checkRuleAndOutputs} for more details. - */ - public static sealed class NameConflictException extends Exception - permits MacroNamespaceViolationException { - public NameConflictException(String message) { - super(message); - } - } - - /** - * An exception used when the name of a target or submacro declared within a symbolic macro - * violates symbolic macro naming rules. - * - *

An example might be a target named "libfoo" declared within a macro named "foo". - */ - public static final class MacroNamespaceViolationException extends NameConflictException { - public MacroNamespaceViolationException(String message) { - super(message); - } - } - - protected TargetDefinitionContext(RepositoryMapping mainRepoMapping) { - super(() -> mainRepoMapping); - } -} diff --git a/src/main/java/com/google/devtools/build/lib/packages/TargetRegistrationEnvironment.java b/src/main/java/com/google/devtools/build/lib/packages/TargetRegistrationEnvironment.java index 9142c4bf51f41e..1af8a3b9035235 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/TargetRegistrationEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/packages/TargetRegistrationEnvironment.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.cmdline.RepositoryMapping; +import com.google.devtools.build.lib.cmdline.StarlarkThreadContext; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.util.HashMap; @@ -40,10 +41,10 @@ * enforcing naming requirements on them. It is used by {@link Package.Builder} as part of package * construction. */ -// TODO: #19922 - Inheritance from TargetDefinitionContext isn't needed if we have Package.Builder +// TODO: #19922 - Inheritance from StarlarkThreadContext isn't needed if we have Package.Builder // compose this class rather than inherit from it. If we change that, we can make the protected APIs // here public. -public class TargetRegistrationEnvironment extends TargetDefinitionContext { +public class TargetRegistrationEnvironment extends StarlarkThreadContext { /** Used for constructing macro namespace violation error messages. */ static final String MACRO_NAMING_RULES = @@ -168,7 +169,7 @@ private enum NameConflictCheckingPolicy { @Nullable private Map outputFilePrefixes = new HashMap<>(); protected TargetRegistrationEnvironment(RepositoryMapping mainRepositoryMapping) { - super(mainRepositoryMapping); + super(() -> mainRepositoryMapping); } protected Map getTargetMap() { @@ -724,4 +725,31 @@ private static NameConflictException overlappingOutputFilePrefixes( existing.getGeneratingRule().getName())); } } + + /** + * An exception used when the name of a target or symbolic macro clashes with another entity + * defined in the package. + * + *

Common examples of conflicts include two targets or symbolic macros sharing the same name, + * and one output file being a prefix of another. See {@link Package.Builder#checkForExistingName} + * and {@link Package.Builder#checkRuleAndOutputs} for more details. + */ + public static sealed class NameConflictException extends Exception + permits MacroNamespaceViolationException { + public NameConflictException(String message) { + super(message); + } + } + + /** + * An exception used when the name of a target or submacro declared within a symbolic macro + * violates symbolic macro naming rules. + * + *

An example might be a target named "libfoo" declared within a macro named "foo". + */ + public static final class MacroNamespaceViolationException extends NameConflictException { + public MacroNamespaceViolationException(String message) { + super(message); + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java index 4f5f96e6f2664b..d2222d7d7ed583 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java @@ -19,7 +19,7 @@ import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.NullEventHandler; -import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException; +import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.NameConflictException; import com.google.devtools.build.lib.server.FailureDetails; import com.google.devtools.build.lib.server.FailureDetails.PackageLoading; import com.google.devtools.build.lib.vfs.Path; diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java index d3d6ed25fd7014..85ab7ecc94b8d9 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java @@ -23,7 +23,7 @@ import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap; -import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException; +import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.NameConflictException; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.util.Map; import java.util.stream.Collectors; diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java index 78a45ae14f01fb..32aa08e7353745 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java +++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java @@ -29,7 +29,7 @@ import com.google.devtools.build.lib.cmdline.TargetParsingException; import com.google.devtools.build.lib.cmdline.TargetPattern; import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException; -import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException; +import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.NameConflictException; import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.starlarkbuildapi.WorkspaceGlobalsApi; import com.google.devtools.build.lib.vfs.PathFragment; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunction.java index e6d90ddf5763df..00713da010d610 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunction.java @@ -25,7 +25,7 @@ import com.google.devtools.build.lib.packages.ErrorDeterminingRepositoryException; import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.Rule; -import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException; +import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.NameConflictException; import com.google.devtools.build.lib.packages.Type; import com.google.devtools.build.lib.packages.WorkspaceFileValue; import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java index ad60175d327365..1c818e9bbd7f27 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java @@ -38,7 +38,7 @@ import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.packages.RuleClassProvider; -import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException; +import com.google.devtools.build.lib.packages.TargetRegistrationEnvironment.NameConflictException; import com.google.devtools.build.lib.packages.WorkspaceFactory; import com.google.devtools.build.lib.packages.WorkspaceFileValue; import com.google.devtools.build.lib.packages.WorkspaceFileValue.WorkspaceFileKey;