-
Notifications
You must be signed in to change notification settings - Fork 124
packaging/rpm: Be sure we pull in chcon (and util-linux) #1391
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
This version is unused. At one time I accidentally bumped it and then kept doing so occasionally. Fixate it so that doesn't happen again. The canonical version is in lib/Cargo.toml.
We execute it, and Anaconda was somehow omitting this. Also pull in util-linux-core because we also depend on that. bootc-dev#1383 (comment) Signed-off-by: Colin Walters <[email protected]>
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.
Code Review
This pull request updates dependencies for bootc
. It adds chcon
as a binary dependency and updates the RPM spec file accordingly. It also changes the version of the bootc
CLI crate to a stub value, presumably to help centralize versioning in the library crate.
My review focuses on the consistency of the dependency declarations. I've found that the RPM spec file is missing some runtime dependencies that are declared elsewhere, and I've suggested a change to make it complete. The other changes look reasonable.
Requires: util-linux-core | ||
Requires: /usr/bin/chcon |
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 adding these dependencies. The new comment on lines 50-51 about keeping this in sync with workspace.metadata.binary-dependencies
is very helpful.
Following that comment, I checked Cargo.toml
and noticed zstd
and systemctl
are also listed as binary dependencies. To make the spec file fully consistent, we should probably add Requires
for them as well. I've suggested adding file-path based Requires
for them to be consistent with the addition of /usr/bin/chcon
.
Requires: util-linux-core
Requires: /usr/bin/chcon
Requires: /usr/bin/systemctl
Requires: /usr/bin/zstd
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.
lgtm
chcon is being used by the bootc and recently added to bootc.spec file as an explicite requirement: bootc-dev/bootc#1391 When it is not presented in the system the result is failure of the `bootc install to-filesystem` command used by Anaconda installer.
chcon is being used by the bootc and recently added to bootc.spec file as an explicite requirement: bootc-dev/bootc#1391 When it is not presented in the system the result is failure of the `bootc install to-filesystem` command used by Anaconda installer.
We execute it, and Anaconda was somehow omitting this.
Also pull in util-linux-core because we also depend on that.
#1383 (comment)