From be2075eef00640f85d62692f061f4b0fe7a12bfd Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 23 Jun 2022 17:36:46 -0400 Subject: [PATCH] repo: Metadata return values from `load_file` are not nullable The pattern this API uses in C is to allow the input parameters pointer targets to be `NULL`, and it doesn't return values in that case. A further complexity here is that the API will still return `NULL` for symbolic links. But Rust can't express this pattern as is, so we were always returning values but in `Option` wrappers that the caller needed to unwrap for the metadata. (We really want an even more efficient API here that avoids the glib objects entirely, e.g. no reason not to pass directly back a type that lets Rust directly read from the fd for bare repos, but that can come later) --- rust-bindings/conf/ostree.toml | 5 +++++ rust-bindings/src/auto/repo.rs | 12 ---------- rust-bindings/src/auto/versions.txt | 2 +- rust-bindings/src/repo.rs | 33 ++++++++++++++++++++++++++++ rust-bindings/tests/functions/mod.rs | 4 ++-- 5 files changed, 41 insertions(+), 15 deletions(-) diff --git a/rust-bindings/conf/ostree.toml b/rust-bindings/conf/ostree.toml index 64abc10ae5..57c2a79315 100644 --- a/rust-bindings/conf/ostree.toml +++ b/rust-bindings/conf/ostree.toml @@ -168,6 +168,11 @@ concurrency = "send" name = "destination_path" string_type = "filename" + # Cases where we use nullable output parameters that shouldn't be `Option` + [[object.function]] + pattern = "^(load_file)$" + ignore = true + [[object]] name = "OSTree.RepoFinder" status = "generate" diff --git a/rust-bindings/src/auto/repo.rs b/rust-bindings/src/auto/repo.rs index ba34e1dc82..4fd2a29c90 100644 --- a/rust-bindings/src/auto/repo.rs +++ b/rust-bindings/src/auto/repo.rs @@ -486,18 +486,6 @@ impl Repo { } } - #[doc(alias = "ostree_repo_load_file")] - pub fn load_file>(&self, checksum: &str, cancellable: Option<&P>) -> Result<(Option, Option, Option), glib::Error> { - unsafe { - let mut out_input = ptr::null_mut(); - let mut out_file_info = ptr::null_mut(); - let mut out_xattrs = ptr::null_mut(); - let mut error = ptr::null_mut(); - let _ = ffi::ostree_repo_load_file(self.to_glib_none().0, checksum.to_glib_none().0, &mut out_input, &mut out_file_info, &mut out_xattrs, cancellable.map(|p| p.as_ref()).to_glib_none().0, &mut error); - if error.is_null() { Ok((from_glib_full(out_input), from_glib_full(out_file_info), from_glib_full(out_xattrs))) } else { Err(from_glib_full(error)) } - } - } - #[doc(alias = "ostree_repo_load_object_stream")] pub fn load_object_stream>(&self, objtype: ObjectType, checksum: &str, cancellable: Option<&P>) -> Result<(gio::InputStream, u64), glib::Error> { unsafe { diff --git a/rust-bindings/src/auto/versions.txt b/rust-bindings/src/auto/versions.txt index 47d437596a..3c7edf9ef5 100644 --- a/rust-bindings/src/auto/versions.txt +++ b/rust-bindings/src/auto/versions.txt @@ -1,2 +1,2 @@ Generated by gir (https://github.com/gtk-rs/gir @ e8f82cf6) -from gir-files (@ 21901c2d) +from gir-files (@ ee5b3c76) diff --git a/rust-bindings/src/repo.rs b/rust-bindings/src/repo.rs index 6947f49b7d..6aeccff55c 100644 --- a/rust-bindings/src/repo.rs +++ b/rust-bindings/src/repo.rs @@ -277,6 +277,39 @@ impl Repo { Ok(self.resolve_rev(refspec, false)?.unwrap()) } + /// Load the contents (for regular files) and metadata for a content object. + #[doc(alias = "ostree_repo_load_file")] + pub fn load_file>( + &self, + checksum: &str, + cancellable: Option<&P>, + ) -> Result<(Option, gio::FileInfo, glib::Variant), glib::Error> { + unsafe { + let mut out_input = ptr::null_mut(); + let mut out_file_info = ptr::null_mut(); + let mut out_xattrs = ptr::null_mut(); + let mut error = ptr::null_mut(); + let _ = ffi::ostree_repo_load_file( + self.to_glib_none().0, + checksum.to_glib_none().0, + &mut out_input, + &mut out_file_info, + &mut out_xattrs, + cancellable.map(|p| p.as_ref()).to_glib_none().0, + &mut error, + ); + if error.is_null() { + Ok(( + from_glib_full(out_input), + from_glib_full(out_file_info), + from_glib_full(out_xattrs), + )) + } else { + Err(from_glib_full(error)) + } + } + } + /// Query metadata for a content object. /// /// This is similar to [`load_file`], but is more efficient if reading the file content is not needed. diff --git a/rust-bindings/tests/functions/mod.rs b/rust-bindings/tests/functions/mod.rs index 9fa3a91841..e041ddfb5c 100644 --- a/rust-bindings/tests/functions/mod.rs +++ b/rust-bindings/tests/functions/mod.rs @@ -59,8 +59,8 @@ fn should_checksum_file_from_input() { .load_file(obj.checksum(), NONE_CANCELLABLE) .expect("load file"); let result = checksum_file_from_input( - file_info.as_ref().unwrap(), - xattrs.as_ref(), + &file_info, + Some(&xattrs), stream.as_ref(), ObjectType::File, NONE_CANCELLABLE,