Skip to content

Conversation

@cgwalters
Copy link
Collaborator

Now that we're doing a "from scratch" build we don't
have the mtime issue, and so we can change our build system
to do everything in a single step.

Assisted-by: OpenCode (Opus 4.5)

@github-actions github-actions bot added the area/documentation Updates to the documentation label Jan 8, 2026
@bootc-bot bootc-bot bot requested a review from jeckersb January 8, 2026 22:57
Copy link
Contributor

@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 is an excellent refactoring of the build system, unifying the standard and sealed image build processes into a single, multi-stage Docker build. This simplifies the build logic, improves maintainability, and likely enhances build caching. The changes are well-structured, breaking down the complex sealing process into smaller, dedicated scripts and Dockerfile stages. The removal of outdated files and the consolidation of logic into a single, more powerful Dockerfile is a significant cleanup. My review includes a few suggestions to improve the robustness of some of the new shell scripts to better handle cases where file globs might match more than one file.

Comment on lines +25 to +29
kver=$(cd /usr/lib/modules && echo *)
if [ -z "$kver" ] || [ "$kver" = "*" ]; then
echo "Error: No kernel found" >&2
exit 1
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The command kver=$(cd /usr/lib/modules && echo *) is not robust if multiple kernel version directories exist under /usr/lib/modules. In such a case, kver would contain a space-separated list of versions, which would break subsequent commands. It's safer to ensure exactly one kernel directory is found and fail otherwise.

Suggested change
kver=$(cd /usr/lib/modules && echo *)
if [ -z "$kver" ] || [ "$kver" = "*" ]; then
echo "Error: No kernel found" >&2
exit 1
fi
kver_paths=(/usr/lib/modules/*)
if [ "${#kver_paths[@]}" -ne 1 ]; then
echo "Error: Expected 1 kernel version directory, but found ${#kver_paths[@]}" >&2
ls -l /usr/lib/modules/
exit 1
fi
kver=$(basename "${kver_paths[0]}")

Comment on lines +24 to +28
kver=$(cd "${target}/usr/lib/modules" && echo *)
if [ -z "$kver" ] || [ "$kver" = "*" ]; then
echo "Error: No kernel found" >&2
exit 1
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The command kver=$(cd "${target}/usr/lib/modules" && echo *) is not robust if multiple kernel version directories exist. In such a case, kver would contain a space-separated list of versions, which would break subsequent commands. It's safer to ensure exactly one kernel directory is found and fail otherwise.

Suggested change
kver=$(cd "${target}/usr/lib/modules" && echo *)
if [ -z "$kver" ] || [ "$kver" = "*" ]; then
echo "Error: No kernel found" >&2
exit 1
fi
kver_paths=("${target}/usr/lib/modules"/*)
if [ "${#kver_paths[@]}" -ne 1 ]; then
echo "Error: Expected 1 kernel version directory, but found ${#kver_paths[@]}" >&2
ls -l "${target}/usr/lib/modules/"
exit 1
fi
kver=$(basename "${kver_paths[0]}")

Comment on lines +99 to +100
sdboot_unsigned=$(ls ./usr/lib/systemd/boot/efi/systemd-boot*.efi)
sdboot_bn=$(basename ${sdboot_unsigned})
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The use of ls with a glob to find the systemd-boot EFI binary can be fragile. If the glob matches more than one file, the sdboot_unsigned variable will contain a multi-line string, which could cause sbsign to behave unexpectedly. It's safer to ensure exactly one file is found.

sdboot_unsigned_files=(./usr/lib/systemd/boot/efi/systemd-boot*.efi)
if [ ${#sdboot_unsigned_files[@]} -ne 1 ]; then
    echo "Error: Expected 1 systemd-boot EFI file, but found ${#sdboot_unsigned_files[@]}" >&2
    ls -l ./usr/lib/systemd/boot/efi/
    exit 1
fi
sdboot_unsigned="${sdboot_unsigned_files[0]}"
sdboot_bn=$(basename "${sdboot_unsigned}")

Comment on lines +21 to +22
sdboot=$(ls /usr/lib/systemd/boot/efi/systemd-boot*.efi)
sdboot_bn=$(basename "${sdboot}")
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The use of ls with a glob to find the systemd-boot EFI binary can be fragile. If the glob matches more than one file, the sdboot variable will contain a multi-line string. This would cause the install command on line 24 to fail because it would be interpreted as multiple destination arguments. It's safer to ensure exactly one file is found.

Suggested change
sdboot=$(ls /usr/lib/systemd/boot/efi/systemd-boot*.efi)
sdboot_bn=$(basename "${sdboot}")
sdboot_files=(/usr/lib/systemd/boot/efi/systemd-boot*.efi)
if [ "${#sdboot_files[@]}" -ne 1 ]; then
echo "Error: Expected 1 systemd-boot EFI file, but found ${#sdboot_files[@]}" >&2
ls -l /usr/lib/systemd/boot/efi/
exit 1
fi
sdboot="${sdboot_files[0]}"
sdboot_bn=$(basename "${sdboot}")

@cgwalters cgwalters force-pushed the sealed-scratch-rebase branch from 0ba5f48 to 4c7882c Compare January 8, 2026 23:57
@cgwalters
Copy link
Collaborator Author

error: Installing to disk: Setting up composefs boot: Setting up UKI boot: Writing 6.12.0-174.el10.x86_64.efi to ESP: The UKI has the wrong composefs= parameter (is 'sha512:1c88433c334194ecbf03356ba12fdcf7bb16c4ba075ff72d44666237136c162b35cc0bcfe923c8ace1fe27d724c18c6dc386d644d98ec5a890e63bab469b21b2', should be sha512:5c8b606415d239698de34b714935a684b38a763eddb573161213e98ed2ad91b69e10f236ded502af078744ce94ddbe353368be3dda5a182c631edd8e1ec0add0)

OK this was working for me at one point but then broke and it took me quite a while to figure out why: containers/storage#743 (comment)

@cgwalters
Copy link
Collaborator Author

OK this will depend on more composefs-rs work so we need to get back on git main, which is #1791

@jeckersb
Copy link
Collaborator

OK this will depend on more composefs-rs work so we need to get back on git main, which is #1791

Merged now, so I think this is unblocked?

@cgwalters
Copy link
Collaborator Author

I'm working on containers/composefs-rs#209 because I was hitting unexpected diffs - will flesh this out more

Now that we're doing a "from scratch" build we don't
have the mtime issue, and so we can change our build system
to do everything in a single step.

Assisted-by: OpenCode (Opus 4.5)
Signed-off-by: Colin Walters <walters@verbum.org>
@cgwalters cgwalters force-pushed the sealed-scratch-rebase branch from 4c7882c to d46fca8 Compare January 13, 2026 18:03
Add support for bind-mounting an extra source directory into container
builds, primarily for developing against a local composefs-rs checkout.

Usage:
  BOOTC_extra_src=$HOME/src/composefs-rs just build

The directory is mounted at /run/extra-src inside the container. When
using this, also patch Cargo.toml to use path dependencies pointing to
/run/extra-src/crates/....

Signed-off-by: Colin Walters <walters@verbum.org>

Assisted-by: OpenCode (Opus 4.5)
Add a convenience target to launch the built image in a transient VM
for manual testing and debugging.

Signed-off-by: Colin Walters <walters@verbum.org>

Assisted-by: OpenCode (Opus 4.5)
See containers/composefs-rs#209

Signed-off-by: Colin Walters <walters@verbum.org>

Assisted-by: OpenCode (Opus 4.5)
Add a command to build a sealed image and compare composefs dumpfiles
between the build-time view (mounted filesystem) and install-time view
(OCI tar layer processing). This helps debug mtime and metadata
discrepancies that cause sealed boot failures.

Usage:
  cargo xtask validate-composefs-digest
  cargo xtask validate-composefs-digest --no-cache  # force clean build

Output files are written to target/validate-digest/:
  - build.dumpfile: from mounted filesystem (what seal-uki sees)
  - storage.dumpfile: from containers-storage (what bootc upgrade sees)
  - diff.txt: differences if any

Signed-off-by: Colin Walters <walters@verbum.org>

Assisted-by: OpenCode (Opus 4.5)
Now that we're doing a "from scratch" build we don't
have the mtime issue, and so we can change our build system
to do everything in a single step.

Assisted-by: OpenCode (Opus 4.5)
Signed-off-by: Colin Walters <walters@verbum.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/documentation Updates to the documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants