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
…19764)

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

Fixes #19641
  • Loading branch information
fmeum authored Oct 9, 2023
1 parent b90ab8b commit d3d2dd2
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 6 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 @@ -53,10 +54,10 @@ public ConfigurationTransitionApi transition(
StarlarkSemantics semantics = thread.getSemantics();
List<String> inputsList = Sequence.cast(inputs, String.class, "inputs");
List<String> outputsList = Sequence.cast(outputs, String.class, "outputs");
validateBuildSettingKeys(inputsList, Settings.INPUTS);
validateBuildSettingKeys(outputsList, Settings.OUTPUTS);
BazelModuleContext moduleContext =
BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(thread));
validateBuildSettingKeys(inputsList, Settings.INPUTS, moduleContext.packageContext());
validateBuildSettingKeys(outputsList, Settings.OUTPUTS, moduleContext.packageContext());
Location location = thread.getCallerLocation();
return StarlarkDefinedConfigTransition.newRegularTransition(
implementation,
Expand All @@ -76,15 +77,17 @@ public ConfigurationTransitionApi analysisTestTransition(
throws EvalException {
Map<String, Object> changedSettingsMap =
Dict.cast(changedSettings, String.class, Object.class, "changed_settings dict");
validateBuildSettingKeys(changedSettingsMap.keySet(), Settings.OUTPUTS);
BazelModuleContext moduleContext =
BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(thread));
validateBuildSettingKeys(
changedSettingsMap.keySet(), Settings.OUTPUTS, moduleContext.packageContext());
Location location = thread.getCallerLocation();
return StarlarkDefinedConfigTransition.newAnalysisTestTransition(
changedSettingsMap, moduleContext.repoMapping(), moduleContext.label(), location);
}

private void validateBuildSettingKeys(Iterable<String> optionKeys, Settings keyErrorDescriptor)
private void validateBuildSettingKeys(
Iterable<String> optionKeys, Settings keyErrorDescriptor, Label.PackageContext packageContext)
throws EvalException {

HashSet<String> processedOptions = Sets.newHashSet();
Expand All @@ -93,8 +96,16 @@ private void validateBuildSettingKeys(Iterable<String> optionKeys, Settings keyE
for (String optionKey : optionKeys) {
if (!optionKey.startsWith(COMMAND_LINE_OPTION_PREFIX)) {
try {
Label.parseAbsoluteUnchecked(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 d3d2dd2

Please sign in to comment.