From 641f947ac2d30cd7b37ba3b74d63d90049d02b17 Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Wed, 11 Oct 2023 16:07:11 +0100 Subject: [PATCH 1/3] fix: apt cache performance (#104) * fix: apt cache performance Use a single call to apt-cache to reduce the time needed to lookup package versions. Also: * Added millisecond details to log timing so slow operations can be more easily identified. * Perform apt update before determining package versions. Fixes #103 * chore: descriptive variable names and use log_err Added the review feedback, updating variable names to be more descriptive and using log_err where appropriate. --- install_and_cache_pkgs.sh | 38 +++++++++----------- lib.sh | 73 ++++++++++++++++++++++++++++++++------- pre_cache_action.sh | 21 +---------- 3 files changed, 78 insertions(+), 54 deletions(-) diff --git a/install_and_cache_pkgs.sh b/install_and_cache_pkgs.sh index bf12b74..f81dacd 100755 --- a/install_and_cache_pkgs.sh +++ b/install_and_cache_pkgs.sh @@ -18,28 +18,6 @@ cache_dir="${1}" # List of the packages to use. input_packages="${@:3}" -# Trim commas, excess spaces, sort, and version syntax. -# -# NOTE: Unless specified, all APT package listings of name and version use -# colon delimited and not equals delimited syntax (i.e. [:=]). -packages="$(get_normalized_package_list "${input_packages}")" - -package_count=$(wc -w <<< "${packages}") -log "Clean installing and caching ${package_count} package(s)." - -log_empty_line - -manifest_main="" -log "Package list:" -for package in ${packages}; do - read package_name package_ver < <(get_package_name_ver "${package}") - manifest_main="${manifest_main}${package_name}=${package_ver}," - log "- ${package_name} (${package_ver})" -done -write_manifest "main" "${manifest_main}" "${cache_dir}/manifest_main.log" - -log_empty_line - if ! apt-fast --version > /dev/null 2>&1; then log "Installing apt-fast for optimized installs..." # Install apt-fast for optimized installs. @@ -59,6 +37,22 @@ fi log_empty_line +packages="$(get_normalized_package_list "${input_packages}")" +package_count=$(wc -w <<< "${packages}") +log "Clean installing and caching ${package_count} package(s)." + +log_empty_line + +manifest_main="" +log "Package list:" +for package in ${packages}; do + manifest_main="${manifest_main}${package}," + log "- ${package}" +done +write_manifest "main" "${manifest_main}" "${cache_dir}/manifest_main.log" + +log_empty_line + # Strictly contains the requested packages. manifest_main="" # Contains all packages including dependencies. diff --git a/lib.sh b/lib.sh index 2144320..fd7764d 100755 --- a/lib.sh +++ b/lib.sh @@ -71,7 +71,7 @@ function get_installed_packages { ############################################################################### # Splits a fully action syntax APT package into the name and version. # Arguments: -# The action syntax colon delimited package pair or just the package name. +# The action syntax equals delimited package pair or just the package name. # Returns: # The package name and version pair. ############################################################################### @@ -81,7 +81,9 @@ function get_package_name_ver { IFS="${ORIG_IFS}" # If version not found in the fully qualified package value. if test -z "${ver}"; then - ver="$(grep "Version:" <<< "$(apt-cache show ${name})" | awk '{print $2}')" + # This is a fallback and should not be used any more as its slow. + log_err "Unexpected version resolution for package '${name}'" + ver="$(apt-cache show ${name} | grep '^Version:' | awk '{print $2}')" fi echo "${name}" "${ver}" } @@ -91,16 +93,63 @@ function get_package_name_ver { # Arguments: # The comma and/or space delimited list of packages. # Returns: -# Sorted list of space delimited packages. +# Sorted list of space delimited package name=version pairs. ############################################################################### function get_normalized_package_list { - # Remove commas, and block scalar folded backslashes. - local stripped=$(echo "${1}" | sed 's/[,\]/ /g') - # Remove extraneous spaces at the middle, beginning, and end. - local trimmed="$(\ - echo "${stripped}" \ - | sed 's/\s\+/ /g; s/^\s\+//g; s/\s\+$//g')" - echo ${trimmed} | tr ' ' '\n' | sort | tr '\n' ' ' + # Remove commas, and block scalar folded backslashes, + # extraneous spaces at the middle, beginning and end + # then sort. + packages=$(echo "${1}" \ + | sed 's/[,\]/ /g; s/\s\+/ /g; s/^\s\+//g; s/\s\+$//g' \ + | sort -t' ') + + # Validate package names and get versions. + log_err "resolving package versions..." + data=$(apt-cache --quiet=0 --no-all-versions show ${packages} 2>&1 | \ + grep -E '^(Package|Version|N):') + log_err "resolved" + + local ORIG_IFS="${IFS}" + IFS=$'\n' + declare -A missing + local package_versions='' + local package='' separator='' + for key_value in ${data}; do + local key="${key_value%%: *}" + local value="${key_value##*: }" + + case $key in + Package) + package=$value + ;; + Version) + package_versions="${package_versions}${separator}"${package}=${value}"" + separator=' ' + ;; + N) + # Warning messages. + case $value in + 'Unable to locate package '*) + package="${value#'Unable to locate package '}" + # Avoid duplicate messages. + if [ -z "${missing[$package]}" ]; then + package="${value#'Unable to locate package '}" + log_err "Package '${package}' not found." + missing[$package]=1 + fi + ;; + esac + ;; + esac + done + IFS="${ORIG_IFS}" + + if [ ${#missing[@]} -gt 0 ]; then + echo "aborted" + exit 5 + fi + + echo "${package_versions}" } ############################################################################### @@ -120,8 +169,8 @@ function get_tar_relpath { fi } -function log { echo "$(date +%H:%M:%S)" "${@}"; } -function log_err { >&2 echo "$(date +%H:%M:%S)" "${@}"; } +function log { echo "$(date +%T.%3N)" "${@}"; } +function log_err { >&2 echo "$(date +%T.%3N)" "${@}"; } function log_empty_line { echo ""; } diff --git a/pre_cache_action.sh b/pre_cache_action.sh index b487743..23a8fbb 100755 --- a/pre_cache_action.sh +++ b/pre_cache_action.sh @@ -53,35 +53,16 @@ log "done" log_empty_line -versioned_packages="" -log "Verifying packages..." -for package in ${packages}; do - if test ! "$(apt-cache show ${package})"; then - echo "aborted" - log "Package '${package}' not found." >&2 - exit 5 - fi - read package_name package_ver < <(get_package_name_ver "${package}") - versioned_packages=""${versioned_packages}" "${package_name}"="${package_ver}"" -done -log "done" - -log_empty_line - # Abort on any failure at this point. set -e log "Creating cache key..." -# TODO Can we prove this will happen again? -normalized_versioned_packages="$(get_normalized_package_list "${versioned_packages}")" -log "- Normalized package list is '${normalized_versioned_packages}'." - # Forces an update in cases where an accidental breaking change was introduced # and a global cache reset is required. force_update_inc="1" -value="${normalized_versioned_packages} @ ${version} ${force_update_inc}" +value="${packages} @ ${version} ${force_update_inc}" log "- Value to hash is '${value}'." key="$(echo "${value}" | md5sum | cut -f1 -d' ')" From 6f9e6a86db4bfb31baf4c7f71526175bc054252e Mon Sep 17 00:00:00 2001 From: Andrew Walsh Date: Wed, 11 Oct 2023 08:11:43 -0700 Subject: [PATCH 2/3] Update README.md --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index d51d75d..66a7372 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,9 @@ This action allows caching of Advanced Package Tool (APT) package dependencies to improve workflow execution time instead of installing the packages on every run. +> [!IMPORTANT] +> Looking for co-maintainers to help review changes, and investigate issues. I haven't had as much time to stay on top of this action as I would like to and want to make sure it is still responsive and reliable for the community. If you are interested, please reach out. + ## Documentation This action is a composition of [actions/cache](https://github.com/actions/cache/) and the `apt` utility. Some actions require additional APT based packages to be installed in order for other steps to be executed. Packages can be installed when ran but can consume much of the execution workflow time. From 44c33b32f808cdddd5ac0366d70595ed63661ed8 Mon Sep 17 00:00:00 2001 From: Andrew Walsh Date: Mon, 30 Oct 2023 11:12:50 -0700 Subject: [PATCH 3/3] Pull staging changes upstream. (#113) * Pull dev upstream to staging. (#112) * Use awk to enclose filename in single quotes tar #99 * Add null field separator so filenames don't get broken up. * Move upload logs up in the action sequence so it captures data before it gets deleted. * Fix awk (#109) --------- Co-authored-by: sn-o-w * Fix awk delimiter. Pull in fix by @sn-o-w in https://github.com/sn-o-w/cache-apt-pkgs-action/commit/d0ee83b497ac30023e51cd526c62e57b07501912 mentioned in issue #99 --------- Co-authored-by: sn-o-w --- action.yml | 14 +++++++------- install_and_cache_pkgs.sh | 3 ++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/action.yml b/action.yml index 035ace6..3e0dd2f 100644 --- a/action.yml +++ b/action.yml @@ -83,6 +83,13 @@ runs: DEBUG: "${{ inputs.debug }}" PACKAGES: "${{ inputs.packages }}" + - id: upload-logs + if: ${{ inputs.debug == 'true' }} + uses: actions/upload-artifact@v3 + with: + name: cache-apt-pkgs-logs_${{ env.CACHE_KEY }} + path: ~/cache-apt-pkgs/*.log + - id: save-cache if: ${{ ! steps.load-cache.outputs.cache-hit }} uses: actions/cache/save@v3 @@ -94,10 +101,3 @@ runs: run: | rm -rf ~/cache-apt-pkgs shell: bash - - - id: upload-logs - if: ${{ inputs.debug == 'true' }} - uses: actions/upload-artifact@v3 - with: - name: cache-apt-pkgs-logs_${{ env.CACHE_KEY }} - path: ~/cache-apt-pkgs/*.log diff --git a/install_and_cache_pkgs.sh b/install_and_cache_pkgs.sh index f81dacd..a35971f 100755 --- a/install_and_cache_pkgs.sh +++ b/install_and_cache_pkgs.sh @@ -92,7 +92,8 @@ for installed_package in ${installed_packages}; do & get_install_script_filepath "" "${package_name}" "preinst" \ & get_install_script_filepath "" "${package_name}" "postinst"; } | while IFS= read -r f; do test -f "${f}" -o -L "${f}" && get_tar_relpath "${f}"; done | - xargs -I {} echo \'{}\' | # Single quotes ensure literals like backslash get captured. + # Single quotes ensure literals like backslash get captured. Use \0 to avoid field separation. + awk -F"\0" '{print "\x27"$1"\x27"}' | sudo xargs tar -cf "${cache_filepath}" -C / log " done (compressed size $(du -h "${cache_filepath}" | cut -f1))."