Improvements to cache invalidation (#133)

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:
heroku/heroku-buildpack-chrome-for-testing#21

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))

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

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
 }