diff options
author | 2018-05-30 02:08:15 -0700 | |
---|---|---|
committer | 2018-05-30 02:09:50 -0700 | |
commit | 2cbcde33478756d265a57e88c89732126f33879a (patch) | |
tree | b90265543bbd2a903ad52c8a9d4d0b395534429a /src/test/java/com/google/devtools/build | |
parent | 8f384fc273e36ecc2a35f0b2f0d69e137651e36c (diff) |
Automated rollback of commit 7fcbc8ffdead028d19606fefa2fa3be13781da98.
*** Reason for rollback ***
According to a post-submit regression test, this
commit increased Bazel's memory usage slightly
beyond some threshold. That event prompted me to
consider and question the necessity of this
commit.
My goal is to make Bazel work on Windows without
requiring MSYS, failing the build only if an
action is a shell action. Toolchainifying the
shell is related to, but not required by this
effort. What is required is to fail the build when
Bazel tries to create a shell action but the shell
is missing, and we have that already with
ShToolchain.getPath (which only considers the
ShellConfiguration fragment).
What's missing for my goal is to reimplement
hardcoded shell actions
(i.e. SpawnAction.builder().setShellCommand(...))
without using the shell where possible.
*** Original change description ***
shell toolchain: genrule now uses it
genrule() now uses the shell toolchain (as well as
the ShellConfiguration config fragment) to look up
the shell interpreter's path.
Also the CommandHelper no longer looks up the
shell interpreter's path itself. Instead its ctor
takes the path as an argument. This allows
supporting rules that already use the shell
toolchain and rules that still only consider the
configuration fragment.
With these changes genrule() is now able to use
the shell interpreter registered as a toolchain,
even if there's no `--shell_executable` flag
specified (or its value is empty).
Subsequent commits will migrate more and more
rules to depend on the shell toolchain.
This commit takes us closer to:
1. be able to detect the local shell interpreter's
location, especially when it's not the default
/bin/bash
2. be able to define different shell toolchains
(different interpreter paths) for remote builds
3. gracefully fail the build if the machine has no
shell installed but the action graph included a
shell action
See https://github.com/bazelbuild/bazel/issues/4319
Change-Id: I0674c7e2d5917643d87b48b19a1cb43e606ad2f7
Closes #5282.
***
RELNOTES: none
PiperOrigin-RevId: 198527351
Diffstat (limited to 'src/test/java/com/google/devtools/build')
4 files changed, 1 insertions, 131 deletions
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(); } |