diff options
9 files changed, 7 insertions, 243 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 482ba88380..c785da410e 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,40 +14,12 @@ 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. * @@ -71,61 +43,13 @@ public final class ShToolchain { /** * Returns the shell executable's path, or reports a rule error if the path is empty. * - * <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()); - - 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; - } - - /** - * 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 + * shell executable's path. If null or empty, the method reports an error against the rule. */ - 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)); - + public static PathFragment getPathOrError(RuleContext ctx) { 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"); @@ -135,14 +59,4 @@ public final class ShToolchain { } 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 bda085ec41..964cfafce1 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,7 +20,6 @@ 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; @@ -45,7 +44,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 ShToolchain.addDependency(builder, env) + return builder .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 ab57723303..52efdf15de 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.getToolchainPathOrError(ruleContext); + PathFragment shExecutable = ShToolchain.getPathOrError(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 d26727a650..af2220d0f6 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,8 +61,7 @@ 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/sh:toolchain')")); + "register_toolchains('@bazel_tools//tools/cpp:all')")); } @Override @@ -191,31 +190,6 @@ 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 267fedbac2..7ced520c12 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,7 +27,6 @@ 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; @@ -476,10 +475,6 @@ 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 deleted file mode 100644 index 9783aad9d7..0000000000 --- a/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleUsesShellToolchainTest.java +++ /dev/null @@ -1,96 +0,0 @@ -// 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 3883aaeebc..ce1e9fddcf 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 @@ -1001,9 +1001,6 @@ public class CcCommonTest extends BuildViewTestCase { ToolchainRules.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(); } diff --git a/tools/sh/BUILD b/tools/sh/BUILD index ea6ea974ff..66f710a926 100644 --- a/tools/sh/BUILD +++ b/tools/sh/BUILD @@ -21,22 +21,3 @@ filegroup( ], visibility = ["//tools:__pkg__"], ) - -# Do not depend on this rule. -# Use @bazel_tools//tools/sh:toolchain_type instead. -# -# This rule is available exclusively for the Bazel bootstrapping process. -# -# As part of bootstrapping Bazel, we set up a mock @bazel_tools repo, by -# creating a directory with a WORKSPACE file and symlinking the real tools -# directory into it. This approach however won't use the BUILD.tools files as -# BUILD files, so the fake repo won't be exactly the same as the real one. To -# work around this issue without having to symlink each file in the entire -# tools/** tree and symlinking BUILD.tools files as BUILD files, the -# bootstrapper just symlinks the top tools directory. Therefore to bootstrap -# Bazel, we need those targets in BUILD.tools files that Bazel need to also -# exist in BUILD files. -toolchain_type( - name = "toolchain_type", - visibility = ["//visibility:public"], -) diff --git a/tools/sh/sh_configure.bzl b/tools/sh/sh_configure.bzl index 03c821a6be..f7c8899420 100644 --- a/tools/sh/sh_configure.bzl +++ b/tools/sh/sh_configure.bzl @@ -54,7 +54,7 @@ load("@bazel_tools//tools/sh:sh_toolchain.bzl", "sh_toolchain") sh_toolchain( name = "local_sh", - path = {sh_path}, + path = "{sh_path}", visibility = ["//visibility:public"], ) @@ -63,7 +63,7 @@ toolchain( toolchain = ":local_sh", toolchain_type = "@bazel_tools//tools/sh:toolchain_type", ) -""".format(sh_path = "\"%s\"" % sh_path if sh_path else "None")) +""".format(sh_path = sh_path)) sh_config = repository_rule( environ = [ |