Skip to content

Commit

Permalink
Raise an early error on invalid labels in transitions inputs/outputs
Browse files Browse the repository at this point in the history
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 bazelbuild#19632

Closes bazelbuild#19634.

PiperOrigin-RevId: 570634319
Change-Id: Icda2946313898db2372484b4f845aafcf5a8b272
  • Loading branch information
fmeum authored and copybara-github committed Oct 4, 2023
1 parent 9b30bf7 commit ab72956
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -79,16 +82,22 @@ public ConfigurationTransitionApi analysisTestTransition(
throws EvalException {
Map<String, Object> 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<String> optionKeys, Settings keyErrorDescriptor, boolean isExecTransition)
Iterable<String> optionKeys,
Settings keyErrorDescriptor,
boolean isExecTransition,
Label.PackageContext packageContext)
throws EvalException {

HashSet<String> processedOptions = Sets.newHashSet();
Expand All @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}

0 comments on commit ab72956

Please sign in to comment.