From 8ff216d8e8a1c3fc5ed33453bb21b53381e295c9 Mon Sep 17 00:00:00 2001 From: Philipp Wollermann Date: Mon, 19 Oct 2015 12:28:03 +0000 Subject: Quick workaround for #473 that reenables hdrs_check in Bazel to unblock the release. Note the following peculiarities of the current situation: - Sandboxed execution still silently falls back to non-sandboxed execution due to LocalGccStrategy explicitly requesting local execution. - However, builds are still correct due to local execution using hdrs_check. - hdrs_check, even though it was intended to default to "strict" in Bazel from the start, is still set to "loose" by default, so you might accidentally be using non-declared header files in your cc_* targets. In this case, your builds will break when the default becomes hdrs_check=strict and sandboxing becomes available. It is recommended that you check whether your builds are affected, by specifying --hdrs_check=strict manually and fixing your BUILD files to explicitly list all used includes in the srcs / hdrs attributes of your cc_* targets. These remaining issues will be fixed in an upcoming change. This change also adds regression tests for the issue. -- MOS_MIGRATED_REVID=105747212 --- .../com/google/devtools/build/lib/Constants.java | 2 +- src/test/shell/bazel/BUILD | 7 + src/test/shell/bazel/bazel_sandboxing_cpp_test.sh | 221 +++++++++++++++++++++ src/test/shell/bazel/bazel_sandboxing_test.sh | 44 +--- .../shell/bazel/bazel_sandboxing_test_utils.sh | 55 +++++ 5 files changed, 288 insertions(+), 41 deletions(-) create mode 100755 src/test/shell/bazel/bazel_sandboxing_cpp_test.sh create mode 100755 src/test/shell/bazel/bazel_sandboxing_test_utils.sh (limited to 'src') diff --git a/src/main/java/com/google/devtools/build/lib/Constants.java b/src/main/java/com/google/devtools/build/lib/Constants.java index 940fa0e535..1606b7e054 100644 --- a/src/main/java/com/google/devtools/build/lib/Constants.java +++ b/src/main/java/com/google/devtools/build/lib/Constants.java @@ -44,5 +44,5 @@ public class Constants { * Whether C++ include scanning should be disabled no matter what the --cc_include_scanning flag * says. */ - public static final boolean HARD_DISABLE_CC_INCLUDE_SCANNING = true; + public static final boolean HARD_DISABLE_CC_INCLUDE_SCANNING = new Boolean(false).booleanValue(); } diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD index 604ec56e1f..613ad4546f 100644 --- a/src/test/shell/bazel/BUILD +++ b/src/test/shell/bazel/BUILD @@ -38,6 +38,7 @@ filegroup( name = "test-deps", testonly = 1, srcs = [ + "bazel_sandboxing_test_utils.sh", "remote_helpers.sh", "test-setup.sh", "testenv.sh", @@ -220,6 +221,12 @@ sh_test( data = [":test-deps"], ) +sh_test( + name = "bazel_sandboxing_cpp_test", + srcs = ["bazel_sandboxing_cpp_test.sh"], + data = [":test-deps"], +) + sh_test( name = "bazel_worker_test", size = "large", diff --git a/src/test/shell/bazel/bazel_sandboxing_cpp_test.sh b/src/test/shell/bazel/bazel_sandboxing_cpp_test.sh new file mode 100755 index 0000000000..169fbc217b --- /dev/null +++ b/src/test/shell/bazel/bazel_sandboxing_cpp_test.sh @@ -0,0 +1,221 @@ +#!/bin/bash +# +# Copyright 2015 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. +# +# Test C++ builds with the sandboxing spawn strategy. +# + +# Load test environment +src_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +source ${src_dir}/test-setup.sh \ + || { echo "test-setup.sh not found!" >&2; exit 1; } +source ${src_dir}/bazel_sandboxing_test_utils.sh \ + || { echo "bazel_sandboxing_test_utils.sh not found!" >&2; exit 1; } + +function set_up { + mkdir -p examples/cpp/{bin,lib} + cat << 'EOF' > examples/cpp/BUILD +cc_library( + name = "hello-lib", + srcs = ["lib/hello-lib.c"], + hdrs = ["lib/hello-lib.h"], +) + +cc_binary( + name = "hello-world", + srcs = ["bin/hello-world.c"], + deps = [":hello-lib"], +) +EOF + cat << 'EOF' > examples/cpp/lib/hello-lib.c +#include "examples/cpp/lib/hello-lib.h" + +void greet(char *greeting) { + printf("hello-lib says: %s\n", greeting); +} +EOF + cat << 'EOF' > examples/cpp/lib/hello-lib.h +#ifndef EXAMPLES_CPP_LIB_HELLO_LIB_H_ +#define EXAMPLES_CPP_LIB_HELLO_LIB_H_ + +#include + +void greet(char *greeting); + +static inline void greet_from_header() { + printf("greetings from the header"); +} + +#endif // EXAMPLES_CPP_LIB_HELLO_LIB_H_ +EOF + cat << 'EOF' > examples/cpp/bin/hello-world.c +#include "examples/cpp/lib/hello-lib.h" + +int main(int argc, char** argv) { + if (argc > 1) { + greet(argv[1]); + } + greet_from_header(); + return 0; +} +EOF +} + +function tear_down() { + rm -rf examples/cpp +} + +function test_sandboxed_cpp_build_rebuilds_on_change() { + bazel --batch clean &> $TEST_log \ + || fail "bazel clean failed" + + bazel build --spawn_strategy=sandboxed //examples/cpp:hello-world &> $TEST_log \ + || fail "Building hello-world failed" + + bazel-bin/examples/cpp/hello-world | fgrep "greetings from the header" \ + || fail "Did not print expected string 'greetings from the header'" + + sed "s/from the header/from the modified header/g" < examples/cpp/lib/hello-lib.h > tmp \ + || fail "modifying hello-lib.h failed" + + mv tmp examples/cpp/lib/hello-lib.h \ + || fail "moving modified header file back to examples/cpp/lib/hello-lib.h failed" + + bazel build --spawn_strategy=sandboxed //examples/cpp:hello-world &> $TEST_log \ + || fail "Building modified hello-world failed" + + bazel-bin/examples/cpp/hello-world | fgrep "greetings from the modified header" \ + || fail "Did not print expected string 'greetings from the modified header'" +} + +function test_sandboxed_cpp_build_catches_missing_header_via_sandbox() { + bazel --batch clean &> $TEST_log \ + || fail "bazel clean failed" + + cat << 'EOF' > examples/cpp/BUILD +cc_library( + name = "hello-lib", + srcs = ["lib/hello-lib.c"], +) +EOF + + bazel build --hdrs_check=strict --spawn_strategy=sandboxed //examples/cpp:hello-lib &> $TEST_log \ + && fail "build should not have succeeded with missing header file" + + fgrep "undeclared inclusion(s) in rule '//examples/cpp:hello-lib'" $TEST_log \ + || fail "could not find 'undeclared inclusion' error message in bazel output" +} + +# TODO(philwo) turns out, we have this special "hdrs" attribute and in theory you can only include +# header files from libraries that are specified in "hdrs" and not "srcs", but we never check that, +# so the test fails. :( +function DISABLED_test_sandboxed_cpp_build_catches_header_only_in_srcs() { + bazel --batch clean &> $TEST_log \ + || fail "bazel clean failed" + + cat << 'EOF' > examples/cpp/BUILD +cc_library( + name = "hello-lib", + srcs = ["hello-lib.c", "hello-lib.h"], +) + +cc_binary( + name = "hello-world", + srcs = ["hello-world.c"], + deps = [":hello-lib"], +) +EOF + + bazel build --spawn_strategy=sandboxed //examples/cpp:hello-lib &> $TEST_log \ + || fail "building hello-lib should have succeeded with header file in srcs" + + bazel build --spawn_strategy=sandboxed //examples/cpp:hello-world &> $TEST_log \ + && fail "building hello-world should not have succeeded with library header file in srcs" + + fgrep "undeclared inclusion(s) in rule '//examples/cpp:hello-world'" $TEST_log \ + || fail "could not find 'undeclared inclusion' error message in bazel output" +} + +function test_standalone_cpp_build_rebuilds_on_change() { + bazel --batch clean &> $TEST_log \ + || fail "bazel clean failed" + + bazel build --hdrs_check=strict --spawn_strategy=standalone //examples/cpp:hello-world &> $TEST_log \ + || fail "Building hello-world failed" + + bazel-bin/examples/cpp/hello-world | fgrep "greetings from the header" \ + || fail "Did not print expected string 'greetings from the header'" + + sed "s/from the header/from the modified header/g" < examples/cpp/lib/hello-lib.h > tmp \ + || fail "modifying hello-lib.h failed" + + mv tmp examples/cpp/lib/hello-lib.h \ + || fail "moving modified header file back to examples/cpp/lib/hello-lib.h failed" + + bazel build --hdrs_check=strict --spawn_strategy=standalone //examples/cpp:hello-world &> $TEST_log \ + || fail "Building modified hello-world failed" + + bazel-bin/examples/cpp/hello-world | fgrep "greetings from the modified header" \ + || fail "Did not print expected string 'greetings from the modified header'" +} + +function test_standalone_cpp_build_catches_missing_header() { + bazel --batch clean &> $TEST_log \ + || fail "bazel clean failed" + + cat << 'EOF' > examples/cpp/BUILD +cc_library( + name = "hello-lib", + srcs = ["lib/hello-lib.c"], +) +EOF + + bazel build --hdrs_check=strict --spawn_strategy=standalone //examples/cpp:hello-lib &> $TEST_log \ + && fail "build should not have succeeded with missing header file" + + fgrep "undeclared inclusion(s) in rule '//examples/cpp:hello-lib'" $TEST_log \ + || fail "could not find 'undeclared inclusion' error message in bazel output" +} + +function DISABLED_test_standalone_cpp_build_catches_header_only_in_srcs() { + bazel --batch clean &> $TEST_log \ + || fail "bazel clean failed" + + cat << 'EOF' > examples/cpp/BUILD +cc_library( + name = "hello-lib", + srcs = ["hello-lib.c", "hello-lib.h"], +) + +cc_binary( + name = "hello-world", + srcs = ["hello-world.c"], + deps = [":hello-lib"], +) +EOF + + bazel build --hdrs_check=strict --spawn_strategy=standalone //examples/cpp:hello-lib &> $TEST_log \ + || fail "building hello-lib should have succeeded with header file in srcs" + + bazel build --hdrs_check=strict --spawn_strategy=standalone //examples/cpp:hello-world &> $TEST_log \ + && fail "building hello-world should not have succeeded with library header file in srcs" + + fgrep "undeclared inclusion(s) in rule '//examples/cpp:hello-world'" $TEST_log \ + || fail "could not find 'undeclared inclusion' error message in bazel output" +} + +check_kernel_version +check_sandbox_allowed || exit 0 +run_suite "sandbox" diff --git a/src/test/shell/bazel/bazel_sandboxing_test.sh b/src/test/shell/bazel/bazel_sandboxing_test.sh index 5cf124af72..aa113cfffa 100755 --- a/src/test/shell/bazel/bazel_sandboxing_test.sh +++ b/src/test/shell/bazel/bazel_sandboxing_test.sh @@ -18,47 +18,11 @@ # # Load test environment -source $(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/test-setup.sh \ +src_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +source ${src_dir}/test-setup.sh \ || { echo "test-setup.sh not found!" >&2; exit 1; } - -# namespaces which are used by the sandbox were introduced in 3.8, so -# test won't run on earlier kernels -function check_kernel_version { - if [ "${PLATFORM-}" = "darwin" ]; then - echo "Test will skip: sandbox is not yet supported on Darwin." - exit 0 - fi - MAJOR=$(uname -r | sed 's/^\([0-9]*\)\.\([0-9]*\)\..*/\1/') - MINOR=$(uname -r | sed 's/^\([0-9]*\)\.\([0-9]*\)\..*/\2/') - if [ $MAJOR -lt 3 ]; then - echo "Test will skip: sandbox requires kernel >= 3.8; got $(uname -r)" - exit 0 - fi - if [ $MAJOR -eq 3 ] && [ $MINOR -lt 8 ]; then - echo "Test will skip: sandbox requires kernel >= 3.8; got $(uname -r)" - exit 0 - fi -} - -# Some CI systems might deactivate sandboxing -function check_sandbox_allowed { - mkdir -p test - # Create a program that check if unshare(2) is allowed. - cat <<'EOF' > test/test.c -#define _GNU_SOURCE -#include -int main() { - return unshare(CLONE_NEWNS | CLONE_NEWUTS | CLONE_NEWIPC | CLONE_NEWUSER); -} -EOF - cat <<'EOF' >test/BUILD -cc_test(name = "sandbox_enabled", srcs = ["test.c"], copts = ["-std=c99"]) -EOF - bazel test //test:sandbox_enabled || { - echo "Sandboxing disabled, skipping..." - return false - } -} +source ${src_dir}/bazel_sandboxing_test_utils.sh \ + || { echo "bazel_sandboxing_test_utils.sh not found!" >&2; exit 1; } function set_up { mkdir -p examples/genrule diff --git a/src/test/shell/bazel/bazel_sandboxing_test_utils.sh b/src/test/shell/bazel/bazel_sandboxing_test_utils.sh new file mode 100755 index 0000000000..e7efd51224 --- /dev/null +++ b/src/test/shell/bazel/bazel_sandboxing_test_utils.sh @@ -0,0 +1,55 @@ +#!/bin/bash +# +# Copyright 2015 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. +# + +# namespaces which are used by the sandbox were introduced in 3.8, so +# test won't run on earlier kernels +function check_kernel_version { + if [ "${PLATFORM-}" = "darwin" ]; then + echo "Test will skip: sandbox is not yet supported on Darwin." + exit 0 + fi + MAJOR=$(uname -r | sed 's/^\([0-9]*\)\.\([0-9]*\)\..*/\1/') + MINOR=$(uname -r | sed 's/^\([0-9]*\)\.\([0-9]*\)\..*/\2/') + if [ $MAJOR -lt 3 ]; then + echo "Test will skip: sandbox requires kernel >= 3.8; got $(uname -r)" + exit 0 + fi + if [ $MAJOR -eq 3 ] && [ $MINOR -lt 8 ]; then + echo "Test will skip: sandbox requires kernel >= 3.8; got $(uname -r)" + exit 0 + fi +} + +# Some CI systems might deactivate sandboxing +function check_sandbox_allowed { + mkdir -p test + # Create a program that check if unshare(2) is allowed. + cat <<'EOF' > test/test.c +#define _GNU_SOURCE +#include +int main() { + return unshare(CLONE_NEWNS | CLONE_NEWUTS | CLONE_NEWIPC | CLONE_NEWUSER); +} +EOF + cat <<'EOF' >test/BUILD +cc_test(name = "sandbox_enabled", srcs = ["test.c"], copts = ["-std=c99"]) +EOF + bazel test //test:sandbox_enabled || { + echo "Sandboxing disabled, skipping..." + return false + } +} -- cgit v1.2.3