Skip to content

rpm: fix non-deterministic stability from HashMap iteration#83

Merged
jlebon merged 2 commits intomainfrom
repro-hum
Mar 9, 2026
Merged

rpm: fix non-deterministic stability from HashMap iteration#83
jlebon merged 2 commits intomainfrom
repro-hum

Conversation

@jlebon
Copy link
Copy Markdown
Member

@jlebon jlebon commented Mar 9, 2026

If a rootfs has packages built from different versions of the same SRPM, (e.g. foo-1.0-1 and foo-libs-1.0-2), current chunkah logic is to still put them in the same component. This seems like the right call because clearly the package set hasn't converged yet and more churn is expected.

If that does happen, we should make sure to have a deterministic stability. Right now it just depends on whichever subpackage is encountered first, which may change between invocations.

Always calculate stability, and use the lower of the two. While we're there, warn if we encounter this since it likely indicates a compose bug on the user's side.

Closes: #80
Assisted-by: OpenCode (Claude Opus 4.6)

This brings in `Clone` support for `Package`, which we need.
@jlebon jlebon marked this pull request as ready for review March 9, 2026 15:52
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly addresses a non-determinism issue in stability calculation by ensuring the minimum stability is used across all sub-packages of a source RPM. The included test effectively validates this fix. I have identified a couple of areas for improvement: the new warning logic introduces its own non-deterministic behavior, and there's an opportunity to reduce code duplication. Please see my detailed comments below.

Comment thread src/components/rpm.rs
Comment thread src/components/rpm.rs Outdated
@jlebon jlebon enabled auto-merge (rebase) March 9, 2026 15:58
Comment thread src/components/rpm.rs Outdated
@jlebon jlebon disabled auto-merge March 9, 2026 16:04
If a rootfs has packages built from different versions of the same SRPM,
(e.g. `foo-1.0-1` and `foo-libs-1.0-2`), current chunkah logic is to
still put them in the same component. This seems like the right call
because clearly the package set hasn't converged yet and more churn
is expected.

If that does happen, we should make sure to have a deterministic
stability. Right now it just depends on whichever subpackage is
encountered first, which may change between invocations.

Always calculate stability, and use the lower of the two. While we're
there, warn if we encounter this since it likely indicates a compose bug
on the user's side.

Closes: #80
Assisted-by: OpenCode (Claude Opus 4.6)
@jlebon jlebon enabled auto-merge (rebase) March 9, 2026 16:22
@jlebon
Copy link
Copy Markdown
Member Author

jlebon commented Mar 9, 2026

Just going to force-merge this. CI has already passed on it and the only change is a comment string.

@jlebon jlebon disabled auto-merge March 9, 2026 16:23
@jlebon jlebon merged commit c05927f into main Mar 9, 2026
6 of 8 checks passed
@jlebon jlebon deleted the repro-hum branch March 9, 2026 16:24
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.

Non-deterministic layer packing from HashMap iteration

2 participants