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