Skip to content

Install only missing packages across all common-utils package sets#1644

Merged
abdurriq merged 8 commits into
mainfrom
copilot/fix-github-actions-job-failure
May 13, 2026
Merged

Install only missing packages across all common-utils package sets#1644
abdurriq merged 8 commits into
mainfrom
copilot/fix-github-actions-job-failure

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 11, 2026

✨ Enhancement

What does this improve?
The previous change only applied “install if missing” behavior to bubblewrap and socat. This expands that behavior to the full distro-specific package sets in common-utils, so prebuilt images avoid redundant installs while still converging to the expected package baseline.

Why is this valuable?
It preserves idempotent behavior for images that already contain part (or most) of the toolchain, reduces unnecessary package operations, and keeps feature behavior consistent across Debian/Ubuntu, RHEL-family, and Alpine.

Implementation approach:

  • Debian/Ubuntu path
    • Build the full intended package list (including conditional entries).
    • Compute missing_package_list via dpkg-query.
    • Install only missing packages with apt-get.
  • RHEL-family path
    • Build the full intended package list (including conditional entries).
    • Compute missing_package_list via rpm -q.
    • Install only missing packages via the detected package manager.
  • Alpine path
    • Build the full intended package list (including conditional entries).
    • Compute missing_package_list via apk info -e.
    • Install only missing packages with apk add.
  • Safety/robustness
    • Parse package lists into arrays before iteration to avoid shell glob expansion issues (notably for bracketed package patterns).
read -r -a packages <<< "${package_list}"
for package in "${packages[@]}"; do
    if ! dpkg-query -W -f='${db:Status-Abbrev}' "${package}" 2>/dev/null | grep -q '^ii'; then
        missing_package_list="${missing_package_list} ${package}"
    fi
done

Copilot AI changed the title [WIP] Fix failing GitHub Actions job in common-utils fix(common-utils): install bubblewrap and socat regardless of PACKAGES_ALREADY_INSTALLED May 11, 2026
Copilot AI requested a review from abdurriq May 11, 2026 09:59
Copilot AI changed the title fix(common-utils): install bubblewrap and socat regardless of PACKAGES_ALREADY_INSTALLED common-utils: install bubblewrap/socat only when missing May 11, 2026
Copilot AI changed the title common-utils: install bubblewrap/socat only when missing Install only missing packages across all common-utils package sets May 11, 2026
@abdurriq abdurriq marked this pull request as ready for review May 12, 2026 16:40
@abdurriq abdurriq requested a review from a team as a code owner May 12, 2026 16:40
Copy link
Copy Markdown
Contributor

@chrmarti chrmarti left a comment

Choose a reason for hiding this comment

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

A few review notes on the per-package install change. The core direction looks great — these are mostly small follow-ups.

Dead variable (issue 2): With the per-package check now inlined, PACKAGES_ALREADY_INSTALLED no longer gates anything inside install_debian_packages / install_redhat_packages / install_alpine_packages (it's still loaded from the marker file in the main section and set at the end of all three functions, but the guards that used to read it are gone — see lines 166, 285, 367 in src/common-utils/main.sh). The variable is effectively dead within these functions. Worth a follow-up cleanup, either here or in a separate PR, to avoid confusion for future readers.

README/NOTES (issue 5): bubblewrap and socat aren't mentioned in src/common-utils/README.md or src/common-utils/NOTES.md. Since the feature advertises a curated set of common command-line utilities, it's probably worth listing them (and noting the bubblewrap dependency, since it's a less obvious addition than socat).

Comment thread src/common-utils/main.sh Outdated
Comment thread test/common-utils/test.sh
…ocs and tests

- Add \n to dpkg-query format string so glob patterns like libicu[0-9][0-9]
  that match multiple packages get each status on its own line
- Remove dead PACKAGES_ALREADY_INSTALLED variable from all three install
  functions and the marker file (per-package checks make it redundant)
- Document bubblewrap and socat in NOTES.md
- Add bubblewrap/socat checks to alpine, alma-9, fedora, and rocky-9 tests#
@abdurriq abdurriq force-pushed the copilot/fix-github-actions-job-failure branch from 8bab70c to f142c4f Compare May 13, 2026 11:06
@abdurriq abdurriq requested a review from chrmarti May 13, 2026 12:22
Copy link
Copy Markdown
Contributor

@chrmarti chrmarti left a comment

Choose a reason for hiding this comment

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

Thanks!

@abdurriq abdurriq merged commit 8c47157 into main May 13, 2026
15 checks passed
@abdurriq abdurriq deleted the copilot/fix-github-actions-job-failure branch May 13, 2026 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants