Skip to content

Commit ce7296c

Browse files
committed
Add a warning when multiple package managers are found
Currently the buildpack will use whichever package manager it finds first, if the files of multiple package managers are found. This occasionally results in support tickets where the user believes the build to not be installing dependencies correctly, when in fact they are adding dependencies to the wrong package manager file. As such, we want to make the presence of multiple package manager files an error, but first we should add a warning so it's not a surprise. (Plus with the recent Poetry support addition, there will be apps using the third party Poetry buildpack that have both a `poetry.lock` and the generated `requirements.txt` until they remove the third-party buildpack.) Towards #1691. GUS-W-17167927.
1 parent 1f6857e commit ce7296c

File tree

5 files changed

+90
-0
lines changed

5 files changed

+90
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
## [Unreleased]
44

5+
- Added a warning when the files for multiple package managers are found. In the future this warning will become an error. ([#1692](https://github.com/heroku/heroku-buildpack-python/pull/1692))
56
- 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))
67
- Improved the error messages and buildpack metrics for package manager related failures. ([#1689](https://github.com/heroku/heroku-buildpack-python/pull/1689))
78
- 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))

lib/output.sh

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
# however, it helps Shellcheck realise the options under which these functions will run.
55
set -euo pipefail
66

7+
ANSI_BLUE='\033[1;34m'
78
ANSI_RED='\033[1;31m'
89
ANSI_YELLOW='\033[1;33m'
910
ANSI_RESET='\033[0m'
@@ -28,6 +29,24 @@ function output::indent() {
2829
sed --unbuffered "s/^/ /"
2930
}
3031

32+
# Output a styled multi-line notice message to stderr.
33+
#
34+
# Usage:
35+
# ```
36+
# output::notice <<-EOF
37+
# Note: The note summary.
38+
#
39+
# Detailed description.
40+
# EOF
41+
# ```
42+
function output::notice() {
43+
echo >&2
44+
while IFS= read -r line; do
45+
echo -e "${ANSI_BLUE} ! ${line}${ANSI_RESET}" >&2
46+
done
47+
echo >&2
48+
}
49+
3150
# Output a styled multi-line warning message to stderr.
3251
#
3352
# Usage:

lib/package_manager.sh

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@ set -euo pipefail
77
function package_manager::determine_package_manager() {
88
local build_dir="${1}"
99
local package_managers_found=()
10+
local package_managers_found_display_text=()
1011

1112
if [[ -f "${build_dir}/Pipfile.lock" ]]; then
1213
package_managers_found+=(pipenv)
14+
package_managers_found_display_text+=("Pipfile.lock (Pipenv)")
1315
meta_set "pipenv_has_lockfile" "true"
1416
elif [[ -f "${build_dir}/Pipfile" ]]; then
1517
# TODO: Start requiring a Pipfile.lock and make this branch a "missing lockfile" error instead.
@@ -20,11 +22,13 @@ function package_manager::determine_package_manager() {
2022
We recommend you commit this into your repository.
2123
EOF
2224
package_managers_found+=(pipenv)
25+
package_managers_found_display_text+=("Pipfile (Pipenv)")
2326
meta_set "pipenv_has_lockfile" "false"
2427
fi
2528

2629
if [[ -f "${build_dir}/requirements.txt" ]]; then
2730
package_managers_found+=(pip)
31+
package_managers_found_display_text+=("requirements.txt (pip)")
2832
fi
2933

3034
# This must be after the requirements.txt check, so that the requirements.txt exported by
@@ -34,12 +38,14 @@ function package_manager::determine_package_manager() {
3438
# ordering here won't matter.
3539
if [[ -f "${build_dir}/poetry.lock" ]]; then
3640
package_managers_found+=(poetry)
41+
package_managers_found_display_text+=("poetry.lock (Poetry)")
3742
fi
3843

3944
# TODO: Deprecate/sunset this fallback, since using setup.py declared dependencies is
4045
# not a best practice, and we can only guess as to which package manager to use.
4146
if ((${#package_managers_found[@]} == 0)) && [[ -f "${build_dir}/setup.py" ]]; then
4247
package_managers_found+=(pip)
48+
package_managers_found_display_text+=("setup.py (pip)")
4349
meta_set "setup_py_only" "true"
4450
else
4551
meta_set "setup_py_only" "false"
@@ -87,6 +93,32 @@ function package_manager::determine_package_manager() {
8793
# aren't taking effect. (We'll need to wait until after Poetry support has landed,
8894
# and people have had a chance to migrate from the Poetry buildpack mentioned above.)
8995
echo "${package_managers_found[0]}"
96+
97+
output::warning <<-EOF
98+
Warning: Multiple Python package manager files were found.
99+
100+
Exactly one package manager file should be present in your app's
101+
source code, however, several were found:
102+
103+
$(printf -- "%s\n" "${package_managers_found_display_text[@]}")
104+
105+
For now, we will build your app using the first package manager
106+
listed above, however, in the future this warning will become
107+
an error.
108+
109+
Decide which package manager you want to use with your app, and
110+
then delete the file(s) and any config from the others.
111+
EOF
112+
113+
if [[ "${package_managers_found[*]}" == *"poetry"* ]]; then
114+
output::notice <<-EOF
115+
Note: We recently added support for the package manager Poetry.
116+
If you are using a third-party Poetry buildpack you must remove
117+
it, otherwise the requirements.txt file it generates will cause
118+
the warning above.
119+
EOF
120+
fi
121+
90122
meta_set "package_manager_multiple_found" "$(
91123
IFS=,
92124
echo "${package_managers_found[*]}"

spec/hatchet/pipenv_spec.rb

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,22 @@
346346
app.deploy do |app|
347347
expect(clean_output(app.output)).to match(Regexp.new(<<~REGEX))
348348
remote: -----> Python app detected
349+
remote:
350+
remote: ! Warning: Multiple Python package manager files were found.
351+
remote: !
352+
remote: ! Exactly one package manager file should be present in your app's
353+
remote: ! source code, however, several were found:
354+
remote: !
355+
remote: ! Pipfile.lock \\(Pipenv\\)
356+
remote: ! requirements.txt \\(pip\\)
357+
remote: !
358+
remote: ! For now, we will build your app using the first package manager
359+
remote: ! listed above, however, in the future this warning will become
360+
remote: ! an error.
361+
remote: !
362+
remote: ! Decide which package manager you want to use with your app, and
363+
remote: ! then delete the file\\(s\\) and any config from the others.
364+
remote:
349365
remote: -----> Using Python 3.12 specified in Pipfile.lock
350366
remote: -----> Installing Python #{LATEST_PYTHON_3_12}
351367
remote: -----> Installing pip #{PIP_VERSION}, setuptools #{SETUPTOOLS_VERSION} and wheel #{WHEEL_VERSION}

spec/hatchet/poetry_spec.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,28 @@
245245
app.deploy do |app|
246246
expect(clean_output(app.output)).to include(<<~OUTPUT)
247247
remote: -----> Python app detected
248+
remote:
249+
remote: ! Warning: Multiple Python package manager files were found.
250+
remote: !
251+
remote: ! Exactly one package manager file should be present in your app's
252+
remote: ! source code, however, several were found:
253+
remote: !
254+
remote: ! requirements.txt (pip)
255+
remote: ! poetry.lock (Poetry)
256+
remote: !
257+
remote: ! For now, we will build your app using the first package manager
258+
remote: ! listed above, however, in the future this warning will become
259+
remote: ! an error.
260+
remote: !
261+
remote: ! Decide which package manager you want to use with your app, and
262+
remote: ! then delete the file(s) and any config from the others.
263+
remote:
264+
remote:
265+
remote: ! Note: We recently added support for the package manager Poetry.
266+
remote: ! If you are using a third-party Poetry buildpack you must remove
267+
remote: ! it, otherwise the requirements.txt file it generates will cause
268+
remote: ! the warning above.
269+
remote:
248270
remote: -----> Using Python #{DEFAULT_PYTHON_MAJOR_VERSION} specified in .python-version
249271
remote: -----> Installing Python #{LATEST_PYTHON_3_12}
250272
remote: -----> Installing pip #{PIP_VERSION}, setuptools #{SETUPTOOLS_VERSION} and wheel #{WHEEL_VERSION}

0 commit comments

Comments
 (0)