Improve output utils for warnings, steps and indented output (#1666)

* Switches warnings to the same approach used for errors
  (which also adds colour)
* Switches to the new style utils for steps + indentation
* Removes usage of the `exec 1>&2` pattern since it breaks
  the output returned by Bash functions, amongst other issues.

GUS-W-17048951.

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [Unreleased]
 
+- Updated buildpack-generated warning messages to use colour and be more consistently formatted. ([#1666](https://github.com/heroku/heroku-buildpack-python/pull/1666))
 
 ## [v261] - 2024-10-14
 

bin/compile

@@ -142,13 +142,6 @@ fi
 package_manager="$(package_manager::determine_package_manager "${BUILD_DIR}")"
 meta_set "package_manager" "${package_manager}"
 
-# TODO: Move this warning to lib/package_manager.sh once `output::warning()` exists
-# (puts-warn outputs to stdout, which would break `determine_package_manager()` as is).
-# TODO: Adjust this warning to mention support for missing Pipfile.lock will be removed soon.
-if [[ "${package_manager}" == "pipenv" && ! -f "${BUILD_DIR}/Pipfile.lock" ]]; then
-	puts-warn "No 'Pipfile.lock' found! We recommend you commit this into your repository."
-fi
-
 # We use the Bash 4.3+ `nameref` feature to pass back multiple values from this function
 # without having to hardcode globals. See: https://stackoverflow.com/a/38997681
 python_version::read_requested_python_version "${BUILD_DIR}" "${package_manager}" "${cached_python_version}" requested_python_version python_version_origin
@@ -159,15 +152,15 @@ meta_set "python_version_reason" "${python_version_origin}"
 # TODO: Add runtime.txt deprecation warning.
 case "${python_version_origin}" in
 	default)
-		puts-step "No Python version was specified. Using the buildpack default: Python ${requested_python_version}"
+		output::step "No Python version was specified. Using the buildpack default: Python ${requested_python_version}"
 		echo "       To use a different version, see: https://devcenter.heroku.com/articles/python-runtimes"
 		;;
 	cached)
-		puts-step "No Python version was specified. Using the same version as the last build: Python ${requested_python_version}"
+		output::step "No Python version was specified. Using the same version as the last build: Python ${requested_python_version}"
 		echo "       To use a different version, see: https://devcenter.heroku.com/articles/python-runtimes"
 		;;
 	*)
-		puts-step "Using Python ${requested_python_version} specified in ${python_version_origin}"
+		output::step "Using Python ${requested_python_version} specified in ${python_version_origin}"
 		;;
 esac
 

bin/detect

@@ -45,7 +45,7 @@ done
 # Note: This error message intentionally doesn't list all of the filetypes above,
 # since during compile the build will still require a package manager file, so it
 # makes sense to describe the stricter requirements upfront.
-display_error <<EOF
+output::error <<EOF
 Error: Your app is configured to use the Python buildpack,
 but we couldn't find any supported Python project files.
 

bin/steps/collectstatic

@@ -16,20 +16,25 @@
 set -euo pipefail
 BUILDPACK_DIR=$(cd "$(dirname "$(dirname "$(dirname "${BASH_SOURCE[0]}")")")" && pwd)
 source "${BUILDPACK_DIR}/bin/utils"
+source "${BUILDPACK_DIR}/lib/output.sh"
 
 # Required for `meta_set`.
 source "${BUILDPACK_DIR}/lib/metadata.sh"
 meta_init "${CACHE_DIR:?}" "python"
 
 if [[ -f .heroku/collectstatic_disabled ]]; then
-	puts-step "Skipping Django collectstatic since the file '.heroku/collectstatic_disabled' exists."
-	puts-warn "This approach is deprecated, please set the env var DISABLE_COLLECTSTATIC=1 instead."
+	output::step "Skipping Django collectstatic since the file '.heroku/collectstatic_disabled' exists."
+	output::warning <<-'EOF'
+		Warning: The .heroku/collectstatic_disabled file is deprecated.
+
+		Please remove the file and set the env var DISABLE_COLLECTSTATIC=1 instead.
+	EOF
 	meta_set "django_collectstatic" "disabled-file"
 	exit 0
 fi
 
 if [[ "${DISABLE_COLLECTSTATIC:-0}" != "0" ]]; then
-	puts-step "Skipping Django collectstatic since the env var DISABLE_COLLECTSTATIC is set."
+	output::step "Skipping Django collectstatic since the env var DISABLE_COLLECTSTATIC is set."
 	meta_set "django_collectstatic" "disabled-env-var"
 	exit 0
 fi
@@ -45,21 +50,21 @@ MANAGE_FILE=$(find . -maxdepth 3 -type f -name 'manage.py' -printf '%d\t%P\n' |
 MANAGE_FILE=${MANAGE_FILE:-fakepath}
 
 if [[ ! -f "${MANAGE_FILE}" ]]; then
-	puts-step "Skipping Django collectstatic since no manage.py file found."
+	output::step "Skipping Django collectstatic since no manage.py file found."
 	meta_set "django_collectstatic" "skipped-no-manage-py"
 	exit 0
 fi
 
 meta_set "django_collectstatic" "enabled"
 
-puts-step "$ python $MANAGE_FILE collectstatic --noinput"
+output::step "$ python $MANAGE_FILE collectstatic --noinput"
 
 PYTHONPATH=${PYTHONPATH:-.}
 export PYTHONPATH
 COLLECTSTATIC_LOG=$(mktemp)
 
 set +e
-python "$MANAGE_FILE" collectstatic --noinput --traceback 2>&1 | tee "$COLLECTSTATIC_LOG" | sed '/^Post-processed/d;/^Copying/d;/^$/d' | indent
+python "$MANAGE_FILE" collectstatic --noinput --traceback 2>&1 | tee "$COLLECTSTATIC_LOG" | sed '/^Post-processed/d;/^Copying/d;/^$/d' | output::indent
 COLLECTSTATIC_STATUS="${PIPESTATUS[0]}"
 set -e
 
@@ -82,22 +87,28 @@ else
 	meta_set "failure_reason" "collectstatic-other"
 fi
 
-echo " !     Error while running '$ python $MANAGE_FILE collectstatic --noinput'."
-echo "       See traceback above for details."
-echo
-echo "       You may need to update application code to resolve this error."
-echo "       Or, you can disable collectstatic for this application:"
-echo
-echo "          $ heroku config:set DISABLE_COLLECTSTATIC=1"
-echo
-echo "       https://devcenter.heroku.com/articles/django-assets"
+output::error <<-EOF
+	Error: Unable to generate Django static files.
+
+	The 'python ${MANAGE_FILE} collectstatic --noinput' Django
+	management command to generate static files failed.
+
+	See the traceback above for details.
+
+	You may need to update application code to resolve this error.
+	Or, you can disable collectstatic for this application:
+
+	   $ heroku config:set DISABLE_COLLECTSTATIC=1
+
+	https://devcenter.heroku.com/articles/django-assets
+EOF
 
 # Additionally, dump out the environment, if debug mode is on.
 if [[ "${DEBUG_COLLECTSTATIC:-0}" == "1" ]]; then
 	echo
 	echo "****** Collectstatic environment variables:"
 	echo
-	env | indent
+	env | output::indent
 fi
 
 exit 1

bin/steps/nltk

@@ -6,6 +6,7 @@
 set -euo pipefail
 BUILDPACK_DIR=$(cd "$(dirname "$(dirname "$(dirname "${BASH_SOURCE[0]}")")")" && pwd)
 source "${BUILDPACK_DIR}/bin/utils"
+source "${BUILDPACK_DIR}/lib/output.sh"
 
 # Required for `meta_set`.
 source "${BUILDPACK_DIR}/lib/metadata.sh"
@@ -18,22 +19,22 @@ EXPORT_PATH="${BUILDPACK_DIR}/export"
 # Check that nltk was installed by pip, otherwise obviously not needed
 # shellcheck disable=SC2310 # TODO: This function is invoked in an 'if' condition so set -e will be disabled.
 if is_module_available 'nltk'; then
-	puts-step "Downloading NLTK corpora..."
+	output::step "Downloading NLTK corpora..."
 
 	nltk_packages_definition="$BUILD_DIR/nltk.txt"
 
 	if [[ -f "$nltk_packages_definition" ]]; then
 		meta_set "nltk_downloader" "enabled"
 
 		readarray -t nltk_packages <"$nltk_packages_definition"
-		puts-step "Downloading NLTK packages: ${nltk_packages[*]}"
+		output::step "Downloading NLTK packages: ${nltk_packages[*]}"
 
-		python -m nltk.downloader -d "$BUILD_DIR/.heroku/python/nltk_data" "${nltk_packages[@]}" | indent
+		python -m nltk.downloader -d "$BUILD_DIR/.heroku/python/nltk_data" "${nltk_packages[@]}" | output::indent
 		set_env NLTK_DATA "/app/.heroku/python/nltk_data"
 
 	else
 		meta_set "nltk_downloader" "skipped-no-nltk-file"
-		puts-warn "'nltk.txt' not found, not downloading any corpora"
-		puts-warn "Learn more: https://devcenter.heroku.com/articles/python-nltk"
+		echo "       'nltk.txt' not found, not downloading any corpora"
+		echo "       Learn more: https://devcenter.heroku.com/articles/python-nltk"
 	fi
 fi

bin/steps/python

@@ -17,7 +17,7 @@ PYTHON_URL="${S3_BASE_URL}/python-${python_full_version}-ubuntu-${UBUNTU_VERSION
 # 3. The user has pinned to an older buildpack version and the S3 bucket location or layout has changed since.
 # TODO: Update this message to be more specific once Python 3.8 support is dropped.
 if ! curl --output /dev/null --silent --head --fail --retry 3 --retry-connrefused --connect-timeout 10 "${PYTHON_URL}"; then
-	display_error <<-EOF
+	output::error <<-EOF
 		Error: Python ${python_full_version} isn't available for this stack (${STACK}).
 
 		For a list of the supported Python versions, see:
@@ -38,10 +38,12 @@ function warn_if_patch_update_available() {
 	# TODO: Update this message to suggest using the .python-version major version syntax to stay up to date,
 	# once runtime.txt is deprecated and sticky-versioning only pins to the major version.
 	if ((requested_patch_number < latest_patch_number)); then
-		puts-warn
-		puts-warn "A Python security update is available! Upgrade as soon as possible to: Python ${latest_patch_version}"
-		puts-warn "See: https://devcenter.heroku.com/articles/python-runtimes"
-		puts-warn
+		output::warning <<-EOF
+			Warning: A Python security update is available!
+
+			Upgrade as soon as possible to: Python ${latest_patch_version}
+			See: https://devcenter.heroku.com/articles/python-runtimes
+		EOF
 		meta_set "python_version_outdated" "true"
 	else
 		meta_set "python_version_outdated" "false"
@@ -52,29 +54,31 @@ function warn_if_patch_update_available() {
 # if there weren't any errors with the version to avoid adding noise to the error messages.
 # TODO: Move this into lib/ as part of the warnings refactor.
 if [[ "${python_major_version}" == "3.8" ]]; then
-	puts-warn
-	puts-warn "Python 3.8 will reach its upstream end-of-life in October 2024, at which"
-	puts-warn "point it will no longer receive security updates:"
-	puts-warn "https://devguide.python.org/versions/#supported-versions"
-	puts-warn
-	puts-warn "Support for Python 3.8 will be removed from this buildpack on December 4th, 2024."
-	puts-warn
-	puts-warn "Upgrade to a newer Python version as soon as possible to keep your app secure."
-	puts-warn "See: https://devcenter.heroku.com/articles/python-runtimes"
-	puts-warn
+	output::warning <<-'EOF'
+		Warning: Support for Python 3.8 is ending soon!
+
+		Python 3.8 will reach its upstream end-of-life in October 2024, at which
+		point it will no longer receive security updates:
+		https://devguide.python.org/versions/#supported-versions
+
+		Support for Python 3.8 will be removed from this buildpack on December 4th, 2024.
+
+		Upgrade to a newer Python version as soon as possible to keep your app secure.
+		See: https://devcenter.heroku.com/articles/python-runtimes
+	EOF
 fi
 
 warn_if_patch_update_available "${python_full_version}" "${python_major_version}"
 
 if [[ "$STACK" != "$CACHED_PYTHON_STACK" ]]; then
-	puts-step "Stack has changed from $CACHED_PYTHON_STACK to $STACK, clearing cache"
+	output::step "Stack has changed from $CACHED_PYTHON_STACK to $STACK, clearing cache"
 	rm -rf .heroku/python-stack .heroku/python-version .heroku/python .heroku/vendor .heroku/python .heroku/python-sqlite3-version
 fi
 
 # TODO: Clean this up as part of the cache refactor.
 if [[ -f .heroku/python-version ]]; then
 	if [[ "${cached_python_version}" != "${python_full_version}" ]]; then
-		puts-step "Python version has changed from ${cached_python_version} to ${python_full_version}, clearing cache"
+		output::step "Python version has changed from ${cached_python_version} to ${python_full_version}, clearing cache"
 		rm -rf .heroku/python
 	else
 		SKIP_INSTALL=1
@@ -92,30 +96,35 @@ if [[ -f "${BUILD_DIR}/requirements.txt" ]]; then
 	else
 		# IF there IS a cached directory, check for differences with the new one
 		if ! diff "$BUILD_DIR/requirements.txt" "$CACHE_DIR/.heroku/requirements.txt" &>/dev/null; then
-			puts-step "Requirements file has been changed, clearing cached dependencies"
+			output::step "Requirements file has been changed, clearing cached dependencies"
 			# if there are any differences, clear the Python cache
 			# Installing Python over again does not take noticably more time
 			cp -R "$BUILD_DIR/requirements.txt" "$CACHE_DIR/.heroku/requirements.txt"
 			rm -rf .heroku/python
 			unset SKIP_INSTALL
 		else
-			puts-step "No change in requirements detected, installing from cache"
+			output::step "No change in requirements detected, installing from cache"
 		fi
 	fi
 fi
 
 if [[ "${SKIP_INSTALL:-0}" == "1" ]]; then
-	puts-step "Using cached install of Python ${python_full_version}"
+	output::step "Using cached install of Python ${python_full_version}"
 else
-	puts-step "Installing Python ${python_full_version}"
+	output::step "Installing Python ${python_full_version}"
 
 	# Prepare destination directory.
 	mkdir -p .heroku/python
 
 	if ! curl --silent --show-error --fail --retry 3 --retry-connrefused --connect-timeout 10 "${PYTHON_URL}" | tar --zstd --extract --directory .heroku/python; then
 		# The Python version was confirmed to exist previously, so any failure here is due to
 		# a networking issue or archive/buildpack bug rather than the runtime not existing.
-		display_error "Error: Failed to download/install Python ${python_full_version}."
+		output::error <<-EOF
+			Error: Failed to download/install Python ${python_full_version}.
+
+			In some cases, this happens due to an unstable network connection.
+			Please try again and to see if the error resolves itself.
+		EOF
 		meta_set "failure_reason" "python-download"
 		exit 1
 	fi

bin/steps/sqlite3

@@ -63,7 +63,7 @@ sqlite3_install() {
 }
 
 buildpack_sqlite3_install() {
-	puts-step "Installing SQLite3"
+	output::step "Installing SQLite3"
 
 	# TODO: This never actual prints failure or even aborts the build, since
 	# the conditional disables `set -e` inside the called function:

bin/utils

@@ -10,26 +10,11 @@ source "${BUILDPACK_DIR:?}/vendor/buildpack-stdlib_v8.sh"
 
 sed() { command sed -u "$@"; }
 
-# Syntax sugar.
-indent() {
-	sed "s/^/       /"
-}
-
 # Clean up pip output
 cleanup() {
 	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'
 }
 
-# Buildpack Steps.
-puts-step() {
-	echo "-----> $*"
-}
-
-# Buildpack Warnings.
-puts-warn() {
-	echo " !     $*"
-}
-
 # Does some serious copying.
 deep-cp() {
 	declare source="$1" target="$2"

bin/warnings

@@ -3,10 +3,12 @@
 gdal-missing() {
 	# shellcheck disable=SC2154 # TODO: Env var is referenced but not assigned.
 	if grep -qi 'Could not find gdal-config' "${WARNINGS_LOG}"; then
-		echo
-		puts-warn "Hello! Package installation failed since the GDAL library was not found."
-		puts-warn "For GDAL, GEOS and PROJ support, use the Geo buildpack alongside the Python buildpack:"
-		puts-warn "https://github.com/heroku/heroku-geo-buildpack"
+		output::error <<-'EOF'
+			Error: Package installation failed since the GDAL library was not found.
+
+			For GDAL, GEOS and PROJ support, use the Geo buildpack alongside the Python buildpack:
+			https://github.com/heroku/heroku-geo-buildpack
+		EOF
 	fi
 }