Skip to content

fix(sles): bootstrap base packages separately - #1217

Open
djshah19 wants to merge 1 commit into
project-dalec:mainfrom
djshah19:dhwanishah/sles-bootstrap-order
Open

fix(sles): bootstrap base packages separately#1217
djshah19 wants to merge 1 commit into
project-dalec:mainfrom
djshah19:dhwanishah/sles-bootstrap-order

Conversation

@djshah19

@djshah19 djshah19 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Install the SLES base userland before the application RPM and its runtime dependency graph. This prevents package scriptlets from invoking a partially installed shell while preserving the existing combined transaction for other RPM targets.

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):
Fixes #

Special notes for your reviewer:

Install the SLES base userland before the application RPM and its runtime dependency graph. This prevents package scriptlets from invoking a partially installed shell while preserving the existing combined transaction for other RPM targets.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: eb0dc7e4-0a3b-4e0b-acdc-0807676acccc
Signed-off-by: Dhwani Shah <dhwanishah@microsoft.com>
Copilot AI balanced review requested due to automatic review settings September 2, 2026 15:29

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.

馃煝 Approval recommended

The scoped implementation preserves existing behavior by default and includes focused coverage for both transaction modes.

Pull request overview

Bootstraps the SLES base userland before installing application RPMs to prevent package scriptlet failures.

Changes:

  • Adds configurable separate base-package installation.
  • Enables it for SLES 15.
  • Tests combined and separate installation ordering.
File summaries
File Description
targets/linux/rpm/suse/sles15.go Enables separate bootstrapping for SLES 15.
targets/linux/rpm/distro/distro.go Adds the configuration option.
targets/linux/rpm/distro/container.go Implements ordered installation transactions.
targets/linux/rpm/distro/container_test.go Verifies installation ordering.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Balanced

馃挕 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.


worker := cfg.Worker(sOpt, dalec.Platform(sOpt.TargetPlatform), dalec.WithConstraints(opts...))

if hasBasePackages && cfg.InstallBasePackagesSeparately {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We could probably do this generically for all.
We'll just end up with 2 layers for images, and potentially share a base layer between builds of the same distro.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Any thoughts on this @invidian

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.

Let me look back at debian implementation.

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.

    opts = append(opts, dalec.ProgressGroup("Install base image packages"))

    // Update the base image to include the base packages.
    // This may include things that are necessary to even install the debSt package.
    // So this must be done separately from the debSt package.
    baseImg = baseImg.Run(
      dalec.WithConstraints(opts...),
      InstallLocalPkg(basePackages(ctx, input), true, opts...),
      aptProxyConfig(input.SOpt),
      dalec.WithMountedAptCache(c.AptCachePrefix, opts...),

I mean, the implementation is quite distro-specific, but if you want to keep it as a separate layer explicitly, maybe that make sense.

Minimal deb images squashes the layers later on anyway to reduce image size.

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.

4 participants