Skip to content

Commit

Permalink
Add build environment validation checks
Browse files Browse the repository at this point in the history
- Adds a warning if the Python buildpack has been run multiple times
  in the same build.
- Adds a warning if an existing `.heroku/python/` directory is found
  in the app source.
- Improves the error message shown if the buildpack is used on an
  unsupported stack. Previously the build would fail with a curl
  download error depending on the availability of assets on S3.
  This scenario can only happen outside of Heroku, or in an old
  buildpack is used with a brand new stack (eg for Heroku-26).

The two warnings are the first step towards #1704 and #1710, and will
be turned into errors in January 2025.

Towards #1704 and #1710.
GUS-W-17386432.
GUS-W-17309709.
  • Loading branch information
edmorley committed Dec 13, 2024
1 parent dfd025b commit 6e5fca0
Show file tree
Hide file tree
Showing 6 changed files with 197 additions and 3 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## [Unreleased]

- Added a warning if the Python buildpack has been run multiple times in the same build. In January 2025 this warning will be made an error. ([#1724](https://github.com/heroku/heroku-buildpack-python/pull/1724))
- Added a warning if an existing `.heroku/python/` directory is found in the app source. In January 2025 this warning will be made an error. ([#1724](https://github.com/heroku/heroku-buildpack-python/pull/1724))
- Improved the error message shown if the buildpack is used on an unsupported stack. ([#1724](https://github.com/heroku/heroku-buildpack-python/pull/1724))
- Fixed Dev Center links to reflect recent article URL changes. ([#1723](https://github.com/heroku/heroku-buildpack-python/pull/1723))

## [v271] - 2024-12-12
Expand Down
14 changes: 12 additions & 2 deletions bin/compile
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ BUILDPACK_DIR=$(cd "$(dirname "$(dirname "${BASH_SOURCE[0]}")")" && pwd)
source "${BUILDPACK_DIR}/bin/utils"
source "${BUILDPACK_DIR}/lib/utils.sh"
source "${BUILDPACK_DIR}/lib/cache.sh"
source "${BUILDPACK_DIR}/lib/checks.sh"
source "${BUILDPACK_DIR}/lib/hooks.sh"
source "${BUILDPACK_DIR}/lib/metadata.sh"
source "${BUILDPACK_DIR}/lib/output.sh"
Expand All @@ -32,10 +33,15 @@ source "${BUILDPACK_DIR}/lib/poetry.sh"

compile_start_time=$(nowms)

# Initialise metadata store.
# Initialise the buildpack metadata store.
# This is used to track state across builds (for cache invalidation and messaging when build
# configuration changes) and also so that `bin/report` can generate the build report.
meta_init "${CACHE_DIR}" "python"
meta_setup

checks::ensure_supported_stack "${STACK:?Required env var STACK is not set}"
checks::warn_if_duplicate_python_buildpack "${BUILD_DIR}"

# Prepend proper path for old-school virtualenv hackery.
# This may not be necessary.
export PATH=:/usr/local/bin:$PATH
Expand Down Expand Up @@ -103,6 +109,10 @@ cd "$BUILD_DIR"
# Runs a `bin/pre_compile` script if found in the app source, allowing build customisation.
hooks::run_hook "pre_compile"

# This check must be after the pre_compile hook, so that we can check not only the original
# app source, but also that the hook hasn't written to '.heroku/python/' either.
checks::warn_if_existing_python_dir_present "${BUILD_DIR}"

package_manager="$(package_manager::determine_package_manager "${BUILD_DIR}")"
meta_set "package_manager" "${package_manager}"

Expand Down Expand Up @@ -135,7 +145,7 @@ python_major_version="${python_full_version%.*}"
meta_set "python_version" "${python_full_version}"
meta_set "python_version_major" "${python_major_version}"

cache::restore "${BUILD_DIR}" "${CACHE_DIR}" "${STACK:?}" "${cached_python_full_version}" "${python_full_version}" "${package_manager}"
cache::restore "${BUILD_DIR}" "${CACHE_DIR}" "${STACK}" "${cached_python_full_version}" "${python_full_version}" "${package_manager}"

# The directory for the .profile.d scripts.
mkdir -p "$(dirname "$PROFILE_PATH")"
Expand Down
2 changes: 2 additions & 0 deletions bin/report
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ ALL_OTHER_FIELDS=(
cache_save_duration
dependencies_install_duration
django_collectstatic_duration
duplicate_python_buildpack
existing_python_dir
nltk_downloader_duration
package_manager_install_duration
pipenv_has_lockfile
Expand Down
2 changes: 1 addition & 1 deletion lib/cache.sh
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ function cache::restore() {
mkdir -p "${build_dir}/.heroku"

# NB: For now this has to handle files already existing in build_dir since some apps accidentally
# run the Python buildpack twice. TODO: Add an explicit check/error for duplicate buildpacks.
# run the Python buildpack twice. TODO: Refactor this once duplicate buildpacks become an error.
# TODO: Investigate why errors are ignored and ideally stop doing so.
# TODO: Compare the performance of moving the directory vs copying files.
cp -R "${cache_dir}/.heroku/python" "${build_dir}/.heroku/" &>/dev/null || true
Expand Down
116 changes: 116 additions & 0 deletions lib/checks.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
#!/usr/bin/env bash

function checks::ensure_supported_stack() {
local stack="${1}"

case "${stack}" in
heroku-20 | heroku-22 | heroku-24)
return 0
;;
cedar* | heroku-16 | heroku-18)
# This error will only ever be seen on non-Heroku environments, since the
# Heroku build system rejects builds using EOL stacks.
output::error <<-EOF
Error: The '${stack}' stack is no longer supported.
This buildpack no longer supports the '${stack}' stack since it has
reached its end-of-life:
https://devcenter.heroku.com/articles/stack#stack-support-details-for-apps-using-classic-buildpacks
Upgrade to a newer stack to continue using this buildpack.
EOF
meta_set "failure_reason" "stack::eol"
exit 1
;;
*)
output::error <<-EOF
Error: The '${stack}' stack is not recognised.
This buildpack does not recognise or support the '${stack}' stack.
If '${stack}' is a valid stack, make sure that you are using the latest
version of this buildpack and have not pinned to an older release:
https://devcenter.heroku.com/articles/managing-buildpacks#view-your-buildpacks
https://devcenter.heroku.com/articles/managing-buildpacks#classic-buildpacks-references
EOF
meta_set "failure_reason" "stack::unknown"
exit 1
;;
esac
}

# TODO: Turn this into an error in January 2025.
function checks::warn_if_duplicate_python_buildpack() {
local build_dir="${1}"

# The check for the `PYTHONHOME` env var prevents this warning triggering in the case
# where the Python install was committed to the Git repo (which will be handled later).
# (The env var can only have come from the `export` file of an earlier buildpack,
# since app provided config vars haven't been exported to the environment here.)
if [[ -f "${build_dir}/.heroku/python/bin/python" && -v PYTHONHOME ]]; then
output::warning <<-EOF
Warning: The Python buildpack has already been run this build.
An existing Python installation was found in the build directory
from a buildpack run earlier in the build.
This normally means there are duplicate Python buildpacks set
on your on your app, which is not supported since it can cause
unexpected errors and slow down builds.
Check the buildpacks set on your app and remove any duplicate
Python buildpack entries:
https://devcenter.heroku.com/articles/managing-buildpacks#view-your-buildpacks
https://devcenter.heroku.com/articles/managing-buildpacks#remove-classic-buildpacks
If you have a use-case that requires duplicate buildpacks,
please comment on:
https://github.com/heroku/heroku-buildpack-python/issues/1704
In January 2025 this warning will be made an error.
EOF
meta_set "duplicate_python_buildpack" "true"
# shellcheck disable=SC2034 # This is used below until we make this check an error.
DUPLICATE_PYTHON_BUILDPACK=1
fi
}

# TODO: Turn this into an error in January 2025.
function checks::warn_if_existing_python_dir_present() {
local build_dir="${1}"

# Avoid warning twice in the case of duplicate buildpacks.
# TODO: Remove this once `warn_if_duplicate_python_buildpack` becomes an error.
if [[ -v DUPLICATE_PYTHON_BUILDPACK ]]; then
return 0
fi

# We use `-e` here to catch the case where `python` is a file rather than a directory.
if [[ -e "${build_dir}/.heroku/python" ]]; then
output::warning <<-EOF
Warning: Existing '.heroku/python/' directory found.
Your app's source code contains an existing directory named
'.heroku/python/', which is where the Python buildpack needs
to install its files. This existing directory contains:
$(find .heroku/python/ -maxdepth 2 || true)
Writing to internal locations used by the Python buildpack
is not supported and can cause unexpected errors.
If you have committed a '.heroku/python/' directory to your
Git repo, you must delete it or use a different location.
Otherwise, check that an earlier buildpack or 'bin/pre_compile'
hook has not created this directory.
If you have a use-case that requires writing to this location,
please comment on:
https://github.com/heroku/heroku-buildpack-python/issues/1704
In January 2025 this warning will be made an error.
EOF
meta_set "existing_python_dir" "true"
fi
}
63 changes: 63 additions & 0 deletions spec/hatchet/checks_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,76 @@
require_relative '../spec_helper'

RSpec.describe 'Buildpack validation checks' do
context 'when there are duplicate Python buildpacks set on the app' do
let(:buildpacks) { [:default, :default] }
let(:app) { Hatchet::Runner.new("spec/fixtures/python_#{DEFAULT_PYTHON_MAJOR_VERSION}", buildpacks:) }

it 'fails detection' do
app.deploy do |app|
expect(clean_output(app.output)).to include(<<~OUTPUT)
remote: -----> Python app detected
remote:
remote: ! Warning: The Python buildpack has already been run this build.
remote: !
remote: ! An existing Python installation was found in the build directory
remote: ! from a buildpack run earlier in the build.
remote: !
remote: ! This normally means there are duplicate Python buildpacks set
remote: ! on your on your app, which is not supported since it can cause
remote: ! unexpected errors and slow down builds.
remote: !
remote: ! Check the buildpacks set on your app and remove any duplicate
remote: ! Python buildpack entries:
remote: ! https://devcenter.heroku.com/articles/managing-buildpacks#view-your-buildpacks
remote: ! https://devcenter.heroku.com/articles/managing-buildpacks#remove-classic-buildpacks
remote: !
remote: ! If you have a use-case that requires duplicate buildpacks,
remote: ! please comment on:
remote: ! https://github.com/heroku/heroku-buildpack-python/issues/1704
remote: !
remote: ! In January 2025 this warning will be made an error.
remote:
remote: -----> Using Python #{DEFAULT_PYTHON_MAJOR_VERSION} specified in .python-version
remote: -----> Restoring cache
remote: -----> Using cached install of Python #{DEFAULT_PYTHON_FULL_VERSION}
OUTPUT
end
end
end

context 'when the app source contains a broken Python install' do
let(:app) { Hatchet::Runner.new('spec/fixtures/python_in_app_source', allow_failure: true) }

it 'fails detection' do
app.deploy do |app|
expect(clean_output(app.output)).to include(<<~OUTPUT)
remote: -----> Python app detected
remote:
remote: ! Warning: Existing '.heroku/python/' directory found.
remote: !
remote: ! Your app's source code contains an existing directory named
remote: ! '.heroku/python/', which is where the Python buildpack needs
remote: ! to install its files. This existing directory contains:
remote: !
remote: ! .heroku/python/
remote: ! .heroku/python/bin
remote: ! .heroku/python/bin/python
remote: !
remote: ! Writing to internal locations used by the Python buildpack
remote: ! is not supported and can cause unexpected errors.
remote: !
remote: ! If you have committed a '.heroku/python/' directory to your
remote: ! Git repo, you must delete it or use a different location.
remote: !
remote: ! Otherwise, check that an earlier buildpack or 'bin/pre_compile'
remote: ! hook has not created this directory.
remote: !
remote: ! If you have a use-case that requires writing to this location,
remote: ! please comment on:
remote: ! https://github.com/heroku/heroku-buildpack-python/issues/1704
remote: !
remote: ! In January 2025 this warning will be made an error.
remote:
remote: -----> Using Python #{DEFAULT_PYTHON_MAJOR_VERSION} specified in .python-version
remote: -----> Using cached install of Python #{DEFAULT_PYTHON_FULL_VERSION}
remote:
Expand Down

0 comments on commit 6e5fca0

Please sign in to comment.