Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add `shellcheck.py` and apply first round of fixes #7698

Merged
merged 19 commits into from May 14, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -20,3 +20,12 @@ python_binary(
],
compatibility = ['CPython>=3.6'],
)

python_binary(
name = 'shellcheck',
sources = 'shellcheck.py',
dependencies = [
':common',
],
compatibility = ['CPython>=3.6'],
)
@@ -3,6 +3,6 @@
set -e

REPO_ROOT="$(git rev-parse --show-toplevel)"
cd ${REPO_ROOT}
cd "${REPO_ROOT}"

./build-support/bin/native/cargo clippy --manifest-path="${REPO_ROOT}/src/rust/engine/Cargo.toml" --all
@@ -4,19 +4,19 @@ set -euo pipefail
IFS=$'\n\t'

REPO_ROOT="$(git rev-parse --show-toplevel)"
cd ${REPO_ROOT}
cd "${REPO_ROOT}"

DIRS_TO_CHECK=("$@")

non_empty_files=$(find ${DIRS_TO_CHECK[@]} -type f -name "__init__.py" -not -empty)
non_empty_files=$(find "${DIRS_TO_CHECK[@]}" -type f -name "__init__.py" -not -empty)

bad_files=()
for package_file in ${non_empty_files}
do
if [[ "$(sed -E -e 's/^[[:space:]]+//' -e 's/[[:space:]]+$//' ${package_file})" != \
if [[ "$(sed -E -e 's/^[[:space:]]+//' -e 's/[[:space:]]+$//' "${package_file}")" != \
"__import__('pkg_resources').declare_namespace(__name__)" ]]
then
bad_files+=(${package_file})
bad_files+=("${package_file}")
fi
done

@@ -36,25 +36,25 @@ done
NATIVE_ROOT="${REPO_ROOT}/src/rust/engine"

cmd=(
${REPO_ROOT}/build-support/bin/native/cargo fmt --all --
"${REPO_ROOT}/build-support/bin/native/cargo" fmt --all --
)
if [[ "${check}" == "true" ]]; then
cmd=("${cmd[@]}" "--check")
fi

bad_files=(
$(
cd "${NATIVE_ROOT}"
cd "${NATIVE_ROOT}" || exit "${PIPESTATUS[0]}"
# Ensure generated code is present since `cargo fmt` needs to do enough parsing to follow use's
# and these will land in generated code.
echo >&2 "Ensuring generated code is present for downstream formatting checks..."
${REPO_ROOT}/build-support/bin/native/cargo check -p bazel_protos
"${REPO_ROOT}/build-support/bin/native/cargo" check -p bazel_protos
${cmd[@]} | \
"${cmd[@]}" | \
awk '$0 ~ /^Diff in/ {print $3}' | \
sort -u
exit ${PIPESTATUS[0]}
exit "${PIPESTATUS[0]}"
)
)
exit_code=$?
@@ -63,15 +63,15 @@ if [[ ${exit_code} -ne 0 ]]; then
if [[ "${check}" == "true" ]]; then
echo >&2 "The following rust files were incorrectly formatted, run \`$0 -f\` to reformat them:"
for bad_file in ${bad_files[*]}; do
echo >&2 ${bad_file}
echo >&2 "${bad_file}"
done
else
cat << EOF >&2
An error occurred while checking the formatting of rust files.
Try running \`(cd "${NATIVE_ROOT}" && ${cmd[@]})\` to investigate.
Its error is:
EOF
cd "${NATIVE_ROOT}" && ${cmd[@]} >/dev/null
cd "${NATIVE_ROOT}" && "${cmd[@]}" >/dev/null
fi
exit 1
fi
@@ -6,8 +6,10 @@ cargo="${REPO_ROOT}/build-support/bin/native/cargo"

"${cargo}" ensure-installed --package cargo-ensure-prefix --version 0.1.3

out="$("${cargo}" ensure-prefix --manifest-path="${REPO_ROOT}/src/rust/engine/Cargo.toml" --prefix-path="${REPO_ROOT}/build-support/rust-target-prefix.txt" --all --exclude=bazel_protos)"
if [[ $? -ne 0 ]]; then
if ! out="$("${cargo}" ensure-prefix \
--manifest-path="${REPO_ROOT}/src/rust/engine/Cargo.toml" \
--prefix-path="${REPO_ROOT}/build-support/rust-target-prefix.txt" \
--all --exclude=bazel_protos)"; then

This comment has been minimized.

Copy link
@cosmicexplorer

cosmicexplorer May 14, 2019

Contributor

I would absolutely make this into a function!

echo >&2 "Rust targets didn't have correct prefix:"
echo >&2 "${out}"
exit 1
@@ -1,5 +1,9 @@
#!/bin/bash -eu
bad_output="$(find * -name '*.sh' -print0 | xargs -0 grep '^ *\(readonly\|declare\) .*\(\$(\|`\)' || :)"

# shellcheck disable=SC2016
bad_output="$(find ./* -name '*.sh' -print0 \
| xargs -0 grep '^ *\(readonly\|declare\) .*\(\$(\|`\)' \
|| :)"

This comment has been minimized.

Copy link
@cosmicexplorer

cosmicexplorer May 14, 2019

Contributor

function


if [[ -n "${bad_output}" ]]; then
echo >&2 "Found bash files with readonly variables defined by invoking subprocesses."
@@ -4,8 +4,8 @@
# fails the build.
set -o pipefail

REPO_ROOT=$(cd $(dirname "${BASH_SOURCE[0]}") && cd "$(git rev-parse --show-toplevel)" && pwd)
cd ${REPO_ROOT}
REPO_ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")" && cd "$(git rev-parse --show-toplevel)" && pwd)
cd "${REPO_ROOT}" || exit 1

source build-support/common.sh

@@ -85,11 +85,11 @@ while getopts "h27fxbmrjlpeasu:ny:ci:tz" opt; do
*) usage "Invalid option: -${OPTARG}" ;;
esac
done
shift $((${OPTIND} - 1))
shift $((OPTIND - 1))

echo
if [[ $# > 0 ]]; then
banner "CI BEGINS: $@"
if [[ $# -gt 0 ]]; then
banner "CI BEGINS: $*"
else
banner "CI BEGINS"
fi
@@ -106,7 +106,7 @@ export PANTS_DEV=1

# We only want to output failures and skips.
# See https://docs.pytest.org/en/latest/usage.html#detailed-summary-report.
export PYTEST_PASSTHRU_ARGS="-q -rfa"
export PYTEST_PASSTHRU_ARGS=(-q -rfa)

# Determine the Python version to use for bootstrapping pants.pex. This would usually not be
# necessary to set when developing locally, because the `./pants` and `./pants2` scripts set
@@ -122,6 +122,7 @@ else
fi
export PY="${PY:-python${py_major_minor}}"

# 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}"

@@ -196,7 +197,7 @@ if [[ "${run_internal_backends:-false}" == "true" ]]; then
start_travis_section "BackendTests" "Running internal backend python tests"
(
./pants.pex test.pytest \
pants-plugins/src/python:: pants-plugins/tests/python:: -- ${PYTEST_PASSTHRU_ARGS}
pants-plugins/src/python:: pants-plugins/tests/python:: -- "${PYTEST_PASSTHRU_ARGS[@]}"
) || die "Internal backend python test failure"
end_travis_section
fi
@@ -208,8 +209,8 @@ if [[ "${run_python:-false}" == "true" ]]; then
start_travis_section "CoreTests" "Running core python tests${shard_desc}"
(
./pants.pex --tag='-integration' test.pytest --chroot \
--test-pytest-test-shard=${python_unit_shard} \
src/python:: tests/python:: -- ${PYTEST_PASSTHRU_ARGS}
"--test-pytest-test-shard=${python_unit_shard}" \
src/python:: tests/python:: -- "${PYTEST_PASSTHRU_ARGS[@]}"
) || die "Core python test failure"
end_travis_section
fi
@@ -221,8 +222,8 @@ if [[ "${run_contrib:-false}" == "true" ]]; then
start_travis_section "ContribTests" "Running contrib python tests${shard_desc}"
(
./pants.pex --exclude-target-regexp='.*/testprojects/.*' test.pytest \
--test-pytest-test-shard=${python_contrib_shard} \
contrib:: -- ${PYTEST_PASSTHRU_ARGS}
"--test-pytest-test-shard=${python_contrib_shard}" \
contrib:: -- "${PYTEST_PASSTHRU_ARGS[@]}"
) || die "Contrib python test failure"
end_travis_section
fi
@@ -269,7 +270,7 @@ if [[ "${test_platform_specific_behavior:-false}" == 'true' ]]; then
"Running platform-specific testing on platform: $(uname)"
(
./pants.pex --tag='+platform_specific_behavior' test \
src/python/:: tests/python:: -- ${PYTEST_PASSTHRU_ARGS}
src/python/:: tests/python:: -- "${PYTEST_PASSTHRU_ARGS[@]}"
) || die "Pants platform-specific test failure"
end_travis_section
fi
@@ -282,8 +283,8 @@ if [[ "${run_integration:-false}" == "true" ]]; then
start_travis_section "IntegrationTests" "Running Pants Integration tests${shard_desc}"
(
./pants.pex --tag='+integration' test.pytest \
--test-pytest-test-shard=${python_intg_shard} \
src/python:: tests/python:: -- ${PYTEST_PASSTHRU_ARGS}
"--test-pytest-test-shard=${python_intg_shard}" \
src/python:: tests/python:: -- "${PYTEST_PASSTHRU_ARGS[@]}"
) || die "Pants Integration test failure"
end_travis_section
fi
@@ -3,6 +3,7 @@
REPO_ROOT="$(git rev-parse --show-toplevel)"
cd "$REPO_ROOT" || exit 1

# shellcheck source=build-support/common.sh
source "${REPO_ROOT}/build-support/common.sh"

function usage() {
@@ -2,10 +2,11 @@

# This script wraps the main() method in binary_util.py.

REPO_ROOT=$(cd $(dirname "${BASH_SOURCE[0]}") && cd ../.. && pwd -P)
REPO_ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")" && cd ../.. && pwd -P)
BINARY_HELPER_SCRIPT="${REPO_ROOT}/src/python/pants/binaries/binary_util.py"

# shellcheck source=build-support/pants_venv
source "${REPO_ROOT}/build-support/pants_venv"

activate_pants_venv 1>&2
PYTHONPATH="${REPO_ROOT}/src/python:${PYTHONPATH}" python "$BINARY_HELPER_SCRIPT" $@
PYTHONPATH="${REPO_ROOT}/src/python:${PYTHONPATH}" python "$BINARY_HELPER_SCRIPT" "$@"
@@ -12,7 +12,7 @@ BOOTSTRAPPED_PEX_URL=s3://${BOOTSTRAPPED_PEX_BUCKET}/${BOOTSTRAPPED_PEX_KEY}

# First check that there's only one version of the object on S3, to detect malicious overwrites.
NUM_VERSIONS=$(aws --no-sign-request --region us-east-1 s3api list-object-versions \
--bucket ${BOOTSTRAPPED_PEX_BUCKET} --prefix ${BOOTSTRAPPED_PEX_KEY} --max-items 2 \
--bucket "${BOOTSTRAPPED_PEX_BUCKET}" --prefix "${BOOTSTRAPPED_PEX_KEY}" --max-items 2 \
| jq '.Versions | length')
[ "${NUM_VERSIONS}" == "1" ] || (echo "Error: Found ${NUM_VERSIONS} versions for ${BOOTSTRAPPED_PEX_URL}" && exit 1)

@@ -11,12 +11,12 @@ mkdir -p logs/jobs
curl=(curl -s -S -H 'Travis-API-Version: 3')

"${curl[@]}" 'https://api.travis-ci.org/repo/pantsbuild%2Fpants/builds?event_type=pull_request&limit=100&sort_by=finished_at:desc' > logs/search
jobs="$(cat logs/search | jq "[ .builds[] | select(.pull_request_number == ${pull}) ][0] | .jobs[].id")"
jobs="$(jq "[ .builds[] | select(.pull_request_number == ${pull}) ][0] | .jobs[].id" logs/search)"

This comment has been minimized.

Copy link
@cosmicexplorer

cosmicexplorer May 14, 2019

Contributor

function

targets=()
for job in ${jobs}; do
mkdir -p "logs/jobs/${job}"
"${curl[@]}" "https://api.travis-ci.org/job/${job}" >"logs/jobs/${job}/info"
state="$(cat logs/jobs/${job}/info | jq -r '.state')"
state="$(jq -r '.state' "logs/jobs/${job}/info")"
case "${state}" in
"passed")
continue
@@ -21,7 +21,7 @@ if [[ ! -x "${AWS_CLI_BIN}" ]]; then

TMPDIR=$(mktemp -d)

pushd ${TMPDIR}
pushd "${TMPDIR}"

curl "https://s3.amazonaws.com/aws-cli/awscli-bundle.zip" -o "awscli-bundle.zip"
unzip awscli-bundle.zip
@@ -9,7 +9,7 @@ REPO_ROOT="$(git rev-parse --show-toplevel)"

HOOK_DIR="${GIT_DIR:-${REPO_ROOT}/.git}/hooks"
HOOK_SRC_DIR="${REPO_ROOT}/build-support/githooks"
HOOK_NAMES="$(ls ${HOOK_SRC_DIR})"
HOOK_NAMES="$(ls "${HOOK_SRC_DIR}")"

RELPATH_PREFIX="$(cat << EOF | python
import os
@@ -24,8 +24,8 @@ function install_hook() {
RELPATH="${RELPATH_PREFIX}/${HOOK}"
(
cd "${HOOK_DIR}" && \
rm -f ${HOOK} && \
ln -s "${RELPATH}" ${HOOK} && \
rm -f "${HOOK}" && \
ln -s "${RELPATH}" "${HOOK}" && \
echo "${HOOK} hook linked to $(pwd)/${HOOK}";
)
}
@@ -38,16 +38,16 @@ function ensure_hook() {

if [[ ! -e "${HOOK_DST}" ]]
then
install_hook ${HOOK}
install_hook "${HOOK}"
else
if cmp --quiet "${HOOK_SRC}" "${HOOK_DST}"
then
echo "${HOOK} hook up to date."
else
read -p "A ${HOOK} hook already exists, replace with ${HOOK_SRC}? [Yn]" ok
read -rp "A ${HOOK} hook already exists, replace with ${HOOK_SRC}? [Yn]" ok
if [[ "${ok:-Y}" =~ ^[yY]([eE][sS])?$ ]]
then
install_hook ${HOOK}
install_hook "${HOOK}"
else
echo "${HOOK} hook not installed"
exit 1
@@ -58,5 +58,5 @@ function ensure_hook() {


for HOOK in ${HOOK_NAMES}; do
ensure_hook ${HOOK}
ensure_hook "${HOOK}"
done
@@ -1,8 +1,9 @@
#!/usr/bin/env bash

REPO_ROOT="$(git rev-parse --show-toplevel)"
cd ${REPO_ROOT}
cd "${REPO_ROOT}" || exit 1

# shellcheck source=build-support/common.sh
source build-support/common.sh

function usage() {
@@ -34,7 +35,7 @@ do
done

# If changes were made or issues found, output with leading whitespace trimmed.
output="$(./pants --changed-parent="$(git_merge_base)" fmt.isort -- ${isort_args[@]})"
output="$(./pants --changed-parent="$(git_merge_base)" fmt.isort -- "${isort_args[@]}")"
echo "${output}" | grep -Eo '(ERROR).*$' && exit 1
echo "${output}" | grep -Eo '(Fixing).*$'
exit 0
@@ -4,6 +4,7 @@ set -e

REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && cd ../../.. && pwd -P)"

# shellcheck source=build-support/pants_venv
source "${REPO_ROOT}/build-support/pants_venv"

if (( $# != 2 )); then
@@ -5,7 +5,9 @@ REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && cd ../../.. && pwd -P)"
# Exposes:
# + die: Exit in a failure state and optionally log an error message to the console.
# + fingerprint_data: Fingerprints the data on stdin.
source ${REPO_ROOT}/build-support/common.sh

# shellcheck source=build-support/common.sh
source "${REPO_ROOT}/build-support/common.sh"

KERNEL=$(uname -s | tr '[:upper:]' '[:lower:]')
case "${KERNEL}" in
@@ -40,7 +42,7 @@ function calculate_current_hash() {
#
# Assumes we're in the venv that will be used to build the native engine.
(
cd ${REPO_ROOT}
cd "${REPO_ROOT}" || exit 1
(echo "${MODE_FLAG}"
echo "${RUST_TOOLCHAIN}"
uname
@@ -69,7 +71,8 @@ function _build_native_code() {

function bootstrap_native_code() {
# Bootstraps the native code only if needed.
local native_engine_version="$(calculate_current_hash)"
local native_engine_version
native_engine_version="$(calculate_current_hash)"
local engine_version_hdr="engine_version: ${native_engine_version}"
local target_binary="${NATIVE_ENGINE_CACHE_DIR}/${native_engine_version}/${NATIVE_ENGINE_BINARY}"
local target_binary_metadata="${target_binary}.metadata"
@@ -90,7 +93,7 @@ function bootstrap_native_code() {
target_binary="${NATIVE_ENGINE_CACHE_DIR}/${native_engine_version}/${NATIVE_ENGINE_BINARY}"
target_binary_metadata="${target_binary}.metadata"

mkdir -p "$(dirname ${target_binary})"
mkdir -p "$(dirname "${target_binary}")"
cp "${native_binary}" "${target_binary}"

local -r metadata_file=$(mktemp -t pants.native_engine.metadata.XXXXXX)
@@ -5,6 +5,8 @@ REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && cd ../../.. && pwd -P)"
# Exposes:
# + log: Log a message to the console.
# + fingerprint_data: Fingerprints the data on stdin.

# shellcheck source=build-support/common.sh
source "${REPO_ROOT}/build-support/common.sh"

rust_toolchain_root="${CACHE_ROOT}/rust"
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.