From 67663beb637d60e2114a6dae3c6da52e3bf9b01d Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Fri, 22 Jul 2022 14:15:31 +0100 Subject: [PATCH 1/2] query: expose `is_executable` It can be useful to know this, and currently the best proxy appears to be to look for `attributes.$is_executable` (which isn't documented), and or-ing that with a match on `rule_class.ends_with("_test")`. Instead, let's have a clearly documented API here. --- .../build/lib/analysis/BaseRuleClasses.java | 3 ++- .../starlark/StarlarkRuleClassFunctions.java | 5 +++-- .../google/devtools/build/lib/packages/Rule.java | 9 +++++++++ .../query/output/ProtoOutputFormatter.java | 3 +++ .../build/lib/rules/genrule/GenRuleBaseRule.java | 10 ++++++---- .../starlarkdocextract/ModuleInfoExtractor.java | 3 ++- .../build/lib/runtime/commands/RunCommand.java | 5 +---- src/main/protobuf/build.proto | 3 +++ .../devtools/build/lib/packages/RuleTest.java | 16 ++++++++++++++++ 9 files changed, 45 insertions(+), 12 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java index eb2a5374f20d2f..b364e9a39f7eaf 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java @@ -45,6 +45,7 @@ import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy; +import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; import com.google.devtools.build.lib.packages.TargetUtils; @@ -500,7 +501,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) .add(attr("env", STRING_DICT)) .add(attr("output_licenses", LICENSE)) .add( - attr("$is_executable", BOOLEAN) + attr(Rule.IS_EXECUTABLE_ATTRIBUTE_NAME, BOOLEAN) .value(true) .nonconfigurable("Called from RunCommand.isExecutable, which takes a Target")) .build(); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java index b7ed56111ae821..d812380a81282e 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java @@ -82,6 +82,7 @@ import com.google.devtools.build.lib.packages.MacroInstance; import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.PredicateWithMessage; +import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; import com.google.devtools.build.lib.packages.RuleFactory; @@ -170,7 +171,7 @@ public class StarlarkRuleClassFunctions implements StarlarkRuleFunctionsApi { .add(attr("args", STRING_LIST)) .add(attr("output_licenses", LICENSE)) .addAttribute( - attr("$is_executable", BOOLEAN) + attr(Rule.IS_EXECUTABLE_ATTRIBUTE_NAME, BOOLEAN) .value(true) .nonconfigurable("Called from RunCommand.isExecutable, which takes a Target") .build()) @@ -268,7 +269,7 @@ public static RuleClass getTestBaseRule(RuleDefinitionEnvironment env) { + BaseRuleClasses.DEFAULT_COVERAGE_REPORT_GENERATOR_VALUE)))) .add(attr(":run_under", LABEL).value(RUN_UNDER)) .addAttribute( - attr("$is_executable", BOOLEAN) + attr(Rule.IS_EXECUTABLE_ATTRIBUTE_NAME, BOOLEAN) .value(true) .nonconfigurable("Called from RunCommand.isExecutable, which takes a Target") .build()); diff --git a/src/main/java/com/google/devtools/build/lib/packages/Rule.java b/src/main/java/com/google/devtools/build/lib/packages/Rule.java index 7089823f692048..24eb6a593a8e8e 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Rule.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Rule.java @@ -94,6 +94,8 @@ public class Rule implements Target, DependencyFilter.AttributeInfoProvider { private static final OutputFile[] NO_OUTPUTS = new OutputFile[0]; + public static final String IS_EXECUTABLE_ATTRIBUTE_NAME = "$is_executable"; + private final Package pkg; private final Label label; private final RuleClass ruleClass; @@ -1281,6 +1283,13 @@ public boolean useToolchainResolution() { } } + public boolean isExecutable() { + if (getRuleClassObject().hasAttr(IS_EXECUTABLE_ATTRIBUTE_NAME, Type.BOOLEAN)) { + return NonconfigurableAttributeMapper.of(this).get(IS_EXECUTABLE_ATTRIBUTE_NAME, Type.BOOLEAN); + } + return false; + } + public RepositoryName getRepository() { return label.getPackageIdentifier().getRepository(); } diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/output/ProtoOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/query/output/ProtoOutputFormatter.java index 5eb7ecee6389be..a2cd9bfe9e07c0 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/query/output/ProtoOutputFormatter.java +++ b/src/main/java/com/google/devtools/build/lib/query2/query/output/ProtoOutputFormatter.java @@ -47,6 +47,7 @@ import com.google.devtools.build.lib.packages.ProtoUtils; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.Target; +import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.packages.Type; import com.google.devtools.build.lib.packages.Types; import com.google.devtools.build.lib.query2.common.CommonQueryOptions; @@ -201,6 +202,8 @@ public Build.Target toTargetProtoBuffer( BaseEncoding.base16().lowerCase().encode(transitiveDigest))); // hexify } + rulePb.setIsExecutable(rule.isExecutable() || TargetUtils.isTestRule(target)); + ImmutableMap> aspectsDependencies = aspectResolver.computeAspectDependencies(target, dependencyFilter); if (!aspectsDependencies.isEmpty()) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBaseRule.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBaseRule.java index bfecb964386e6b..0ee191b088432c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBaseRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBaseRule.java @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.BuildType; +import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; import com.google.devtools.build.lib.util.FileTypeSet; @@ -276,12 +277,13 @@ expanded to Windows style paths (with backslash). attr("executable", BOOLEAN) .value(false) .nonconfigurable( - "Used in computed default for $is_executable, which is itself non-configurable" + "Used in computed default for " + Rule.IS_EXECUTABLE_ATTRIBUTE_NAME + + " which is itself non-configurable" + " (and thus expects its dependencies to be non-configurable), because" - + " $is_executable is called from RunCommand.isExecutable, which has no" - + " configuration context")) + + " " + Rule.IS_EXECUTABLE_ATTRIBUTE_NAME + " is called from " + + " RunCommand.isExecutable, which has no configuration context")) .add( - attr("$is_executable", BOOLEAN) + attr(Rule.IS_EXECUTABLE_ATTRIBUTE_NAME, BOOLEAN) .nonconfigurable("Called from RunCommand.isExecutable, which takes a Target") .value( new Attribute.ComputedDefault() { diff --git a/src/main/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractor.java b/src/main/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractor.java index 4c3e2512be9527..8875bd370f7902 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractor.java +++ b/src/main/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractor.java @@ -29,6 +29,7 @@ import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.BuiltinProvider; import com.google.devtools.build.lib.packages.MacroClass; +import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; import com.google.devtools.build.lib.packages.StarlarkDefinedAspect; @@ -401,7 +402,7 @@ protected void visitRule(String qualifiedName, StarlarkRuleFunction ruleFunction if (ruleClass.getRuleClassType() == RuleClassType.TEST) { ruleInfoBuilder.setTest(true); } - if (ruleClass.hasAttr("$is_executable", Type.BOOLEAN)) { + if (ruleClass.hasAttr(Rule.IS_EXECUTABLE_ATTRIBUTE_NAME, Type.BOOLEAN)) { ruleInfoBuilder.setExecutable(true); } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java index 399f3af6aafb9b..b3683e12e5cd21 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java @@ -1123,10 +1123,7 @@ private static boolean isExecutableNonTestRule(Target target) { if (!(target instanceof Rule rule)) { return false; } - if (rule.getRuleClassObject().hasAttr("$is_executable", Type.BOOLEAN)) { - return NonconfigurableAttributeMapper.of(rule).get("$is_executable", Type.BOOLEAN); - } - return false; + return rule.isExecutable(); } private static boolean isPlainFile(Target target) { diff --git a/src/main/protobuf/build.proto b/src/main/protobuf/build.proto index c4e94e97141415..9bd39219ac42e3 100644 --- a/src/main/protobuf/build.proto +++ b/src/main/protobuf/build.proto @@ -289,6 +289,9 @@ message Rule { // The rule class (e.g., java_library) required string rule_class = 2; + // Whether the rule can be `bazel run` (i.e. is a binary or test rule). + optional bool is_executable = 16; + // The BUILD file and line number of the location (formatted as // ::) in the rule's package's // BUILD file where the rule instance was instantiated. The line number will diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleTest.java index aa7fb3729a3de1..3d87cd55e4e319 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/RuleTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/RuleTest.java @@ -279,4 +279,20 @@ private TargetData roundTrip(Target target) throws SerializationException, IOExc ImmutableClassToInstanceMap.of( RuleClassProvider.class, skyframeExecutor.getRuleClassProviderForTesting())); } + + @Test + public void testIsExecutable() throws Exception { + scratch.file( + "x/BUILD", + "cc_binary(name = 'cb')", + "cc_library(name = 'cl')", + "java_binary(name = 'jb')", + "java_library(name = 'jl')"); + Package pkg = getTarget("//x:BUILD").getPackage(); + assertThat(pkg.getRule("cb").isExecutable()).isTrue(); + assertThat(pkg.getRule("jb").isExecutable()).isTrue(); + + assertThat(pkg.getRule("cl").isExecutable()).isFalse(); + assertThat(pkg.getRule("jl").isExecutable()).isFalse(); + } } From 31d43ad1915e856e6174009747b192992d6d5c53 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Fri, 17 May 2024 10:36:46 +0100 Subject: [PATCH 2/2] Respond to review comments --- .../google/devtools/build/lib/analysis/BaseRuleClasses.java | 4 ++++ .../build/lib/query2/query/output/ProtoOutputFormatter.java | 2 +- src/main/protobuf/build.proto | 2 ++ .../java/com/google/devtools/build/lib/packages/RuleTest.java | 2 ++ 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java index b364e9a39f7eaf..06a9b4b4b4ca2a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java @@ -209,6 +209,10 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) .add(attr("args", STRING_LIST)) .add(attr("env", STRING_DICT)) .add(attr("env_inherit", STRING_LIST)) + .add( + attr(Rule.IS_EXECUTABLE_ATTRIBUTE_NAME, BOOLEAN) + .value(true) + .nonconfigurable("Called from RunCommand.isExecutable, which takes a Target")) // Input files for every test action .add( attr("$test_wrapper", LABEL) diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/output/ProtoOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/query/output/ProtoOutputFormatter.java index a2cd9bfe9e07c0..1b76923264375e 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/query/output/ProtoOutputFormatter.java +++ b/src/main/java/com/google/devtools/build/lib/query2/query/output/ProtoOutputFormatter.java @@ -202,7 +202,7 @@ public Build.Target toTargetProtoBuffer( BaseEncoding.base16().lowerCase().encode(transitiveDigest))); // hexify } - rulePb.setIsExecutable(rule.isExecutable() || TargetUtils.isTestRule(target)); + rulePb.setIsExecutable(rule.isExecutable()); ImmutableMap> aspectsDependencies = aspectResolver.computeAspectDependencies(target, dependencyFilter); diff --git a/src/main/protobuf/build.proto b/src/main/protobuf/build.proto index 9bd39219ac42e3..f4e2f6a7ef101a 100644 --- a/src/main/protobuf/build.proto +++ b/src/main/protobuf/build.proto @@ -290,6 +290,8 @@ message Rule { required string rule_class = 2; // Whether the rule can be `bazel run` (i.e. is a binary or test rule). + // Note: This is optional for compatibility requirements. + // TODO: Make this required after Bazel 8 ships. optional bool is_executable = 16; // The BUILD file and line number of the location (formatted as diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleTest.java index 3d87cd55e4e319..cf1c3e558518da 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/RuleTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/RuleTest.java @@ -288,7 +288,9 @@ public void testIsExecutable() throws Exception { "cc_library(name = 'cl')", "java_binary(name = 'jb')", "java_library(name = 'jl')"); + Package pkg = getTarget("//x:BUILD").getPackage(); + assertThat(pkg.getRule("cb").isExecutable()).isTrue(); assertThat(pkg.getRule("jb").isExecutable()).isTrue();