diff options
Diffstat (limited to 'src')
7 files changed, 223 insertions, 5 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ShToolchain.java b/src/main/java/com/google/devtools/build/lib/analysis/ShToolchain.java index c785da410e..482ba88380 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ShToolchain.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ShToolchain.java @@ -14,12 +14,40 @@ package com.google.devtools.build.lib.analysis; +import static com.google.devtools.build.lib.packages.Attribute.attr; +import static com.google.devtools.build.lib.packages.BuildType.LABEL; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; +import com.google.devtools.build.lib.analysis.ToolchainContext.ResolvedToolchainProviders; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; +import com.google.devtools.build.lib.analysis.platform.ToolchainInfo; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.packages.Attribute; +import com.google.devtools.build.lib.packages.RuleClass; +import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.vfs.PathFragment; /** Class to work with the shell toolchain, e.g. get the shell interpreter's path. */ public final class ShToolchain { + private static final String TOOLCHAIN_TYPE_ATTR = "$sh_toolchain_type"; + private static final String TOOLCHAIN_TYPE_LABEL = "//tools/sh:toolchain_type"; + private static final String TOOLCHAIN_TYPE_PACKAGE = "tools/sh"; + + public static RuleClass.Builder addDependency( + RuleClass.Builder builder, RuleDefinitionEnvironment env) { + return addDependency(builder, env.getToolsLabel(TOOLCHAIN_TYPE_LABEL)); + } + + private static RuleClass.Builder addDependency(RuleClass.Builder builder, Label toolchainType) { + return builder.addRequiredToolchains(toolchainType).add(createAttribute(toolchainType)); + } + + private static Attribute.Builder createAttribute(Label toolchainType) { + return attr(TOOLCHAIN_TYPE_ATTR, LABEL).value(toolchainType); + } + /** * Returns the shell executable's path, or an empty path if not set. * @@ -43,9 +71,11 @@ public final class ShToolchain { /** * Returns the shell executable's path, or reports a rule error if the path is empty. * - * <p>This method checks the rule's configuration's {@link ShellConfiguration} fragment for the - * shell executable's path. If null or empty, the method reports an error against the rule. + * <p>DO NOT USE THIS METHOD, use {@link #getToolchainPathOrError} instead. This method exists to + * allow incremental migration to {@link #getToolchainPathOrError}. */ + // TODO(laszlocsomor): update every rule that calls getPathOrError to depend on the shell + // toolchain, and change their use of getPathOrError to getToolchainPathOrError. public static PathFragment getPathOrError(RuleContext ctx) { PathFragment result = getPath(ctx.getConfiguration()); @@ -58,5 +88,61 @@ public final class ShToolchain { return result; } + /** + * Returns the shell executable's path, or reports a rule error if the path is empty. + * + * <p>This method checks the rule's configuration's {@link ShellConfiguration} fragment for the + * shell executable's path. If null or empty, this method gets the path from the selected shell + * toolchain. If the path is still null or empty, the method reports an error against the rule. + * + * @throws IllegalArgumentException if the rule does not depend on the shell toolchain + */ + public static PathFragment getToolchainPathOrError(RuleContext ctx) { + // TODO(laszlocsomor): update every rule that calls getPathOrError to depend on the shell + // toolchain, and change their use of getPathOrError to getToolchainPathOrError. + Preconditions.checkState(ctx.attributes().has(TOOLCHAIN_TYPE_ATTR, LABEL)); + + PathFragment result = getPath(ctx.getConfiguration()); + + if (result.isEmpty()) { + ResolvedToolchainProviders toolchains = + (ResolvedToolchainProviders) ctx.getToolchainContext().getResolvedToolchainProviders(); + + ToolchainInfo activeToolchain = + toolchains.getForToolchainType(ctx.attributes().get(TOOLCHAIN_TYPE_ATTR, LABEL)); + + if (activeToolchain != null) { + String path = null; + try { + path = (String) activeToolchain.getValue("path"); + } catch (EvalException e) { + throw new IllegalStateException(e); + } + + if (path != null && !path.isEmpty()) { + result = PathFragment.create(path); + } + } + } + + if (result.isEmpty()) { + ctx.ruleError( + "This rule needs a shell interpreter. Use the --shell_executable=<path> flag to specify" + + " the interpreter's path, e.g. --shell_executable=/usr/local/bin/bash"); + } + + return result; + } + private ShToolchain() {} + + @VisibleForTesting + public static String getToolchainTypeLabelForTesting() { + return TOOLCHAIN_TYPE_LABEL; + } + + @VisibleForTesting + public static String getToolchainTypePackageForTesting() { + return TOOLCHAIN_TYPE_PACKAGE; + } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRuleRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRuleRule.java index 964cfafce1..bda085ec41 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRuleRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRuleRule.java @@ -20,6 +20,7 @@ import static com.google.devtools.build.lib.syntax.Type.BOOLEAN; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; +import com.google.devtools.build.lib.analysis.ShToolchain; import com.google.devtools.build.lib.analysis.config.HostTransition; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.rules.cpp.CcToolchain; @@ -44,7 +45,7 @@ public final class BazelGenRuleRule implements RuleDefinition { rules. If the rule generates source files, you should use the <code>srcs</code> attribute. <!-- #END_BLAZE_RULE.NAME --> */ - return builder + return ShToolchain.addDependency(builder, env) .setOutputToGenfiles() .add( attr("$genrule_setup", LABEL) diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java index 52efdf15de..ab57723303 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java +++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java @@ -196,7 +196,7 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory { FilesToRunProvider genruleSetup = ruleContext.getPrerequisite("$genrule_setup", Mode.HOST, FilesToRunProvider.class); inputs.addTransitive(genruleSetup.getFilesToRun()); - PathFragment shExecutable = ShToolchain.getPathOrError(ruleContext); + PathFragment shExecutable = ShToolchain.getToolchainPathOrError(ruleContext); if (ruleContext.hasErrors()) { return null; } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java index af2220d0f6..d26727a650 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java @@ -61,7 +61,8 @@ public final class BazelAnalysisMock extends AnalysisMock { "local_repository(name = 'com_google_protobuf', path = '/protobuf')", "bind(name = 'android/sdk', actual='@bazel_tools//tools/android:sdk')", "bind(name = 'tools/python', actual='//tools/python')", - "register_toolchains('@bazel_tools//tools/cpp:all')")); + "register_toolchains('@bazel_tools//tools/cpp:all')", + "register_toolchains('@bazel_tools//tools/sh:toolchain')")); } @Override @@ -190,6 +191,31 @@ public final class BazelAnalysisMock extends AnalysisMock { config.create("/bazel_tools_workspace/objcproto/empty.cc"); config.create("/bazel_tools_workspace/objcproto/well_known_type.proto"); + config.create( + "/bazel_tools_workspace/tools/sh/BUILD", + "package(default_visibility = ['//visibility:public'])", + "load(':sh_toolchain.bzl', 'sh_toolchain')", + "", + "toolchain_type(name = 'toolchain_type')", + "sh_toolchain(", + " name = 'dummy_toolchain',", + " path = '/mock/shell/toolchain',", + ")", + "toolchain(", + " name = 'toolchain',", + " toolchain = ':dummy_toolchain',", + " toolchain_type = ':toolchain_type',", + ")"); + config.create( + "/bazel_tools_workspace/tools/sh/sh_toolchain.bzl", + "def _sh_toolchain_impl(ctx):", + " return [platform_common.ToolchainInfo(path = ctx.attr.path)]", + "", + "sh_toolchain = rule(", + " attrs = {'path': attr.string()},", + " implementation = _sh_toolchain_impl,", + ")"); + ccSupport().setup(config); } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java index 7ced520c12..267fedbac2 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.ShToolchain; import com.google.devtools.build.lib.analysis.ShellConfiguration; import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.analysis.configuredtargets.FileConfiguredTarget; @@ -475,6 +476,10 @@ public class GenRuleConfiguredTargetTest extends BuildViewTestCase { boolean foundTool = false; boolean foundSetup = false; for (ConfiguredTarget prereq : prereqs) { + String label = prereq.getLabel().toString(); + if (label.endsWith(ShToolchain.getToolchainTypeLabelForTesting())) { + continue; + } String name = prereq.getLabel().getName(); if (name.contains("cc-") || name.contains("jdk")) { // Ignore these, they are present due to the implied genrule dependency on crosstool and diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleUsesShellToolchainTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleUsesShellToolchainTest.java new file mode 100644 index 0000000000..9783aad9d7 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleUsesShellToolchainTest.java @@ -0,0 +1,96 @@ +// Copyright 2018 The Bazel Authors. 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.genrule; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.devtools.build.lib.testutil.TestConstants.GENRULE_SETUP; + +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.actions.SpawnAction; +import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Assert that genrule uses the shell toolchain. */ +@RunWith(JUnit4.class) +public class GenRuleUsesShellToolchainTest extends BuildViewTestCase { + + private static final Pattern SETUP_COMMAND_PATTERN = + Pattern.compile(".*/genrule-setup.sh;\\s+(?<command>.*)"); + + @Test + public void testActionIsShellCommandUsingShellFromShellConfig() throws Exception { + useConfiguration("--shell_executable=/custom/shell/executable"); + assertActionIsShellCommand("/custom/shell/executable"); + } + + @Test + public void testActionIsShellCommandUsingShellFromShellToolchain() throws Exception { + useConfiguration("--shell_executable="); + assertActionIsShellCommand("/mock/shell/toolchain"); + } + + @Test + public void testActionIsShellCommandUsingShellFromExtraToolchain() throws Exception { + useConfiguration("--shell_executable=", "--extra_toolchains=//pkg1:tc"); + scratch.file( + "pkg1/BUILD", + "load('@bazel_tools//tools/sh:sh_toolchain.bzl', 'sh_toolchain')", + "", + "sh_toolchain(", + " name = 'tc_def',", + " path = '/foo/bar/baz',", + " visibility = ['//visibility:public'],", + ")", + "", + "toolchain(", + " name = 'tc',", + " toolchain = 'tc_def',", + " toolchain_type = '@bazel_tools//tools/sh:toolchain_type',", + ")"); + + assertActionIsShellCommand("/foo/bar/baz"); + } + + private void assertActionIsShellCommand(String expectedShell) throws Exception { + scratch.file( + "genrule1/BUILD", + "genrule(", + " name = 'genrule1',", + " srcs = ['input.txt'],", + " outs = ['output.txt'],", + " cmd = 'dummy >$@',", + ")"); + + Artifact input = getFileConfiguredTarget("//genrule1:input.txt").getArtifact(); + Artifact genruleSetup = getFileConfiguredTarget(GENRULE_SETUP).getArtifact(); + + Artifact out = getFileConfiguredTarget("//genrule1:output.txt").getArtifact(); + SpawnAction shellAction = (SpawnAction) getGeneratingAction(out); + assertThat(shellAction).isNotNull(); + assertThat(shellAction.getInputs()).containsExactly(input, genruleSetup); + assertThat(shellAction.getOutputs()).containsExactly(out); + + String expected = "dummy >" + out.getExecPathString(); + assertThat(shellAction.getArguments().get(0)).isEqualTo(expectedShell); + assertThat(shellAction.getArguments().get(1)).isEqualTo("-c"); + Matcher m = SETUP_COMMAND_PATTERN.matcher(shellAction.getArguments().get(2)); + assertThat(m.matches()).isTrue(); + assertThat(m.group("command")).isEqualTo(expected); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java index f94c047667..74f614599b 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java @@ -36,6 +36,7 @@ import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.bazel.rules.BazelRuleClassProvider; import com.google.devtools.build.lib.bazel.rules.CcRules; import com.google.devtools.build.lib.bazel.rules.GenericRules; +import com.google.devtools.build.lib.bazel.rules.ToolchainRules; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.cmdline.RepositoryName; @@ -999,6 +1000,9 @@ public class CcCommonTest extends BuildViewTestCase { PlatformRules.INSTANCE.init(builder); GenericRules.INSTANCE.init(builder); CcRules.INSTANCE.init(builder); + // Some tests use genrules so they need the shell toolchain (//tools/sh:toolchain_type) so + // we need to support the toolchain rules. + ToolchainRules.INSTANCE.init(builder); return builder.build(); } |