Skip to content

docs(debian): annotate apt package roles in base image - #374

Open
wdconinc wants to merge 2 commits into
masterfrom
wdconinc-document-debian-apt-packages
Open

docs(debian): annotate apt package roles in base image#374
wdconinc wants to merge 2 commits into
masterfrom
wdconinc-document-debian-apt-packages

Conversation

@wdconinc

Copy link
Copy Markdown
Contributor

This documents why the Debian base image installs these packages with apt instead of leaving the question to Spack-managed environments. Reviewers should be able to see which packages are OS setup, Spack prerequisites, or deliberate system externals at a glance.

The Dockerfile now feeds the apt package lists through a commented heredoc using sed and xargs, so each package can carry an inline justification without changing the installed package set. The same pattern is used for the base tools block and the Spack runtime dependency block.

Notable details:

  • keeps the installed package set equivalent
  • shortens the justifications to the minimum useful rationale
  • calls out the GL development packages as system externals to avoid bundling Mesa/LLVM swrast in Spack

Validation:

  • docker buildx build -f containers/debian/Dockerfile containers/debian

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 31, 2026 00:11
@wdconinc
wdconinc enabled auto-merge (squash) July 31, 2026 00:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the readability/maintainability of the Debian base image by documenting the role of each apt-installed package (OS setup vs Spack prerequisite vs intentional system external), while keeping the installed package set the same.

Changes:

  • Replace long apt-get install line continuations with a commented package-list heredoc, filtered through sed and fed to apt-get via xargs.
  • Add short inline rationales for each apt package in both the “base tools” and “Spack runtime deps” apt blocks.
Comments suppressed due to low confidence (1)

containers/debian/Dockerfile:160

  • Using xargs -r here can hide mistakes by turning an empty package list into a no-op (the build would succeed but Spack runtime deps wouldn’t be installed). Consider dropping -r so the build fails fast if the list is accidentally empty.
sed -e 's/[[:space:]]*#.*$//' -e '/^[[:space:]]*$/d' <<'EOF_APT_PACKAGES' \
  | xargs -r apt-get -yqq install --no-install-recommends

Comment thread containers/debian/Dockerfile Outdated
Comment thread containers/debian/Dockerfile Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Wouter Deconinck <wdconinc@gmail.com>
@wdconinc
wdconinc requested review from Copilot and removed request for Copilot July 31, 2026 00:15
@github-actions

Copy link
Copy Markdown

Capybara summary for PR 374

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.

2 participants