Skip to content

Commit 1f6857e

Browse files
edmorleyedmorley
authored
Log output and error handling improvements (#1689)
This backports a number of the package manager log output and error handling improvements from the new Poetry implementation to the existing pip and Pipenv implementations, along with a few other misc improvements to metrics and error handling. GUS-W-8059892. GUS-W-17166168.
1 parent eed2dc7 commit 1f6857e

17 files changed

+210
-100
lines changed

‎CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
## [Unreleased]
44

5+
- Updated the build log message shown when installing dependencies to include the package manager command being run. ([#1689](https://github.com/heroku/heroku-buildpack-python/pull/1689))
6+
- Improved the error messages and buildpack metrics for package manager related failures. ([#1689](https://github.com/heroku/heroku-buildpack-python/pull/1689))
7+
- Changed test dependency installation on Heroku CI to now install `requirements.txt` and `requirements-test.txt` in a single `pip install` invocation rather than separately. This allows pip's resolver to resolve any version conflicts between the two files. ([#1689](https://github.com/heroku/heroku-buildpack-python/pull/1689))
58

69
## [v265] - 2024-11-06
710

‎bin/compile

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@ GUNICORN_PROFILE_PATH="$BUILD_DIR/.profile.d/python.gunicorn.sh"
7878
WEB_CONCURRENCY_PROFILE_PATH="$BUILD_DIR/.profile.d/WEB_CONCURRENCY.sh"
7979
python_home='/app/.heroku/python'
8080

81+
# NB: Python must be added to PATH using the symlinked `/app` location and not its actual location
82+
# in BUILD_DIR, so that Python reports its location (via `sys.prefix`, `sys.executable` and others)
83+
# using `/app` paths which will still work at run-time after relocation. Amongst other things, this
84+
# ensures that the shebang lines in the entrypoint scripts of installed packages are correct.
8185
export PATH="/app/.heroku/python/bin:${PATH}"
8286
# Tell Python to not buffer it's stdin/stdout.
8387
export PYTHONUNBUFFERED=1

‎bin/steps/collectstatic

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ export PYTHONPATH
6464
COLLECTSTATIC_LOG=$(mktemp)
6565

6666
set +e
67-
python "$MANAGE_FILE" collectstatic --noinput --traceback 2>&1 | tee "$COLLECTSTATIC_LOG" | sed '/^Post-processed/d;/^Copying/d;/^$/d' | output::indent
67+
python "$MANAGE_FILE" collectstatic --noinput --traceback 2>&1 | tee "$COLLECTSTATIC_LOG" | sed --unbuffered '/^Post-processed/d;/^Copying/d;/^$/d' | output::indent
6868
COLLECTSTATIC_STATUS="${PIPESTATUS[0]}"
6969
set -e
7070

‎bin/utils

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,6 @@ shopt -s nullglob
88

99
source "${BUILDPACK_DIR:?}/vendor/buildpack-stdlib_v8.sh"
1010

11-
sed() { command sed -u "$@"; }
12-
13-
# Clean up pip output
14-
cleanup() {
15-
sed -e 's/\.\.\.\+/.../g' | sed -e '/already satisfied/Id' | sed -e '/No files were found to uninstall/Id' | sed -e '/Overwriting/Id' | sed -e '/python executable/Id' | sed -e '/no previously-included files/Id'
16-
}
17-
1811
# Does some serious copying.
1912
deep-cp() {
2013
declare source="$1" target="$2"

‎lib/package_manager.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,8 @@ function package_manager::determine_package_manager() {
7777
https://devcenter.heroku.com/articles/getting-started-with-python
7878
https://devcenter.heroku.com/articles/python-support
7979
EOF
80-
meta_set "failure_reason" "package-manager-not-found"
81-
return 1
80+
meta_set "failure_reason" "package-manager::none-found"
81+
exit 1
8282
;;
8383
*)
8484
# If multiple package managers are found, use the first found above.

‎lib/pip.sh

Lines changed: 58 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,30 @@ function pip::install_pip_setuptools_wheel() {
4646
# app's requirements.txt in the last build). The install will be a no-op if the versions match.
4747
output::step "Installing ${packages_display_text}"
4848

49-
/app/.heroku/python/bin/python "${bundled_pip_module_path}" install --quiet --disable-pip-version-check --no-cache-dir \
50-
"${packages_to_install[@]}"
49+
if ! {
50+
python "${bundled_pip_module_path}" \
51+
install \
52+
--disable-pip-version-check \
53+
--no-cache-dir \
54+
--no-input \
55+
--quiet \
56+
"${packages_to_install[@]}"
57+
}; then
58+
output::error <<-EOF
59+
Error: Unable to install pip.
60+
61+
Try building again to see if the error resolves itself.
62+
63+
If that does not help, check the status of PyPI (the Python
64+
package repository service), here:
65+
https://status.python.org
66+
EOF
67+
meta_set "failure_reason" "install-package-manager::pip"
68+
exit 1
69+
fi
5170
}
5271

5372
function pip::install_dependencies() {
54-
output::step "Installing requirements with pip"
55-
5673
# Make select pip config vars set on the Heroku app available to pip.
5774
# TODO: Expose all config vars (after suitable checks are added for unsafe env vars)
5875
# to allow for the env var interpolation feature of requirements files to work.
@@ -64,31 +81,49 @@ function pip::install_dependencies() {
6481
export PIP_EXTRA_INDEX_URL
6582
fi
6683

84+
local pip_install_command=(
85+
pip
86+
install
87+
)
88+
6789
# TODO: Deprecate/sunset this missing requirements file fallback.
6890
if [[ -f setup.py && ! -f requirements.txt ]]; then
69-
args=(--editable .)
91+
pip_install_command+=(--editable .)
7092
else
71-
args=(-r requirements.txt)
93+
pip_install_command+=(-r requirements.txt)
7294
fi
7395

74-
set +e
75-
# shellcheck disable=SC2154 # TODO: Env var is referenced but not assigned.
76-
/app/.heroku/python/bin/pip install "${args[@]}" --exists-action=w --src='/app/.heroku/src' --disable-pip-version-check --no-cache-dir --progress-bar off 2>&1 | tee "${WARNINGS_LOG}" | cleanup | output::indent
77-
local PIP_STATUS="${PIPESTATUS[0]}"
78-
set -e
79-
80-
show-warnings
81-
82-
if [[ ! ${PIP_STATUS} -eq 0 ]]; then
83-
meta_set "failure_reason" "pip-install"
84-
return 1
96+
# Install test dependencies too when the buildpack is invoked via `bin/test-compile` on Heroku CI.
97+
# We install both requirements files at the same time to allow pip to resolve version conflicts.
98+
if [[ -v INSTALL_TEST && -f requirements-test.txt ]]; then
99+
pip_install_command+=(-r requirements-test.txt)
85100
fi
86101

87-
# Install test dependencies, for Heroku CI.
88-
if [[ "${INSTALL_TEST:-0}" == "1" ]]; then
89-
if [[ -f requirements-test.txt ]]; then
90-
output::step "Installing test dependencies..."
91-
/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 | output::indent
92-
fi
102+
# We only display the most relevant command args here, to improve the signal to noise ratio.
103+
output::step "Installing dependencies using '${pip_install_command[*]}'"
104+
105+
# shellcheck disable=SC2310 # This function is invoked in an 'if' condition so set -e will be disabled.
106+
if ! {
107+
"${pip_install_command[@]}" \
108+
--disable-pip-version-check \
109+
--exists-action=w \
110+
--no-cache-dir \
111+
--no-input \
112+
--progress-bar off \
113+
--src='/app/.heroku/src' \
114+
|& tee "${WARNINGS_LOG:?}" \
115+
|& sed --unbuffered --expression '/Requirement already satisfied/d' \
116+
|& output::indent
117+
}; then
118+
# TODO: Overhaul warnings and combine them with error handling.
119+
show-warnings
120+
121+
output::error <<-EOF
122+
Error: Unable to install dependencies using pip.
123+
124+
See the log output above for more information.
125+
EOF
126+
meta_set "failure_reason" "install-dependencies::pip"
127+
exit 1
93128
fi
94129
}

‎lib/pipenv.sh

Lines changed: 59 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,34 @@ function pipenv::install_pipenv() {
1818
# TODO: Install Pipenv into a venv so it isn't leaked into the app environment.
1919
# TODO: Skip installing Pipenv if its version hasn't changed (once it's installed into a venv).
2020
# TODO: Explore viability of making Pipenv only be available during the build, to reduce slug size.
21-
/app/.heroku/python/bin/pip install --quiet --disable-pip-version-check --no-cache-dir "pipenv==${PIPENV_VERSION}"
21+
if ! {
22+
pip \
23+
install \
24+
--disable-pip-version-check \
25+
--no-cache-dir \
26+
--no-input \
27+
--quiet \
28+
"pipenv==${PIPENV_VERSION}"
29+
}; then
30+
output::error <<-EOF
31+
Error: Unable to install Pipenv.
32+
33+
Try building again to see if the error resolves itself.
34+
35+
If that does not help, check the status of PyPI (the Python
36+
package repository service), here:
37+
https://status.python.org
38+
EOF
39+
meta_set "failure_reason" "install-package-manager::pipenv"
40+
exit 1
41+
fi
2242
}
2343

2444
# Previous versions of the buildpack used to cache the checksum of the lockfile to allow
2545
# for skipping pipenv install if the lockfile was unchanged. However, this is not always safe
2646
# to do (the lockfile can refer to dependencies that can change independently of the lockfile,
2747
# for example, when using a local non-editable file dependency), so we no longer ever skip
28-
# install, and instead defer to pipenv to determine whether install is actually a no-op.
48+
# install, and instead defer to Pipenv to determine whether install is actually a no-op.
2949
function pipenv::install_dependencies() {
3050
# Make select pip config vars set on the Heroku app available to the pip used by Pipenv.
3151
# TODO: Expose all config vars (after suitable checks are added for unsafe env vars).
@@ -37,19 +57,44 @@ function pipenv::install_dependencies() {
3757
export PIP_EXTRA_INDEX_URL
3858
fi
3959

40-
# Install the test dependencies, for CI.
41-
# TODO: This is currently inconsistent with the non-test path, since it assumes (but doesn't check for) a lockfile.
42-
if [[ "${INSTALL_TEST:-0}" == "1" ]]; then
43-
output::step "Installing test dependencies with Pipenv"
44-
/app/.heroku/python/bin/pipenv install --dev --system --deploy --extra-pip-args='--src=/app/.heroku/src' 2>&1 | cleanup | output::indent
45-
46-
# Install the dependencies.
47-
elif [[ ! -f Pipfile.lock ]]; then
48-
output::step "Installing dependencies with Pipenv"
49-
/app/.heroku/python/bin/pipenv install --system --skip-lock --extra-pip-args='--src=/app/.heroku/src' 2>&1 | output::indent
60+
# Note: We can't use `pipenv sync` since it doesn't validate that the lockfile is up to date.
61+
local pipenv_install_command=(
62+
pipenv
63+
install
64+
)
5065

66+
# TODO: Make Pipfile.lock mandatory during package manager selection.
67+
if [[ ! -f Pipfile.lock ]]; then
68+
pipenv_install_command+=(--skip-lock)
5169
else
52-
output::step "Installing dependencies with Pipenv"
53-
/app/.heroku/python/bin/pipenv install --system --deploy --extra-pip-args='--src=/app/.heroku/src' 2>&1 | output::indent
70+
pipenv_install_command+=(--deploy)
71+
fi
72+
73+
# Install test dependencies too when the buildpack is invoked via `bin/test-compile` on Heroku CI.
74+
if [[ -v INSTALL_TEST ]]; then
75+
pipenv_install_command+=(--dev)
76+
fi
77+
78+
# We only display the most relevant command args here, to improve the signal to noise ratio.
79+
output::step "Installing dependencies using '${pipenv_install_command[*]}'"
80+
81+
# shellcheck disable=SC2310 # This function is invoked in an 'if' condition so set -e will be disabled.
82+
if ! {
83+
"${pipenv_install_command[@]}" \
84+
--extra-pip-args='--src=/app/.heroku/src' \
85+
--system \
86+
|& tee "${WARNINGS_LOG:?}" \
87+
|& output::indent
88+
}; then
89+
# TODO: Overhaul warnings and combine them with error handling.
90+
show-warnings
91+
92+
output::error <<-EOF
93+
Error: Unable to install dependencies using Pipenv.
94+
95+
See the log output above for more information.
96+
EOF
97+
meta_set "failure_reason" "install-dependencies::pipenv"
98+
exit 1
5499
fi
55100
}

‎lib/poetry.sh

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,18 @@ function poetry::install_poetry() {
4545
# are still using outdated patch releases of those Python versions, whose bundled pip
4646
# can be older (for example Python 3.9.0 ships with pip v20.2.1). Once Python 3.10 EOLs
4747
# we can switch back to the previous approach since Python 3.11.0 ships with pip v22.3.
48-
python -m venv "${poetry_venv_dir}"
48+
if ! python -m venv "${poetry_venv_dir}"; then
49+
output::error <<-EOF
50+
Internal Error: Unable to create virtual environment for Poetry.
51+
52+
The 'python -m venv' command to create a virtual environment did
53+
not exit successfully.
54+
55+
See the log output above for more information.
56+
EOF
57+
meta_set "failure_reason" "create-venv::poetry"
58+
exit 1
59+
fi
4960

5061
if ! {
5162
"${poetry_venv_dir}/bin/pip" \
@@ -65,8 +76,8 @@ function poetry::install_poetry() {
6576
package repository service), here:
6677
https://status.python.org
6778
EOF
68-
meta_set "failure_reason" "install-poetry"
69-
return 1
79+
meta_set "failure_reason" "install-package-manager::poetry"
80+
exit 1
7081
fi
7182

7283
mkdir -p "${poetry_bin_dir}"
@@ -112,11 +123,15 @@ function poetry::install_dependencies() {
112123
# codes for redrawing lines, which renders badly in persisted build logs.
113124
# shellcheck disable=SC2310 # This function is invoked in an 'if' condition so set -e will be disabled.
114125
if ! {
115-
"${poetry_install_command[@]}" --compile --no-ansi --no-interaction \
126+
"${poetry_install_command[@]}" \
127+
--compile \
128+
--no-ansi \
129+
--no-interaction \
116130
|& tee "${WARNINGS_LOG:?}" \
117-
|& grep --invert-match 'Skipping virtualenv creation' \
131+
|& sed --unbuffered --expression '/Skipping virtualenv creation/d' \
118132
|& output::indent
119133
}; then
134+
# TODO: Overhaul warnings and combine them with error handling.
120135
show-warnings
121136

122137
output::error <<-EOF
@@ -125,6 +140,6 @@ function poetry::install_dependencies() {
125140
See the log output above for more information.
126141
EOF
127142
meta_set "failure_reason" "install-dependencies::poetry"
128-
return 1
143+
exit 1
129144
fi
130145
}

‎lib/python_version.sh

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ function python_version::parse_runtime_txt() {
115115
python-${DEFAULT_PYTHON_MAJOR_VERSION}
116116
EOF
117117
meta_set "failure_reason" "runtime-txt::invalid-version"
118-
return 1
118+
exit 1
119119
fi
120120
}
121121

@@ -160,7 +160,7 @@ function python_version::parse_python_version_file() {
160160
${DEFAULT_PYTHON_MAJOR_VERSION}
161161
EOF
162162
meta_set "failure_reason" "python-version-file::invalid-version"
163-
return 1
163+
exit 1
164164
fi
165165
;;
166166
0)
@@ -176,7 +176,7 @@ function python_version::parse_python_version_file() {
176176
begin with a '#', otherwise it will be treated as a comment.
177177
EOF
178178
meta_set "failure_reason" "python-version-file::no-version"
179-
return 1
179+
exit 1
180180
;;
181181
*)
182182
output::error <<-EOF
@@ -196,7 +196,7 @@ function python_version::parse_python_version_file() {
196196
those lines with '#'.
197197
EOF
198198
meta_set "failure_reason" "python-version-file::multiple-versions"
199-
return 1
199+
exit 1
200200
;;
201201
esac
202202
}
@@ -228,7 +228,7 @@ function python_version::read_pipenv_python_version() {
228228
Run 'pipenv lock' to regenerate/fix the lockfile.
229229
EOF
230230
meta_set "failure_reason" "pipfile-lock::invalid-json"
231-
return 1
231+
exit 1
232232
fi
233233

234234
# Neither of the optional fields were found.
@@ -262,7 +262,7 @@ function python_version::read_pipenv_python_version() {
262262
https://pipenv.pypa.io/en/latest/specifiers.html#specifying-versions-of-python
263263
EOF
264264
meta_set "failure_reason" "pipfile-lock::invalid-version"
265-
return 1
265+
exit 1
266266
fi
267267
}
268268

@@ -319,7 +319,7 @@ function python_version::resolve_python_version() {
319319
EOF
320320
fi
321321
meta_set "failure_reason" "python-version::eol"
322-
return 1
322+
exit 1
323323
fi
324324

325325
if (((major == 3 && minor > 13) || major >= 4)); then
@@ -358,7 +358,7 @@ function python_version::resolve_python_version() {
358358
EOF
359359
fi
360360
meta_set "failure_reason" "python-version::unknown-major"
361-
return 1
361+
exit 1
362362
fi
363363

364364
# If an exact Python version was requested, there's nothing to resolve.

‎lib/utils.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@ function utils::bundled_pip_module_path() {
3131

3232
if [[ -z "${bundled_pip_wheel}" ]]; then
3333
output::error <<-'EOF'
34-
Error: Failed to locate the bundled pip wheel.
34+
Internal Error: Unable to locate the ensurepip pip wheel file.
3535
EOF
3636
meta_set "failure_reason" "bundled-pip-not-found"
37-
return 1
37+
exit 1
3838
fi
3939

4040
echo "${bundled_pip_wheel}/pip"

0 commit comments

Comments
 (0)