Make labels in .bzl files in remote repos resolve relative to their repo
For example, if you have a BUILD file that does:
load('@foo//bar:baz.bzl', 'my_rule')
my_rule(...)
If baz.bzl uses Label('//whatever'), this change makes //whatever resolve to
@foo//whatever. Previous to this change, it would be resolved to the repository
the BUILD file using my_rule was in.
RELNOTES[INC]: Labels in .bzl files in remote repositories will be resolved
relative to their repository (instead of the repository the Skylark rule is
used in).
--
MOS_MIGRATED_REVID=117720181
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 798b14d..efe4573 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
@@ -518,7 +518,7 @@
EventHandler eventHandler,
String astFileContentHashCode,
Map<String, Extension> importMap) {
- Environment env = Environment.builder(mutability)
+ return Environment.builder(mutability)
.setSkylark()
.setGlobals(globals)
.setEventHandler(eventHandler)
@@ -526,17 +526,17 @@
.setImportedExtensions(importMap)
.setLoadingPhase()
.build();
- return env;
}
@Override
public Environment createSkylarkRuleClassEnvironment(
- Mutability mutability,
+ Label extensionLabel, Mutability mutability,
EventHandler eventHandler,
String astFileContentHashCode,
Map<String, Extension> importMap) {
return createSkylarkRuleClassEnvironment(
- mutability, globals, eventHandler, astFileContentHashCode, importMap);
+ mutability, globals.setLabel(extensionLabel),
+ eventHandler, astFileContentHashCode, importMap);
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java
index f6529a6..2f0988f 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java
@@ -49,12 +49,15 @@
* Returns a new Skylark Environment instance for rule creation.
* Implementations need to be thread safe.
* Be sure to close() the mutability before you return the results of said evaluation.
+ *
+ * @param label the location of the rule.
* @param mutability the Mutability for the current evaluation context
* @param eventHandler the EventHandler for warnings, errors, etc.
* @param astFileContentHashCode the hash code identifying this environment.
* @return an Environment, in which to evaluate load time skylark forms.
*/
Environment createSkylarkRuleClassEnvironment(
+ Label label,
Mutability mutability,
EventHandler eventHandler,
@Nullable String astFileContentHashCode,
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 8ca4b64..a671496 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
@@ -42,6 +42,7 @@
import com.google.devtools.build.lib.analysis.config.RunUnder;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.cmdline.LabelValidator;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
@@ -163,7 +164,7 @@
// rules to skylark extensions. Using the same instance would require a large refactoring.
// If we don't want to support old built-in rules and Skylark simultaneously
// (except for transition phase) it's probably OK.
- private static LoadingCache<String, Label> labelCache =
+ private static final LoadingCache<String, Label> labelCache =
CacheBuilder.newBuilder().build(new CacheLoader<String, Label>() {
@Override
public Label load(String from) throws Exception {
@@ -613,16 +614,36 @@
+ "Example: <br><pre class=language-python>Label(\"//tools:default\")</pre>",
returnType = Label.class,
mandatoryPositionals = {@Param(name = "label_string", type = String.class,
- doc = "the label string")},
- useLocation = true)
+ doc = "the label string")},
+ optionalNamedOnly = {@Param(
+ name = "relative_to_caller_repository",
+ type = Boolean.class,
+ defaultValue = "False",
+ doc = "whether the label should be resolved relative to the label of the file this "
+ + "function is called from.")},
+ useLocation = true,
+ useEnvironment = true)
private static final BuiltinFunction label = new BuiltinFunction("Label") {
- public Label invoke(String labelString,
- Location loc) throws EvalException, ConversionException {
- try {
- return labelCache.get(labelString);
- } catch (ExecutionException e) {
- throw new EvalException(loc, "Illegal absolute label syntax: " + labelString);
+ @SuppressWarnings({"unchecked", "unused"})
+ public Label invoke(
+ String labelString, Boolean relativeToCallerRepository, Location loc, Environment env)
+ throws EvalException {
+ Label parentLabel = null;
+ if (relativeToCallerRepository) {
+ parentLabel = env.getCallerLabel();
+ } else {
+ parentLabel = env.getGlobals().label();
+ }
+ try {
+ if (parentLabel != null) {
+ LabelValidator.parseAbsoluteLabel(labelString);
+ labelString = parentLabel.getRelative(labelString)
+ .getUnambiguousCanonicalForm();
}
+ return labelCache.get(labelString);
+ } catch (LabelValidator.BadLabelException | LabelSyntaxException | ExecutionException e) {
+ throw new EvalException(loc, "Illegal absolute label syntax: " + labelString);
+ }
}
};
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
index a264278..db0f970 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
@@ -69,6 +69,7 @@
SkylarkRuleContext skylarkRuleContext = new SkylarkRuleContext(ruleContext, Kind.RULE);
Environment env = Environment.builder(mutability)
.setSkylark()
+ .setCallerLabel(ruleContext.getLabel())
.setGlobals(
ruleContext.getRule().getRuleClassObject().getRuleDefinitionEnvironment().getGlobals())
.setEventHandler(ruleContext.getAnalysisEnvironment().getEventHandler())
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java
index c030ccc..e0bafdf 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java
@@ -105,6 +105,7 @@
ast = BuildFileAST.parseSkylarkFile(path, astFileSize, env.getListener(),
new ValidationEnvironment(
ruleClassProvider.createSkylarkRuleClassEnvironment(
+ fileLabel,
mutability,
env.getListener(),
// the two below don't matter for extracting the ValidationEnvironment:
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
index 47ee3ef..c4889fa 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
@@ -360,7 +360,7 @@
com.google.devtools.build.lib.syntax.Environment extensionEnv =
ruleClassProvider
.createSkylarkRuleClassEnvironment(
- mutability, eventHandler, ast.getContentHashCode(), importMap)
+ extensionLabel, mutability, eventHandler, ast.getContentHashCode(), importMap)
.setupOverride("native", packageFactory.getNativeModule(inWorkspace));
ast.exec(extensionEnv, eventHandler);
try {
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 f25aed6..2338b31 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
@@ -18,6 +18,7 @@
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.EventKind;
@@ -98,10 +99,14 @@
private final Mutability mutability;
final Frame parent;
final Map<String, Object> bindings = new HashMap<>();
+ // The label for the target this frame is defined in (e.g., //foo:bar.bzl).
+ @Nullable
+ private Label label;
- Frame(Mutability mutability, Frame parent) {
+ private Frame(Mutability mutability, Frame parent) {
this.mutability = mutability;
this.parent = parent;
+ this.label = parent == null ? null : parent.label;
}
@Override
@@ -110,6 +115,25 @@
}
/**
+ * Attaches a label to an existing frame. This is used to get the repository a Skylark
+ * extension is actually defined in.
+ * @param label the label to attach.
+ * @return a new Frame with the existing frame's properties plus the label.
+ */
+ public Frame setLabel(Label label) {
+ this.label = label;
+ return this;
+ }
+
+ /**
+ * Returns the label for this frame.
+ */
+ @Nullable
+ public final Label label() {
+ return label;
+ }
+
+ /**
* Gets a binding from the current frame or if not found its parent.
* @param varname the name of the variable to be bound
* @return the value bound to variable
@@ -325,6 +349,13 @@
@Nullable private Continuation continuation;
/**
+ * Gets the label of the BUILD file that is using this environment. For example, if a target
+ * //foo has a dependency on //bar which is a Skylark rule defined in //rules:my_rule.bzl being
+ * evaluated in this environment, then this would return //foo.
+ */
+ @Nullable private final Label callerLabel;
+
+ /**
* 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
@@ -394,7 +425,7 @@
/**
* @return the current Frame, in which variable side-effects happen.
*/
- private Frame currentFrame() {
+ public Frame currentFrame() {
return isGlobal() ? globalFrame : lexicalFrame;
}
@@ -450,6 +481,7 @@
* @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 callerLabel the label this environment came from
*/
private Environment(
Frame globalFrame,
@@ -458,7 +490,8 @@
Map<String, Extension> importedExtensions,
boolean isSkylark,
@Nullable String fileContentHashCode,
- boolean isLoadingPhase) {
+ boolean isLoadingPhase,
+ @Nullable Label callerLabel) {
this.globalFrame = Preconditions.checkNotNull(globalFrame);
this.dynamicFrame = Preconditions.checkNotNull(dynamicFrame);
Preconditions.checkArgument(globalFrame.mutability().isMutable());
@@ -468,6 +501,7 @@
this.isSkylark = isSkylark;
this.fileContentHashCode = fileContentHashCode;
this.isLoadingPhase = isLoadingPhase;
+ this.callerLabel = callerLabel;
}
/**
@@ -481,6 +515,7 @@
@Nullable private EventHandler eventHandler;
@Nullable private Map<String, Extension> importedExtensions;
@Nullable private String fileContentHashCode;
+ private Label label;
Builder(Mutability mutability) {
this.mutability = mutability;
@@ -545,7 +580,13 @@
importedExtensions,
isSkylark,
fileContentHashCode,
- isLoadingPhase);
+ isLoadingPhase,
+ label);
+ }
+
+ public Builder setCallerLabel(Label label) {
+ this.label = label;
+ return this;
}
}
@@ -554,6 +595,13 @@
}
/**
+ * Returns the caller's label.
+ */
+ public Label getCallerLabel() {
+ return callerLabel;
+ }
+
+ /**
* Sets a binding for a special dynamic variable in this Environment.
* This is not for end-users, and will throw an AssertionError in case of conflict.
* @param varname the name of the dynamic variable to be bound
@@ -856,7 +904,7 @@
/**
* Parses some String input without a supporting file, returning statements and comments.
- * @param input a list of lines of code
+ * @param inputLines a list of lines of code
*/
@VisibleForTesting
Parser.ParseResult parseFileWithComments(String... inputLines) {
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index 113ba74..61a6ba8 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -789,7 +789,6 @@
@Test
public void testRelativeLabelInExternalRepository() throws Exception {
- scratch.file("BUILD");
scratch.file("external_rule.bzl",
"def _impl(ctx):",
" return",
@@ -800,6 +799,37 @@
" }",
")");
+ scratch.file("BUILD",
+ "filegroup(name='dep')");
+
+ scratch.file("/r/a/BUILD",
+ "load('/external_rule', 'external_rule')",
+ "external_rule(name='r')");
+
+ scratch.overwriteFile("WORKSPACE",
+ "local_repository(name='r', path='/r')");
+
+ invalidatePackages();
+ SkylarkRuleContext context = createRuleContext("@r//a:r");
+ Label depLabel = (Label) evalRuleContextCode(context, "ruleContext.attr.internal_dep.label");
+ assertThat(depLabel).isEqualTo(Label.parseAbsolute("//:dep"));
+ }
+
+ @Test
+ public void testCallerRelativeLabelInExternalRepository() throws Exception {
+ scratch.file("BUILD");
+ scratch.file("external_rule.bzl",
+ "def _impl(ctx):",
+ " return",
+ "external_rule = rule(",
+ " implementation = _impl,",
+ " attrs = {",
+ " 'internal_dep': attr.label(",
+ " default = Label('//:dep', relative_to_caller_repository = True)",
+ " )",
+ " }",
+ ")");
+
scratch.file("/r/BUILD",
"filegroup(name='dep')");
diff --git a/src/test/shell/bazel/external_skylark_load_test.sh b/src/test/shell/bazel/external_skylark_load_test.sh
index 5f221c0..bd8318a 100755
--- a/src/test/shell/bazel/external_skylark_load_test.sh
+++ b/src/test/shell/bazel/external_skylark_load_test.sh
@@ -123,6 +123,117 @@
"LOCAL!"
}
+function test_skylark_repository_relative_label() {
+ repo2=$TEST_TMPDIR/repo2
+ mkdir -p $repo2
+ touch $repo2/WORKSPACE $repo2/BUILD
+ cat > $repo2/remote.bzl <<EOF
+def _impl(ctx):
+ print(Label("//foo:bar"))
+
+remote_rule = rule(
+ implementation = _impl,
+)
+EOF
+
+ cat > WORKSPACE <<EOF
+local_repository(
+ name = "r",
+ path = "$repo2",
+)
+EOF
+ cat > BUILD <<EOF
+load('@r//:remote.bzl', 'remote_rule')
+
+remote_rule(name = 'local')
+EOF
+
+ bazel build //:local &> $TEST_log || fail "Building local failed"
+ expect_log "@r//foo:bar"
+
+ cat > $repo2/remote.bzl <<EOF
+def _impl(ctx):
+ print(Label("//foo:bar", relative_to_caller_repository = True))
+
+remote_rule = rule(
+ implementation = _impl,
+)
+EOF
+ bazel build //:local &> $TEST_log || fail "Building local failed"
+ expect_log "//foo:bar"
+}
+
+# Going one level deeper: if we have:
+# local/
+# BUILD
+# r1/
+# BUILD
+# r2/
+# BUILD
+# remote.bzl
+# If //foo in local depends on //bar in r1, which is a Skylark rule
+# defined in r2/remote.bzl, then a Label in remote.bzl should either
+# resolve to @r2//whatever or @r1//whatever.
+function test_skylark_repository_nested_relative_label() {
+ repo1=$TEST_TMPDIR/repo1
+ repo2=$TEST_TMPDIR/repo2
+ mkdir -p $repo1 $repo2
+
+ # local
+ cat > WORKSPACE <<EOF
+local_repository(
+ name = "r1",
+ path = "$repo1",
+)
+local_repository(
+ name = "r2",
+ path = "$repo2",
+)
+EOF
+ cat > BUILD <<'EOF'
+genrule(
+ name = "foo",
+ srcs = ["@r1//:bar"],
+ outs = ["foo.out"],
+ cmd = "echo '$(SRCS)' > $@",
+)
+EOF
+
+ # r1
+ touch $repo1/WORKSPACE
+ cat > $repo1/BUILD <<EOF
+load('@r2//:remote.bzl', 'remote_rule')
+
+remote_rule(
+ name = 'bar',
+ visibility = ["//visibility:public"]
+)
+EOF
+
+ # r2
+ touch $repo2/WORKSPACE $repo2/BUILD
+ cat > $repo2/remote.bzl <<EOF
+def _impl(ctx):
+ print(Label("//foo:bar"))
+
+remote_rule = rule(
+ implementation = _impl,
+)
+EOF
+
+ bazel build //:foo &> $TEST_log || fail "Building local failed"
+ expect_log "@r2//foo:bar"
+
+ cat > $repo2/remote.bzl <<EOF
+def _impl(ctx):
+ print(Label("//foo:bar", relative_to_caller_repository = True))
+
+remote_rule = rule(
+ implementation = _impl,
+)
+EOF
+ bazel build //:foo &> $TEST_log || fail "Building local failed"
+ expect_log "@r1//foo:bar"
+}
+
run_suite "Test Skylark loads from/in external repositories"
-
-
diff --git a/src/test/shell/bazel/skylark_repository_test.sh b/src/test/shell/bazel/skylark_repository_test.sh
index 16f52ab..a79feb7 100755
--- a/src/test/shell/bazel/skylark_repository_test.sh
+++ b/src/test/shell/bazel/skylark_repository_test.sh
@@ -305,9 +305,8 @@
function test_skylark_repository_which_and_execute() {
setup_skylark_repository
- bazel info
-
# Test we are using the client environment, not the server one
+ bazel info &> /dev/null # Start up the server.
echo "#!/bin/bash" > bin.sh
echo "exit 0" >> bin.sh
chmod +x bin.sh
