Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix repart definition for ESP builds #3125

Merged
merged 1 commit into from
Oct 12, 2024
Merged

Conversation

septatrix
Copy link
Contributor

Currently ESP builds succeed but do not actually include any UKI as sd-repart silently ignore that they are missing. I am not sure if this is the correct place to add that fix. Maybe a --copy-from in the repart invocation would be cleaner.

Also, disk builds with UKI bootloaders are still broken - not sure if anything else is affected, too

Copy link
Contributor

@DaanDeMeyer DaanDeMeyer left a comment

Choose a reason for hiding this comment

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

Yikes, good catch

Copy link
Contributor

@DaanDeMeyer DaanDeMeyer left a comment

Choose a reason for hiding this comment

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

We also need to pass something like --ro-bind, uki, workdir(uki) to the sandbox options of make_image()

@septatrix
Copy link
Contributor Author

We also need to pass something like --ro-bind, uki, workdir(uki) to the sandbox options of make_image()

Do we? The uki is implicitly mounted as part of the staging dir under /work. Or do you suggest that as an alternative to adjusting the repart definition?

I am really not sure where the best place to put this fix is.

@septatrix
Copy link
Contributor Author

And I took a look at the logs when building a disk image with a UKI as a bootloader but was unable to find the error in that case...

@DaanDeMeyer
Copy link
Contributor

We also need to pass something like --ro-bind, uki, workdir(uki) to the sandbox options of make_image()

Do we? The uki is implicitly mounted as part of the staging dir under /work. Or do you suggest that as an alternative to adjusting the repart definition?

I am really not sure where the best place to put this fix is.

You are correct that it's available in the sandbox, it's the "implicit" that's the problem. We mount in the staging directory for another unrelated reason and that might change in the future so I'd like to make it explicit. Just adding an options argument to make_image() and then passing it through is good enough.

@septatrix septatrix marked this pull request as ready for review October 11, 2024 17:40
@behrmann behrmann merged commit 2195d95 into systemd:main Oct 12, 2024
35 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants