Skip to content

Commit

Permalink
Add the (no-op) 'exec_groups' attribute to the rule() function.
Browse files Browse the repository at this point in the history
    Currently not linked up to any logic behind the scenes. Usage of attribute hidden behind new --experimental_execution_groups flag.

    PiperOrigin-RevId: 302672390
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent 16f5cc4 commit 2e3c8d3
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 65 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,9 @@ public BaseFunction rule(
(Label) Module.ofInnermostEnclosingStarlarkFunction(thread).getLabel(),
thread.getTransitiveContentHashCode());

builder.addRequiredToolchains(parseToolchains(toolchains, thread));
builder.addRequiredToolchains(
collectToolchainLabels(
toolchains.getContents(String.class, "toolchains"), bazelContext.getRepoMapping()));

if (!buildSetting.equals(Starlark.NONE) && !cfg.equals(Starlark.NONE)) {
throw Starlark.errorf(
Expand Down Expand Up @@ -399,7 +401,10 @@ public BaseFunction rule(
}

if (!execCompatibleWith.isEmpty()) {
builder.addExecutionPlatformConstraints(parseExecCompatibleWith(execCompatibleWith, thread));
builder.addExecutionPlatformConstraints(
collectConstraintLabels(
execCompatibleWith.getContents(String.class, "exec_compatile_with"),
bazelContext.getRepoMapping()));
}

return new SkylarkRuleFunction(builder, type, attributes, thread.getCallerLocation());
Expand Down Expand Up @@ -437,46 +442,37 @@ private static void addAttribute(RuleClass.Builder builder, Attribute attribute)
}
}

/**
* Parses a sequence of label strings with a repo mapping.
*
* @param inputs sequence of input strings
* @param mapping repository mapping
* @param adjective describes the purpose of the label; used for errors
* @throws EvalException if the label can't be parsed
*/
private static ImmutableList<Label> parseLabels(
Iterable<String> inputs,
ImmutableMap<RepositoryName, RepositoryName> mapping,
String adjective)
private static ImmutableList<Label> collectToolchainLabels(
Iterable<String> rawLabels, ImmutableMap<RepositoryName, RepositoryName> mapping)
throws EvalException {
ImmutableList.Builder<Label> parsedLabels = new ImmutableList.Builder<>();
for (String input : inputs) {
ImmutableList.Builder<Label> requiredToolchains = new ImmutableList.Builder<>();
for (String rawLabel : rawLabels) {
try {
Label label = Label.parseAbsolute(input, mapping);
parsedLabels.add(label);
Label toolchainLabel = Label.parseAbsolute(rawLabel, mapping);
requiredToolchains.add(toolchainLabel);
} catch (LabelSyntaxException e) {
throw Starlark.errorf(
"Unable to parse %s label '%s': %s", adjective, input, e.getMessage());
throw new EvalException(
null, String.format("Unable to parse toolchain %s: %s", rawLabel, e.getMessage()), e);
}
}
return parsedLabels.build();

return requiredToolchains.build();
}

private static ImmutableList<Label> parseToolchains(Sequence<?> inputs, StarlarkThread thread)
private static ImmutableList<Label> collectConstraintLabels(
Iterable<String> rawLabels, ImmutableMap<RepositoryName, RepositoryName> mapping)
throws EvalException {
return parseLabels(
inputs.getContents(String.class, "toolchains"),
BazelStarlarkContext.from(thread).getRepoMapping(),
"toolchain");
}
ImmutableList.Builder<Label> constraintLabels = new ImmutableList.Builder<>();
for (String rawLabel : rawLabels) {
try {
Label constraintLabel = Label.parseAbsolute(rawLabel, mapping);
constraintLabels.add(constraintLabel);
} catch (LabelSyntaxException e) {
throw Starlark.errorf("Unable to parse constraint %s: %s", rawLabel, e.getMessage());
}
}

private static ImmutableList<Label> parseExecCompatibleWith(
Sequence<?> inputs, StarlarkThread thread) throws EvalException {
return parseLabels(
inputs.getContents(String.class, "exec_compatible_with"),
BazelStarlarkContext.from(thread).getRepoMapping(),
"constraint");
return constraintLabels.build();
}

@Override
Expand Down Expand Up @@ -573,7 +569,9 @@ public SkylarkAspect aspect(
ImmutableSet.copyOf(fragments.getContents(String.class, "fragments")),
HostTransition.INSTANCE,
ImmutableSet.copyOf(hostFragments.getContents(String.class, "host_fragments")),
parseToolchains(toolchains, thread),
collectToolchainLabels(
toolchains.getContents(String.class, "toolchains"),
BazelStarlarkContext.from(thread).getRepoMapping()),
applyToGeneratingRules);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,20 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
+ "migration instructions.")
public boolean incompatibleUseCcConfigureFromRulesCc;

@Option(
name = "incompatible_restrict_named_params",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help =
"If set to true, restricts a number of Starlark built-in function parameters to be "
+ "only specifiable positionally (and not by keyword).")
public boolean incompatibleRestrictNamedParams;

@Option(
name = "incompatible_depset_for_libraries_to_link_getter",
defaultValue = "true",
Expand Down Expand Up @@ -691,6 +705,7 @@ public StarlarkSemantics toSkylarkSemantics() {
.incompatibleNoRuleOutputsParam(incompatibleNoRuleOutputsParam)
.incompatibleNoSupportToolsInActionInputs(incompatibleNoSupportToolsInActionInputs)
.incompatibleNoTargetOutputGroup(incompatibleNoTargetOutputGroup)
.incompatibleRestrictNamedParams(incompatibleRestrictNamedParams)
.incompatibleRunShellCommandString(incompatibleRunShellCommandString)
.incompatibleVisibilityPrivateAttributesAtDefinition(
incompatibleVisibilityPrivateAttributesAtDefinition)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,7 @@ SkylarkAspectApi aspect(
@Param(
name = "label_string",
type = String.class,
legacyNamed = true,
doc = "the label string."),
@Param(
name = "relative_to_caller_repository",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,37 +22,15 @@
import java.util.List;

/**
* Options that affect the dynamic behavior of Starlark execution and operators.
* Options that affect Starlark semantics.
*
* <p>For descriptions of what these options do, see {@link packages.StarlarkSemanticsOptions}.
*
* <p>For options that affect the static behavior of the Starlark frontend (lexer, parser,
* validator, compiler), see FileOptions.
* <p>For descriptions of what these options do, see {@link StarlarkSemanticsOptions}.
*/
// TODO(brandjon): User error messages that reference options should maybe be substituted with the
// option name outside of the core Starlark interpreter?
// TODO(brandjon): Eventually these should be documented in full here, and StarlarkSemanticsOptions
// should refer to this class for documentation. But this doesn't play nice with the options
// parser's annotation mechanism.
//
// TODO(adonovan): nearly all of these options are Bazel-isms.
// The only ones that affect the Starlark interpreter directly are are:
// - incompatibleRestrictNamedParams, which affects calls to many built-ins,
// but is used only by copybara and will be deleted soon (CL 298871155);
// - incompatibleRestrictStringEscapes, which affects the lexer and is thus
// properly one of the FileOptions, but piggybacks on the command-line flag
// plumbing of StarlarkSemantics; and
// - internalSkylarkFlagTestCanary, which is used to test propagation of Bazel
// command-line flags to the 'print' built-in, but this could easily be
// achieved using some other Bazel-specific built-in.
// Most of the rest are used generically to disable parameters to built-ins,
// or to disable fields of modules, based on flags. In both of those cases,
// a generic set-of-feature-strings representation would do.
// A few could be expressed as Bazel-specific thread state,
// though several are inspected by the implementations of operations
// such as SkylarkIndexable, SkylarkQueryable, and SkylarkClassObject.
// TODO(adonovan): move to lib.packages.BuildLanguageSemantics.
//
@AutoValue
public abstract class StarlarkSemantics {

Expand Down Expand Up @@ -269,6 +247,8 @@ boolean isFeatureEnabledBasedOnTogglingFlags(String enablingFlag, String disabli

public abstract boolean incompatibleNoTargetOutputGroup();

public abstract boolean incompatibleRestrictNamedParams();

public abstract boolean incompatibleRunShellCommandString();

public abstract boolean incompatibleVisibilityPrivateAttributesAtDefinition();
Expand Down Expand Up @@ -360,6 +340,7 @@ public static Builder builderWithDefaults() {
.incompatibleNoSupportToolsInActionInputs(true)
.incompatibleNoTargetOutputGroup(true)
.incompatibleRunShellCommandString(false)
.incompatibleRestrictNamedParams(true)
.incompatibleVisibilityPrivateAttributesAtDefinition(false)
.internalSkylarkFlagTestCanary(false)
.incompatibleDoNotSplitLinkingCmdline(true)
Expand Down Expand Up @@ -443,6 +424,8 @@ public abstract static class Builder {

public abstract Builder incompatibleNoTargetOutputGroup(boolean value);

public abstract Builder incompatibleRestrictNamedParams(boolean value);

public abstract Builder incompatibleRunShellCommandString(boolean value);

public abstract Builder incompatibleVisibilityPrivateAttributesAtDefinition(boolean value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.skylarkbuildapi.ExecGroupApi;
import com.google.devtools.build.lib.skylarkbuildapi.FileApi;
import com.google.devtools.build.lib.skylarkbuildapi.SkylarkAspectApi;
import com.google.devtools.build.lib.skylarkbuildapi.SkylarkRuleFunctionsApi;
Expand Down Expand Up @@ -238,12 +237,6 @@ public SkylarkAspectApi aspect(
return fakeAspect;
}

@Override
public ExecGroupApi execGroup(
Sequence<?> execCompatibleWith, Sequence<?> toolchains, StarlarkThread thread) {
return new FakeExecGroup();
}

/**
* A fake {@link BaseFunction} implementation which serves as an identifier for a rule definition.
* A skylark invocation of 'rule()' should spawn a unique instance of this class and return it.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E
"--incompatible_no_rule_outputs_param=" + rand.nextBoolean(),
"--incompatible_no_support_tools_in_action_inputs=" + rand.nextBoolean(),
"--incompatible_no_target_output_group=" + rand.nextBoolean(),
"--incompatible_restrict_named_params=" + rand.nextBoolean(),
"--incompatible_run_shell_command_string=" + rand.nextBoolean(),
"--incompatible_visibility_private_attributes_at_definition=" + rand.nextBoolean(),
"--incompatible_require_linker_input_cc_api=" + rand.nextBoolean(),
Expand Down Expand Up @@ -212,6 +213,7 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
.incompatibleNoRuleOutputsParam(rand.nextBoolean())
.incompatibleNoSupportToolsInActionInputs(rand.nextBoolean())
.incompatibleNoTargetOutputGroup(rand.nextBoolean())
.incompatibleRestrictNamedParams(rand.nextBoolean())
.incompatibleRunShellCommandString(rand.nextBoolean())
.incompatibleVisibilityPrivateAttributesAtDefinition(rand.nextBoolean())
.incompatibleRequireLinkerInputCcApi(rand.nextBoolean())
Expand Down

0 comments on commit 2e3c8d3

Please sign in to comment.