-
Notifications
You must be signed in to change notification settings - Fork 14
boot: Empty /sysroot too #169
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs DCO but otherwise 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the change, and the comments. I more or less agree. Two (related notes):
- how intentional is the added
continue
? Should we not create/boot
if it doesn't exist? Or is that an error? How about/sysroot
? - if we do decide to always create/sysroot then we can simplify the examples to skip that step
I was trying to avoid introducing a new failure if somehow the image didn't have
The only rationale I can think of for not making it mandatory is to support having it be somewhere else (say So I can change both here and the mount code to make it mandatory? |
And then it makes sense in the code to have That said, there is I think a rationale for having that be configurable; the spec argues for mounting the ESP at |
Last I heard you're supposed to put the ESP at I agree that If we did want another (fixed) spot for this under Also: everyone knows what But: if someone wants it to be optional to avoid having that mount by default, I think that's legit... |
84a048f
to
373b3e7
Compare
Yeah you're right, see https://uapi-group.org/specifications/specs/boot_loader_specification/#mount-points But that's mostly talking about the client state which is different from the container image state. Eh I think we can just require OK so...I updated things to make both required and empty both. |
373b3e7
to
773ab67
Compare
This aids our compatibility with existing ostree-containers. Closes: containers#164 Signed-off-by: Colin Walters <walters@verbum.org>
773ab67
to
80203b3
Compare
I think we'll never have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't argue with that. We can adjust this later, if needed.
Thanks!
This aids our compatibility with existing ostree-containers.
Closes: #164