From ab7295663cae0a940ddb89125a0f411be9dad8c2 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Wed, 4 Oct 2023 02:42:52 -0700 Subject: [PATCH] Raise an early error on invalid labels in transitions inputs/outputs When a label pointing to an unavailable repository is specified as a transition's input or output, an early error is raised instead of crashing during transition evaluation. Fixes #19632 Closes #19634. PiperOrigin-RevId: 570634319 Change-Id: Icda2946313898db2372484b4f845aafcf5a8b272 --- .../lib/rules/config/ConfigGlobalLibrary.java | 31 ++++++++++--- .../lib/rules/LabelBuildSettingTest.java | 44 +++++++++++++++++++ 2 files changed, 68 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigGlobalLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigGlobalLibrary.java index cb278d768c0c53..1138c849c615ef 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigGlobalLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigGlobalLibrary.java @@ -21,6 +21,7 @@ import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition.Settings; import com.google.devtools.build.lib.cmdline.BazelModuleContext; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.starlarkbuildapi.config.ConfigGlobalLibraryApi; import com.google.devtools.build.lib.starlarkbuildapi.config.ConfigurationTransitionApi; import java.util.HashSet; @@ -57,9 +58,11 @@ public ConfigurationTransitionApi transition( // that info through StarlarkSemantics) or add an "exec = True" paramter to Starlark's // transition() function. boolean isExecTransition = implementation.getLocation().file().endsWith("_exec_platforms.bzl"); - validateBuildSettingKeys(inputsList, Settings.INPUTS, isExecTransition); - validateBuildSettingKeys(outputsList, Settings.OUTPUTS, isExecTransition); BazelModuleContext moduleContext = BazelModuleContext.ofInnermostBzlOrThrow(thread); + validateBuildSettingKeys( + inputsList, Settings.INPUTS, isExecTransition, moduleContext.packageContext()); + validateBuildSettingKeys( + outputsList, Settings.OUTPUTS, isExecTransition, moduleContext.packageContext()); Location location = thread.getCallerLocation(); return StarlarkDefinedConfigTransition.newRegularTransition( implementation, @@ -79,16 +82,22 @@ public ConfigurationTransitionApi analysisTestTransition( throws EvalException { Map changedSettingsMap = Dict.cast(changedSettings, String.class, Object.class, "changed_settings dict"); - validateBuildSettingKeys( - changedSettingsMap.keySet(), Settings.OUTPUTS, /* isExecTransition= */ false); BazelModuleContext moduleContext = BazelModuleContext.ofInnermostBzlOrThrow(thread); + validateBuildSettingKeys( + changedSettingsMap.keySet(), + Settings.OUTPUTS, + /* isExecTransition= */ false, + moduleContext.packageContext()); Location location = thread.getCallerLocation(); return StarlarkDefinedConfigTransition.newAnalysisTestTransition( changedSettingsMap, moduleContext.repoMapping(), moduleContext.label(), location); } private void validateBuildSettingKeys( - Iterable optionKeys, Settings keyErrorDescriptor, boolean isExecTransition) + Iterable optionKeys, + Settings keyErrorDescriptor, + boolean isExecTransition, + Label.PackageContext packageContext) throws EvalException { HashSet processedOptions = Sets.newHashSet(); @@ -97,8 +106,16 @@ private void validateBuildSettingKeys( for (String optionKey : optionKeys) { if (!optionKey.startsWith(COMMAND_LINE_OPTION_PREFIX)) { try { - var unused = Label.parseCanonicalUnchecked(optionKey); - } catch (IllegalArgumentException e) { + Label label = Label.parseWithRepoContext(optionKey, packageContext); + if (!label.getRepository().isVisible()) { + throw Starlark.errorf( + "invalid transition %s '%s': no repo visible as @%s from %s", + singularErrorDescriptor, + label, + label.getRepository().getName(), + label.getRepository().getOwnerRepoDisplayString()); + } + } catch (LabelSyntaxException e) { throw Starlark.errorf( "invalid transition %s '%s'. If this is intended as a native option, " + "it must begin with //command_line_option: %s", diff --git a/src/test/java/com/google/devtools/build/lib/rules/LabelBuildSettingTest.java b/src/test/java/com/google/devtools/build/lib/rules/LabelBuildSettingTest.java index 5f3254d21a75d4..5d6c0ffb873ab8 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/LabelBuildSettingTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/LabelBuildSettingTest.java @@ -339,4 +339,48 @@ public void transitionOutput_otherRepo() throws Exception { assertThat(getConfiguredTarget("//test:buildme")).isNotNull(); assertNoEvents(); } + + @Test + public void testInvisibleRepoInLabelResultsInEarlyError() throws Exception { + setBuildLanguageOptions("--enable_bzlmod"); + + scratch.file("MODULE.bazel"); + scratch.file( + "test/defs.bzl", + "def _setting_impl(ctx):", + " return []", + "string_flag = rule(", + " implementation = _setting_impl,", + " build_setting = config.string(flag=True),", + ")", + "def _transition_impl(settings, attr):", + " return {'//test:formation': 'mesa'}", + "formation_transition = transition(", + " implementation = _transition_impl,", + " inputs = ['@foobar//test:formation'],", // invalid repo name + " outputs = ['//test:formation'],", + ")", + "def _impl(ctx):", + " return []", + "state = rule(", + " implementation = _impl,", + " cfg = formation_transition,", + " attrs = {", + " '_allowlist_function_transition': attr.label(", + " default = '//tools/allowlists/function_transition_allowlist',", + " ),", + " })"); + scratch.file( + "test/BUILD", + "load('//test:defs.bzl', 'state', 'string_flag')", + "state(name = 'arizona')", + "string_flag(name = 'formation', build_setting_default = 'canyon')"); + + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//test:arizona"); + + assertContainsEvent( + "Error in transition: invalid transition input '@[unknown repo 'foobar' requested from @]" + + "//test:formation': no repo visible as @foobar from main repository"); + } }