Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Ban bad readonly shell pattern (#5924)
This subverts `set -e` and means that failed commands don't fail the
script, which is responsible for late failures on CI such as
https://travis-ci.org/pantsbuild/pants/jobs/389174049 which failed to
download protoc, but only failed when something tried to use it.
  • Loading branch information
illicitonion committed Jun 8, 2018
1 parent d221967 commit 16bd8ff
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 10 deletions.
11 changes: 11 additions & 0 deletions build-support/bin/check_shell.sh
@@ -0,0 +1,11 @@
#!/bin/bash -eu
bad_output="$(find * -name '*.sh' -print0 | xargs -0 grep '^ *\(readonly\|declare\) .*\(\$(\|`\)' || :)"

if [[ -n "${bad_output}" ]]; then
echo >&2 "Found bash files with readonly variables defined by invoking subprocesses."
echo >&2 "This is bad because \`set -e\` doesn't exit if these fail."
echo >&2 "Make your variable non-readonly, or define it and then mark it readonly."
echo >&2 "Matches:"
echo "${bad_output}"
exit 1
fi
2 changes: 1 addition & 1 deletion build-support/bin/download_binary.sh
Expand Up @@ -25,7 +25,7 @@ readonly util_name="$2"
readonly version="$3"
readonly filename="${4:-${util_name}}"

readonly os=$("${REPO_ROOT}/build-support/bin/get_os.sh")
os=$("${REPO_ROOT}/build-support/bin/get_os.sh")
readonly path="bin/${util_name}/${os}/${version}/${filename}"
readonly cache_path="${CACHE_ROOT}/${path}"

Expand Down
2 changes: 1 addition & 1 deletion build-support/bin/native/bootstrap_code.sh
Expand Up @@ -7,7 +7,7 @@ REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && cd ../../.. && pwd -P)"
# + fingerprint_data: Fingerprints the data on stdin.
source ${REPO_ROOT}/build-support/common.sh

readonly KERNEL=$(uname -s | tr '[:upper:]' '[:lower:]')
KERNEL=$(uname -s | tr '[:upper:]' '[:lower:]')
case "${KERNEL}" in
linux)
readonly LIB_EXTENSION=so
Expand Down
10 changes: 5 additions & 5 deletions build-support/bin/native/cargo.sh
Expand Up @@ -12,20 +12,20 @@ REPO_ROOT=$(cd $(dirname "${BASH_SOURCE[0]}") && cd ../../.. && pwd -P)
source "${REPO_ROOT}/build-support/bin/native/bootstrap_rust.sh"
ensure_native_build_prerequisites >&2

readonly download_binary="${REPO_ROOT}/build-support/bin/download_binary.sh"
download_binary="${REPO_ROOT}/build-support/bin/download_binary.sh"

# The following is needed by grpcio-sys and we have no better way to hook its build.rs than this;
# ie: wrapping cargo.
readonly cmakeroot="$("${download_binary}" "binaries.pantsbuild.org" "cmake" "3.9.5" "cmake.tar.gz")"
readonly goroot="$("${download_binary}" "binaries.pantsbuild.org" "go" "1.7.3" "go.tar.gz")/go"
cmakeroot="$("${download_binary}" "binaries.pantsbuild.org" "cmake" "3.9.5" "cmake.tar.gz")"
goroot="$("${download_binary}" "binaries.pantsbuild.org" "go" "1.7.3" "go.tar.gz")/go"

# Code generation in the bazel_protos crate needs to be able to find protoc on the PATH.
readonly protoc="$("${download_binary}" "binaries.pantsbuild.org" "protobuf" "3.4.1" "protoc")"
protoc="$("${download_binary}" "binaries.pantsbuild.org" "protobuf" "3.4.1" "protoc")"

export GOROOT="${goroot}"
export PATH="${cmakeroot}/bin:${goroot}/bin:${CARGO_HOME}/bin:$(dirname "${protoc}"):${PATH}"

readonly cargo_bin="${CARGO_HOME}/bin/cargo"
cargo_bin="${CARGO_HOME}/bin/cargo"

if [[ -n "${CARGO_WRAPPER_DEBUG}" ]]; then
cat << DEBUG >&2
Expand Down
2 changes: 1 addition & 1 deletion build-support/bin/native/rust_toolchain.sh
Expand Up @@ -18,7 +18,7 @@ of the \$CARGO_HOME binary to run.
EOF
exit 1
fi
readonly binary="$(basename "$0")"
binary="$(basename "$0")"

ensure_native_build_prerequisites >&2
exec "${CARGO_HOME}/bin/${binary}" "$@"
1 change: 1 addition & 0 deletions build-support/bin/pre-commit.sh
Expand Up @@ -13,6 +13,7 @@ echo "* Checking packages" && ./build-support/bin/check_packages.sh || exit 1
echo "* Checking headers" && ./build-support/bin/check_header.sh || exit 1
echo "* Checking for banned imports" && ./build-support/bin/check_banned_imports.sh || exit 1
echo "* Checking formatting of rust files" && "$(pwd)/build-support/bin/check_rust_formatting.sh" || exit 1
echo "* Checking for bad shell patterns" && ./build-support/bin/check_shell.sh || exit 1

$(git rev-parse --verify master > /dev/null 2>&1)
if [[ $? -eq 0 ]]; then
Expand Down
4 changes: 2 additions & 2 deletions build-support/bin/release.sh
Expand Up @@ -18,8 +18,8 @@ function run_local_pants() {
# NB: Pants core does not have the ability to change its own version, so we compute the
# suffix here and mutate the VERSION_FILE to affect the current version.
readonly VERSION_FILE="${ROOT}/src/python/pants/VERSION"
readonly PANTS_STABLE_VERSION="$(cat "${VERSION_FILE}")"
readonly HEAD_SHA=$(git rev-parse --verify HEAD)
PANTS_STABLE_VERSION="$(cat "${VERSION_FILE}")"
HEAD_SHA=$(git rev-parse --verify HEAD)
readonly PANTS_UNSTABLE_VERSION="${PANTS_STABLE_VERSION}+${HEAD_SHA:0:8}"

readonly DEPLOY_DIR="${ROOT}/dist/deploy"
Expand Down

0 comments on commit 16bd8ff

Please sign in to comment.