diff options
author | Luis Fernando Pino Duque <lpino@google.com> | 2016-04-28 15:47:29 +0000 |
---|---|---|
committer | Yun Peng <pcloudy@google.com> | 2016-04-29 08:06:28 +0000 |
commit | 3fedf9e618cbce3dbdd00559b2de0bb8e2d43171 (patch) | |
tree | cbe0c132f0a815bbf0bc5a85560cf33cd7118484 /src/main/java/com/google | |
parent | 60166c5dbef131b2d0104b514fe596bd5b572f7e (diff) |
Inject the Constants.TOOLS_REPOSITORY in SkylarkRuleClassFunctions.testBaseRule
via the Skylark and delete the constant. Also, change the isLoadingPhase in the
Skylark environment an enum Phase in order to:
- Decide whether testRules are enabled or not and,
- Check that the toolsRepository is set when in the LOADING phase.
Finally, a few tests that were using ConfiguredRuleClassProvider directly
had to be updated to set a tools repository, otherwise createGlobals() fails.
--
MOS_MIGRATED_REVID=121022804
Diffstat (limited to 'src/main/java/com/google')
6 files changed, 94 insertions, 61 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/Constants.java b/src/main/java/com/google/devtools/build/lib/Constants.java index 556a742612..03547219ea 100644 --- a/src/main/java/com/google/devtools/build/lib/Constants.java +++ b/src/main/java/com/google/devtools/build/lib/Constants.java @@ -27,7 +27,4 @@ public final class Constants { // Native Java deps are all linked into a single file, which is named with this value + ".so". public static final String NATIVE_DEPS_LIB_SUFFIX = "_nativedeps"; - - // Most other tools dependencies use this; we plan to split it into per-language repositories. - public static final String TOOLS_REPOSITORY = "@bazel_tools"; } 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 bfc8b5bc2e..a39737ce7c 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 @@ -44,6 +44,7 @@ import com.google.devtools.build.lib.rules.SkylarkModules; import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy; import com.google.devtools.build.lib.syntax.Environment; import com.google.devtools.build.lib.syntax.Environment.Extension; +import com.google.devtools.build.lib.syntax.Environment.Phase; import com.google.devtools.build.lib.syntax.Mutability; import com.google.devtools.common.options.OptionsClassProvider; @@ -537,7 +538,8 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { .setEventHandler(eventHandler) .setFileContentHashCode(astFileContentHashCode) .setImportedExtensions(importMap) - .setLoadingPhase() + .setToolsRepository(toolsRepository) + .setPhase(Phase.LOADING) .build(); } 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 d2f70902ec..6f86bbd76d 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 @@ -44,6 +44,7 @@ import com.google.devtools.build.lib.syntax.ClassObject; import com.google.devtools.build.lib.syntax.Environment; import com.google.devtools.build.lib.syntax.Environment.Extension; import com.google.devtools.build.lib.syntax.Environment.NoSuchVariableException; +import com.google.devtools.build.lib.syntax.Environment.Phase; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.EvalUtils; import com.google.devtools.build.lib.syntax.Expression; @@ -1504,7 +1505,8 @@ public final class PackageFactory { .setGlobals(Environment.BUILD) .setEventHandler(eventHandler) .setImportedExtensions(imports) - .setLoadingPhase() + .setToolsRepository(ruleClassProvider.getToolsRepository()) + .setPhase(Phase.LOADING) .build(); pkgBuilder.setFilename(buildFilePath) @@ -1576,7 +1578,8 @@ public final class PackageFactory { Environment pkgEnv = Environment.builder(mutability) .setGlobals(Environment.BUILD) .setEventHandler(NullEventHandler.INSTANCE) - .setLoadingPhase() + .setToolsRepository(ruleClassProvider.getToolsRepository()) + .setPhase(Phase.LOADING) .build(); Package.LegacyBuilder pkgBuilder = new Package.LegacyBuilder(packageId, 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 7608c31293..f8d885fc01 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 @@ -37,6 +37,7 @@ import com.google.devtools.build.lib.syntax.ClassObject; import com.google.devtools.build.lib.syntax.Environment; import com.google.devtools.build.lib.syntax.Environment.Extension; import com.google.devtools.build.lib.syntax.Environment.Frame; +import com.google.devtools.build.lib.syntax.Environment.Phase; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.FuncallExpression; import com.google.devtools.build.lib.syntax.FunctionSignature; @@ -73,7 +74,7 @@ public class WorkspaceFactory { PackageFactory.PKG_CONTEXT); private final LegacyBuilder builder; - + private final Path installDir; private final Path workspaceDir; private final Mutability mutability; @@ -190,7 +191,7 @@ public class WorkspaceFactory { importMap = parentImportMap; } environmentBuilder.setImportedExtensions(importMap); - Environment workspaceEnv = environmentBuilder.setLoadingPhase().build(); + Environment workspaceEnv = environmentBuilder.setPhase(Phase.WORKSPACE).build(); addWorkspaceFunctions(workspaceEnv, localReporter); for (Map.Entry<String, Object> binding : parentVariableBindings.entrySet()) { try { diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java index a5f3d646fe..c8c0c86494 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java @@ -33,7 +33,6 @@ import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.devtools.build.lib.Constants; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.OutputGroupProvider; @@ -194,42 +193,43 @@ public class SkylarkRuleClassFunctions { .build(); /** Parent rule class for test Skylark rules. */ - public static final RuleClass testBaseRule = - new RuleClass.Builder("$test_base_rule", RuleClassType.ABSTRACT, true, baseRule) - .add(attr("size", STRING).value("medium").taggable() - .nonconfigurable("used in loading phase rule validation logic")) - .add(attr("timeout", STRING).taggable() - .nonconfigurable("used in loading phase rule validation logic").value( - new Attribute.ComputedDefault() { - @Override - public Object getDefault(AttributeMap rule) { - TestSize size = TestSize.getTestSize(rule.get("size", Type.STRING)); - if (size != null) { - String timeout = size.getDefaultTimeout().toString(); - if (timeout != null) { - return timeout; + public static final RuleClass getTestBaseRule(String toolsRespository) { + return new RuleClass.Builder("$test_base_rule", RuleClassType.ABSTRACT, true, baseRule) + .add(attr("size", STRING).value("medium").taggable() + .nonconfigurable("used in loading phase rule validation logic")) + .add(attr("timeout", STRING).taggable() + .nonconfigurable("used in loading phase rule validation logic").value( + new Attribute.ComputedDefault() { + @Override + public Object getDefault(AttributeMap rule) { + TestSize size = TestSize.getTestSize(rule.get("size", Type.STRING)); + if (size != null) { + String timeout = size.getDefaultTimeout().toString(); + if (timeout != null) { + return timeout; + } } + return "illegal"; } - return "illegal"; - } - })) - .add(attr("flaky", BOOLEAN).value(false).taggable() - .nonconfigurable("taggable - called in Rule.getRuleTags")) - .add(attr("shard_count", INTEGER).value(-1)) - .add(attr("local", BOOLEAN).value(false).taggable() - .nonconfigurable("policy decision: this should be consistent across configurations")) - .add(attr("args", STRING_LIST) - .nonconfigurable("policy decision: should be consistent across configurations")) - .add(attr("$test_runtime", LABEL_LIST).cfg(HOST).value(ImmutableList.of( - labelCache.getUnchecked(Constants.TOOLS_REPOSITORY + "//tools/test:runtime")))) - .add(attr(":run_under", LABEL).cfg(DATA).value(RUN_UNDER)) - .add(attr(":gcov", LABEL_LIST).cfg(HOST).value(GCOV)) - .add(attr(":coverage_support", LABEL_LIST).cfg(HOST).value(COVERAGE_SUPPORT)) - .add( - attr(":coverage_report_generator", LABEL_LIST) - .cfg(HOST) - .value(COVERAGE_REPORT_GENERATOR)) - .build(); + })) + .add(attr("flaky", BOOLEAN).value(false).taggable() + .nonconfigurable("taggable - called in Rule.getRuleTags")) + .add(attr("shard_count", INTEGER).value(-1)) + .add(attr("local", BOOLEAN).value(false).taggable() + .nonconfigurable("policy decision: this should be consistent across configurations")) + .add(attr("args", STRING_LIST) + .nonconfigurable("policy decision: should be consistent across configurations")) + .add(attr("$test_runtime", LABEL_LIST).cfg(HOST).value(ImmutableList.of( + labelCache.getUnchecked(toolsRespository + "//tools/test:runtime")))) + .add(attr(":run_under", LABEL).cfg(DATA).value(RUN_UNDER)) + .add(attr(":gcov", LABEL_LIST).cfg(HOST).value(GCOV)) + .add(attr(":coverage_support", LABEL_LIST).cfg(HOST).value(COVERAGE_SUPPORT)) + .add( + attr(":coverage_report_generator", LABEL_LIST) + .cfg(HOST) + .value(COVERAGE_REPORT_GENERATOR)) + .build(); + } /** * In native code, private values start with $. @@ -313,7 +313,8 @@ public class SkylarkRuleClassFunctions { throws EvalException, ConversionException { funcallEnv.checkLoadingPhase("rule", ast.getLocation()); RuleClassType type = test ? RuleClassType.TEST : RuleClassType.NORMAL; - RuleClass parent = test ? testBaseRule : (executable ? binaryBaseRule : baseRule); + RuleClass parent = test ? getTestBaseRule(funcallEnv.getToolsRepository()) + : (executable ? binaryBaseRule : baseRule); // We'll set the name later, pass the empty string for now. RuleClass.Builder builder = new RuleClass.Builder("", type, true, parent); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java index 8c1def42b2..8ef298a697 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java @@ -14,6 +14,8 @@ package com.google.devtools.build.lib.syntax; +import static com.google.common.base.Preconditions.checkState; + import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; @@ -75,6 +77,11 @@ import javax.annotation.Nullable; public final class Environment implements Freezable { /** + * A phase for enabling or disabling certain builtin functions + */ + public enum Phase { WORKSPACE, LOADING, ANALYSIS } + + /** * A Frame is a Map of bindings, plus a {@link Mutability} and a parent Frame * from which to inherit bindings. * @@ -331,7 +338,7 @@ public final class Environment implements Freezable { * Is this Environment being executed during the loading phase? * Many builtin functions are only enabled during the loading phase, and check this flag. */ - private final boolean isLoadingPhase; + private Phase phase; /** * When in a lexical (Skylark) Frame, this set contains the variable names that are global, @@ -357,6 +364,11 @@ public final class Environment implements Freezable { @Nullable private final Label callerLabel; /** + * The path to the tools repository. + */ + private final String toolsRepository; + + /** * Enters a scope by saving state to a new Continuation * @param function the function whose scope to enter * @param caller the source AST node for the caller @@ -391,11 +403,10 @@ public final class Environment implements Freezable { /** * Is this Environment being evaluated during the loading phase? * This is fixed during Environment setup, and enables various functions - * that are not available during the analysis phase. - * @return true if this Environment corresponds to code during the loading phase. + * that are not available during the analysis or workspace phase. */ - private boolean isLoadingPhase() { - return isLoadingPhase; + public Phase getPhase() { + return phase; } /** @@ -403,7 +414,7 @@ public final class Environment implements Freezable { * @param symbol name of the function being only authorized thus. */ public void checkLoadingPhase(String symbol, Location loc) throws EvalException { - if (!isLoadingPhase()) { + if (phase != Phase.LOADING) { throw new EvalException(loc, symbol + "() can only be called during the loading phase"); } } @@ -481,7 +492,7 @@ public final class Environment implements Freezable { * @param importedExtensions Extension-s from which to import bindings with load() * @param isSkylark true if in Skylark context * @param fileContentHashCode a hash for the source file being evaluated, if any - * @param isLoadingPhase true if in loading phase + * @param phase the current phase * @param callerLabel the label this environment came from */ private Environment( @@ -491,8 +502,9 @@ public final class Environment implements Freezable { Map<String, Extension> importedExtensions, boolean isSkylark, @Nullable String fileContentHashCode, - boolean isLoadingPhase, - @Nullable Label callerLabel) { + Phase phase, + @Nullable Label callerLabel, + String toolsRepository) { this.globalFrame = Preconditions.checkNotNull(globalFrame); this.dynamicFrame = Preconditions.checkNotNull(dynamicFrame); Preconditions.checkArgument(globalFrame.mutability().isMutable()); @@ -501,8 +513,9 @@ public final class Environment implements Freezable { this.importedExtensions = importedExtensions; this.isSkylark = isSkylark; this.fileContentHashCode = fileContentHashCode; - this.isLoadingPhase = isLoadingPhase; + this.phase = phase; this.callerLabel = callerLabel; + this.toolsRepository = toolsRepository; } /** @@ -511,12 +524,13 @@ public final class Environment implements Freezable { public static class Builder { private final Mutability mutability; private boolean isSkylark = false; - private boolean isLoadingPhase = false; + private Phase phase = Phase.ANALYSIS; @Nullable private Frame parent; @Nullable private EventHandler eventHandler; @Nullable private Map<String, Extension> importedExtensions; @Nullable private String fileContentHashCode; private Label label; + private String toolsRepository; Builder(Mutability mutability) { this.mutability = mutability; @@ -529,10 +543,10 @@ public final class Environment implements Freezable { return this; } - /** Enables loading phase only functions in this Environment. */ - public Builder setLoadingPhase() { - Preconditions.checkState(!isLoadingPhase); - isLoadingPhase = true; + /** Enables loading or workspace phase only functions in this Environment. */ + public Builder setPhase(Phase phase) { + Preconditions.checkState(this.phase == Phase.ANALYSIS); + this.phase = phase; return this; } @@ -563,6 +577,12 @@ public final class Environment implements Freezable { return this; } + /** Sets the path to the tools repository */ + public Builder setToolsRepository(String toolsRepository) { + this.toolsRepository = toolsRepository; + return this; + } + /** Builds the Environment. */ public Environment build() { Preconditions.checkArgument(mutability.isMutable()); @@ -574,6 +594,9 @@ public final class Environment implements Freezable { if (importedExtensions == null) { importedExtensions = ImmutableMap.of(); } + if (phase == Phase.LOADING) { + Preconditions.checkState(this.toolsRepository != null); + } return new Environment( globalFrame, dynamicFrame, @@ -581,8 +604,9 @@ public final class Environment implements Freezable { importedExtensions, isSkylark, fileContentHashCode, - isLoadingPhase, - label); + phase, + label, + toolsRepository); } public Builder setCallerLabel(Label label) { @@ -941,4 +965,9 @@ public final class Environment implements Freezable { } return last; } + + public String getToolsRepository() { + checkState(toolsRepository != null); + return toolsRepository; + } } |