Use setLoadingOrWorkspacePhase instead of setLoadingPhase where relevant,
commit 3fedf9e618cbce3dbdd00559b2de0bb8e2d43171 introduced the distinction between the loading phase and the workspace phase.
This check broke the use of native.existing_rule(s) in skylark remote repository, added a regression test for it.
Fixes #1277
--
MOS_MIGRATED_REVID=122988569
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 344d9c7..4939e32 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
@@ -101,7 +101,7 @@
FuncallExpression ast,
com.google.devtools.build.lib.syntax.Environment funcallEnv)
throws EvalException {
- funcallEnv.checkLoadingPhase("repository_rule", ast.getLocation());
+ funcallEnv.checkLoadingOrWorkspacePhase("repository_rule", ast.getLocation());
// We'll set the name later, pass the empty string for now.
Builder builder = new Builder("", RuleClassType.WORKSPACE, true);
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 6dc5577..f869315 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
@@ -1158,7 +1158,7 @@
public Runtime.NoneType invoke(Map<String, Object> kwargs,
FuncallExpression ast, Environment env)
throws EvalException, InterruptedException {
- env.checkLoadingPhase(ruleClass, ast.getLocation());
+ env.checkLoadingOrWorkspacePhase(ruleClass, ast.getLocation());
try {
addRule(ruleFactory, ruleClass, getContext(env, ast), kwargs, ast, env);
} catch (RuleFactory.InvalidRuleException | Package.NameConflictException e) {
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 91e5e81..41762fe 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
@@ -111,7 +111,7 @@
new BuiltinFunction("existing_rule") {
public Object invoke(String name, FuncallExpression ast, Environment env)
throws EvalException, InterruptedException {
- env.checkLoadingPhase("native.existing_rule", ast.getLocation());
+ env.checkLoadingOrWorkspacePhase("native.existing_rule", ast.getLocation());
SkylarkDict<String, Object> rule = PackageFactory.callGetRuleFunction(name, ast, env);
if (rule != null) {
return rule;
@@ -142,7 +142,7 @@
public SkylarkDict<String, SkylarkDict<String, Object>> invoke(
FuncallExpression ast, Environment env)
throws EvalException, InterruptedException {
- env.checkLoadingPhase("native.existing_rules", ast.getLocation());
+ env.checkLoadingOrWorkspacePhase("native.existing_rules", ast.getLocation());
return PackageFactory.callGetRulesFunction(ast, env);
}
};
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
index 95f4bd4..2fd7e1a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
@@ -298,7 +298,7 @@
Environment env)
throws EvalException {
// TODO(bazel-team): Replace literal strings with constants.
- env.checkLoadingPhase("attr.int", ast.getLocation());
+ env.checkLoadingOrWorkspacePhase("attr.int", ast.getLocation());
return createAttrDescriptor(
EvalUtils.<String, Object>optionMap(
env, DEFAULT_ARG, defaultInt, MANDATORY_ARG, mandatory, VALUES_ARG, values),
@@ -342,7 +342,7 @@
FuncallExpression ast,
Environment env)
throws EvalException {
- env.checkLoadingPhase("attr.string", ast.getLocation());
+ env.checkLoadingOrWorkspacePhase("attr.string", ast.getLocation());
return createAttrDescriptor(
EvalUtils.<String, Object>optionMap(
env, DEFAULT_ARG, defaultString, MANDATORY_ARG, mandatory, VALUES_ARG, values),
@@ -431,7 +431,7 @@
FuncallExpression ast,
Environment env)
throws EvalException {
- env.checkLoadingPhase("attr.label", ast.getLocation());
+ env.checkLoadingOrWorkspacePhase("attr.label", ast.getLocation());
return createAttrDescriptor(
EvalUtils.<String, Object>optionMap(
env,
@@ -488,7 +488,7 @@
FuncallExpression ast,
Environment env)
throws EvalException {
- env.checkLoadingPhase("attr.string_list", ast.getLocation());
+ env.checkLoadingOrWorkspacePhase("attr.string_list", ast.getLocation());
return createAttrDescriptor(
EvalUtils.<String, Object>optionMap(
env,
@@ -534,7 +534,7 @@
FuncallExpression ast,
Environment env)
throws EvalException {
- env.checkLoadingPhase("attr.int_list", ast.getLocation());
+ env.checkLoadingOrWorkspacePhase("attr.int_list", ast.getLocation());
return createAttrDescriptor(
EvalUtils.<String, Object>optionMap(
env,
@@ -634,7 +634,7 @@
FuncallExpression ast,
Environment env)
throws EvalException {
- env.checkLoadingPhase("attr.label_list", ast.getLocation());
+ env.checkLoadingOrWorkspacePhase("attr.label_list", ast.getLocation());
SkylarkDict<String, Object> kwargs = EvalUtils.<String, Object>optionMap(
env,
DEFAULT_ARG,
@@ -683,7 +683,7 @@
public Descriptor invoke(
Boolean defaultO, Boolean mandatory, FuncallExpression ast, Environment env)
throws EvalException {
- env.checkLoadingPhase("attr.bool", ast.getLocation());
+ env.checkLoadingOrWorkspacePhase("attr.bool", ast.getLocation());
return createAttrDescriptor(
EvalUtils.<String, Object>optionMap(
env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
@@ -720,7 +720,7 @@
public Descriptor invoke(
Object defaultO, Boolean mandatory, FuncallExpression ast, Environment env)
throws EvalException {
- env.checkLoadingPhase("attr.output", ast.getLocation());
+ env.checkLoadingOrWorkspacePhase("attr.output", ast.getLocation());
return createNonconfigurableAttrDescriptor(
EvalUtils.<String, Object>optionMap(
env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
@@ -763,7 +763,7 @@
FuncallExpression ast,
Environment env)
throws EvalException {
- env.checkLoadingPhase("attr.output_list", ast.getLocation());
+ env.checkLoadingOrWorkspacePhase("attr.output_list", ast.getLocation());
return createAttrDescriptor(
EvalUtils.<String, Object>optionMap(
env,
@@ -804,7 +804,7 @@
FuncallExpression ast,
Environment env)
throws EvalException {
- env.checkLoadingPhase("attr.string_dict", ast.getLocation());
+ env.checkLoadingOrWorkspacePhase("attr.string_dict", ast.getLocation());
return createAttrDescriptor(
EvalUtils.<String, Object>optionMap(
env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
@@ -840,7 +840,7 @@
FuncallExpression ast,
Environment env)
throws EvalException {
- env.checkLoadingPhase("attr.string_list_dict", ast.getLocation());
+ env.checkLoadingOrWorkspacePhase("attr.string_list_dict", ast.getLocation());
return createAttrDescriptor(
EvalUtils.<String, Object>optionMap(
env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
@@ -870,7 +870,7 @@
public Descriptor invoke(
Object defaultO, Boolean mandatory, FuncallExpression ast, Environment env)
throws EvalException {
- env.checkLoadingPhase("attr.license", ast.getLocation());
+ env.checkLoadingOrWorkspacePhase("attr.license", ast.getLocation());
return createNonconfigurableAttrDescriptor(
EvalUtils.<String, Object>optionMap(
env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
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 8d452b6..abf4cf1 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
@@ -287,7 +287,7 @@
Object implicitOutputs, Boolean executable, Boolean outputToGenfiles, SkylarkList fragments,
SkylarkList hostFragments, FuncallExpression ast, Environment funcallEnv)
throws EvalException, ConversionException {
- funcallEnv.checkLoadingPhase("rule", ast.getLocation());
+ funcallEnv.checkLoadingOrWorkspacePhase("rule", ast.getLocation());
RuleClassType type = test ? RuleClassType.TEST : RuleClassType.NORMAL;
RuleClass parent = test ? getTestBaseRule(funcallEnv.getToolsRepository())
: (executable ? binaryBaseRule : baseRule);
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 e26b257..d9a8b27 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
@@ -399,6 +399,16 @@
}
/**
+ * Checks that the current Environment is in the loading or the workspace phase.
+ * @param symbol name of the function being only authorized thus.
+ */
+ public void checkLoadingOrWorkspacePhase(String symbol, Location loc) throws EvalException {
+ if (phase == Phase.ANALYSIS) {
+ throw new EvalException(loc, symbol + "() cannot be called during the analysis phase");
+ }
+ }
+
+ /**
* Checks that the current Environment is in the loading phase.
* @param symbol name of the function being only authorized thus.
*/
diff --git a/src/test/shell/bazel/skylark_repository_test.sh b/src/test/shell/bazel/skylark_repository_test.sh
index e512b70..d20cb8b 100755
--- a/src/test/shell/bazel/skylark_repository_test.sh
+++ b/src/test/shell/bazel/skylark_repository_test.sh
@@ -596,6 +596,42 @@
expect_log ": ${version}."
}
+
+# Test native.existing_rule(s), regression test for #1277
+function test_existing_rule() {
+ create_new_workspace
+ repo2=$new_workspace_dir
+
+ cat > BUILD
+ cat > WORKSPACE
+
+ cd ${WORKSPACE_DIR}
+ cat > WORKSPACE <<EOF
+local_repository(name = 'existing', path='$repo2')
+load('/test', 'macro')
+
+macro()
+EOF
+
+ # Empty package for the .bzl file
+ echo -n >BUILD
+
+ # Our macro
+ cat >test.bzl <<EOF
+def test(s):
+ print("%s = %s,%s" % (s,
+ native.existing_rule(s) != None,
+ s in native.existing_rules()))
+def macro():
+ test("existing")
+ test("non_existing")
+EOF
+
+ bazel query //... >& $TEST_log || fail "Failed to build"
+ expect_log "existing = True,True"
+ expect_log "non_existing = False,False"
+}
+
function tear_down() {
shutdown_server
if [ -d "${TEST_TMPDIR}/server_dir" ]; then
