Follow unconfigured `alias`es during options parsing
Fixes #20582
RELNOTES: Starlark command-line flags can now be referred to through `alias` targets.
Closes #22192.
PiperOrigin-RevId: 629865954
Change-Id: I6215c8484ddc08e75507191bfa1eb5bc709c5fc6
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java b/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java
index 7461ae4..976cf73 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java
@@ -17,6 +17,7 @@
import static com.google.devtools.build.lib.analysis.config.CoreOptionConverters.BUILD_SETTING_CONVERTERS;
import static com.google.devtools.build.lib.packages.RuleClass.Builder.STARLARK_BUILD_SETTING_DEFAULT_ATTR_NAME;
import static com.google.devtools.build.lib.packages.Type.BOOLEAN;
+import static java.util.stream.Collectors.joining;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
@@ -27,7 +28,7 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.packages.BuildSetting;
-import com.google.devtools.build.lib.packages.Rule;
+import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.packages.Types;
@@ -38,10 +39,13 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import java.util.SequencedSet;
import java.util.TreeMap;
+import java.util.stream.Stream;
import javax.annotation.Nullable;
/**
@@ -282,7 +286,7 @@
}
/**
- * Returns the given build setting's {@link Target}.
+ * Returns the given build setting's {@link Target}, following (unconfigured) aliases if needed.
*
* @return the target, or null if the {@link BuildSettingLoader} needs to do more work to retrieve
* the target
@@ -293,23 +297,71 @@
return buildSettings.get(targetToBuild);
}
- Target buildSetting;
- try {
- buildSetting = buildSettingLoader.loadBuildSetting(targetToBuild);
- if (buildSetting == null) {
- return null;
+ Target target;
+ String targetToLoadNext = targetToBuild;
+ SequencedSet<Label> aliasChain = new LinkedHashSet<>();
+ while (true) {
+ try {
+ target = buildSettingLoader.loadBuildSetting(targetToLoadNext);
+ if (target == null) {
+ return null;
+ }
+ } catch (InterruptedException | TargetParsingException e) {
+ Thread.currentThread().interrupt();
+ throw new OptionsParsingException(
+ "Error loading option " + targetToBuild + ": " + e.getMessage(), targetToBuild, e);
}
- } catch (InterruptedException | TargetParsingException e) {
- Thread.currentThread().interrupt();
+ if (!aliasChain.add(target.getLabel())) {
+ throw new OptionsParsingException(
+ String.format(
+ "Failed to load build setting '%s' due to a cycle in alias chain: %s",
+ targetToBuild,
+ formatAliasChain(Stream.concat(aliasChain.stream(), Stream.of(target.getLabel())))),
+ targetToBuild);
+ }
+ if (target.getAssociatedRule() == null) {
+ throw new OptionsParsingException(
+ String.format("Unrecognized option: %s", formatAliasChain(aliasChain.stream())),
+ targetToBuild);
+ }
+ if (target.getAssociatedRule().isBuildSetting()) {
+ break;
+ }
+ // Follow the unconfigured values of aliases.
+ if (target.getAssociatedRule().getRuleClass().equals("alias")) {
+ targetToLoadNext =
+ switch (target.getAssociatedRule().getAttr("actual")) {
+ case Label label -> label.getUnambiguousCanonicalForm();
+ case BuildType.SelectorList<?> ignored ->
+ throw new OptionsParsingException(
+ String.format(
+ "Failed to load build setting '%s' as it resolves to an alias with an"
+ + " actual value that uses select(): %s. This is not supported as"
+ + " build settings are needed to determine the configuration the"
+ + " select is evaluated in.",
+ targetToBuild, formatAliasChain(aliasChain.stream())),
+ targetToBuild);
+ case null, default ->
+ throw new IllegalStateException(
+ String.format(
+ "Alias target '%s' with 'actual' attr value not equals to a label or a"
+ + " selectorlist",
+ target.getLabel()));
+ };
+ continue;
+ }
throw new OptionsParsingException(
- "Error loading option " + targetToBuild + ": " + e.getMessage(), targetToBuild, e);
+ String.format("Unrecognized option: %s", formatAliasChain(aliasChain.stream())),
+ targetToBuild);
}
- Rule associatedRule = buildSetting.getAssociatedRule();
- if (associatedRule == null || associatedRule.getRuleClassObject().getBuildSetting() == null) {
- throw new OptionsParsingException("Unrecognized option: " + targetToBuild, targetToBuild);
- }
- buildSettings.put(targetToBuild, buildSetting);
- return buildSetting;
+ ;
+
+ buildSettings.put(targetToBuild, target);
+ return target;
+ }
+
+ private static String formatAliasChain(Stream<Label> aliasChain) {
+ return aliasChain.map(Label::getCanonicalForm).collect(joining(" -> "));
}
@VisibleForTesting
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkOptionsParsingTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkOptionsParsingTest.java
index e36f782..3375361 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkOptionsParsingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkOptionsParsingTest.java
@@ -567,4 +567,146 @@
.hasMessageThat()
.contains("//test:all: user-defined flags must reference exactly one target");
}
+
+ @Test
+ public void flagIsAlias() throws Exception {
+ scratch.file(
+ "test/build_setting.bzl",
+ """
+ string_flag = rule(
+ implementation = lambda ctx: [],
+ build_setting = config.string(flag = True),
+ )
+ """);
+ scratch.file(
+ "test/BUILD",
+ """
+ load("//test:build_setting.bzl", "string_flag")
+
+ alias(
+ name = "one",
+ actual = "//test/pkg:two",
+ )
+
+ string_flag(
+ name = "three",
+ build_setting_default = "",
+ )
+ """);
+ scratch.file(
+ "test/pkg/BUILD",
+ """
+ alias(
+ name = "two",
+ actual = "//test:three",
+ )
+ """);
+
+ OptionsParsingResult result = parseStarlarkOptions("--//test:one=one --//test/pkg:two=two");
+
+ assertThat(result.getStarlarkOptions()).containsExactly("//test:three", "two");
+ }
+
+ @Test
+ public void flagIsAlias_cycle() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ """
+ alias(
+ name = "one",
+ actual = "//test/pkg:two",
+ )
+
+ alias(
+ name = "three",
+ actual = ":one",
+ )
+ """);
+ scratch.file(
+ "test/pkg/BUILD",
+ """
+ alias(
+ name = "two",
+ actual = "//test:three",
+ )
+ """);
+
+ OptionsParsingException e =
+ assertThrows(OptionsParsingException.class, () -> parseStarlarkOptions("--//test:one=one"));
+
+ assertThat(e)
+ .hasMessageThat()
+ .isEqualTo(
+ "Failed to load build setting '//test:one' due to a cycle in alias chain: //test:one"
+ + " -> //test/pkg:two -> //test:three -> //test:one");
+ }
+
+ @Test
+ public void flagIsAlias_usesSelect() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ """
+ alias(
+ name = "one",
+ actual = "//test/pkg:two",
+ )
+
+ alias(
+ name = "three",
+ actual = ":one",
+ )
+ """);
+ scratch.file(
+ "test/pkg/BUILD",
+ """
+ alias(
+ name = "two",
+ actual = select({"//conditions:default": "//test:three"}),
+ )
+ """);
+
+ OptionsParsingException e =
+ assertThrows(OptionsParsingException.class, () -> parseStarlarkOptions("--//test:one=one"));
+
+ assertThat(e)
+ .hasMessageThat()
+ .isEqualTo(
+ "Failed to load build setting '//test:one' as it resolves to an alias with an actual"
+ + " value that uses select(): //test:one -> //test/pkg:two. This is not supported"
+ + " as build settings are needed to determine the configuration the select is"
+ + " evaluated in.");
+ }
+
+ @Test
+ public void flagIsAlias_resolvesToNonBuildSettingTarget() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ """
+ alias(
+ name = "one",
+ actual = "//test/pkg:two",
+ )
+
+ genrule(
+ name = "three",
+ outs = ["out"],
+ cmd = "echo hello > $@",
+ )
+ """);
+ scratch.file(
+ "test/pkg/BUILD",
+ """
+ alias(
+ name = "two",
+ actual = "//test:three",
+ )
+ """);
+
+ OptionsParsingException e =
+ assertThrows(OptionsParsingException.class, () -> parseStarlarkOptions("--//test:one=one"));
+
+ assertThat(e)
+ .hasMessageThat()
+ .isEqualTo("Unrecognized option: //test:one -> //test/pkg:two -> //test:three");
+ }
}
