Skip to content

Conversation

@wdconinc
Copy link
Contributor

@wdconinc wdconinc commented Jan 8, 2026

Briefly, what does this PR introduce?

This PR reverts commit 4e49b9c, #115.

The offending commit appears to cause the second stage concretization (custom) to do a complete rebuild of everything. That makes the entire image grow by a factor 2, which is not acceptable. E.g. https://github.com/eic/containers/actions/runs/20766901576/job/59634999346#step:15:11828 which should not have new package builds, but only build eicrecon etc.

What kind of change does this PR introduce?

  • Bug fix (issue #__)
  • New feature (issue #__)
  • Documentation update
  • Other: __

Please check if this PR fulfills the following:

  • Tests for the changes have been added
  • Documentation has been added / updated
  • Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

No.

Does this PR change default behavior?

No.

Copilot AI review requested due to automatic review settings January 8, 2026 01:51
@wdconinc wdconinc enabled auto-merge (squash) January 8, 2026 01:52
Copy link
Contributor

Copilot AI left a comment

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 reverts the three-track container build approach introduced in PR #115 back to the original two-track approach. The three-track approach caused the custom concretization stage to trigger a complete rebuild of all packages, doubling the final image size. The revert fixes this by ensuring builder_concretization_custom is based on builder_installation_default (which has packages already built) rather than builder_concretization_default (which only has concretization).

Key changes:

  • Restored two-track build structure where custom concretization builds on top of default installation
  • Split runtime stages into separate concretization and installation phases to maintain proper layering
  • Updated CI workflow to test default concretization stage for CUDA builds

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
containers/eic/Dockerfile Reverted build structure from three-track to two-track, making builder_concretization_custom depend on builder_installation_default to prevent duplicate package builds; added separate runtime concretization/installation stages
docs/architecture.md Updated architecture diagram to reflect two-track build approach with proper stage dependencies and data flow
.github/workflows/build-push.yml Changed CUDA build target from builder_concretization_custom to builder_concretization_default to align with two-track workflow

@wdconinc wdconinc requested a review from veprbl January 8, 2026 03:08
@wdconinc
Copy link
Contributor Author

wdconinc commented Jan 8, 2026

Hmm, so I guess the issue of #115 is that the reuse criterion only applies when something is already installed, not when it is concretized but not installed... That's why doing two stage concretization and only then installation is not the same as concretization and installation repeated twice... Arguably a spack issue.

@wdconinc wdconinc merged commit 161a8ec into master Jan 8, 2026
34 checks passed
wdconinc added a commit that referenced this pull request Jan 18, 2026
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