Skip to content

Commit

Permalink
Enable Bash exit on undefined variables (set -u) (#1646)
Browse files Browse the repository at this point in the history
Since it's a best practice that helps make bugs more visible.

Aside from the `DEBUG_COLLECTSTATIC` change, all of the
others only affect buildpack internals.

GUS-W-16864420.
  • Loading branch information
edmorley authored Sep 30, 2024
1 parent 92c9328 commit 4a264fc
Show file tree
Hide file tree
Showing 11 changed files with 14 additions and 18 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## [Unreleased]

- Fixed Django collectstatic debug output being shown if `DEBUG_COLLECTSTATIC` was set to `0` or the empty string. ([#1646](https://github.com/heroku/heroku-buildpack-python/pull/1646))
- Stopped adding a trailing `:` to `C_INCLUDE_PATH`, `CPLUS_INCLUDE_PATH`, `LIBRARY_PATH`, `LD_LIBRARY_PATH` and `PKG_CONFIG_PATH`. ([#1645](https://github.com/heroku/heroku-buildpack-python/pull/1645))
- Removed remnants of the unused `.heroku/vendor/` directory. ([#1644](https://github.com/heroku/heroku-buildpack-python/pull/1644))

Expand Down
7 changes: 5 additions & 2 deletions bin/compile
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@
# Usage: bin/compile <build-dir> <cache-dir> <env-dir>
# See: https://devcenter.heroku.com/articles/buildpack-api

set -eo pipefail
set -euo pipefail

[[ -n "$BUILDPACK_XTRACE" ]] && set -o xtrace
# Note: This can't be enabled via app config vars, since at this point they haven't been loaded from ENV_DIR.
if [[ "${BUILDPACK_XTRACE:-0}" == "1" ]]; then
set -o xtrace
fi

BUILD_DIR="${1}"
CACHE_DIR="${2}"
Expand Down
4 changes: 0 additions & 4 deletions bin/release
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,8 @@ BUILDPACK_DIR=$(cd "$(dirname "$(dirname "${BASH_SOURCE[0]}")")" && pwd)
# Unfortunately the build system doesn't source the `export` script before
# running `bin/release`, so we have to do so manually to ensure the buildpack
# Python is used by `is_module_available` instead of system Python.
# We also have to disable Bash error on undefined variables, since not all env
# vars used in the export script will be set by default (eg `LIBRARY_PATH`).
set +u
# shellcheck source=/dev/null
source "${BUILDPACK_DIR}/export"
set -u

source "${BUILDPACK_DIR}/bin/utils"

Expand Down
4 changes: 0 additions & 4 deletions bin/report
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,10 @@ BUILDPACK_DIR=$(cd "$(dirname "$(dirname "${BASH_SOURCE[0]}")")" && pwd)

# The build system doesn't source the `export` script before running this script, so we have to do
# so manually (if it exists) so that buildpack Python/pip can be found (if the build succeeded).
# We have to disable Bash error on undefined variables for now, since not all env vars used in the
# export script will be set by default (eg `LIBRARY_PATH`).
EXPORT_FILE="${BUILDPACK_DIR}/export"
if [[ -f "${EXPORT_FILE}" ]]; then
set +u
# shellcheck source=/dev/null
source "${EXPORT_FILE}"
set -u
fi

source "${BUILDPACK_DIR}/lib/metadata.sh"
Expand Down
4 changes: 2 additions & 2 deletions bin/steps/collectstatic
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

# 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
set -euo pipefail
BUILDPACK_DIR=$(cd "$(dirname "$(dirname "$(dirname "${BASH_SOURCE[0]}")")")" && pwd)
source "${BUILDPACK_DIR}/bin/utils"

Expand Down Expand Up @@ -92,7 +92,7 @@ echo
echo " https://devcenter.heroku.com/articles/django-assets"

# Additionally, dump out the environment, if debug mode is on.
if [[ -n "$DEBUG_COLLECTSTATIC" ]]; then
if [[ "${DEBUG_COLLECTSTATIC:-0}" == "1" ]]; then
echo
echo "****** Collectstatic environment variables:"
echo
Expand Down
2 changes: 1 addition & 1 deletion bin/steps/nltk
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

# 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
set -euo pipefail
BUILDPACK_DIR=$(cd "$(dirname "$(dirname "$(dirname "${BASH_SOURCE[0]}")")")" && pwd)
source "${BUILDPACK_DIR}/bin/utils"

Expand Down
2 changes: 1 addition & 1 deletion bin/steps/python
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ if [[ -f "${BUILD_DIR}/requirements.txt" ]]; then
fi
fi

if [[ -n "${SKIP_INSTALL}" ]]; then
if [[ "${SKIP_INSTALL:-0}" == "1" ]]; then
puts-step "Using cached install of ${PYTHON_VERSION}"
else
puts-step "Installing ${PYTHON_VERSION}"
Expand Down
2 changes: 1 addition & 1 deletion bin/steps/sqlite3
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

sqlite3_install() {
HEROKU_PYTHON_DIR="$1"
HEADERS_ONLY="$3"
HEADERS_ONLY="${3:-}"

mkdir -p "$HEROKU_PYTHON_DIR"

Expand Down
2 changes: 1 addition & 1 deletion lib/pip.sh
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function pip::install_dependencies() {
fi

# Install test dependencies, for Heroku CI.
if [[ -n "${INSTALL_TEST}" ]]; then
if [[ "${INSTALL_TEST:-0}" == "1" ]]; 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
Expand Down
2 changes: 1 addition & 1 deletion lib/pipenv.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ function pipenv::install_dependencies() {

# Install the test dependencies, for CI.
# TODO: This is currently inconsistent with the non-test path, since it assumes (but doesn't check for) a lockfile.
if [[ -n "${INSTALL_TEST}" ]]; then
if [[ "${INSTALL_TEST:-0}" == "1" ]]; then
puts-step "Installing test dependencies with Pipenv"
/app/.heroku/python/bin/pipenv install --dev --system --deploy --extra-pip-args='--src=/app/.heroku/src' 2>&1 | cleanup | indent

Expand Down
2 changes: 1 addition & 1 deletion vendor/buildpack-stdlib_v8.sh
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ sub_env() {
(
# TODO: Fix https://github.com/heroku/buildpack-stdlib/issues/37
# shellcheck disable=SC2153
export_env "$ENV_DIR" "$WHITELIST" "$BLACKLIST"
export_env "$ENV_DIR" "${WHITELIST:-}" "${BLACKLIST:-}"

"$@"
)
Expand Down

0 comments on commit 4a264fc

Please sign in to comment.