From 59d347df11c26bc85865f205245494cc33811f87 Mon Sep 17 00:00:00 2001 From: Ed Morley <501702+edmorley@users.noreply.github.com> Date: Wed, 19 Jun 2024 15:36:41 +0100 Subject: [PATCH] Improvements to cache invalidation Now: - If the stack version file is missing, we don't assume the cached files belong to the current stack (since that could cause breakage), and instead invalidate the cache. (The cache actually offers little benefit in practice for this buildpack, so invalidating is cheap.) - The whole `${CACHE_DIR}/apt` directory is removed rather than only the `${CACHE_DIR}/apt/cache` directory. This ensures the APT indexes and other files from old stack versions are cleaned up too. - Some build output logs have been adjusted to be more accurate. - A test has been added for cache re-use, since it wasn't previously tested. These papercuts were noticed whilst working on: https://github.com/heroku/heroku-buildpack-chrome-for-testing/pull/21 --- CHANGELOG.md | 3 +++ bin/compile | 21 +++++++++++++-------- test/run | 38 ++++++++++++++++++++++++++++++++------ 3 files changed, 48 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6666e72..2da55f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- The cache is now correctly invalidated if the stack version of an existing cache cannot be determined. ([#133](https://github.com/heroku/heroku-buildpack-apt/pull/133)) +- When the cache is invalidated, all APT-related files are now removed, rather than only some of them. In particular, this means old APT package index files are cleaned up. ([#133](https://github.com/heroku/heroku-buildpack-apt/pull/133)) + ## 2024-03-28 - Warn when Aptfile contains no packages ([#126](https://github.com/heroku/heroku-buildpack-apt/pull/126)) diff --git a/bin/compile b/bin/compile index 711690a..ac34854 100755 --- a/bin/compile +++ b/bin/compile @@ -31,7 +31,7 @@ function indent() { 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, +! 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 @@ -40,15 +40,20 @@ if ! grep --invert-match -e "^\s*#" -e "^\s*$" -e "^:repo:" -q "${BUILD_DIR}/Apt 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") +# This file really should be inside "${CACHE_DIR}/apt" not "${CACHE_DIR}/.apt", however, this +# buildpack has used the wrong directory name for it for some time, so there are many APT-related +# forks that use this path, so for backwards compatibility we're not changing the path, so as to +# not affect apps that have multiple APT-using buildpacks active at the same time. +STACK_VERSION_FILE="${CACHE_DIR}/.apt/STACK" +if [[ -f "${STACK_VERSION_FILE}" ]]; then + CACHED_STACK=$(cat "${STACK_VERSION_FILE}") else - CACHED_STACK=$STACK + CACHED_STACK= fi # Ensure we store the STACK in the cache for next time. -mkdir -p "$CACHE_DIR/.apt" -echo "$STACK" > "$CACHE_DIR/.apt/STACK" +mkdir -p "${CACHE_DIR}/.apt" +echo "${STACK}" > "${STACK_VERSION_FILE}" APT_CACHE_DIR="$CACHE_DIR/apt/cache" APT_STATE_DIR="$CACHE_DIR/apt/state" @@ -70,7 +75,7 @@ if [[ -f "$APT_CACHE_DIR/Aptfile" ]] && cmp -s "$BUILD_DIR/Aptfile" "$APT_CACHE_ else # Aptfile changed or does not exist or STACK changed topic "Detected Aptfile or Stack changes, flushing cache" - rm -rf "$APT_CACHE_DIR" + rm -rf "${CACHE_DIR}/apt" mkdir -p "$APT_CACHE_DIR/archives/partial" mkdir -p "$APT_STATE_DIR/lists/partial" mkdir -p "$APT_SOURCELIST_DIR" # make dir for sources @@ -89,7 +94,7 @@ APT_OPTIONS=("-o" "debug::nolocking=true" "-o" "dir::cache=$APT_CACHE_DIR" "-o" # Override the use of /etc/apt/sources.list (sourcelist) and /etc/apt/sources.list.d/* (sourceparts). APT_OPTIONS+=("-o" "dir::etc::sourcelist=$APT_SOURCES" "-o" "dir::etc::sourceparts=$APT_SOURCEPARTS_DIR") -topic "Updating apt caches" +topic "Updating APT package index" apt-get "${APT_OPTIONS[@]}" update 2>&1 | indent while IFS= read -r PACKAGE; do diff --git a/test/run b/test/run index 243237e..83b62c2 100755 --- a/test/run +++ b/test/run @@ -2,7 +2,7 @@ testCompilePackageNames() { compile "package-names" - assertCaptured "Updating apt caches" + assertCaptured "Updating APT package index" assertCaptured "Fetching .debs for xmlsec1" assertCaptured "Fetching .debs for s3cmd wget" assertCaptured "Fetching .debs for mysql-client-*" @@ -16,6 +16,32 @@ testCompilePackageNames() { assertCapturedSuccess } +testCacheInvalidation() { + cache_dir=$(mktmpdir) + + # Cold cache + compile "package-names" "${cache_dir}" + assertCaptured "Detected Aptfile or Stack changes, flushing cache" + assertCapturedSuccess + + # Warm cache + compile "package-names" "${cache_dir}" + assertCaptured "Reusing cache" + assertCapturedSuccess + + # Cache invalidated on stack change + echo 'some-old-stack' > "${cache_dir}/.apt/STACK" + compile "package-names" "${cache_dir}" + assertCaptured "Detected Aptfile or Stack changes, flushing cache" + assertCapturedSuccess + + # Cache invalidated if stack version file missing (eg cache from old buildpack version) + rm "${cache_dir}/.apt/STACK" + compile "package-names" "${cache_dir}" + assertCaptured "Detected Aptfile or Stack changes, flushing cache" + assertCapturedSuccess +} + testReportPackageNames() { report "package-names" assertCaptured "packages: \"mysql-client-*,s3cmd,wget,xmlsec1\"" @@ -32,7 +58,7 @@ testCompileCustomPackageUrl() { [heroku-24]="https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb" ) compile "custom-package-url-$STACK" - assertCaptured "Updating apt caches" + assertCaptured "Updating APT package index" assertCaptured "Fetching ${download_urls[$STACK]}" assertCaptured "Installing wkhtmltox" assertCaptured "Writing profile script" @@ -62,7 +88,7 @@ testCompileCustomRepository() { ) compile "custom-repository-$STACK" assertCaptured "Adding custom repositories" - assertCaptured "Updating apt caches" + assertCaptured "Updating APT package index" assertCaptured "http://us.archive.ubuntu.com/ubuntu ${ubuntu_release_names[$STACK]}/multiverse amd64 Packages" assertCaptured "Fetching .debs for fasttracker2" assertCaptured "Installing fasttracker2" @@ -87,7 +113,7 @@ testReportCustomRepository() { testCompileEmpty() { compile "empty" assertCaptured "You have no packages listed in your Aptfile" - assertNotCaptured "Updating apt caches" + assertNotCaptured "Updating APT package index" assertCapturedSuccess } @@ -102,7 +128,7 @@ testReportEmpty() { testCompileOnlyComments() { compile "only-comments" assertCaptured "You have no packages listed in your Aptfile" - assertNotCaptured "Updating apt caches" + assertNotCaptured "Updating APT package index" assertCapturedSuccess } @@ -117,7 +143,7 @@ testReportOnlyComments() { testCompileCustomRepositoryNoPackages() { compile "custom-repository-no-packages" assertCaptured "You have no packages listed in your Aptfile" - assertNotCaptured "Updating apt caches" + assertNotCaptured "Updating APT package index" assertCapturedSuccess }