Skip to content
Permalink
Browse files

Apply final set of Shellcheck fixes and turn on in CI (#7832)

Builds off of the work from #7698 and #7719.

Several of the problems are ignored because they are very difficult to correctly fix, and the behavior is not broken when ran locally and in CI so the cost of fixing is not deemed worth it.
  • Loading branch information...
Eric-Arellano committed Jun 3, 2019
1 parent cf95c0a commit e2b18b94942758395a0dac8769030edcdfdc0f33
@@ -148,6 +148,7 @@ base_linux_config: &base_linux_config
- libssl-dev
- jq
- unzip
- shellcheck
language: python
before_install:
- ./build-support/bin/install_aws_cli_for_ci.sh
@@ -42,6 +42,8 @@ if [[ "${check}" == "true" ]]; then
cmd=("${cmd[@]}" "--check")
fi

# WONTFIX: fixing the array expansion is too difficult to be worth it. See https://github.com/koalaman/shellcheck/wiki/SC2207.
# shellcheck disable=SC2207
bad_files=(
$(
cd "${NATIVE_ROOT}" || exit "${PIPESTATUS[0]}"
@@ -2,6 +2,7 @@

exit_code=0

# NB: we intentionally don't want quote expansion here. See https://github.com/koalaman/shellcheck/wiki/SC2016.
# shellcheck disable=SC2016
bad_output="$(find ./* -name '*.sh' -print0 \
| xargs -0 grep '^ *\(readonly\|declare\) .*\(\$(\|`\)' \
@@ -120,6 +120,7 @@ else
fi
export PY="${PY:-python${py_major_minor}}"

# NB: we intentionally don't want quote expansion here. See https://github.com/koalaman/shellcheck/wiki/SC2016.
# shellcheck disable=SC2016
export PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS="${PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS:-['CPython==${py_major_minor}.*']}"
banner "Setting interpreter constraints to ${PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS}"
@@ -6,4 +6,4 @@
# Check for copies (-C) and moves (-M), so we don't get false positives when people do
# refactorings. -l50 bounds the time git takes to search for these non-additions.
# See git-diff(1) and https://stackoverflow.com/a/2299672/2518889 for discussion of these options.
exec git diff --cached --name-only --diff-filter=A -C -M -l50
exec git --no-pager diff --cached --name-only --diff-filter=A -C -M -l50
@@ -23,6 +23,8 @@ for job in ${jobs}; do
;;
"failed")
"${curl[@]}" "https://api.travis-ci.org/job/${job}/log.txt" > "logs/jobs/${job}/txt"
# WONTFIX: fixing the array expansion is too difficult to be worth it. See https://github.com/koalaman/shellcheck/wiki/SC2207.
# shellcheck disable=SC2207
targets=("${targets[@]}" $(cat -v "logs/jobs/${job}/txt" | awk '$2 == "....." && $3 ~ /^FAILURE/ {print $1}'))
;;
*)
@@ -75,7 +75,7 @@ function bootstrap_rust() {
fi
done

ln -fs "$(dirname "${cargo_bin_dir}")/lib/rustlib/src/rust/src"
ln -fs "$(dirname "${cargo_bin_dir}")/lib/rustlib/src/rust/src" src
ln -fs cargo.sh cargo
ln -fs cargo.sh ".${cargo_versioned}"
)
@@ -74,6 +74,8 @@ do
echo "* ${subject}"

urls=()
# WONTFIX: fixing the array expansion is too difficult to be worth it. See https://github.com/koalaman/shellcheck/wiki/SC2207.
# shellcheck disable=SC2207
urls=(
"${urls[@]}"
$(
@@ -82,6 +84,8 @@ do
sed -Ee "s|^\(#([0-9]+)\)$|https://github.com/pantsbuild/pants/pull/\1|"
)
)
# WONTFIX: fixing the array expansion is too difficult to be worth it. See https://github.com/koalaman/shellcheck/wiki/SC2207.
# shellcheck disable=SC2207
urls=(
"${urls[@]}"
$(
@@ -228,6 +228,8 @@ function build_pants_packages() {
pants_version_set "${version}"

start_travis_section "${NAME}" "Building packages"
# WONTFIX: fixing the array expansion is too difficult to be worth it. See https://github.com/koalaman/shellcheck/wiki/SC2207.
# shellcheck disable=SC2207
packages=(
$(run_packages_script build_and_print "${version}")
) || die "Failed to build packages at ${version}!"
@@ -315,6 +317,8 @@ function install_and_test_packages() {
export PANTS_PLUGIN_CACHE_DIR
trap 'rm -rf "${PANTS_PLUGIN_CACHE_DIR}"' EXIT

# WONTFIX: fixing the array expansion is too difficult to be worth it. See https://github.com/koalaman/shellcheck/wiki/SC2207.
# shellcheck disable=SC2207
packages=(
$(run_packages_script list | grep '.' | awk '{print $1}')
) || die "Failed to list packages!"
@@ -515,11 +519,14 @@ function fetch_and_check_prebuilt_wheels() {
fetch_prebuilt_wheels "${check_dir}"

local missing=()
# WONTFIX: fixing the array expansion is too difficult to be worth it. See https://github.com/koalaman/shellcheck/wiki/SC2207.
# shellcheck disable=SC2207
RELEASE_PACKAGES=(
$(run_packages_script list | grep '.' | awk '{print $1}')
) || die "Failed to get a list of packages to release!"
for PACKAGE in "${RELEASE_PACKAGES[@]}"
do
for PACKAGE in "${RELEASE_PACKAGES[@]}"; do
# WONTFIX: fixing the array expansion is too difficult to be worth it. See https://github.com/koalaman/shellcheck/wiki/SC2207.
# shellcheck disable=SC2207
packages=($(find_pkg "${PACKAGE}" "${PANTS_UNSTABLE_VERSION}" "${check_dir}"))
if [ ${#packages[@]} -eq 0 ]; then
missing+=("${PACKAGE}")
@@ -528,8 +535,7 @@ function fetch_and_check_prebuilt_wheels() {

# Confirm that if the package is not cross platform that we have whls for two platforms.
local cross_platform=""
for package in "${packages[@]}"
do
for package in "${packages[@]}"; do
if [[ "${package}" =~ -none-any.whl ]]
then
cross_platform="true"
@@ -559,7 +565,7 @@ function adjust_wheel_platform() {
local src_plat="$1"
local dst_plat="$2"
local dir="$3"
for src_whl in $(find "${dir}" -name '*'"${src_plat}.whl"); do
find "$dir" -type f -name "*${src_plat}.whl" | while read -r src_whl; do
local dst_whl=${src_whl/$src_plat/$dst_plat}
mv -f "${src_whl}" "${dst_whl}"
done
@@ -6,7 +6,7 @@
import subprocess
from glob import glob

from common import die, green
from common import die


def main() -> None:
@@ -37,8 +37,6 @@ def run_shellcheck() -> None:
subprocess.run(command, check=True)
except subprocess.CalledProcessError:
die("Please fix the above errors and run again.")
else:
green("./pants passed the shellcheck!")


if __name__ == "__main__":
@@ -1,3 +1,4 @@
# NB: shellcheck complains this is unused, but it's used by callers. See https://github.com/koalaman/shellcheck/wiki/SC2034.
# shellcheck disable=SC2034
CACHE_ROOT=${XDG_CACHE_HOME:-$HOME/.cache}/pants

@@ -19,8 +19,7 @@ DIRS_TO_CHECK=(
pants-plugins
examples
contrib
# TODO(6071): Add `build-support/bin` once we update the check_header_helper.py script to
# allow Python 3 headers (i.e. without __future__ and coding=utf-8).
build-support/bin
)

# TODO(#7068): Fix all these checks to only act on staged files with
@@ -30,22 +29,24 @@ DIRS_TO_CHECK=(
# integration test!
set -e

# You can use ("$()") with double quotes in zsh, I assume this splits by IFS...
# WONTFIX: fixing the array expansion is too difficult to be worth it. See https://github.com/koalaman/shellcheck/wiki/SC2207.
# shellcheck disable=SC2207
ADDED_FILES=($(./build-support/bin/get_added_files.sh))
MERGE_BASE="$(git_merge_base)"

echo "* Checking packages"
# TODO: Determine the most *hygienic* way to split an array on the command line in portable bash,
# and stick to it.
./build-support/bin/check_packages.sh "${DIRS_TO_CHECK[@]}"
./build-support/bin/check_packages.sh "${DIRS_TO_CHECK[@]}" || exit 1

echo "* Checking headers"
./build-support/bin/check_header.py "${DIRS_TO_CHECK[@]}" --files-added "${ADDED_FILES[@]}"
./build-support/bin/check_header.py "${DIRS_TO_CHECK[@]}" --files-added "${ADDED_FILES[@]}" || exit 1

echo "* Checking for banned imports"
./build-support/bin/check_banned_imports.py
./build-support/bin/check_banned_imports.py || exit 1

echo "* Checking for bad shell patterns"
echo "* Checking shell scripts via shellcheck"
./build-support/bin/shellcheck.py || exit 1

echo "* Checking shell scripts via our custom linter"
./build-support/bin/check_shell.sh || exit 1

# When travis builds a tag, it does so in a shallow clone without master fetched, which
@@ -141,6 +141,7 @@ base_linux_config: &base_linux_config
- libssl-dev
- jq
- unzip
- shellcheck
language: python
before_install:
- ./build-support/bin/install_aws_cli_for_ci.sh
@@ -19,8 +19,8 @@ function pkg_scrooge_install_test() {
function pkg_buildgen_install_test() {
local version=$1
shift
local PIP_ARGS="$@"
pip install ${PIP_ARGS} "pantsbuild.pants.contrib.buildgen==${version}" && \
local PIP_ARGS=("$@")
pip install "${PIP_ARGS[@]}" "pantsbuild.pants.contrib.buildgen==${version}" && \
python -c "from pants.contrib.buildgen.build_file_manipulator import *"
}

9 pants
@@ -67,12 +67,15 @@ export PY="${PY:-python3}"
# Pants will try to use Python 2 for subprocesses. Without setting minor version constraints, when using Python 3
# we could end up using a different Python minor version for subprocesses than we do for Pants.
py_major_minor_patch=$(${PY} -c 'import sys; print(".".join(map(str, sys.version_info[0:3])))')
# NB: We intentionally don't want quote expansion here. See https://github.com/koalaman/shellcheck/wiki/SC2016.
# shellcheck disable=SC2016
export PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS="${PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS:-['CPython==${py_major_minor_patch}']}"

PANTS_EXE="${HERE}/src/python/pants/bin/pants_loader.py"

if [[ -n "${WRAPPER_REQUIREMENTS}" ]]; then
# WONTFIX: fixing the array expansion is too difficult to be worth it. See https://github.com/koalaman/shellcheck/wiki/SC2207.
# shellcheck disable=SC2207
REQUIREMENTS=(
$(echo "${WRAPPER_REQUIREMENTS}" | tr : ' ')
"${REQUIREMENTS[@]}"
@@ -83,18 +86,20 @@ PANTS_SRCPATH=(
"${HERE}/src/python"
)
if [[ -n "${WRAPPER_SRCPATH}" ]]; then
# WONTFIX: fixing the array expansion is too difficult to be worth it. See https://github.com/koalaman/shellcheck/wiki/SC2207.
# shellcheck disable=SC2207
PANTS_SRCPATH=(
$(echo "${WRAPPER_SRCPATH}" | tr : ' ')
"${PANTS_SRCPATH[@]}"
)
fi
PANTS_SRCPATH="$(echo "${PANTS_SRCPATH[@]}" | tr ' ' :)"
PANTS_SRCPATH_STR="$(echo "${PANTS_SRCPATH[@]}" | tr ' ' :)"

function exec_pants_bare() {
# Redirect activation and native bootstrap to ensure that they don't interfere with stdout.
activate_pants_venv 1>&2
bootstrap_native_code 1>&2
PYTHONPATH="${PANTS_SRCPATH}:${PYTHONPATH}" \
PYTHONPATH="${PANTS_SRCPATH_STR}:${PYTHONPATH}" \
exec python "${PANTS_EXE}" "$@"
}

@@ -23,7 +23,7 @@ set -eo pipefail
# If a file "went away", this won't remove it.

root=$(
cd "$(dirname $0)"
cd "$(dirname "$0")"
/bin/pwd
)

@@ -32,7 +32,7 @@ path_within_url=$2
out=/tmp/pantsdoc.$$

# When done, clean up tmp dir:
trap "rm -fr $out" 0 1 2
trap 'rm -fr $out' 0 1 2

mkdir -p $out
cd $out

0 comments on commit e2b18b9

Please sign in to comment.
You can’t perform that action at this time.