Skip to content

Conversation

cgwalters
Copy link
Member

This was a general principle cleanup, preparation
for handling VFAT for /boot for systemd-boot/BLS
support.

However I ran into an ugly corner case in our
unit tests that pointed at a sysroot without a
boot directory. The previous logic handled
ENOENT for boot/loader but not /boot.
Continue to cope with that degenerate situation.

@cgwalters cgwalters requested a review from Copilot April 8, 2025 21:18
Copy link

@Copilot 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.

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

Comments suppressed due to low confidence (1)

src/libostree/ostree-sysroot.c:648

  • Ensure that unit tests cover the scenario where the boot directory is missing, triggering this branch and returning an empty loader configuration set.
if (self->boot_fd == -1)

This was a general principle cleanup, preparation
for handling VFAT for /boot for systemd-boot/BLS
support.

However I ran into an ugly corner case in our
unit tests that pointed at a sysroot without a
boot directory. The previous logic handled
ENOENT for boot/loader but not /boot.
Continue to cope with that degenerate situation.

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

@allisonkarlitskaya allisonkarlitskaya left a comment

Choose a reason for hiding this comment

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

Looks good, but you've got narrow text width in your commit messages again :)

@@ -338,9 +338,38 @@ ensure_sysroot_fd (OstreeSysroot *self, GError **error)
return TRUE;
}

/* Require that both self->sysroot_fd is set.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Both self->sysroot_fd and... what?

@cgwalters cgwalters merged commit 30f36dc into ostreedev:main Apr 16, 2025
26 checks passed
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