From ffb89f4bf91718c45ec6ae58509ba6220165ede7 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 12 Mar 2015 13:20:20 +0000 Subject: Bazel sh_* rules are documented and cleared up. - Unused attribute bash_version is removed from all sh_* rules. - ShTest.java is deleted. Shell binary and test rules have the same implementation. -- MOS_MIGRATED_REVID=88434794 --- .../lib/bazel/rules/sh/BazelShBinaryRule.java | 8 +--- .../lib/bazel/rules/sh/BazelShLibraryRule.java | 52 +++++++-------------- .../lib/bazel/rules/sh/BazelShRuleClasses.java | 42 ++++++++++++----- .../build/lib/bazel/rules/sh/BazelShTestRule.java | 11 +---- .../build/lib/bazel/rules/sh/ShBinary.java | 15 +----- .../devtools/build/lib/bazel/rules/sh/ShTest.java | 53 ---------------------- 6 files changed, 52 insertions(+), 129 deletions(-) delete mode 100644 src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShTest.java (limited to 'src/main/java') diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShBinaryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShBinaryRule.java index 39d4a0c15c..9a2aafdcc7 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShBinaryRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShBinaryRule.java @@ -13,9 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.bazel.rules.sh; -import static com.google.devtools.build.lib.packages.Attribute.attr; -import static com.google.devtools.build.lib.packages.Type.STRING; - import com.google.devtools.build.lib.analysis.BlazeRule; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; @@ -33,9 +30,6 @@ import com.google.devtools.build.lib.packages.RuleClass.Builder; public final class BazelShBinaryRule implements RuleDefinition { @Override public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) { - return builder.add( - attr("bash_version", STRING) - .value(BazelShRuleClasses.DEFAULT_BASH_VERSION) - .allowedValues(BazelShRuleClasses.BASH_VERSION_ALLOWED_VALUES)).build(); + return builder.build(); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShLibraryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShLibraryRule.java index 9d9640b7b3..552d99aaef 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShLibraryRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShLibraryRule.java @@ -33,29 +33,16 @@ public final class BazelShLibraryRule implements RuleDefinition { @Override public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) { return builder - /* - The list of other targets to be aggregated in to this "library" target. - (List of labels; optional)
- See general comments about deps - at Attributes common to all build rules. - You should use this attribute to list other - sh_library or proto_library rules that provide - interpreted program source code depended on by the code in - srcs. If you depend on a proto_library target, - the proto sources in that target will be included in this library, but - no generated files will be built. - */ - /* The list of input files. - (List of labels, - optional)
- You should use this attribute to list interpreted program - source files that belong to this package, such as additional - files containing Bourne shell subroutines, loaded via the shell's - source or . command. + ${SYNOPSIS} +

+ This attribute should be used to list shell script source files that belong to + this library. Scripts can load other scripts using the shell's source + or . command. +

*/ - .override(attr("srcs", LABEL_LIST).legacyAllowAnyFileType()) + .override(attr("srcs", LABEL_LIST).allowedFileTypes(BazelShRuleClasses.SH_FILES)) .build(); } } @@ -69,31 +56,24 @@ ${ATTRIBUTE_SIGNATURE} "library" consisting of related scripts—programs in an interpreted language that does not require compilation or linking, such as the Bourne shell—and any data those programs need at - run-time. Such "libraries" can then be used from + run-time. Such "libraries" can then be used from the data attribute of one or more sh_binary rules.

- Historically, a second use was to aggregate a collection of data files - together, to ensure that they are available at runtime in - the .runfiles area of one or more *_binary - rules (not necessarily sh_binary). - However, the filegroup() rule - should be used now; it is intended to replace this use of - sh_library. + You can use the filegroup rule to aggregate data files.

In interpreted programming languages, there's not always a clear distinction between "code" and "data": after all, the program is - just "data" from the interpreter's point of view. For this reason - (and historical accident) this rule has three attributes which are - all essentially equivalent: srcs, deps - and data. - The recommended usage of each attribute is mentioned below. The - current implementation does not distinguish the elements of these lists. - All three attributes accept rules, source files and derived files. + just "data" from the interpreter's point of view. For this reason + this rule has three attributes which are all essentially equivalent: + srcs, deps and data. + The current implementation does not distinguish between the elements of these lists. + All three attributes accept rules, source files and generated files. + It is however good practice to use the attributes for their usual purpose (as with other rules).

${ATTRIBUTE_DEFINITION} @@ -104,7 +84,7 @@ ${ATTRIBUTE_DEFINITION} sh_library( name = "foo", data = [ - ":foo_service_script", # a sh_binary with srcs + ":foo_service_script", # an sh_binary with srcs ":deploy_foo", # another sh_binary with srcs ], ) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShRuleClasses.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShRuleClasses.java index 6f66465726..c1692afc27 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShRuleClasses.java @@ -15,21 +15,22 @@ package com.google.devtools.build.lib.bazel.rules.sh; import static com.google.devtools.build.lib.packages.Attribute.attr; -import static com.google.devtools.build.lib.packages.ImplicitOutputsFunction.fromTemplates; import static com.google.devtools.build.lib.packages.Type.LABEL_LIST; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; + import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.BlazeRule; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet; -import com.google.devtools.build.lib.packages.ImplicitOutputsFunction; import com.google.devtools.build.lib.packages.PredicateWithMessage; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; +import com.google.devtools.build.lib.util.FileType; +import com.google.devtools.build.lib.util.FileTypeSet; import java.util.Collection; import java.util.Map; @@ -42,7 +43,9 @@ import javax.annotation.Nullable; public final class BazelShRuleClasses { static final Collection ALLOWED_RULES_IN_DEPS_WITH_WARNING = ImmutableSet.of( - "filegroup", "Fileset", "genrule", "sh_binary", "sh_test", "test_suite"); + "filegroup", "genrule", "sh_binary", "sh_test", "test_suite"); + + static final FileTypeSet SH_FILES = FileTypeSet.of(FileType.of(".sh")); /** * Common attributes for shell rules. @@ -54,21 +57,38 @@ public final class BazelShRuleClasses { @Override public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) { return builder - .add(attr("srcs", LABEL_LIST).mandatory().legacyAllowAnyFileType()) + /* + The file containing the shell script. + ${SYNOPSIS} +

+ This attribute must be a singleton list, whose element is the shell script. + This script must be executable, and may be a source file or a generated file. + All other files required at runtime (whether scripts or data) belong in the + data attribute. +

+ */ + .add(attr("srcs", LABEL_LIST) + .mandatory() + .allowedFileTypes(SH_FILES)) + /* + The list of "library" targets to be aggregated into this target. + ${SYNOPSIS} + See general comments about deps + at Attributes common to all build rules. +

+ This attribute should be used to list other sh_library rules that provide + interpreted program source code depended on by the code in srcs. The files + provided by these rules will be present among the runfiles of this target. +

+ */ .override(builder.copy("deps") - .allowedRuleClasses("sh_library", "proto_library") + .allowedRuleClasses("sh_library") .allowedRuleClassesWithWarning(ALLOWED_RULES_IN_DEPS_WITH_WARNING) .allowedFileTypes()) .build(); } } - /** - * Defines the file name of an sh_binary's implicit .sar (script package) output. - */ - static final ImplicitOutputsFunction SAR_PACKAGE_FILENAME = - fromTemplates("%{name}.sar"); - /** * Convenience structure for the bash dependency combinations defined * by BASH_BINARY_BINDINGS. diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShTestRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShTestRule.java index 4a1e51f019..4472d53d22 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShTestRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShTestRule.java @@ -13,9 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.bazel.rules.sh; -import static com.google.devtools.build.lib.packages.Attribute.attr; -import static com.google.devtools.build.lib.packages.Type.STRING; - import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.BlazeRule; import com.google.devtools.build.lib.analysis.RuleDefinition; @@ -31,14 +28,10 @@ import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; @BlazeRule(name = "sh_test", type = RuleClassType.TEST, ancestors = { ShRule.class, BaseRuleClasses.TestBaseRule.class }, - factoryClass = ShTest.class) + factoryClass = ShBinary.class) public final class BazelShTestRule implements RuleDefinition { @Override public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) { - return builder - .add(attr("bash_version", STRING) - .value(BazelShRuleClasses.DEFAULT_BASH_VERSION) - .allowedValues(BazelShRuleClasses.BASH_VERSION_ALLOWED_VALUES)) - .build(); + return builder.build(); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java index 4e6ba814a3..acdba76fc0 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java @@ -41,8 +41,8 @@ public class ShBinary implements RuleConfiguredTargetFactory { } Artifact symlink = ruleContext.createOutputArtifact(); + // Note that src is used as the executable script too Artifact src = srcs.get(0); - Artifact executableScript = getExecutableScript(ruleContext, src); // The interpretation of this deceptively simple yet incredibly generic rule is complicated // by the distinction between targets and (not properly encapsulated) artifacts. It depends // on the notion of other rule's "files-to-build" sets, which are undocumented, making it @@ -50,11 +50,10 @@ public class ShBinary implements RuleConfiguredTargetFactory { // happens when srcs = ['x', 'y'] but 'x' is an empty filegroup?). This is a pervasive // problem in Blaze. ruleContext.registerAction( - new ExecutableSymlinkAction(ruleContext.getActionOwner(), executableScript, symlink)); + new ExecutableSymlinkAction(ruleContext.getActionOwner(), src, symlink)); NestedSet filesToBuild = NestedSetBuilder.stableOrder() .add(src) - .add(executableScript) // May be the same as src, in which case set semantics apply. .add(symlink) .build(); Runfiles runfiles = new Runfiles.Builder() @@ -69,14 +68,4 @@ public class ShBinary implements RuleConfiguredTargetFactory { .addProvider(RunfilesProvider.class, RunfilesProvider.simple(runfiles)) .build(); } - - /** - * Hook for sh_test to provide the executable. - * - * @param ruleContext - * @param src - */ - protected Artifact getExecutableScript(RuleContext ruleContext, Artifact src) { - return src; - } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShTest.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShTest.java deleted file mode 100644 index cc965aa405..0000000000 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShTest.java +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright 2014 Google Inc. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -package com.google.devtools.build.lib.bazel.rules.sh; - -import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.analysis.RuleContext; -import com.google.devtools.build.lib.analysis.actions.FileWriteAction; -import com.google.devtools.build.lib.packages.Type; -import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory; -import com.google.devtools.build.lib.vfs.PathFragment; - -/** - * Implementation for sh_test rules. - */ -public class ShTest extends ShBinary implements RuleConfiguredTargetFactory { - - @Override - protected Artifact getExecutableScript(RuleContext ruleContext, Artifact src) { - if (ruleContext.attributes().get("bash_version", Type.STRING) - .equals(BazelShRuleClasses.SYSTEM_BASH_VERSION)) { - return src; - } - - // What *will* this script run with the wrapper? - PathFragment newOutput = src.getRootRelativePath().getParentDirectory().getRelative( - ruleContext.getLabel().getName() + "_runner.sh"); - Artifact testRunner = ruleContext.getAnalysisEnvironment().getDerivedArtifact( - newOutput, ruleContext.getConfiguration().getBinDirectory()); - - String bashPath = BazelShRuleClasses.BASH_BINARY_BINDINGS - .get(BazelShRuleClasses.SYSTEM_BASH_VERSION).execPath; - - // Generate the runner contents. - String runnerContents = - "#!/bin/bash\n" - + bashPath + " \"" + src.getRootRelativePath().getPathString() + "\" \"$@\"\n"; - - ruleContext.registerAction( - new FileWriteAction(ruleContext.getActionOwner(), testRunner, runnerContents, true)); - return testRunner; - } -} -- cgit v1.2.3