diff --git a/.gitignore b/.gitignore index 1339cb4aa..e415b62c2 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,8 @@ __pycache__/ .hatchet/repos/ .venv/ +# The setup-ruby GitHub Action creates this directory when caching is enabled, so we ignore +# it here so it does not show up in the output of `git ls-files` for `make lint-scripts`. +vendor/bundle/ .DS_Store .rspec_status diff --git a/.shellcheckrc b/.shellcheckrc new file mode 100644 index 000000000..44e54caa5 --- /dev/null +++ b/.shellcheckrc @@ -0,0 +1,15 @@ +# Enable all checks, including the optional ones that are off by default. +enable=all + +# TODO: Triage and potentially enable more of these optional checks. + +# SC2154 (warning): var is referenced but not assigned. +disable=SC2154 +# SC2250 (style): Prefer putting braces around variable references even when not strictly required. +disable=SC2250 +# SC2310 (info): This function is invoked in an 'if' condition so set -e will be disabled. +disable=SC2310 +# SC2311 (info): Bash implicitly disabled set -e for this function invocation because it's inside a command substitution. +disable=SC2311 +# SC2312 (info): Consider invoking this command separately to avoid masking its return value +disable=SC2312 diff --git a/Makefile b/Makefile index bf2ac154e..19043d26f 100644 --- a/Makefile +++ b/Makefile @@ -9,15 +9,8 @@ STACK_IMAGE_TAG := heroku/$(subst -,:,$(STACK))-build lint: lint-scripts lint-ruby -# TODO: Enable scanning for files that are currently missed and/or restructure repo -# layout to make it more viable to use wildcards here, given: -# https://github.com/koalaman/shellcheck/issues/962 lint-scripts: - @shellcheck -x bin/compile bin/detect bin/release bin/test-compile bin/utils bin/warnings bin/default_pythons - @shellcheck -x bin/steps/collectstatic bin/steps/nltk bin/steps/pip-install bin/steps/pipenv bin/steps/pipenv-python-version bin/steps/python - @shellcheck -x bin/steps/hooks/* - @shellcheck -x vendor/WEB_CONCURRENCY.sh - @shellcheck -x builds/*.sh + @git ls-files -z --cached --others --exclude-standard 'bin/*' '*/bin/*' '*.sh' | xargs -0 shellcheck --check-sourced --color=always lint-ruby: @bundle exec rubocop diff --git a/bin/compile b/bin/compile index 5ffb15e05..453322464 100755 --- a/bin/compile +++ b/bin/compile @@ -8,7 +8,7 @@ set -eo pipefail export BPLOG_PREFIX="buildpack.python" export BUILDPACK_LOG_FILE=${BUILDPACK_LOG_FILE:-/dev/null} -[ "$BUILDPACK_XTRACE" ] && set -o xtrace +[[ -n "$BUILDPACK_XTRACE" ]] && set -o xtrace BUILD_DIR="${1}" CACHE_DIR="${2}" @@ -59,7 +59,7 @@ source "${BUILDPACK_DIR}/bin/warnings" mkdir -p /app/.heroku PROFILE_PATH="$BUILD_DIR/.profile.d/python.sh" -EXPORT_PATH="${BUILDPACK_DIR}/bin/../export" +EXPORT_PATH="${BUILDPACK_DIR}/export" GUNICORN_PROFILE_PATH="$BUILD_DIR/.profile.d/python.gunicorn.sh" WEB_CONCURRENCY_PROFILE_PATH="$BUILD_DIR/.profile.d/WEB_CONCURRENCY.sh" @@ -120,12 +120,12 @@ source "${BUILDPACK_DIR}/bin/steps/hooks/pre_compile" # Sticky runtimes. If there was a previous build, and it used a given version of Python, # continue to use that version of Python in perpetuity. -if [ -f "$CACHE_DIR/.heroku/python-version" ]; then +if [[ -f "$CACHE_DIR/.heroku/python-version" ]]; then CACHED_PYTHON_VERSION=$(cat "$CACHE_DIR/.heroku/python-version") fi # We didn't always record the stack version. This code is in place because of that. -if [ -f "$CACHE_DIR/.heroku/python-stack" ]; then +if [[ -f "$CACHE_DIR/.heroku/python-stack" ]]; then CACHED_PYTHON_STACK=$(cat "$CACHE_DIR/.heroku/python-stack") else CACHED_PYTHON_STACK=$STACK @@ -184,7 +184,7 @@ source "${BUILDPACK_DIR}/bin/steps/pipenv" # If no requirements.txt file given, assume `setup.py develop` is intended. # This allows for people to ship a setup.py application to Heroku -if [ ! -f requirements.txt ] && [ ! -f Pipfile ]; then +if [[ ! -f requirements.txt ]] && [[ ! -f Pipfile ]]; then echo "-e ." > requirements.txt fi diff --git a/bin/detect b/bin/detect index 118248a6c..97a6dd9a3 100755 --- a/bin/detect +++ b/bin/detect @@ -2,10 +2,12 @@ # Usage: bin/compile # See: https://devcenter.heroku.com/articles/buildpack-api -BUILD_DIR=$1 +set -euo pipefail + +BUILD_DIR="${1}" # Exit early if app is clearly not Python. -if [ ! -f "$BUILD_DIR/requirements.txt" ] && [ ! -f "$BUILD_DIR/setup.py" ] && [ ! -f "$BUILD_DIR/Pipfile" ]; then +if [[ ! -f "$BUILD_DIR/requirements.txt" ]] && [[ ! -f "$BUILD_DIR/setup.py" ]] && [[ ! -f "$BUILD_DIR/Pipfile" ]]; then exit 1 fi diff --git a/bin/steps/collectstatic b/bin/steps/collectstatic index 3f43adaa7..c0227d704 100755 --- a/bin/steps/collectstatic +++ b/bin/steps/collectstatic @@ -10,7 +10,9 @@ # - $DISABLE_COLLECTSTATIC: disables this functionality. # - $DEBUG_COLLECTSTATIC: upon failure, print out environment variables. -# This script is run in a subshell via sub_env so doesn't inherit the vars/utils from `bin/compile`. +# This script is run in a subshell via sub_env so doesn't inherit the options/vars/utils from `bin/compile`. +# TODO: Stop running this script in a subshell. +set -eo pipefail BUILDPACK_DIR=$(cd "$(dirname "$(dirname "$(dirname "${BASH_SOURCE[0]}")")")" && pwd) source "${BUILDPACK_DIR}/bin/utils" @@ -86,7 +88,7 @@ echo echo " https://devcenter.heroku.com/articles/django-assets" # Additionally, dump out the environment, if debug mode is on. -if [ "$DEBUG_COLLECTSTATIC" ]; then +if [[ -n "$DEBUG_COLLECTSTATIC" ]]; then echo echo "****** Collectstatic environment variables:" echo diff --git a/bin/steps/hooks/post_compile b/bin/steps/hooks/post_compile index 014f791c2..89ba2bbcd 100755 --- a/bin/steps/hooks/post_compile +++ b/bin/steps/hooks/post_compile @@ -1,6 +1,6 @@ #!/usr/bin/env bash -if [ -f bin/post_compile ]; then +if [[ -f bin/post_compile ]]; then echo "-----> Running post-compile hook" chmod +x bin/post_compile sub_env bin/post_compile diff --git a/bin/steps/hooks/pre_compile b/bin/steps/hooks/pre_compile index af788d9ac..fd00779f4 100755 --- a/bin/steps/hooks/pre_compile +++ b/bin/steps/hooks/pre_compile @@ -1,6 +1,6 @@ #!/usr/bin/env bash -if [ -f bin/pre_compile ]; then +if [[ -f bin/pre_compile ]]; then echo "-----> Running pre-compile hook" chmod +x bin/pre_compile sub_env bin/pre_compile diff --git a/bin/steps/nltk b/bin/steps/nltk index 27571ce61..718f3b25f 100755 --- a/bin/steps/nltk +++ b/bin/steps/nltk @@ -1,25 +1,22 @@ #!/usr/bin/env bash -# This script serves as the NLTK build step of the -# [**Python Buildpack**](https://github.com/heroku/heroku-buildpack-python) -# compiler. -# -# A [buildpack](https://devcenter.heroku.com/articles/buildpacks) is an -# adapter between a Python application and Heroku's runtime. -# -# This script is invoked by [`bin/compile`](/). - -# This script is run in a subshell via sub_env so doesn't inherit the vars/utils from `bin/compile`. +# This script is run in a subshell via sub_env so doesn't inherit the options/vars/utils from `bin/compile`. +# TODO: Stop running this script in a subshell. +set -eo pipefail BUILDPACK_DIR=$(cd "$(dirname "$(dirname "$(dirname "${BASH_SOURCE[0]}")")")" && pwd) source "${BUILDPACK_DIR}/bin/utils" +# These are required by `set_env`. +PROFILE_PATH="${BUILD_DIR}/.profile.d/python.sh" +EXPORT_PATH="${BUILDPACK_DIR}/export" + # Check that nltk was installed by pip, otherwise obviously not needed if is_module_available 'nltk'; then puts-step "Downloading NLTK corpora..." nltk_packages_definition="$BUILD_DIR/nltk.txt" - if [ -f "$nltk_packages_definition" ]; then + if [[ -f "$nltk_packages_definition" ]]; then readarray -t nltk_packages < "$nltk_packages_definition" puts-step "Downloading NLTK packages: ${nltk_packages[*]}" diff --git a/bin/steps/pip-install b/bin/steps/pip-install index 40707ab8e..3dbf32e21 100755 --- a/bin/steps/pip-install +++ b/bin/steps/pip-install @@ -1,6 +1,6 @@ #!/usr/bin/env bash -if [ ! "$SKIP_PIP_INSTALL" ]; then +if [[ -z "$SKIP_PIP_INSTALL" ]]; then puts-step "Installing requirements with pip" @@ -20,7 +20,7 @@ if [ ! "$SKIP_PIP_INSTALL" ]; then # Measure that we're using pip. mcount "tool.pip" - if [ ! -f "$BUILD_DIR/.heroku/python/bin/pip" ]; then + if [[ ! -f "$BUILD_DIR/.heroku/python/bin/pip" ]]; then exit 1 fi @@ -39,7 +39,7 @@ if [ ! "$SKIP_PIP_INSTALL" ]; then /app/.heroku/python/bin/pip freeze --disable-pip-version-check > .heroku/python/requirements-installed.txt # Install test dependencies, for CI. - if [ "$INSTALL_TEST" ]; then + if [[ -n "$INSTALL_TEST" ]]; then if [[ -f requirements-test.txt ]]; then puts-step "Installing test dependencies..." /app/.heroku/python/bin/pip install -r requirements-test.txt --exists-action=w --src='/app/.heroku/src' --disable-pip-version-check --no-cache-dir 2>&1 | cleanup | indent diff --git a/bin/steps/pipenv b/bin/steps/pipenv index 056657c0d..6b4435671 100755 --- a/bin/steps/pipenv +++ b/bin/steps/pipenv @@ -33,7 +33,7 @@ if [[ -f Pipfile ]]; then /app/.heroku/python/bin/pip install --quiet --disable-pip-version-check --no-cache-dir "pipenv==${PIPENV_VERSION}" # Install the test dependencies, for CI. - if [ "$INSTALL_TEST" ]; then + if [[ -n "$INSTALL_TEST" ]]; then puts-step "Installing test dependencies" /app/.heroku/python/bin/pipenv install --dev --system --deploy --extra-pip-args='--src=/app/.heroku/src' 2>&1 | cleanup | indent diff --git a/bin/steps/pipenv-python-version b/bin/steps/pipenv-python-version index 20a314330..9adba9002 100755 --- a/bin/steps/pipenv-python-version +++ b/bin/steps/pipenv-python-version @@ -52,6 +52,8 @@ if [[ -f $BUILD_DIR/Pipfile ]]; then 3.12) echo "${LATEST_312}" > "${BUILD_DIR}/runtime.txt" ;; + # TODO: Make this case an error + *) ;; esac fi diff --git a/bin/steps/python b/bin/steps/python index 52462c1ff..e0e5d69f1 100755 --- a/bin/steps/python +++ b/bin/steps/python @@ -33,6 +33,7 @@ case "${PYTHON_VERSION}" in python-3.6.+([0-9])) eol_python_version_error "3.6" "December 23rd, 2021" ;; + *) ;; esac # The Python runtime archive filename is of form: 'python-X.Y.Z-ubuntu-22.04-amd64.tar.zst' @@ -93,6 +94,8 @@ case "${PYTHON_VERSION}" in puts-warn warn_if_patch_update_available "${PYTHON_VERSION}" "${LATEST_38}" ;; + # TODO: Make this case an error, since it should be unreachable. + *) ;; esac mcount "version.python.${PYTHON_VERSION}" @@ -102,8 +105,8 @@ if [[ "$STACK" != "$CACHED_PYTHON_STACK" ]]; then rm -rf .heroku/python-stack .heroku/python-version .heroku/python .heroku/vendor .heroku/python .heroku/python-sqlite3-version fi -if [ -f .heroku/python-version ]; then - if [ ! "$(cat .heroku/python-version)" = "$PYTHON_VERSION" ]; then +if [[ -f .heroku/python-version ]]; then + if [[ ! "$(cat .heroku/python-version)" == "$PYTHON_VERSION" ]]; then puts-step "Python version has changed from $(cat .heroku/python-version) to ${PYTHON_VERSION}, clearing cache" rm -rf .heroku/python else diff --git a/bin/steps/sqlite3 b/bin/steps/sqlite3 index d22455fde..14e1c8ae0 100755 --- a/bin/steps/sqlite3 +++ b/bin/steps/sqlite3 @@ -15,16 +15,18 @@ sqlite3_install() { mkdir -p "$APT_CACHE_DIR/archives/partial" mkdir -p "$APT_STATE_DIR/lists/partial" - APT_OPTIONS="-o debug::nolocking=true" - APT_OPTIONS="$APT_OPTIONS -o dir::cache=$APT_CACHE_DIR" - APT_OPTIONS="$APT_OPTIONS -o dir::state=$APT_STATE_DIR" - APT_OPTIONS="$APT_OPTIONS -o dir::etc::sourcelist=/etc/apt/sources.list" - - apt-get $APT_OPTIONS update > /dev/null 2>&1 - if [ -z "$HEADERS_ONLY" ]; then - apt-get $APT_OPTIONS -y -d --reinstall install libsqlite3-dev sqlite3 > /dev/null 2>&1 + APT_OPTIONS=( + "--option=debug::nolocking=true" + "--option=dir::cache=${APT_CACHE_DIR}" + "--option=dir::state=${APT_STATE_DIR}" + "--option=dir::etc::sourcelist=/etc/apt/sources.list" + ) + + apt-get "${APT_OPTIONS[@]}" update > /dev/null 2>&1 + if [[ -z "$HEADERS_ONLY" ]]; then + apt-get "${APT_OPTIONS[@]}" -y -d --reinstall install libsqlite3-dev sqlite3 > /dev/null 2>&1 else - apt-get $APT_OPTIONS -y -d --reinstall install libsqlite3-dev + apt-get "${APT_OPTIONS[@]}" -y -d --reinstall install libsqlite3-dev fi find "$APT_CACHE_DIR/archives/" -name "*.deb" -exec dpkg -x {} "$HEROKU_PYTHON_DIR/sqlite3/" \; @@ -50,7 +52,7 @@ sqlite3_install() { # need to point the libsqlite3.so to the stack image library for /usr/bin/ld -lsqlite3 SQLITE3_LIBFILE="/usr/lib/${GNU_ARCH}-linux-gnu/$(readlink -n "$HEROKU_PYTHON_DIR/sqlite3/usr/lib/${GNU_ARCH}-linux-gnu/libsqlite3.so")" ln -s "$SQLITE3_LIBFILE" "$HEROKU_PYTHON_DIR/lib/libsqlite3.so" - if [ -z "$HEADERS_ONLY" ]; then + if [[ -z "$HEADERS_ONLY" ]]; then mv "$HEROKU_PYTHON_DIR/sqlite3/usr/bin"/* "$HEROKU_PYTHON_DIR/bin/" fi @@ -60,13 +62,6 @@ sqlite3_install() { } buildpack_sqlite3_install() { - HEROKU_PYTHON_DIR="$BUILD_DIR/.heroku/python" - - SQLITE3_VERSION_FILE="$BUILD_DIR/.heroku/python-sqlite3-version" - if [ -f "$SQLITE3_VERSION_FILE" ]; then - INSTALLED_SQLITE3_VERSION=$(cat "$SQLITE3_VERSION_FILE") - fi - puts-step "Installing SQLite3" if sqlite3_install "$BUILD_DIR/.heroku/python" ; then diff --git a/bin/utils b/bin/utils index 3d44fd54b..5ce22d866 100755 --- a/bin/utils +++ b/bin/utils @@ -7,7 +7,7 @@ shopt -s nullglob source "${BUILDPACK_DIR}/vendor/buildpack-stdlib_v8.sh" -if [ "$(uname)" == Darwin ]; then +if [[ "$(uname)" == "Darwin" ]]; then sed() { command sed -l "$@"; } else sed() { command sed -u "$@"; } @@ -52,7 +52,7 @@ deep-cp() { # Measure the size of the Python installation. measure-size() { - echo "$(du -s .heroku/python 2>/dev/null || echo 0) | awk '{print $1}')" + { du -s .heroku/python 2>/dev/null || echo 0; } | awk '{print $1}' } # Returns 0 if the specified module exists, otherwise returns 1. diff --git a/etc/publish.sh b/etc/publish.sh index dac63515b..c7801efe2 100755 --- a/etc/publish.sh +++ b/etc/publish.sh @@ -7,7 +7,7 @@ BP_NAME=${1:-"heroku/python"} curVersion=$(heroku buildpacks:versions "$BP_NAME" | awk 'FNR == 3 { print $1 }') newVersion="v$((curVersion + 1))" -read -p "Deploy as version: $newVersion [y/n]? " choice +read -r -p "Deploy as version: $newVersion [y/n]? " choice case "$choice" in y|Y ) echo "";; n|N ) exit 0;; @@ -18,7 +18,7 @@ git fetch origin originMain=$(git rev-parse origin/main) echo "Tagging commit $originMain with $newVersion... " git tag "$newVersion" "${originMain:?}" -git push origin refs/tags/$newVersion +git push origin "refs/tags/${newVersion}" echo -e "\nPublishing to the buildpack registry..." heroku buildpacks:publish "$BP_NAME" "$newVersion" diff --git a/spec/fixtures/hooks/bin/post_compile b/spec/fixtures/hooks/bin/post_compile index 6b3aaedc2..373d33889 100644 --- a/spec/fixtures/hooks/bin/post_compile +++ b/spec/fixtures/hooks/bin/post_compile @@ -1,3 +1,5 @@ +#!/usr/bin/env bash + set -euo pipefail echo 'post_compile ran with env vars:' diff --git a/spec/fixtures/hooks/bin/pre_compile b/spec/fixtures/hooks/bin/pre_compile index 5085363f3..40d1a402e 100644 --- a/spec/fixtures/hooks/bin/pre_compile +++ b/spec/fixtures/hooks/bin/pre_compile @@ -1,3 +1,5 @@ +#!/usr/bin/env bash + set -euo pipefail echo 'pre_compile ran with env vars:' diff --git a/spec/fixtures/pipenv_editable/bin/test-entrypoints b/spec/fixtures/pipenv_editable/bin/test-entrypoints index 139eedd87..4c27ba0f4 100755 --- a/spec/fixtures/pipenv_editable/bin/test-entrypoints +++ b/spec/fixtures/pipenv_editable/bin/test-entrypoints @@ -5,7 +5,7 @@ set -euo pipefail cd .heroku/python/lib/python*/site-packages/ # List any path like strings in .egg-link, .pth, and finder files in site-packages. -grep --extended-regexp --only-matching '/\S+' *.egg-link *.pth __editable___*_finder.py | sort +grep --extended-regexp --only-matching -- '/\S+' *.egg-link *.pth __editable___*_finder.py | sort echo echo -n "Running entrypoint for the pyproject.toml-based local package: " diff --git a/spec/fixtures/requirements_editable/bin/test-entrypoints b/spec/fixtures/requirements_editable/bin/test-entrypoints index 139eedd87..4c27ba0f4 100755 --- a/spec/fixtures/requirements_editable/bin/test-entrypoints +++ b/spec/fixtures/requirements_editable/bin/test-entrypoints @@ -5,7 +5,7 @@ set -euo pipefail cd .heroku/python/lib/python*/site-packages/ # List any path like strings in .egg-link, .pth, and finder files in site-packages. -grep --extended-regexp --only-matching '/\S+' *.egg-link *.pth __editable___*_finder.py | sort +grep --extended-regexp --only-matching -- '/\S+' *.egg-link *.pth __editable___*_finder.py | sort echo echo -n "Running entrypoint for the pyproject.toml-based local package: " diff --git a/spec/hatchet/nltk_spec.rb b/spec/hatchet/nltk_spec.rb index 5565085dc..0ed2a167f 100644 --- a/spec/hatchet/nltk_spec.rb +++ b/spec/hatchet/nltk_spec.rb @@ -19,6 +19,8 @@ remote: \\[nltk_data\\] /tmp/build_.*/.heroku/python/nltk_data... remote: \\[nltk_data\\] Unzipping corpora/stopwords.zip. REGEX + + # TODO: Add a test that the downloaded corpora can be found at runtime. end end end diff --git a/vendor/buildpack-stdlib_v8.sh b/vendor/buildpack-stdlib_v8.sh index ca7db3a97..e3c60ec90 100755 --- a/vendor/buildpack-stdlib_v8.sh +++ b/vendor/buildpack-stdlib_v8.sh @@ -92,7 +92,7 @@ un_set_env() { # Outputs a regex of default blacklist env vars. _env_blacklist() { local regex=${1:-''} - if [ -n "$regex" ]; then + if [[ -n "$regex" ]]; then regex="|$regex" fi echo "^(PATH|CPATH|CPPATH|LD_PRELOAD|LIBRARY_PATH|LD_LIBRARY_PATH|PYTHONHOME$regex)$" @@ -105,7 +105,7 @@ export_env() { local whitelist=${2:-''} local blacklist blacklist="$(_env_blacklist "$3")" - if [ -d "$env_dir" ]; then + if [[ -d "$env_dir" ]]; then # Environment variable names won't contain characters affected by: # shellcheck disable=SC2045 for e in $(ls "$env_dir"); do diff --git a/vendor/python.gunicorn.sh b/vendor/python.gunicorn.sh index 3a91e3ac1..2ac5e9e1e 100755 --- a/vendor/python.gunicorn.sh +++ b/vendor/python.gunicorn.sh @@ -1,3 +1,8 @@ +#!/usr/bin/env bash + +# Note: Since this is a .profile.d/ script it will be sourced, meaning that we cannot enable +# exit on error, have to use return not exit, and returning non-zero doesn't have an effect. + # Automatic configuration for Gunicorn's ForwardedAllowIPS setting. export FORWARDED_ALLOW_IPS='*'