From b23b8c778f2e077f1830f63b37083d9318324e5b Mon Sep 17 00:00:00 2001 From: adonovan Date: Thu, 12 Dec 2019 11:42:14 -0800 Subject: [PATCH] bazel syntax: delete SkylarkUtils The Phase is now recorded in lib.packages.BazelStarlarkContext. It is now always explicit whereas, before, failure to set it caused it to assume a default of ANALYSIS. In SkylarkRepositoryFunction, the "fetch" thread now uses phase=LOADING where before it has none (=ANALYSIS), which I suspect was an oversight. PiperOrigin-RevId: 285236359 --- .../analysis/ConfiguredRuleClassProvider.java | 4 +- .../lib/analysis/skylark/SkylarkAttr.java | 28 ++++----- .../skylark/SkylarkRuleClassFunctions.java | 7 +-- .../SkylarkRuleConfiguredTargetUtil.java | 6 +- .../skylark/SkylarkRepositoryFunction.java | 7 ++- .../skylark/SkylarkRepositoryModule.java | 13 ++-- .../lib/packages/BazelStarlarkContext.java | 44 ++++++++++++- .../build/lib/packages/PackageFactory.java | 8 +-- .../lib/packages/SkylarkNativeModule.java | 16 +++-- .../lib/packages/StarlarkBuildLibrary.java | 2 +- .../build/lib/packages/WorkspaceFactory.java | 4 +- .../lib/skyframe/SkylarkAspectFactory.java | 3 +- .../google/devtools/build/lib/syntax/BUILD | 1 - .../build/lib/syntax/SkylarkUtils.java | 62 ------------------- ...kylarkRuleImplementationFunctionsTest.java | 2 +- .../lib/skylark/util/SkylarkTestCase.java | 5 +- 16 files changed, 87 insertions(+), 125 deletions(-) delete mode 100644 src/main/java/com/google/devtools/build/lib/syntax/SkylarkUtils.java diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java index 09fb65b8c41c21..b7c205eb5c85b7 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java @@ -59,8 +59,6 @@ import com.google.devtools.build.lib.syntax.ClassObject; import com.google.devtools.build.lib.syntax.Module; import com.google.devtools.build.lib.syntax.Mutability; -import com.google.devtools.build.lib.syntax.SkylarkUtils; -import com.google.devtools.build.lib.syntax.SkylarkUtils.Phase; import com.google.devtools.build.lib.syntax.StarlarkSemantics; import com.google.devtools.build.lib.syntax.StarlarkThread; import com.google.devtools.build.lib.syntax.StarlarkThread.Extension; @@ -819,6 +817,7 @@ public StarlarkThread createRuleClassStarlarkThread( .build(); new BazelStarlarkContext( + BazelStarlarkContext.Phase.LOADING, toolsRepository, configurationFragmentMap, repoMapping, @@ -826,7 +825,6 @@ public StarlarkThread createRuleClassStarlarkThread( /* analysisRuleLabel= */ null) .storeInThread(thread); - SkylarkUtils.setPhase(thread, Phase.LOADING); return thread; } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java index e7ef9b6fc4ee5b..8b2cdd17d82274 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java @@ -51,7 +51,6 @@ import com.google.devtools.build.lib.syntax.Printer; import com.google.devtools.build.lib.syntax.Sequence; import com.google.devtools.build.lib.syntax.SkylarkType; -import com.google.devtools.build.lib.syntax.SkylarkUtils; import com.google.devtools.build.lib.syntax.Starlark; import com.google.devtools.build.lib.syntax.StarlarkFunction; import com.google.devtools.build.lib.syntax.StarlarkThread; @@ -458,7 +457,7 @@ public Descriptor intAttribute( StarlarkThread thread) throws EvalException { // TODO(bazel-team): Replace literal strings with constants. - SkylarkUtils.checkLoadingOrWorkspacePhase(thread, "attr.int", ast.getLocation()); + BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase("attr.int"); return createAttrDescriptor( "int", EvalUtils.optionMap( @@ -477,7 +476,7 @@ public Descriptor stringAttribute( FuncallExpression ast, StarlarkThread thread) throws EvalException { - SkylarkUtils.checkLoadingOrWorkspacePhase(thread, "attr.string", ast.getLocation()); + BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase("attr.string"); return createAttrDescriptor( "string", EvalUtils.optionMap( @@ -503,7 +502,7 @@ public Descriptor labelAttribute( FuncallExpression ast, StarlarkThread thread) throws EvalException { - SkylarkUtils.checkLoadingOrWorkspacePhase(thread, "attr.label", ast.getLocation()); + BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase("attr.label"); try { ImmutableAttributeFactory attribute = createAttributeFactory( @@ -550,7 +549,7 @@ public Descriptor stringListAttribute( FuncallExpression ast, StarlarkThread thread) throws EvalException { - SkylarkUtils.checkLoadingOrWorkspacePhase(thread, "attr.string_list", ast.getLocation()); + BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase("attr.string_list"); return createAttrDescriptor( "string_list", EvalUtils.optionMap( @@ -578,7 +577,7 @@ public Descriptor intListAttribute( FuncallExpression ast, StarlarkThread thread) throws EvalException { - SkylarkUtils.checkLoadingOrWorkspacePhase(thread, "attr.int_list", ast.getLocation()); + BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase("attr.int_list"); return createAttrDescriptor( "int_list", EvalUtils.optionMap( @@ -612,7 +611,7 @@ public Descriptor labelListAttribute( FuncallExpression ast, StarlarkThread thread) throws EvalException { - SkylarkUtils.checkLoadingOrWorkspacePhase(thread, "attr.label_list", ast.getLocation()); + BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase("attr.label_list"); Dict kwargs = EvalUtils.optionMap( thread, @@ -661,8 +660,7 @@ public Descriptor labelKeyedStringDictAttribute( FuncallExpression ast, StarlarkThread thread) throws EvalException { - SkylarkUtils.checkLoadingOrWorkspacePhase( - thread, "attr.label_keyed_string_dict", ast.getLocation()); + BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase("attr.label_keyed_string_dict"); Dict kwargs = EvalUtils.optionMap( thread, @@ -705,7 +703,7 @@ public Descriptor labelKeyedStringDictAttribute( public Descriptor boolAttribute( Boolean defaultO, String doc, Boolean mandatory, FuncallExpression ast, StarlarkThread thread) throws EvalException { - SkylarkUtils.checkLoadingOrWorkspacePhase(thread, "attr.bool", ast.getLocation()); + BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase("attr.bool"); return createAttrDescriptor( "bool", EvalUtils.optionMap( @@ -719,7 +717,7 @@ public Descriptor boolAttribute( public Descriptor outputAttribute( Object defaultO, String doc, Boolean mandatory, FuncallExpression ast, StarlarkThread thread) throws EvalException { - SkylarkUtils.checkLoadingOrWorkspacePhase(thread, "attr.output", ast.getLocation()); + BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase("attr.output"); return createNonconfigurableAttrDescriptor( "output", @@ -740,7 +738,7 @@ public Descriptor outputListAttribute( FuncallExpression ast, StarlarkThread thread) throws EvalException { - SkylarkUtils.checkLoadingOrWorkspacePhase(thread, "attr.output_list", ast.getLocation()); + BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase("attr.output_list"); return createAttrDescriptor( "output_list", @@ -769,7 +767,7 @@ public Descriptor stringDictAttribute( FuncallExpression ast, StarlarkThread thread) throws EvalException { - SkylarkUtils.checkLoadingOrWorkspacePhase(thread, "attr.string_dict", ast.getLocation()); + BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase("attr.string_dict"); return createAttrDescriptor( "string_dict", EvalUtils.optionMap( @@ -797,7 +795,7 @@ public Descriptor stringListDictAttribute( FuncallExpression ast, StarlarkThread thread) throws EvalException { - SkylarkUtils.checkLoadingOrWorkspacePhase(thread, "attr.string_list_dict", ast.getLocation()); + BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase("attr.string_list_dict"); return createAttrDescriptor( "string_list_dict", EvalUtils.optionMap( @@ -819,7 +817,7 @@ public Descriptor stringListDictAttribute( public Descriptor licenseAttribute( Object defaultO, String doc, Boolean mandatory, FuncallExpression ast, StarlarkThread thread) throws EvalException { - SkylarkUtils.checkLoadingOrWorkspacePhase(thread, "attr.license", ast.getLocation()); + BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase("attr.license"); return createNonconfigurableAttrDescriptor( "license", EvalUtils.optionMap( diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java index da65072de959c3..98d24d0fc00ea9 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java @@ -89,7 +89,6 @@ import com.google.devtools.build.lib.syntax.Printer; import com.google.devtools.build.lib.syntax.Sequence; import com.google.devtools.build.lib.syntax.SkylarkType; -import com.google.devtools.build.lib.syntax.SkylarkUtils; import com.google.devtools.build.lib.syntax.Starlark; import com.google.devtools.build.lib.syntax.StarlarkFunction; import com.google.devtools.build.lib.syntax.StarlarkThread; @@ -297,9 +296,9 @@ public BaseFunction rule( FuncallExpression ast, StarlarkThread thread) throws EvalException { - SkylarkUtils.checkLoadingOrWorkspacePhase(thread, "rule", ast.getLocation()); - BazelStarlarkContext bazelContext = BazelStarlarkContext.from(thread); + bazelContext.checkLoadingOrWorkspacePhase("rule"); + // analysis_test=true implies test=true. test |= Boolean.TRUE.equals(analysisTest); @@ -644,7 +643,7 @@ public String getName() { public Object call(Object[] args, FuncallExpression astForLocation, StarlarkThread thread) throws EvalException, InterruptedException, ConversionException { Location loc = astForLocation.getLocation(); - SkylarkUtils.checkLoadingPhase(thread, getName(), loc); + BazelStarlarkContext.from(thread).checkLoadingPhase(getName()); if (ruleClass == null) { throw new EvalException(loc, "Invalid rule class hasn't been exported by a bzl file"); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java index b3459a7b4a38b2..27952e18e72692 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java @@ -102,12 +102,12 @@ public static ConfiguredTarget buildRule( StarlarkThread.builder(mutability) .setSemantics(starlarkSemantics) .setEventHandler(ruleContext.getAnalysisEnvironment().getEventHandler()) - .build(); // NB: loading phase functions are not available: this is analysis already, - // so we do *not* setLoadingPhase(). + .build(); new BazelStarlarkContext( + BazelStarlarkContext.Phase.ANALYSIS, toolsRepository, - /* fragmentNameToClass= */ null, + /*fragmentNameToClass=*/ null, ruleContext.getTarget().getPackage().getRepositoryMapping(), ruleContext.getSymbolGenerator(), ruleContext.getLabel()) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java index a1b8a0ab4541e7..3d3b3ed034dcd8 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java @@ -130,11 +130,12 @@ public RepositoryDirectoryValue.Builder fetch( // The fetch phase does not need the tools repository // or the fragment map because it happens before analysis. new BazelStarlarkContext( - /* toolsRepository = */ null, - /* fragmentNameToClass = */ null, + BazelStarlarkContext.Phase.LOADING, // ("fetch") + /*toolsRepository=*/ null, + /*fragmentNameToClass=*/ null, rule.getPackage().getRepositoryMapping(), new SymbolGenerator<>(key), - /* analysisRuleLabel= */ null) + /*analysisRuleLabel=*/ null) .storeInThread(thread); SkylarkRepositoryContext skylarkRepositoryContext = diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java index fdf1b9ed386fa6..8649a5c7caca9a 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.packages.AttributeValueSource; +import com.google.devtools.build.lib.packages.BazelStarlarkContext; import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.Package.NameConflictException; import com.google.devtools.build.lib.packages.PackageFactory; @@ -47,10 +48,8 @@ import com.google.devtools.build.lib.syntax.Identifier; import com.google.devtools.build.lib.syntax.Printer; import com.google.devtools.build.lib.syntax.Sequence; -import com.google.devtools.build.lib.syntax.SkylarkUtils; import com.google.devtools.build.lib.syntax.Starlark; import com.google.devtools.build.lib.syntax.StarlarkFunction; -import com.google.devtools.build.lib.syntax.StarlarkSemantics; import com.google.devtools.build.lib.syntax.StarlarkThread; import java.util.Map; @@ -70,16 +69,15 @@ public BaseFunction repositoryRule( Boolean remotable, String doc, FuncallExpression ast, - StarlarkThread funcallThread) + StarlarkThread thread) throws EvalException { - SkylarkUtils.checkLoadingOrWorkspacePhase(funcallThread, "repository_rule", ast.getLocation()); - StarlarkSemantics semantics = funcallThread.getSemantics(); + BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase("repository_rule"); // We'll set the name later, pass the empty string for now. RuleClass.Builder builder = new RuleClass.Builder("", RuleClassType.WORKSPACE, true); builder.addOrOverrideAttribute(attr("$local", BOOLEAN).defaultValue(local).build()); builder.addOrOverrideAttribute(attr("$configure", BOOLEAN).defaultValue(configure).build()); - if (semantics.experimentalRepoRemoteExec()) { + if (thread.getSemantics().experimentalRepoRemoteExec()) { builder.addOrOverrideAttribute(attr("$remotable", BOOLEAN).defaultValue(remotable).build()); BaseRuleClasses.execPropertiesAttribute(builder); } @@ -99,8 +97,7 @@ public BaseFunction repositoryRule( } builder.setConfiguredTargetFunction(implementation); builder.setRuleDefinitionEnvironmentLabelAndHashCode( - (Label) funcallThread.getGlobals().getLabel(), - funcallThread.getTransitiveContentHashCode()); + (Label) thread.getGlobals().getLabel(), thread.getTransitiveContentHashCode()); builder.setWorkspaceOnly(); return new RepositoryRuleFunction(builder, ast.getLocation()); } diff --git a/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkContext.java b/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkContext.java index 9bd2299ac2d1bf..df8d5e4ad9cd25 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkContext.java +++ b/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkContext.java @@ -19,11 +19,19 @@ import com.google.devtools.build.lib.analysis.RuleDefinitionContext; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.RepositoryName; +import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.StarlarkThread; import javax.annotation.Nullable; /** Contextual information associated with each Starlark thread created by Bazel. */ -public class BazelStarlarkContext implements RuleDefinitionContext, Label.HasRepoMapping { +public final class BazelStarlarkContext implements RuleDefinitionContext, Label.HasRepoMapping { + + /** The phase to which this Starlark thread belongs. */ + public enum Phase { + WORKSPACE, + LOADING, + ANALYSIS + } /** Return the Bazel information associated with the specified Starlark thread. */ public static BazelStarlarkContext from(StarlarkThread thread) { @@ -36,6 +44,7 @@ public void storeInThread(StarlarkThread thread) { thread.setThreadLocal(Label.HasRepoMapping.class, this); } + private final Phase phase; private final String toolsRepository; @Nullable private final ImmutableMap> fragmentNameToClass; private final ImmutableMap repoMapping; @@ -43,6 +52,7 @@ public void storeInThread(StarlarkThread thread) { @Nullable private final Label analysisRuleLabel; /** + * @param phase the phase to which this Starlark thread belongs * @param toolsRepository the name of the tools repository, such as "@bazel_tools" * @param fragmentNameToClass a map from configuration fragment name to configuration fragment * class, such as "apple" to AppleConfiguration.class @@ -56,15 +66,18 @@ public void storeInThread(StarlarkThread thread) { // analysis, workspace, implicit outputs, computed defaults, etc), perhaps by splitting these into // separate structs, exactly one of which is populated (plus the common fields). And eliminate // SkylarkUtils.Phase. + // TODO(adonovan): move PackageFactory.PackageContext in here, for loading-phase threads. // TODO(adonovan): add a PackageIdentifier here, for use by the Starlark Label function. // TODO(adonovan): is there any reason not to put the entire RuleContext in this thread, for // analysis threads? public BazelStarlarkContext( + Phase phase, String toolsRepository, @Nullable ImmutableMap> fragmentNameToClass, ImmutableMap repoMapping, SymbolGenerator symbolGenerator, @Nullable Label analysisRuleLabel) { + this.phase = phase; this.toolsRepository = toolsRepository; this.fragmentNameToClass = fragmentNameToClass; this.repoMapping = repoMapping; @@ -72,6 +85,11 @@ public BazelStarlarkContext( this.analysisRuleLabel = analysisRuleLabel; } + /** Returns the phase to which this Starlark thread belongs. */ + public Phase getPhase() { + return phase; + } + /** Returns the name of the tools repository, such as "@bazel_tools". */ @Override public String getToolsRepository() { @@ -105,4 +123,28 @@ public SymbolGenerator getSymbolGenerator() { public Label getAnalysisRuleLabel() { return analysisRuleLabel; } + + /** + * Checks that the Starlark thread is in the loading or the workspace phase. + * + * @param function name of a function that requires this check + */ + public void checkLoadingOrWorkspacePhase(String function) throws EvalException { + if (phase == Phase.ANALYSIS) { + throw new EvalException( + null, "'" + function + "' cannot be called during the analysis phase"); + } + } + + /** + * Checks that the current StarlarkThread is in the loading phase. + * + * @param function name of a function that requires this check + */ + public void checkLoadingPhase(String function) throws EvalException { + if (phase != Phase.LOADING) { + throw new EvalException( + null, "'" + function + "' can only be called during the loading phase"); + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java index e5446bee1b7c83..44a9a5e0dff052 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java @@ -56,8 +56,6 @@ import com.google.devtools.build.lib.syntax.NoneType; import com.google.devtools.build.lib.syntax.ParserInput; import com.google.devtools.build.lib.syntax.Printer; -import com.google.devtools.build.lib.syntax.SkylarkUtils; -import com.google.devtools.build.lib.syntax.SkylarkUtils.Phase; import com.google.devtools.build.lib.syntax.Starlark; import com.google.devtools.build.lib.syntax.StarlarkFile; import com.google.devtools.build.lib.syntax.StarlarkSemantics; @@ -605,7 +603,7 @@ public NoneType callImpl( throw new EvalException(null, "unexpected positional arguments"); } Location loc = call != null ? call.getLocation() : Location.BUILTIN; - SkylarkUtils.checkLoadingOrWorkspacePhase(thread, ruleClass.getName(), loc); + BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase(ruleClass.getName()); try { addRule(getContext(thread, loc), kwargs, loc, thread); } catch (RuleFactory.InvalidRuleException | Package.NameConflictException e) { @@ -1004,14 +1002,14 @@ public Package.Builder evaluateBuildFile( .setEventHandler(eventHandler) .setImportedExtensions(imports) .build(); - SkylarkUtils.setPhase(thread, Phase.LOADING); // TODO(adonovan): save this as a field in BazelSkylarkContext. - // It needn't be a third thread-local. + // It needn't be a second thread-local. thread.setThreadLocal( PackageContext.class, new PackageContext(pkgBuilder, globber, eventHandler)); new BazelStarlarkContext( + BazelStarlarkContext.Phase.LOADING, ruleClassProvider.getToolsRepository(), /*fragmentNameToClass=*/ null, repositoryMapping, diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java index 5e68ae6ba9e08a..f267a59c033aaa 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java +++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java @@ -38,7 +38,6 @@ import com.google.devtools.build.lib.syntax.Mutability; import com.google.devtools.build.lib.syntax.NoneType; import com.google.devtools.build.lib.syntax.Sequence; -import com.google.devtools.build.lib.syntax.SkylarkUtils; import com.google.devtools.build.lib.syntax.Starlark; import com.google.devtools.build.lib.syntax.StarlarkList; import com.google.devtools.build.lib.syntax.StarlarkThread; @@ -82,8 +81,7 @@ public Sequence glob( Location loc, StarlarkThread thread) throws EvalException, ConversionException, InterruptedException { - SkylarkUtils.checkLoadingPhase(thread, "native.glob", loc); - + BazelStarlarkContext.from(thread).checkLoadingPhase("native.glob"); PackageContext context = getContext(thread, loc); List includes = Type.STRING_LIST.convert(include, "'glob' argument"); @@ -126,7 +124,7 @@ public Sequence glob( @Override public Object existingRule(String name, Location loc, StarlarkThread thread) throws EvalException, InterruptedException { - SkylarkUtils.checkLoadingOrWorkspacePhase(thread, "native.existing_rule", loc); + BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase("native.existing_rule"); PackageContext context = getContext(thread, loc); Target target = context.pkgBuilder.getTarget(name); Dict rule = targetDict(target, loc, thread.mutability()); @@ -140,7 +138,7 @@ If necessary, we could allow filtering by tag (anytag, alltags), name (regexp?), @Override public Dict> existingRules(Location loc, StarlarkThread thread) throws EvalException, InterruptedException { - SkylarkUtils.checkLoadingOrWorkspacePhase(thread, "native.existing_rules", loc); + BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase("native.existing_rules"); PackageContext context = getContext(thread, loc); Collection targets = context.pkgBuilder.getTargets(); Mutability mu = thread.mutability(); @@ -164,7 +162,7 @@ public NoneType packageGroup( Location loc, StarlarkThread thread) throws EvalException { - SkylarkUtils.checkLoadingPhase(thread, "native.package_group", loc); + BazelStarlarkContext.from(thread).checkLoadingPhase("native.package_group"); PackageContext context = getContext(thread, loc); List packages = @@ -188,7 +186,7 @@ public NoneType packageGroup( public NoneType exportsFiles( Sequence srcs, Object visibilityO, Object licensesO, Location loc, StarlarkThread thread) throws EvalException { - SkylarkUtils.checkLoadingPhase(thread, "native.exports_files", loc); + BazelStarlarkContext.from(thread).checkLoadingPhase("native.exports_files"); Package.Builder pkgBuilder = getContext(thread, loc).pkgBuilder; List files = Type.STRING_LIST.convert(srcs, "'exports_files' operand"); @@ -262,7 +260,7 @@ public NoneType exportsFiles( @Override public String packageName(Location loc, StarlarkThread thread) throws EvalException { - SkylarkUtils.checkLoadingPhase(thread, "native.package_name", loc); + BazelStarlarkContext.from(thread).checkLoadingPhase("native.package_name"); PackageIdentifier packageId = PackageFactory.getContext(thread, loc).getBuilder().getPackageIdentifier(); return packageId.getPackageFragment().getPathString(); @@ -270,7 +268,7 @@ public String packageName(Location loc, StarlarkThread thread) throws EvalExcept @Override public String repositoryName(Location location, StarlarkThread thread) throws EvalException { - SkylarkUtils.checkLoadingPhase(thread, "native.repository_name", location); + BazelStarlarkContext.from(thread).checkLoadingPhase("native.repository_name"); PackageIdentifier packageId = PackageFactory.getContext(thread, location).getBuilder().getPackageIdentifier(); return packageId.getRepository().toString(); diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkBuildLibrary.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkBuildLibrary.java index 4839695e7dc6b5..f8ee7c80a4e8d6 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkBuildLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkBuildLibrary.java @@ -135,7 +135,7 @@ public NoneType environmentGroup( documented = false, useStarlarkThread = true, useLocation = true) - public NoneType invoke( + public NoneType licenses( Sequence licensesList, // list of license strings Location loc, StarlarkThread thread) 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 af97687480c4a2..eaf3f6c82a885e 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 @@ -36,8 +36,6 @@ import com.google.devtools.build.lib.syntax.Module; import com.google.devtools.build.lib.syntax.Mutability; import com.google.devtools.build.lib.syntax.ParserInput; -import com.google.devtools.build.lib.syntax.SkylarkUtils; -import com.google.devtools.build.lib.syntax.SkylarkUtils.Phase; import com.google.devtools.build.lib.syntax.Starlark; import com.google.devtools.build.lib.syntax.StarlarkFile; import com.google.devtools.build.lib.syntax.StarlarkSemantics; @@ -168,7 +166,6 @@ private void execute( .setEventHandler(localReporter) .setImportedExtensions(importMap) .build(); - SkylarkUtils.setPhase(thread, Phase.WORKSPACE); thread.setThreadLocal( PackageFactory.PackageContext.class, new PackageFactory.PackageContext(builder, null, localReporter)); @@ -178,6 +175,7 @@ private void execute( // repository mapping because calls to the Label constructor in the WORKSPACE file // are, by definition, not in an external repository and so they don't need the mapping new BazelStarlarkContext( + BazelStarlarkContext.Phase.WORKSPACE, /* toolsRepository= */ null, /* fragmentNameToClass= */ null, /* repoMapping= */ ImmutableMap.of(), diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java index db8479df6f98a5..ac5f2f287727e2 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java @@ -77,10 +77,9 @@ public ConfiguredAspect create( .setSemantics(analysisEnv.getSkylarkSemantics()) .setEventHandler(analysisEnv.getEventHandler()) .build(); - // NB: loading phase functions are not available: this is analysis already, so we do - // *not* setLoadingPhase(). new BazelStarlarkContext( + BazelStarlarkContext.Phase.ANALYSIS, toolsRepository, /* fragmentNameToClass=*/ null, ruleContext.getRule().getPackage().getRepositoryMapping(), diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BUILD b/src/main/java/com/google/devtools/build/lib/syntax/BUILD index 770e47a3749567..03cca6f7426c29 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/BUILD +++ b/src/main/java/com/google/devtools/build/lib/syntax/BUILD @@ -120,7 +120,6 @@ java_library( "SkylarkIndexable.java", "SkylarkQueryable.java", "SkylarkType.java", - "SkylarkUtils.java", "Starlark.java", "StarlarkCallable.java", "StarlarkFunction.java", diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkUtils.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkUtils.java deleted file mode 100644 index d8289cdedf2f87..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkUtils.java +++ /dev/null @@ -1,62 +0,0 @@ -// Copyright 2016 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.syntax; - -import com.google.devtools.build.lib.events.Location; - -/** This class contains Bazel-specific functions to extend or interoperate with Skylark. */ -// TODO(adonovan): move this into BazelStarlarkContext. -public final class SkylarkUtils { - - /** A phase for enabling or disabling certain builtin functions */ - public enum Phase { - WORKSPACE, - LOADING, - ANALYSIS - } - - public static void setPhase(StarlarkThread thread, Phase phase) { - thread.setThreadLocal(Phase.class, phase); - } - - private static Phase getPhase(StarlarkThread thread) { - Phase phase = thread.getThreadLocal(Phase.class); - return phase == null ? Phase.ANALYSIS : phase; - } - - /** - * Checks that the current StarlarkThread is in the loading or the workspace phase. - * - * @param symbol name of the function being only authorized thus. - */ - public static void checkLoadingOrWorkspacePhase( - StarlarkThread thread, String symbol, Location loc) throws EvalException { - if (getPhase(thread) == Phase.ANALYSIS) { - throw new EvalException(loc, symbol + "() cannot be called during the analysis phase"); - } - } - - /** - * Checks that the current StarlarkThread is in the loading phase. - * - * @param symbol name of the function being only authorized thus. - */ - public static void checkLoadingPhase(StarlarkThread thread, String symbol, Location loc) - throws EvalException { - if (getPhase(thread) != Phase.LOADING) { - throw new EvalException(loc, symbol + "() can only be called during the loading phase"); - } - } -} diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java index 0dadd377353169..e01dd6b67fa16d 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java @@ -1936,7 +1936,7 @@ public void testGlobInImplicitOutputs() throws Exception { " srcs = ['foo.bar', 'other_foo.bar'])"); reporter.removeHandler(failFastHandler); getConfiguredTarget("//test:my_glob"); - assertContainsEvent("native.glob() can only be called during the loading phase"); + assertContainsEvent("The native module can be accessed only from a BUILD thread."); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/skylark/util/SkylarkTestCase.java b/src/test/java/com/google/devtools/build/lib/skylark/util/SkylarkTestCase.java index 941ddf0dc4fd07..8805f9c265e707 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/util/SkylarkTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/util/SkylarkTestCase.java @@ -25,8 +25,6 @@ import com.google.devtools.build.lib.packages.SymbolGenerator; import com.google.devtools.build.lib.rules.platform.PlatformCommon; import com.google.devtools.build.lib.syntax.Module; -import com.google.devtools.build.lib.syntax.SkylarkUtils; -import com.google.devtools.build.lib.syntax.SkylarkUtils.Phase; import com.google.devtools.build.lib.syntax.Starlark; import com.google.devtools.build.lib.syntax.StarlarkThread; import com.google.devtools.build.lib.syntax.util.EvaluationTestCase; @@ -67,12 +65,11 @@ public StarlarkThread newStarlarkThread() throws Exception { Label.parseAbsoluteUnchecked("//test:label", /*defaultToMain=*/ false))) .build(); - SkylarkUtils.setPhase(thread, Phase.LOADING); - // This StarlarkThread has no PackageContext, so attempts to create a rule will fail. // Rule creation is tested by SkylarkIntegrationTest. new BazelStarlarkContext( + BazelStarlarkContext.Phase.LOADING, TestConstants.TOOLS_REPOSITORY, /*fragmentNameToClass=*/ null, /*repoMapping=*/ ImmutableMap.of(),