From 24174d139b21c3f0331949208b66ce9117187227 Mon Sep 17 00:00:00 2001 From: Mahyar McDonald Date: Mon, 3 Nov 2025 18:52:54 -0800 Subject: [PATCH] clean up stuff --- README.md | 2 +- lib.sh | 22 +-- src/internal/common/apt.go | 23 +-- .../test_get_normalized_package_list.sh | 141 ------------------ 4 files changed, 12 insertions(+), 176 deletions(-) delete mode 100755 test_shell/test_get_normalized_package_list.sh diff --git a/README.md b/README.md index b1f3f0f..00ada5d 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ There are three kinds of version labels you can use. ### Inputs -- `packages` - Space delimited list of packages to install. If not provided, packages will be read from `Aptfile` at the repository root if it exists. Packages from both the input and `Aptfile` will be merged if both are provided. +- `packages` - Space delimited list of packages to install. If not provided, packages will be read from `Aptfile` at the repository root if it exists and `use_aptfile` is true. Packages from both the input and `Aptfile` will be merged if both are provided. - `version` - Version of cache to load. Each version will have its own cache. Note, all characters except spaces are allowed. - `execute_install_scripts` - Execute Debian package pre and post install script upon restore. See [Caveats / Non-file Dependencies](#non-file-dependencies) for more information. - `empty_packages_behavior` - Desired behavior when the given `packages` is empty. `'error'` (default), `'warn'` or `'ignore'`. diff --git a/lib.sh b/lib.sh index e82a478..601a303 100755 --- a/lib.sh +++ b/lib.sh @@ -115,25 +115,19 @@ function get_normalized_package_list { local architecture architecture=$(dpkg --print-architecture) local result - local temp_file - temp_file=$(mktemp) - + #IMPORTANT: we rely on a list style input to the apt_query binary with ${packages}, do remove this lint disable! if [ "${architecture}" == "arm64" ]; then - # shellcheck disable=SC2086 we rely on a list style input - "${script_dir}/apt_query-arm64" normalized-list ${packages} > "${temp_file}" 2>&1 + # shellcheck disable=SC2086 + result=$("${script_dir}/apt_query-arm64" normalized-list ${packages} 2>&1) else - # shellcheck disable=SC2086 we rely on a list style input - "${script_dir}/apt_query-x86" normalized-list ${packages} > "${temp_file}" 2>&1 + # shellcheck disable=SC2086 + result=$("${script_dir}/apt_query-x86" normalized-list ${packages} 2>&1) fi - local exit_code=$? - result=$(cat "${temp_file}") - rm -f "${temp_file}" - # Check if the command failed or if output looks like an error message - if [ ${exit_code} -ne 0 ] || [ -z "${result}" ] || echo "${result}" | grep -qiE "^exit status|^error|^fatal|^unable"; then - echo "apt_query failed with exit code ${exit_code}" >&2 + if [ -z "${result}" ] || echo "${result}" | grep -qiE "^exit status|^error|^fatal|^unable"; then + echo "apt_query failed" >&2 echo "Output: ${result}" >&2 # Return empty string to indicate failure echo "" @@ -142,7 +136,7 @@ function get_normalized_package_list { # Remove "Reverse=Provides: " prefix from strings if present local clean_result - clean_result=$(echo "${result}" | sed 's/Reverse=Provides: //g') + clean_result="${result//Reverse=Provides: /}" # Debug logging to stderr (won't interfere with return value captured via command substitution) if [[ "${-}" == *x* ]] || [ "${DEBUG:-${debug}}" = "true" ]; then diff --git a/src/internal/common/apt.go b/src/internal/common/apt.go index 1ccd41d..b497a66 100644 --- a/src/internal/common/apt.go +++ b/src/internal/common/apt.go @@ -33,10 +33,7 @@ func isErrLine(line string) bool { // Resolves virtual packages names to their concrete one. func getNonVirtualPackage(executor exec.Executor, name string) (pkg *AptPackage, err error) { - // Use grep with -A to get the line after "Reverse Provides", then filter out the header line itself - // The -v flag filters out lines containing "Reverse Provides" to exclude the header - // Then use tail -1 to get the last (first non-header) line, matching original behavior - execution := executor.Exec("bash", "-c", fmt.Sprintf("apt-cache showpkg %s | grep -A 1 \"Reverse Provides\" | grep -v \"Reverse Provides\" | tail -1", name)) + execution := executor.Exec("bash", "-c", fmt.Sprintf("apt-cache showpkg %s | grep -A 1 \"Reverse Provides\" | tail -1", name)) err = execution.Error() if err != nil { logging.Fatal(err) @@ -45,25 +42,11 @@ func getNonVirtualPackage(executor exec.Executor, name string) (pkg *AptPackage, if isErrLine(execution.CombinedOut) { return pkg, execution.Error() } - // Trim whitespace and handle the output - output := strings.TrimSpace(execution.CombinedOut) - if output == "" { - return pkg, fmt.Errorf("empty output from apt-cache showpkg for virtual package '%s'", name) - } - // Skip if the output is the header line "Reverse Provides:" itself (defensive check) - if strings.HasPrefix(output, "Reverse Provides") { - return pkg, fmt.Errorf("unable to find concrete package for virtual package '%s'. Output was header line: %s", name, output) - } - splitLine := GetSplitLine(output, " ", 3) + splitLine := GetSplitLine(execution.CombinedOut, " ", 3) if len(splitLine.Words) < 2 { return pkg, fmt.Errorf("unable to parse space delimited line's package name and version from apt-cache showpkg output below:\n%s", execution.CombinedOut) } - // Validate that we got a valid package name (not "Reverse" or "Provides") - packageName := splitLine.Words[0] - if packageName == "Reverse" || packageName == "Provides" || strings.HasPrefix(packageName, "Reverse") { - return pkg, fmt.Errorf("unable to parse valid package name from apt-cache showpkg output for virtual package '%s'. Output: %s", name, output) - } - return &AptPackage{Name: packageName, Version: splitLine.Words[1]}, nil + return &AptPackage{Name: splitLine.Words[0], Version: splitLine.Words[1]}, nil } func getPackage(executor exec.Executor, paragraph string) (pkg *AptPackage, err error) { diff --git a/test_shell/test_get_normalized_package_list.sh b/test_shell/test_get_normalized_package_list.sh deleted file mode 100755 index a8f9a72..0000000 --- a/test_shell/test_get_normalized_package_list.sh +++ /dev/null @@ -1,141 +0,0 @@ -#!/bin/bash - -# Test script for get_normalized_package_list function -# This test validates the function with a large package list -# On macOS, this will run via orbctl in a Ubuntu container - -set -e - -# Get the script directory (parent directory where lib.sh is located) -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" - -# Detect if we're on macOS and should use orbctl -if [[ "$(uname)" == "Darwin" ]]; then - # Check if orbctl is available - if ! command -v orbctl &> /dev/null; then - echo "โŒ ERROR: orbctl is not installed. Please install OrbStack from https://orbstack.dev/" - exit 1 - fi - - echo "๐Ÿณ Detected macOS - running test in Linux VM via orbctl" - echo "" - - # Get the absolute path and translate it for Linux - # orbctl automatically translates macOS paths, but we need to ensure it's absolute - ABS_SCRIPT_DIR="${SCRIPT_DIR}" - - # Run the test script inside the Linux VM - # orbctl automatically translates paths, so we can use the macOS path - orbctl run -w "${ABS_SCRIPT_DIR}" bash -c " - set -e - # Check if dpkg is available (should be on Ubuntu/Debian) - if ! command -v dpkg &> /dev/null; then - echo '๐Ÿ“ฆ Installing dpkg...' - sudo apt-get update -qq > /dev/null 2>&1 - sudo apt-get install -y -qq dpkg apt-utils > /dev/null 2>&1 - fi - - # Make the test script executable and run it - # The script will detect it's running in Linux (not macOS) and execute normally - chmod +x test_shell/test_get_normalized_package_list.sh - test_shell/test_get_normalized_package_list.sh - " - exit $? -fi - -# Test input: Large package list from user -TEST_INPUT="moreutils protobuf-compiler ripgrep libnss3-tools mkcert cmake autoconf git gh curl expect psmisc coreutils tmux moreutils util-linux mkcert gettext libsodium23 libsodium-dev postgresql-client redis-tools mysql-client awscli build-essential procps file pkg-config libssl-dev libffi-dev python3-dev python3-pip libkrb5-dev libx11-dev x11proto-core-dev libxkbfile-dev libpng-dev libjpeg-dev libwebp-dev git wget ca-certificates gnupg software-properties-common apt-transport-https ripgrep jq ruff" - -echo "โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”" -echo "๐Ÿงช Test: get_normalized_package_list" -echo "โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”" -echo "" - -echo "Input packages:" -echo "${TEST_INPUT}" -echo "" - -# Check if apt_query binaries exist -architecture=$(dpkg --print-architecture 2>/dev/null || echo "x86_64") -if [ "${architecture}" == "arm64" ]; then - APT_QUERY_BIN="${SCRIPT_DIR}/apt_query-arm64" -else - APT_QUERY_BIN="${SCRIPT_DIR}/apt_query-x86" -fi - -if [ ! -f "${APT_QUERY_BIN}" ]; then - echo "โŒ ERROR: apt_query binary not found at ${APT_QUERY_BIN}" - echo " Please ensure the binary exists in the project root." - exit 1 -fi - -if [ ! -x "${APT_QUERY_BIN}" ]; then - echo "โš ๏ธ WARNING: apt_query binary is not executable. Making it executable..." - chmod +x "${APT_QUERY_BIN}" -fi - -# Source lib.sh to get the function -# Note: The get_normalized_package_list function uses ${0} to find the apt_query binaries. -# Since ${0} will be this test script (in test_shell/), we override the function to use -# SCRIPT_DIR directly where the binaries are actually located. -source "${SCRIPT_DIR}/lib.sh" - -# Override get_normalized_package_list to use the correct script_dir -# This is necessary for testing since ${0} points to the test script, not lib.sh's location -get_normalized_package_list() { - local packages=$(echo "${1}" \ - | sed 's/[,\]/ /g; s/\s\+/ /g; s/^\s\+//g; s/\s\+$//g' \ - | sort -t' ') - local script_dir="${SCRIPT_DIR}" - - local architecture=$(dpkg --print-architecture) - if [ "${architecture}" == "arm64" ]; then - ${script_dir}/apt_query-arm64 normalized-list ${packages} - else - ${script_dir}/apt_query-x86 normalized-list ${packages} - fi -} - -# Call the function -echo "Calling get_normalized_package_list..." -result=$(get_normalized_package_list "${TEST_INPUT}") - -# Check if result is non-empty -if [ -z "${result}" ]; then - echo "โŒ ERROR: get_normalized_package_list returned empty output" - exit 1 -fi - -echo "โœ… Success: get_normalized_package_list returned output" -echo "" -echo "Normalized output:" -echo "${result}" -echo "" - -# Count packages in input vs output -input_count=$(echo "${TEST_INPUT}" | tr ' ' '\n' | sort -u | grep -v '^$' | wc -l) -output_count=$(echo "${result}" | tr ' ' '\n' | grep -v '^$' | wc -l) - -echo "Input package count (unique): ${input_count}" -echo "Output package count: ${output_count}" - -# Verify output format (should be space-delimited package=version pairs) -if echo "${result}" | grep -qvE '^[a-zA-Z0-9._+-]+=[a-zA-Z0-9.:~+-]+([[:space:]]+[a-zA-Z0-9._+-]+=[a-zA-Z0-9.:~+-]+)*$'; then - echo "โš ๏ธ WARNING: Output format may not match expected pattern (package=version pairs)" -else - echo "โœ… Output format validation passed" -fi - -# Check for duplicates in output (should be none) -duplicate_check=$(echo "${result}" | tr ' ' '\n' | sed 's/=.*$//' | sort | uniq -d) -if [ -n "${duplicate_check}" ]; then - echo "โš ๏ธ WARNING: Found duplicate packages in output:" - echo "${duplicate_check}" -else - echo "โœ… No duplicate packages found in output" -fi - -echo "" -echo "โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”" -echo "โœ… Test completed successfully" -echo "โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”"