Skip to content

Commit

Permalink
Automated rollback of commit c266ac966761c4b3d8a408a03e407505c93effdd.
Browse files Browse the repository at this point in the history
    *** Reason for rollback ***

    Broke Bazel Toolchain on RBE
    https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/1743#1dba3b6d-8ea5-4efc-b696-f48e62b035fa

    *** Original change description ***

    Work towards #12006 Allow exec groups to inherit from the rule or other exec groups

    Add a "test" exec group for TestRunnerActions. This will allow users to set {"test.key", "value"} inside their exec properties and {"key", "value"} will propagate as to just TestRunnerActions.

    This addresses user request bazelbuild/bazel#10799

    PiperOrigin-RevId: 340635429
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent f23778a commit b6ebca7
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.HostTransition;
import com.google.devtools.build.lib.analysis.config.RunUnder;
Expand Down Expand Up @@ -193,8 +194,6 @@ public Object getDefault(AttributeMap rule) {
.taggable()
.nonconfigurable("policy decision: should be consistent across configurations"))
.add(attr("args", STRING_LIST))
.add(attr("env", STRING_DICT))
.add(attr("env_inherit", STRING_LIST))
// Input files for every test action
.add(
attr("$test_wrapper", LABEL)
Expand Down Expand Up @@ -247,7 +246,7 @@ public Metadata getMetadata() {
return RuleDefinition.Metadata.builder()
.name("$test_base_rule")
.type(RuleClassType.ABSTRACT)
.ancestors(MakeVariableExpandingRule.class)
.ancestors(RootRule.class, MakeVariableExpandingRule.class)
.build();
}
}
Expand All @@ -274,9 +273,6 @@ public static synchronized ImmutableList<Label> getTestRuntimeLabelList(
public static final String TAGGED_TRIMMING_ATTR = "transitive_configs";

/** Share common attributes across both base and Starlark base rules. */
// TODO(bazel-team): replace this with a common RuleDefinition ancestor of NativeBuildRule
// and StarlarkRuleClassFunctions.baseRule. This requires refactoring StarlarkRuleClassFunctions
// to instantiate its RuleClasses through RuleDefinition.
public static RuleClass.Builder commonCoreAndStarlarkAttributes(RuleClass.Builder builder) {
return builder
// The visibility attribute is special: it is a nodep label, and loading the
Expand Down Expand Up @@ -351,24 +347,41 @@ public static RuleClass.Builder commonCoreAndStarlarkAttributes(RuleClass.Builde
.nonconfigurable("applicable_licenses is not configurable"));
}

public static RuleClass.Builder nameAttribute(RuleClass.Builder builder) {
return builder.add(attr("name", STRING).nonconfigurable("Rule name"));
}

public static RuleClass.Builder execPropertiesAttribute(RuleClass.Builder builder)
throws ConversionException {
return builder.add(
attr(RuleClass.EXEC_PROPERTIES, STRING_DICT).defaultValue(ImmutableMap.of()));
}

/**
* Ancestor of every native rule in BUILD files (not WORKSPACE files).
*
* <p>This includes:
* Ancestor of every rule.
*
* <ul>
* <li>rules that create actions ({@link NativeActionCreatingRule})
* <li>rules that encapsulate toolchain and build environment context
* <li>rules that aggregate other rules (like file groups, test suites, or aliases)
* </ul>
* <p>Adds the name attribute to every rule.
*/
public static final class NativeBuildRule implements RuleDefinition {
public static final class RootRule implements RuleDefinition {

@Override
public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment environment) {
return nameAttribute(builder).build();
}

@Override
public Metadata getMetadata() {
return RuleDefinition.Metadata.builder()
.name("$root_rule")
.type(RuleClassType.ABSTRACT)
.build();
}
}

/**
* Common parts of some rules.
*/
public static final class BaseRule implements RuleDefinition {
@Override
public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
return commonCoreAndStarlarkAttributes(builder)
Expand All @@ -384,8 +397,9 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
@Override
public Metadata getMetadata() {
return RuleDefinition.Metadata.builder()
.name("$native_build_rule")
.name("$base_rule")
.type(RuleClassType.ABSTRACT)
.ancestors(RootRule.class)
.build();
}
}
Expand Down Expand Up @@ -417,13 +431,9 @@ public Metadata getMetadata() {
}

/**
* Ancestor of every native BUILD rule that creates actions.
*
* <p>This is a subset of all BUILD rules. Filegroups and aliases, for example, simply encapsulate
* other rules. Toolchain rules provide metadata for actions of other rules. See {@link
* NativeBuildRule} for these.
* Common ancestor class for some rules.
*/
public static final class NativeActionCreatingRule implements RuleDefinition {
public static final class RuleBase implements RuleDefinition {
@Override
public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
return builder
Expand All @@ -449,20 +459,22 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
@Override
public Metadata getMetadata() {
return RuleDefinition.Metadata.builder()
.name("$native_buildable_rule")
.name("$rule")
.type(RuleClassType.ABSTRACT)
.ancestors(BaseRuleClasses.NativeBuildRule.class)
.ancestors(BaseRule.class)
.build();
}
}

public static final ImmutableSet<String> ALLOWED_RULE_CLASSES =
ImmutableSet.of("filegroup", "genrule", "Fileset");

/** A base rule for all binary rules. */
public static final class BinaryBaseRule implements RuleDefinition {
@Override
public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
return builder
.add(attr("args", STRING_LIST))
.add(attr("env", STRING_DICT))
.add(attr("output_licenses", LICENSE))
.add(
attr("$is_executable", BOOLEAN)
Expand All @@ -476,7 +488,7 @@ public Metadata getMetadata() {
return RuleDefinition.Metadata.builder()
.name("$binary_base_rule")
.type(RuleClassType.ABSTRACT)
.ancestors(MakeVariableExpandingRule.class)
.ancestors(RootRule.class, MakeVariableExpandingRule.class)
.build();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public TestActionBuilder(RuleContext ruleContext) {
*
* @return ordered list of test status artifacts
*/
public TestParams build() throws InterruptedException {
public TestParams build() {
Preconditions.checkNotNull(runfilesSupport);
TestShardingStrategy strategy =
ruleContext.getConfiguration().getFragment(TestConfiguration.class).testShardingStrategy();
Expand Down Expand Up @@ -148,14 +148,14 @@ private boolean isPersistentTestRunner() {
}

/**
* Creates a test action and artifacts for the given rule. The test action will use the specified
* executable and runfiles.
* Creates a test action and artifacts for the given rule. The test action will
* use the specified executable and runfiles.
*
* @return ordered list of test artifacts, one per action. These are used to drive execution in
* Skyframe, and by AggregatingTestListener and TestResultAnalyzer to keep track of completed
* and pending test runs.
* @return ordered list of test artifacts, one per action. These are used to drive
* execution in Skyframe, and by AggregatingTestListener and
* TestResultAnalyzer to keep track of completed and pending test runs.
*/
private TestParams createTestAction(int shards) throws InterruptedException {
private TestParams createTestAction(int shards) {
PathFragment targetName = PathFragment.create(ruleContext.getLabel().getName());
BuildConfiguration config = ruleContext.getConfiguration();
TestConfiguration testConfiguration = config.getFragment(TestConfiguration.class);
Expand Down

0 comments on commit b6ebca7

Please sign in to comment.