Skip to content

Commit

Permalink
Format shell scripts using shfmt (#1638)
Browse files Browse the repository at this point in the history
Shell scripts are now formatted using shfmt:
https://github.com/mvdan/sh/blob/master/cmd/shfmt/shfmt.1.scd

This is enforced in CI, and formatting can be applied locally
using a new `make format` target.

Notably, this also reformats the scripts to use tabs instead of spaces,
since tabs are sadly the only way to be able to properly indent bash
here documents (which is something the scripts here will be using a
lot of soon) without having to resort to mixed tabs and spaces
indentation in the same file. (Plus tabs is also the shfmt default style.)

GUS-W-16808198.
  • Loading branch information
edmorley authored Sep 23, 2024
1 parent b9f7f3f commit 3da3b3a
Show file tree
Hide file tree
Showing 26 changed files with 893 additions and 845 deletions.
35 changes: 35 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# https://editorconfig.org
root = true

[*]
charset = utf-8
end_of_line = lf
indent_style = space
insert_final_newline = true
trim_trailing_whitespace = true

[*.sh]
binary_next_line = true
# We sadly have to use tabs in shell scripts otherwise we can't indent here documents:
# https://www.gnu.org/software/bash/manual/html_node/Redirections.html#Here-Documents
indent_style = tab
shell_variant = bash
switch_case_indent = true

# Catches scripts that we can't give a .sh file extension, such as the Buildpack API scripts.
[**/bin/**]
binary_next_line = true
indent_style = tab
shell_variant = bash
switch_case_indent = true

[.hatchet/repos/**]
ignore = true

# The setup-ruby GitHub Action creates this directory when caching is enabled, and if
# its not ignored will cause false positives when running shfmt in the CI lint job.
[vendor/bundle/**]
ignore = true

[Makefile]
indent_style = tab
8 changes: 8 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ on:
permissions:
contents: read

env:
# Used by shfmt and more.
FORCE_COLOR: 1

jobs:
lint:
runs-on: ubuntu-24.04
Expand All @@ -23,6 +27,10 @@ jobs:
ruby-version: "3.3"
- name: Run ShellCheck
run: make lint-scripts
- name: Run shfmt
uses: docker://mvdan/shfmt:latest
with:
args: "--diff ."
- name: Run Rubocop
run: bundle exec rubocop

Expand Down
10 changes: 8 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,20 +1,26 @@
# These targets are not files
.PHONY: lint lint-scripts lint-ruby run publish
.PHONY: lint lint-scripts lint-ruby check-format format run publish

STACK ?= heroku-24
FIXTURE ?= spec/fixtures/python_version_unspecified

# Converts a stack name of `heroku-NN` to its build Docker image tag of `heroku/heroku:NN-build`.
STACK_IMAGE_TAG := heroku/$(subst -,:,$(STACK))-build

lint: lint-scripts lint-ruby
lint: lint-scripts check-format lint-ruby

lint-scripts:
@git ls-files -z --cached --others --exclude-standard 'bin/*' '*/bin/*' '*.sh' | xargs -0 shellcheck --check-sourced --color=always

lint-ruby:
@bundle exec rubocop

check-format:
@shfmt --diff .

format:
@shfmt --write --list .

run:
@echo "Running buildpack using: STACK=$(STACK) FIXTURE=$(FIXTURE)"
@docker run --rm -it -v $(PWD):/src:ro --tmpfs /app -e "HOME=/app" -e "STACK=$(STACK)" "$(STACK_IMAGE_TAG)" \
Expand Down
128 changes: 63 additions & 65 deletions bin/compile
Original file line number Diff line number Diff line change
Expand Up @@ -103,16 +103,16 @@ mkdir -p "$CACHE_DIR/.heroku"
mkdir -p .heroku

# The Python installation.
cp -R "$CACHE_DIR/.heroku/python" .heroku/ &> /dev/null || true
cp -R "$CACHE_DIR/.heroku/python" .heroku/ &>/dev/null || true
# A plain text file which contains the current stack being used (used for cache busting).
cp -R "$CACHE_DIR/.heroku/python-stack" .heroku/ &> /dev/null || true
cp -R "$CACHE_DIR/.heroku/python-stack" .heroku/ &>/dev/null || true
# A plain text file which contains the current python version being used (used for cache busting).
cp -R "$CACHE_DIR/.heroku/python-version" .heroku/ &> /dev/null || true
cp -R "$CACHE_DIR/.heroku/python-version" .heroku/ &>/dev/null || true
# A plain text file which contains the current sqlite3 version being used (used for cache busting).
cp -R "$CACHE_DIR/.heroku/python-sqlite3-version" .heroku/ &> /dev/null || true
cp -R "$CACHE_DIR/.heroku/python-sqlite3-version" .heroku/ &>/dev/null || true
# "editable" installations of code repositories, via pip or pipenv.
if [[ -d "$CACHE_DIR/.heroku/src" ]]; then
cp -R "$CACHE_DIR/.heroku/src" .heroku/ &> /dev/null || true
cp -R "$CACHE_DIR/.heroku/src" .heroku/ &>/dev/null || true
fi

# The pre_compile hook. Customers rely on this. Don't remove it.
Expand All @@ -124,47 +124,47 @@ source "${BUILDPACK_DIR}/bin/steps/hooks/pre_compile"
# Sticky runtimes. If there was a previous build, and it used a given version of Python,
# continue to use that version of Python in perpetuity.
if [[ -f "$CACHE_DIR/.heroku/python-version" ]]; then
CACHED_PYTHON_VERSION=$(cat "$CACHE_DIR/.heroku/python-version")
CACHED_PYTHON_VERSION=$(cat "$CACHE_DIR/.heroku/python-version")
fi

# We didn't always record the stack version. This code is in place because of that.
if [[ -f "$CACHE_DIR/.heroku/python-stack" ]]; then
CACHED_PYTHON_STACK=$(cat "$CACHE_DIR/.heroku/python-stack")
CACHED_PYTHON_STACK=$(cat "$CACHE_DIR/.heroku/python-stack")
else
CACHED_PYTHON_STACK=$STACK
CACHED_PYTHON_STACK=$STACK
fi

# TODO: Move this into a new package manager handling implementation when adding Poetry support.
# We intentionally don't mention `setup.py` here since it's being removed soon.
if [[ ! -f requirements.txt && ! -f Pipfile && ! -f setup.py ]]; then
puts-warn
puts-warn "Error: Couldn't find any supported Python package manager files."
puts-warn
puts-warn "A Python app on Heroku must have either a 'requirements.txt' or"
puts-warn "'Pipfile' package manager file in the root directory of its"
puts-warn "source code."
puts-warn
puts-warn "Currently the root directory of your app contains:"
puts-warn
# TODO: Overhaul logging helpers so they can handle prefixing multi-line strings, and switch to them.
# shellcheck disable=SC2012 # Using `ls` instead of `find` is absolutely fine for this use case.
ls -1 --indicator-style=slash "${BUILD_DIR}" | sed 's/^/ ! /'
puts-warn
puts-warn "If your app already has a package manager file, check that it:"
puts-warn
puts-warn "1. Is in the top level directory (not a subdirectory)."
puts-warn "2. Has the correct spelling (the filenames are case-sensitive)."
puts-warn "3. Isn't listed in '.gitignore' or '.slugignore'."
puts-warn
puts-warn "Otherwise, add a package manager file to your app. If your app has"
puts-warn "no dependencies, then create an empty 'requirements.txt' file."
puts-warn
puts-warn "For help with using Python on Heroku, see:"
puts-warn "https://devcenter.heroku.com/articles/getting-started-with-python"
puts-warn "https://devcenter.heroku.com/articles/python-support"
puts-warn
meta_set "failure_reason" "package-manager-not-found"
exit 1
puts-warn
puts-warn "Error: Couldn't find any supported Python package manager files."
puts-warn
puts-warn "A Python app on Heroku must have either a 'requirements.txt' or"
puts-warn "'Pipfile' package manager file in the root directory of its"
puts-warn "source code."
puts-warn
puts-warn "Currently the root directory of your app contains:"
puts-warn
# TODO: Overhaul logging helpers so they can handle prefixing multi-line strings, and switch to them.
# shellcheck disable=SC2012 # Using `ls` instead of `find` is absolutely fine for this use case.
ls -1 --indicator-style=slash "${BUILD_DIR}" | sed 's/^/ ! /'
puts-warn
puts-warn "If your app already has a package manager file, check that it:"
puts-warn
puts-warn "1. Is in the top level directory (not a subdirectory)."
puts-warn "2. Has the correct spelling (the filenames are case-sensitive)."
puts-warn "3. Isn't listed in '.gitignore' or '.slugignore'."
puts-warn
puts-warn "Otherwise, add a package manager file to your app. If your app has"
puts-warn "no dependencies, then create an empty 'requirements.txt' file."
puts-warn
puts-warn "For help with using Python on Heroku, see:"
puts-warn "https://devcenter.heroku.com/articles/getting-started-with-python"
puts-warn "https://devcenter.heroku.com/articles/python-support"
puts-warn
meta_set "failure_reason" "package-manager-not-found"
exit 1
fi

# Pipenv Python version support.
Expand All @@ -173,21 +173,21 @@ fi
source "${BUILDPACK_DIR}/bin/steps/pipenv-python-version"

if [[ -f runtime.txt ]]; then
# PYTHON_VERSION_SOURCE may have already been set by the pipenv-python-version step.
# TODO: Refactor this and stop pipenv-python-version using runtime.txt as an API.
PYTHON_VERSION_SOURCE=${PYTHON_VERSION_SOURCE:-"runtime.txt"}
puts-step "Using Python version specified in ${PYTHON_VERSION_SOURCE}"
meta_set "python_version_reason" "specified"
# PYTHON_VERSION_SOURCE may have already been set by the pipenv-python-version step.
# TODO: Refactor this and stop pipenv-python-version using runtime.txt as an API.
PYTHON_VERSION_SOURCE=${PYTHON_VERSION_SOURCE:-"runtime.txt"}
puts-step "Using Python version specified in ${PYTHON_VERSION_SOURCE}"
meta_set "python_version_reason" "specified"
elif [[ -n "${CACHED_PYTHON_VERSION:-}" ]]; then
puts-step "No Python version was specified. Using the same version as the last build: ${CACHED_PYTHON_VERSION}"
echo " To use a different version, see: https://devcenter.heroku.com/articles/python-runtimes"
meta_set "python_version_reason" "cached"
echo "${CACHED_PYTHON_VERSION}" > runtime.txt
puts-step "No Python version was specified. Using the same version as the last build: ${CACHED_PYTHON_VERSION}"
echo " To use a different version, see: https://devcenter.heroku.com/articles/python-runtimes"
meta_set "python_version_reason" "cached"
echo "${CACHED_PYTHON_VERSION}" >runtime.txt
else
puts-step "No Python version was specified. Using the buildpack default: ${DEFAULT_PYTHON_VERSION}"
echo " To use a different version, see: https://devcenter.heroku.com/articles/python-runtimes"
meta_set "python_version_reason" "default"
echo "${DEFAULT_PYTHON_VERSION}" > runtime.txt
puts-step "No Python version was specified. Using the buildpack default: ${DEFAULT_PYTHON_VERSION}"
echo " To use a different version, see: https://devcenter.heroku.com/articles/python-runtimes"
meta_set "python_version_reason" "default"
echo "${DEFAULT_PYTHON_VERSION}" >runtime.txt
fi

# Create the directory for .profile.d, if it doesn't exist.
Expand All @@ -201,11 +201,11 @@ mkdir -p /app/.heroku/src
# This is (hopefully obviously) because apps end up running from `/app` in production.
# Realpath is used to support use-cases where one of the locations is a symlink to the other.
if [[ "$(realpath "${BUILD_DIR}")" != "$(realpath /app)" ]]; then
# python expects to reside in /app, so set up symlinks
# we will not remove these later so subsequent buildpacks can still invoke it
ln -nsf "$BUILD_DIR/.heroku/python" /app/.heroku/python
ln -nsf "$BUILD_DIR/.heroku/vendor" /app/.heroku/vendor
# Note: .heroku/src is copied in later.
# python expects to reside in /app, so set up symlinks
# we will not remove these later so subsequent buildpacks can still invoke it
ln -nsf "$BUILD_DIR/.heroku/python" /app/.heroku/python
ln -nsf "$BUILD_DIR/.heroku/vendor" /app/.heroku/vendor
# Note: .heroku/src is copied in later.
fi

# Download / Install Python, from pre-build binaries available on Amazon S3.
Expand All @@ -221,10 +221,10 @@ source "${BUILDPACK_DIR}/bin/steps/pipenv"
# This allows for people to ship a setup.py application to Heroku

if [[ ! -f requirements.txt ]] && [[ ! -f Pipfile ]]; then
meta_set "setup_py_only" "true"
echo "-e ." > requirements.txt
meta_set "setup_py_only" "true"
echo "-e ." >requirements.txt
else
meta_set "setup_py_only" "false"
meta_set "setup_py_only" "false"
fi

# SQLite3 support.
Expand All @@ -251,11 +251,10 @@ meta_time "nltk_downloader_duration" "${nltk_downloader_start_time}"
# In CI, $BUILD_DIR is /app.
# Realpath is used to support use-cases where one of the locations is a symlink to the other.
if [[ "$(realpath "${BUILD_DIR}")" != "$(realpath /app)" ]]; then
rm -rf "$BUILD_DIR/.heroku/src"
deep-cp /app/.heroku/src "$BUILD_DIR/.heroku/src"
rm -rf "$BUILD_DIR/.heroku/src"
deep-cp /app/.heroku/src "$BUILD_DIR/.heroku/src"
fi


# Django collectstatic support.
# The buildpack automatically runs collectstatic for Django applications.
# This is the cause for the majority of build failures on the Python platform.
Expand All @@ -265,7 +264,6 @@ collectstatic_start_time=$(nowms)
sub_env "${BUILDPACK_DIR}/bin/steps/collectstatic"
meta_time "django_collectstatic_duration" "${collectstatic_start_time}"


# Programmatically create .profile.d script for application runtime environment variables.

# Set the PATH to include Python / pip / pipenv / etc.
Expand All @@ -286,7 +284,7 @@ set_default_env PYTHONPATH "\$HOME"

# Python expects to be in /app, if at runtime, it is not, set
# up symlinks… this can occur when the subdir buildpack is used.
cat <<EOT >> "$PROFILE_PATH"
cat <<EOT >>"$PROFILE_PATH"
if [[ \$HOME != "/app" ]]; then
mkdir -p /app/.heroku
ln -nsf "\$HOME/.heroku/python" /app/.heroku/python
Expand All @@ -298,7 +296,7 @@ EOT
# (such as `/tmp/build_<hash>`) back to `/app`. This is not done during the build itself, since later
# buildpacks still need the build time paths.
if [[ "${BUILD_DIR}" != "/app" ]]; then
cat <<EOT >> "$PROFILE_PATH"
cat <<EOT >>"$PROFILE_PATH"
find .heroku/python/lib/python*/site-packages/ -type f -and \( -name '*.egg-link' -or -name '*.pth' -or -name '__editable___*_finder.py' \) -exec sed -i -e 's#${BUILD_DIR}#/app#' {} \+
EOT
fi
Expand All @@ -320,9 +318,9 @@ rm -rf "$CACHE_DIR/.heroku/src"
mkdir -p "$CACHE_DIR/.heroku"
cp -R .heroku/python "$CACHE_DIR/.heroku/"
cp -R .heroku/python-version "$CACHE_DIR/.heroku/"
cp -R .heroku/python-stack "$CACHE_DIR/.heroku/" &> /dev/null || true
cp -R .heroku/python-stack "$CACHE_DIR/.heroku/" &>/dev/null || true
if [[ -d .heroku/src ]]; then
cp -R .heroku/src "$CACHE_DIR/.heroku/" &> /dev/null || true
cp -R .heroku/src "$CACHE_DIR/.heroku/" &>/dev/null || true
fi

meta_time "total_duration" "${compile_start_time}"
38 changes: 19 additions & 19 deletions bin/detect
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,27 @@ BUILD_DIR="${1}"
# so that Python projects that are missing some of the required files still pass detection,
# allowing us to show a helpful error message during the build phase.
KNOWN_PYTHON_PROJECT_FILES=(
.python-version
app.py
main.py
manage.py
pdm.lock
Pipfile
Pipfile.lock
poetry.lock
pyproject.toml
requirements.txt
runtime.txt
setup.cfg
setup.py
uv.lock
.python-version
app.py
main.py
manage.py
pdm.lock
Pipfile
Pipfile.lock
poetry.lock
pyproject.toml
requirements.txt
runtime.txt
setup.cfg
setup.py
uv.lock
)

for filename in "${KNOWN_PYTHON_PROJECT_FILES[@]}"; do
if [[ -f "${BUILD_DIR}/${filename}" ]]; then
echo "Python"
exit 0
fi
if [[ -f "${BUILD_DIR}/${filename}" ]]; then
echo "Python"
exit 0
fi
done

# Cytokine incorrectly indents the first line, so we have to leave it empty.
Expand All @@ -43,7 +43,7 @@ echo 1>&2
# since during compile the build will still require a package manager file, so it
# makes sense to describe the stricter requirements up front.
# TODO: Overhaul logging helpers so they can handle prefixing multi-line strings, and switch to them.
sed 's/^/ ! /' 1>&2 << EOF
sed 's/^/ ! /' 1>&2 <<EOF
Error: Your app is configured to use the Python buildpack,
but we couldn't find any supported Python project files.
Expand Down
10 changes: 5 additions & 5 deletions bin/release
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ set -u
source "${BUILDPACK_DIR}/bin/utils"

if [[ -f "${BUILD_DIR}/manage.py" ]] && is_module_available 'django' && is_module_available 'psycopg2'; then
cat <<EOF
---
addons:
- heroku-postgresql
EOF
cat <<-'EOF'
---
addons:
- heroku-postgresql
EOF
fi
Loading

0 comments on commit 3da3b3a

Please sign in to comment.