-
Notifications
You must be signed in to change notification settings - Fork 128
ostree-ext: update ostree rust bindings and use new feature #1401
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
base: main
Are you sure you want to change the base?
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.
Code Review
The pull request updates the ostree rust bindings and enables a new feature, v2025_3
. It also downgrades the windows-sys
dependency to version 0.52.0. The changes include updates to the Cargo.lock
and ostree-ext/Cargo.toml
files. It's important to verify the compatibility and functionality of the updated dependencies and the new feature.
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 but this is going to fail until we have the new ostree upstream release shipped everywhere, which is going to be really painful 😢
So tempting again to just vendor ostree, or at least vendor the sysroot parts...
With some nontrivial work what we could do is detect at build time (via a build.rs script) whether a new enough ostree is available, and then only access the updated APIs via a #[cfg()]
.
We need to ship anywhere either way... I will go do that. |
8d9c2b5
to
4708643
Compare
|
4708643
to
5c44b24
Compare
I'm going to take a quick run at making this a build time conditional |
5c44b24
to
d74531c
Compare
OK well, I made it work...except that it only works because we messed up the |
0da907a
to
9b69dc1
Compare
OK I reworked this...the dependency on an updated ostree is now an opt-in feature. We now build with and without it. As you can see in the patch I started to stub out "wrapper functions" that make the soft reboot functionality also opt-in. This will be mildly tedious to deal with but I think it's viable for now. Very definitely though in the medium/long term I am really tempted to either support vendoring ostree and/or just some of the key sysroot bits in this project. |
Blocked CI until centos stream composes sync (ref https://gitlab.com/redhat/centos-stream/containers/bootc/-/issues/1174 ) |
Eh let's just hard require the new version... |
This will enable soft reboots. Signed-off-by: Colin Walters <[email protected]>
9b69dc1
to
d0729b0
Compare
OK right...because the updated ostree isn't in the c9s base image because https://gitlab.com/redhat/centos-stream/containers/bootc/-/issues?show=eyJpaWQiOiIxMTkzIiwiZnVsbF9wYXRoIjoicmVkaGF0L2NlbnRvcy1zdHJlYW0vY29udGFpbmVycy9ib290YyIsImlkIjoxNzA3NzMzNDR9 |
This allows us to enable soft-reboot