Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty Aptfile Fix #118

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
name: CI

on:
push:
branches: ["main"]
pull_request:

permissions:
contents: read

jobs:
shell-lint:
runs-on: ubuntu-22.04
container:
image: koalaman/shellcheck-alpine:v0.9.0
steps:
- uses: actions/checkout@v4
- name: shellcheck
run: shellcheck -x bin/compile bin/detect bin/release bin/report

functional-test:
runs-on: ubuntu-22.04
container:
image: heroku/heroku:${{ matrix.stack_number }}-build
strategy:
matrix:
stack_number: ["20", "22"]
env:
STACK: heroku-${{ matrix.stack_number }}
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Functional tests on heroku:${{ matrix.stack_number }}-build
run: test/run

2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Unreleased

- Warn when Aptfile contains no packages ([#118](https://github.com/heroku/heroku-buildpack-apt/pull/118))

## 2024-03-14

- Shell hardening ([#115](https://github.com/heroku/heroku-buildpack-apt/pull/115))
Expand Down
14 changes: 14 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
test: heroku-22-build heroku-20-build

shellcheck:
@shellcheck -x bin/compile bin/detect bin/release bin/report

heroku-22-build:
@echo "Running tests in docker (heroku-22-build)..."
@docker run -v $(shell pwd):/buildpack:ro --rm -it -e "STACK=heroku-22" heroku/heroku:22-build bash -c 'cp -r /buildpack /buildpack_test; cd /buildpack_test/; test/run;'
@echo ""

heroku-20-build:
@echo "Running tests in docker (heroku-20-build)..."
@docker run -v $(shell pwd):/buildpack:ro --rm -it -e "STACK=heroku-20" heroku/heroku:20-build bash -c 'cp -r /buildpack /buildpack_test; cd /buildpack_test/; test/run;'
@echo ""
22 changes: 17 additions & 5 deletions bin/compile
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,17 @@ function indent() {
esac
}

# exit early because we don't need to do any work if the aptfile has no packages
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're repeating these grep patterns a lot all over the place now.

I'd suggest doing this check a bit later, down before the for DEB in "$APT_CACHE_DIR/archives/"*.deb; do line, like this:

shopt -s nullglob
debs=( "$APT_CACHE_DIR/archives/"*.deb )

if ! (( ${#debs[@]} )); then
  // no packages warning and exit 0
fi

for DEB in "${debs[@]}"; do

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree. If there are no packages to install then we don't need to do any work including running apt-get update so I think the exit here makes more sense.

And that way, when packages are installed, the for DEB in "$APT_CACHE_DIR/archives/"*.deb line will not cause a failure so it can remain unchanged.

if ! grep --invert-match -e "^\s*#" -e "^\s*$" -e "^:repo:" -q "${BUILD_DIR}/Aptfile"; then
echo "
! You have no packages listed in your Aptfile. If you don't need custom Apt packages,
! delete your Aptfile and remove the buildpack with:
!
! $ heroku buildpacks:remove heroku-community/apt
"
exit 0
fi

# Store which STACK we are running on in the cache to bust the cache if it changes
if [[ -f "$CACHE_DIR/.apt/STACK" ]]; then
CACHED_STACK=$(cat "$CACHE_DIR/.apt/STACK")
Expand Down Expand Up @@ -69,7 +80,7 @@ else
# like>> :repo:deb http://cz.archive.ubuntu.com/ubuntu artful main universe
if grep -q -e "^:repo:" "$BUILD_DIR/Aptfile"; then
topic "Adding custom repositories"
cat "$BUILD_DIR/Aptfile" | grep -s -e "^:repo:" | sed 's/^:repo:\(.*\)\s*$/\1/g' >> "$APT_SOURCES"
grep -s -e "^:repo:" "$BUILD_DIR/Aptfile" | sed 's/^:repo:\(.*\)\s*$/\1/g' >> "$APT_SOURCES"
fi
fi

Expand All @@ -78,9 +89,9 @@ APT_OPTIONS=("-o" "debug::nolocking=true" "-o" "dir::cache=$APT_CACHE_DIR" "-o"
APT_OPTIONS+=("-o" "dir::etc::sourcelist=$APT_SOURCES" "-o" "dir::etc::sourceparts=/dev/null")

topic "Updating apt caches"
apt-get "${APT_OPTIONS[@]}" update | indent
apt-get "${APT_OPTIONS[@]}" update 2>&1 | indent

for PACKAGE in $(cat "$BUILD_DIR/Aptfile" | grep -v -s -e '^#' | grep -v -s -e "^:repo:"); do
while IFS= read -r PACKAGE; do
if [[ $PACKAGE == *deb ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we're cleaning things up, this should be == *.deb ;)

PACKAGE_NAME=$(basename "$PACKAGE" .deb)
PACKAGE_FILE=$APT_CACHE_DIR/archives/$PACKAGE_NAME.deb
Expand All @@ -89,9 +100,10 @@ for PACKAGE in $(cat "$BUILD_DIR/Aptfile" | grep -v -s -e '^#' | grep -v -s -e "
curl --silent --show-error --fail -L -z "$PACKAGE_FILE" -o "$PACKAGE_FILE" "$PACKAGE" 2>&1 | indent
else
topic "Fetching .debs for $PACKAGE"
apt-get "${APT_OPTIONS[@]}" -y "${APT_FORCE_YES[@]}" -d install --reinstall "$PACKAGE" | indent
IFS=$' \t' read -ra PACKAGE_NAMES <<< "$PACKAGE"
apt-get "${APT_OPTIONS[@]}" -y "${APT_FORCE_YES[@]}" -d install --reinstall "${PACKAGE_NAMES[@]}" | indent
fi
done
done < <(grep --invert-match -e "^\s*#" -e "^\s*$" -e "^:repo:" "${BUILD_DIR}/Aptfile")

mkdir -p "$BUILD_DIR/.apt"

Expand Down
2 changes: 1 addition & 1 deletion bin/report
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ while IFS= read -r line; do
packages+=("${package_name}")
done
fi
done < <(grep --invert-match -e "^#" -e "^\s*$" "${BUILD_DIR}/Aptfile")
done < <(grep --invert-match -e "^\s*#" -e "^\s*$" "${BUILD_DIR}/Aptfile")

output_key_value() {
local key value
Expand Down
59 changes: 0 additions & 59 deletions test/compile_test.sh

This file was deleted.

3 changes: 0 additions & 3 deletions test/fixtures/Aptfile/Aptfile

This file was deleted.

2 changes: 2 additions & 0 deletions test/fixtures/custom-package-url/Aptfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# only works on heroku-22 stack
https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb
1 change: 1 addition & 0 deletions test/fixtures/custom-repository-no-packages/Aptfile
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
:repo:deb http://us.archive.ubuntu.com/ubuntu/ jammy multiverse
3 changes: 3 additions & 0 deletions test/fixtures/custom-repository/Aptfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# only works on heroku-22 stack
:repo:deb http://us.archive.ubuntu.com/ubuntu/ jammy multiverse
fasttracker2
Empty file added test/fixtures/empty/Aptfile
Empty file.
4 changes: 4 additions & 0 deletions test/fixtures/only-comments/Aptfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# no packages
# only comments

# and whitespace
10 changes: 10 additions & 0 deletions test/fixtures/package-names/Aptfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# single package
xmlsec1

# comment with bad indent

# multiple packages on single line
s3cmd wget

# globbed package
mysql-client-*
153 changes: 153 additions & 0 deletions test/run
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
#!/usr/bin/env bash

testCompilePackageNames() {
compile "package-names"
assertCaptured "Updating apt caches"
assertCaptured "Fetching .debs for xmlsec1"
assertCaptured "Fetching .debs for s3cmd wget"
assertCaptured "Fetching .debs for mysql-client-*"
assertCaptured "Installing xmlsec1"
assertCaptured "Installing s3cmd"
assertCaptured "Installing wget"
assertCaptured "Installing mysql-client"
assertCaptured "Installing mysql-client-core"
assertCaptured "Writing profile script"
assertCaptured "Rewrite package-config files"
assertCapturedSuccess
}

testReportPackageNames() {
report "package-names"
assertCaptured "packages: \"mysql-client-*,s3cmd,wget,xmlsec1\""
assertNotCaptured "custom_packages"
assertNotCaptured "custom_repositories"
assertCapturedSuccess
}

testCompileCustomPackageUrl() {
if [[ "$STACK" == "heroku-22" ]]; then
compile "custom-package-url"
assertCaptured "Updating apt caches"
assertCaptured "Fetching https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb"
assertCaptured "Installing wkhtmltox"
assertCaptured "Writing profile script"
assertCaptured "Rewrite package-config files"
assertCapturedSuccess
fi
}

testReportCustomPackageUrl() {
report "custom-package-url"
assertNotCaptured "^packages"
assertCaptured "custom_packages: \"https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb\""
assertNotCaptured "custom_repositories"
assertCapturedSuccess
}

testCompileCustomRepository() {
if [[ "$STACK" == "heroku-22" ]]; then
compile "custom-repository"
assertCaptured "Adding custom repositories"
assertCaptured "Updating apt caches"
assertCaptured "http://us.archive.ubuntu.com/ubuntu jammy/multiverse amd64 Packages"
assertCaptured "Fetching .debs for fasttracker2"
assertCaptured "Installing fasttracker2"
assertCaptured "Writing profile script"
assertCaptured "Rewrite package-config files"
assertCapturedSuccess
fi
}

testReportCustomRepository() {
report "custom-repository"
assertCaptured "packages: \"fasttracker2\""
assertNotCaptured "custom_packages"
assertCaptured "custom_repositories: \"deb http://us.archive.ubuntu.com/ubuntu/ jammy multiverse\""
assertCapturedSuccess
}

testCompileEmpty() {
compile "empty"
assertCaptured "You have no packages listed in your Aptfile"
assertNotCaptured "Updating apt caches"
assertCapturedSuccess
}

testReportEmpty() {
report "empty"
assertNotCaptured "^packages"
assertNotCaptured "custom_packages"
assertNotCaptured "custom_repositories"
assertCapturedSuccess
}

testCompileOnlyComments() {
compile "only-comments"
assertCaptured "You have no packages listed in your Aptfile"
assertNotCaptured "Updating apt caches"
assertCapturedSuccess
}

testReportOnlyComments() {
report "only-comments"
assertNotCaptured "^packages"
assertNotCaptured "custom_packages"
assertNotCaptured "custom_repositories"
assertCapturedSuccess
}

testCompileCustomRepositoryNoPackages() {
compile "custom-repository-no-packages"
assertCaptured "You have no packages listed in your Aptfile"
assertNotCaptured "Updating apt caches"
assertCapturedSuccess
}

testReportCustomRepositoryNoPackages() {
report "custom-repository-no-packages"
assertNotCaptured "^packages"
assertNotCaptured "custom_packages"
assertCaptured "custom_repositories: \"deb http://us.archive.ubuntu.com/ubuntu/ jammy multiverse\""
assertCapturedSuccess
}

pushd "$(dirname 0)" >/dev/null || exit 1
popd >/dev/null || exit 1

source "$(pwd)"/test/utils

compile() {
default_process_types_cleanup
bp_dir=$(mktmpdir)
compile_dir=$(mktmpdir)
cp -a "$(pwd)"/* "${bp_dir}"
cp -a "${bp_dir}"/test/fixtures/"$1"/. "${compile_dir}"
capture "${bp_dir}"/bin/compile "${compile_dir}" "${2:-$(mktmpdir)}" "$3"
}

report() {
default_process_types_cleanup
compile_dir=${1:-$(mktmpdir)}
cache_dir=${2:-$(mktmpdir)}
env_dir=${3:-$(mktmpdir)}
bp_dir=$(mktmpdir)
cp -a "$(pwd)"/* "${bp_dir}"
cp -a "${bp_dir}"/test/fixtures/"$1"/. "${compile_dir}"
capture "${bp_dir}"/bin/report "${compile_dir}" "${cache_dir}" "${env_dir}"
}

mktmpdir() {
dir=$(mktemp -t testXXXXX)
rm -rf "$dir"
mkdir "$dir"
echo "$dir"
}

default_process_types_cleanup() {
file="/tmp/default_process_types"
if [ -f "$file" ]; then
rm "$file"
fi
}

source "$(pwd)"/test/shunit2
Loading