diff --git a/CHANGELOG.md b/CHANGELOG.md index 05734c0b1..ffccb62c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [Unreleased] +- 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)) - 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)) - Improved the error messages and buildpack metrics for package manager related failures. ([#1689](https://github.com/heroku/heroku-buildpack-python/pull/1689)) - 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)) diff --git a/lib/output.sh b/lib/output.sh index a94074a1c..6893a5319 100644 --- a/lib/output.sh +++ b/lib/output.sh @@ -4,6 +4,7 @@ # however, it helps Shellcheck realise the options under which these functions will run. set -euo pipefail +ANSI_BLUE='\033[1;34m' ANSI_RED='\033[1;31m' ANSI_YELLOW='\033[1;33m' ANSI_RESET='\033[0m' @@ -28,6 +29,24 @@ function output::indent() { sed --unbuffered "s/^/ /" } +# Output a styled multi-line notice message to stderr. +# +# Usage: +# ``` +# output::notice <<-EOF +# Note: The note summary. +# +# Detailed description. +# EOF +# ``` +function output::notice() { + echo >&2 + while IFS= read -r line; do + echo -e "${ANSI_BLUE} ! ${line}${ANSI_RESET}" >&2 + done + echo >&2 +} + # Output a styled multi-line warning message to stderr. # # Usage: diff --git a/lib/package_manager.sh b/lib/package_manager.sh index c4c9350ff..c140542af 100644 --- a/lib/package_manager.sh +++ b/lib/package_manager.sh @@ -7,9 +7,11 @@ set -euo pipefail function package_manager::determine_package_manager() { local build_dir="${1}" local package_managers_found=() + local package_managers_found_display_text=() if [[ -f "${build_dir}/Pipfile.lock" ]]; then package_managers_found+=(pipenv) + package_managers_found_display_text+=("Pipfile.lock (Pipenv)") meta_set "pipenv_has_lockfile" "true" elif [[ -f "${build_dir}/Pipfile" ]]; then # 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() { We recommend you commit this into your repository. EOF package_managers_found+=(pipenv) + package_managers_found_display_text+=("Pipfile (Pipenv)") meta_set "pipenv_has_lockfile" "false" fi if [[ -f "${build_dir}/requirements.txt" ]]; then package_managers_found+=(pip) + package_managers_found_display_text+=("requirements.txt (pip)") fi # 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() { # ordering here won't matter. if [[ -f "${build_dir}/poetry.lock" ]]; then package_managers_found+=(poetry) + package_managers_found_display_text+=("poetry.lock (Poetry)") fi # TODO: Deprecate/sunset this fallback, since using setup.py declared dependencies is # not a best practice, and we can only guess as to which package manager to use. if ((${#package_managers_found[@]} == 0)) && [[ -f "${build_dir}/setup.py" ]]; then package_managers_found+=(pip) + package_managers_found_display_text+=("setup.py (pip)") meta_set "setup_py_only" "true" else meta_set "setup_py_only" "false" @@ -87,6 +93,32 @@ function package_manager::determine_package_manager() { # aren't taking effect. (We'll need to wait until after Poetry support has landed, # and people have had a chance to migrate from the Poetry buildpack mentioned above.) echo "${package_managers_found[0]}" + + output::warning <<-EOF + Warning: Multiple Python package manager files were found. + + Exactly one package manager file should be present in your app's + source code, however, several were found: + + $(printf -- "%s\n" "${package_managers_found_display_text[@]}") + + For now, we will build your app using the first package manager + listed above, however, in the future this warning will become + an error. + + Decide which package manager you want to use with your app, and + then delete the file(s) and any config from the others. + EOF + + if [[ "${package_managers_found[*]}" == *"poetry"* ]]; then + output::notice <<-EOF + Note: We recently added support for the package manager Poetry. + If you are using a third-party Poetry buildpack you must remove + it, otherwise the requirements.txt file it generates will cause + the warning above. + EOF + fi + meta_set "package_manager_multiple_found" "$( IFS=, echo "${package_managers_found[*]}" diff --git a/spec/hatchet/pipenv_spec.rb b/spec/hatchet/pipenv_spec.rb index befe00b72..494add751 100644 --- a/spec/hatchet/pipenv_spec.rb +++ b/spec/hatchet/pipenv_spec.rb @@ -346,6 +346,22 @@ app.deploy do |app| expect(clean_output(app.output)).to match(Regexp.new(<<~REGEX)) remote: -----> Python app detected + remote: + remote: ! Warning: Multiple Python package manager files were found. + remote: ! + remote: ! Exactly one package manager file should be present in your app's + remote: ! source code, however, several were found: + remote: ! + remote: ! Pipfile.lock \\(Pipenv\\) + remote: ! requirements.txt \\(pip\\) + remote: ! + remote: ! For now, we will build your app using the first package manager + remote: ! listed above, however, in the future this warning will become + remote: ! an error. + remote: ! + remote: ! Decide which package manager you want to use with your app, and + remote: ! then delete the file\\(s\\) and any config from the others. + remote: remote: -----> Using Python 3.12 specified in Pipfile.lock remote: -----> Installing Python #{LATEST_PYTHON_3_12} remote: -----> Installing pip #{PIP_VERSION}, setuptools #{SETUPTOOLS_VERSION} and wheel #{WHEEL_VERSION} diff --git a/spec/hatchet/poetry_spec.rb b/spec/hatchet/poetry_spec.rb index 69ddfc2e1..ae875027c 100644 --- a/spec/hatchet/poetry_spec.rb +++ b/spec/hatchet/poetry_spec.rb @@ -245,6 +245,28 @@ app.deploy do |app| expect(clean_output(app.output)).to include(<<~OUTPUT) remote: -----> Python app detected + remote: + remote: ! Warning: Multiple Python package manager files were found. + remote: ! + remote: ! Exactly one package manager file should be present in your app's + remote: ! source code, however, several were found: + remote: ! + remote: ! requirements.txt (pip) + remote: ! poetry.lock (Poetry) + remote: ! + remote: ! For now, we will build your app using the first package manager + remote: ! listed above, however, in the future this warning will become + remote: ! an error. + remote: ! + remote: ! Decide which package manager you want to use with your app, and + remote: ! then delete the file(s) and any config from the others. + remote: + remote: + remote: ! Note: We recently added support for the package manager Poetry. + remote: ! If you are using a third-party Poetry buildpack you must remove + remote: ! it, otherwise the requirements.txt file it generates will cause + remote: ! the warning above. + remote: remote: -----> Using Python #{DEFAULT_PYTHON_MAJOR_VERSION} specified in .python-version remote: -----> Installing Python #{LATEST_PYTHON_3_12} remote: -----> Installing pip #{PIP_VERSION}, setuptools #{SETUPTOOLS_VERSION} and wheel #{WHEEL_VERSION}