Release 2025.7 & regenerate bindings#3548
Conversation
github-actions
bot
added
the
area/rust-bindings
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request primarily focuses on bumping the project's version and regenerating the Rust bindings. The changes include updating dependencies like gio and glib, and adding a significant amount of documentation to the auto-generated binding files, which is a great improvement for code clarity and maintainability. However, I've identified two potential issues that need attention: a version number mismatch in configure.ac compared to the PR title, and a change in a function signature in the generated bindings that could lead to a panic. Addressing these points will ensure the release is consistent and the bindings are robust.
| #[cfg_attr(docsrs, doc(cfg(feature = "v2016_5")))] | ||
| #[doc(alias = "ostree_blob_reader_read_blob")] | ||
| fn read_blob(&self, cancellable: Option<&impl IsA<gio::Cancellable>>) -> Result<Option<glib::Bytes>, glib::Error> { | ||
| fn read_blob(&self, cancellable: Option<&impl IsA<gio::Cancellable>>) -> Result<glib::Bytes, glib::Error> { |
There was a problem hiding this comment.
The return type of read_blob has been changed from Result<Option<glib::Bytes>, glib::Error> to Result<glib::Bytes, glib::Error>. The underlying C function ostree_blob_reader_read_blob can return NULL on success to indicate the end of the stream. The previous Rust binding correctly represented this with an Option<glib::Bytes>.
The new binding will likely cause a panic when from_glib_full(ret) is called with a NULL pointer from the C function, as from_glib_full on a non-nullable pointer type will panic on NULL.
Was the underlying C API or its nullability annotation in the GIR file changed intentionally? If the C function can still return NULL on success, this binding appears incorrect and may need to be reverted to return an Option.
There was a problem hiding this comment.
Well spotted...we very clearly have in the source
/**
* ostree_blob_reader_read_blob
...
* Returns: (nullable): A #GBytes blob, or %NULL if there are no more
*
so it definitely looks like something went wrong in the introspection -> Rust here
There was a problem hiding this comment.
What build environment are you using? fedora-43, centos-10 etc?
#3549 doesn't change anything for me locally but I am very curious if it does for you
There was a problem hiding this comment.
Fedora 42 with the latest gir.
There was a problem hiding this comment.
did not change anything for me
There was a problem hiding this comment.
I'll re-do this inside a clean container and see if it's different.
jmarrero
force-pushed
the
release-20257
branch
from
697b87d to
fe4a6e8
Compare
This is SUCH a giant trap. I am not totally sure why it's working for me in a fedora-42 build env, but it seems like it may have broken in a different build environment in ostreedev#3548 (comment) I used Sonnet to audit for similar instances beyond `read_blob` and it found some, fix those too. Signed-off-by: Colin Walters <walters@verbum.org>
This is SUCH a giant trap. I am not totally sure why it's working for me in a fedora-42 build env, but it seems like it may have broken in a different build environment in ostreedev#3548 (comment) I used Sonnet to audit for similar instances beyond `read_blob` and it found some, fix those too. Signed-off-by: Colin Walters <walters@verbum.org>
rust-bindings/src/kernel_args.rs
Outdated
|
|
||
| /// Append the entire contents of the currently booted kernel commandline. | ||
| // rustdoc-stripper-ignore-next-stop | ||
| /// Appends the command line arguments in the file "/proc/cmdline" |
There was a problem hiding this comment.
cool that the C docs are copied to Rust now
There was a problem hiding this comment.
I think probably this happened because I did : $ make merge-lgpl-docs which https://github.com/ostreedev/ostree/blob/main/rust-bindings/README.md#documentation says I should not have done
There was a problem hiding this comment.
Now I just did: make gir-files && make gir
Fix formatting with clang-format and rust-fmt
jmarrero
force-pushed
the
release-20257
branch
from
fe4a6e8 to
cc11fb8
Compare
|
|
||
| #[doc(alias = "ostree_bootconfig_parser_get_tries_done")] | ||
| #[doc(alias = "get_tries_done")] | ||
| pub fn tries_done(&self) -> u64 { |
There was a problem hiding this comment.
It also looks wrong to me that these APIs are going away?
2 participants
No description provided.