Support for custom translation

[cgwalters]

I'm looking at using cxx-rs in https://github.com/coreos/rpm-ostree/pull/2336

What I'd really like is support for custom bindings for types that already have a Rust wrapper. In our case we use GObject and https://github.com/gtk-rs/glib - so on the Rust side we already have bindings to a C API. Ultimately it's just a wrapper for a raw pointer with auto and hand-generated Rust function bindings.

What I'd like to be able to do is have some sort of "plugin" API in cxx-rs so that I can say if on the Rust side the object e.g. implements a particular trait (in this case, the glib::ObjectType, in the C++ header we write out its C type name, and convert internally to the glib-rs Rust wrapper type.

#[cxx::bridge(namespace = "rpmostreecxx")]
mod ffi {\
    extern "Rust" {
        fn inputdemo(repo: &ostree::Repo);
    }
}

Obviously this case is fairly niche to us, but it probably generalizes to "support mixing existing usage of bindgen" or so? (glib-rs doesn't use bindgen, it uses gobject-introspection, but the idea is broadly similar).

I think in this case actually the bulk of the lifting would be in generating the Rust wrapper code which accepts the C pointer type and wraps it via let repo: ostree::Repo = unsafe { from_glib_none(repo) }; as we do in e.g. code like this.

If this feels too niche (or too hard to generalize at first) one thing I'm considering is forking cxx-rs to add the functionality. It feels like a lot of projects out there are going to have various specialized requirements, and perhaps temporarily forking is a sane thing to do. If taking this path I'd definitely want to be able to pick up things like the forthcoming BTreeMap and Option bindings, but hopefully custom code wouldn't be too much of a conflict-fest.

Or I may be missing something and this is easy to do today?

(Sorry about filing this as an issue; I know that's kind of a standard way to approach GH projects that don't have their own forums; maybe users.rust-lang.org could have a "FFI" topic or something?)

[dtolnay]

This is already supported. Did you get a chance to see https://cxx.rs/extern-c++.html#reusing-existing-binding-types?

#[cxx::bridge]
mod ffi {
    extern "C++" {
        type Repo = ostree::Repo;
    }
    extern "Rust" {
        fn inputdemo(repo: &Repo);
    }
}

[cgwalters]

I have no idea how I missed that before, thanks! My main angle here is improved safety of exposing Rust functions and structures and less writing new code in C/C++, so I think I'd just skimmed extern "C++".

OK so the first stumbling block I hit here is that the Rust bindings are in a separate crate and the C types are also in external libraries (which again probably puts me in an unusual situation). The orphan rule means I needed to add a newtype, and it also took me a bit to understand I needed a typedef on the C++ side (the verifier doesn't let me just do type_id!("OstreeRepo")).

I ended up with this:

diff --git a/rust/rpmostree-cxxrs-prelude.h b/rust/rpmostree-cxxrs-prelude.h
new file mode 100644
index 00000000..5cb0f3ec
--- /dev/null
+++ b/rust/rpmostree-cxxrs-prelude.h
+#pragma once
+
+#include <ostree.h>
+
+namespace rpmostreecxx {
+    typedef ::OstreeRepo OstreeRepo;
+}
diff --git a/rust/src/lib.rs b/rust/src/lib.rs
index 9ae93397..c971e6cd 100644
--- a/rust/src/lib.rs
+++ b/rust/src/lib.rs
@@ -8,12 +8,30 @@
 mod ffiutil;
 mod includes;

+use cxx::{type_id, ExternType};
+
+pub struct FFIOstreeRepo(ostree::Repo);
+
+unsafe impl ExternType for FFIOstreeRepo {
+    type Id = type_id!("rpmostreecxx::OstreeRepo");
+    type Kind = cxx::kind::Trivial;
+}
+
 #[cxx::bridge(namespace = "rpmostreecxx")]
 mod ffi {
     // utils.rs
     extern "Rust" {
         fn download_to_fd(url: &str) -> Result<i32>;
     }
+
+    extern "C++" {
+        include!("rust/rpmostree-cxxrs-prelude.h");
+
+        type OstreeRepo = crate::FFIOstreeRepo;
+    }
+    extern "Rust" {
+        fn passthrough_repo_test(repo: &OstreeRepo);
+    }
 }

 mod cliwrap;
diff --git a/rust/src/utils.rs b/rust/src/utils.rs
index 0b49d8d6..0e2f6dc4 100644
--- a/rust/src/utils.rs
+++ b/rust/src/utils.rs
@@ -198,10 +198,16 @@ mod tests {
 mod cxxffi {
     use super::*;
     use std::os::unix::io::IntoRawFd;
+    use glib::ObjectType;

     pub fn download_to_fd(url: &str) -> Result<i32> {
         Ok(download_url_to_tmpfile(url)?.into_raw_fd())
     }
+
+    pub fn passthrough_repo_test(repo: &crate::ffi::OstreeRepo) {
+        dbg!(repo.0.as_ptr());
+        dbg!(repo.0.get_mode());
+    }
 }
 pub use cxxffi::*;

diff --git a/src/app/rpmostree-compose-builtin-tree.cxx b/src/app/rpmostree-compose-builtin-tree.cxx
index cffebef9..266c28c2 100644
--- a/src/app/rpmostree-compose-builtin-tree.cxx
+++ b/src/app/rpmostree-compose-builtin-tree.cxx
@@ -48,6 +48,7 @@
 #include "rpmostree-libbuiltin.h"
 #include "rpmostree-rpm-util.h"
 #include "rpmostree-rust.h"
+#include "rpmostree-cxxrs.h"

 #include "libglnx.h"

@@ -604,6 +605,8 @@ rpm_ostree_compose_context_new (const char    *treefile_pathstr,
   if (!self->repo)
     return FALSE;

+  rpmostreecxx::passthrough_repo_test(*self->repo);
+
   /* sanity check upfront we can even write to this repo; e.g. might be a mount */
   if (!ostree_repo_is_writable (self->repo, error))
     return glnx_prefix_error (error, "Cannot write to repository");

But I was getting an invalid pointer on the Rust side and looking at it, I just don't understand how it could work because we're effectively "casting" the C type into a Rust object. Here the Rust wrapper is its own type that holds a refcount.

Which is basically what this section is saying:

By writing the unsafe ExternType impl, the programmer asserts that the C++ namespace and type name given in the type id refers to a C++ type that is equivalent to Rust type that is the Self type of the impl.

I think this is probably the other angle that led me to look more at the Rust side of the bindings; basically we need to auto-generate conversion glue on the Rust side, right? That's what led my thinking down the path of something more fully custom; it seems like it'd need to be part of the proc macro.

Now I thought about using ostree_sys::OstreeRepo which is the raw bindgen-style wrapper...ok yes this works (on top of the previous):

diff --git a/rust/src/lib.rs b/rust/src/lib.rs
index c971e6cd..70869efc 100644
--- a/rust/src/lib.rs
+++ b/rust/src/lib.rs
@@ -10,7 +10,8 @@ mod includes;

 use cxx::{type_id, ExternType};

-pub struct FFIOstreeRepo(ostree::Repo);
+#[repr(transparent)]
+pub struct FFIOstreeRepo(ostree_sys::OstreeRepo);

 unsafe impl ExternType for FFIOstreeRepo {
     type Id = type_id!("rpmostreecxx::OstreeRepo");
@@ -30,7 +31,7 @@ mod ffi {
         type OstreeRepo = crate::FFIOstreeRepo;
     }
     extern "Rust" {
-        fn passthrough_repo_test(repo: &OstreeRepo);
+        fn passthrough_repo_test(repo: Pin<&mut OstreeRepo>);
     }
 }

diff --git a/rust/src/utils.rs b/rust/src/utils.rs
index 0e2f6dc4..b3f0812a 100644
--- a/rust/src/utils.rs
+++ b/rust/src/utils.rs
@@ -9,6 +9,7 @@ use std::collections::HashMap;
 use std::io::prelude::*;
 use std::path::Path;
 use std::{fs, io};
+use std::pin::Pin;
 use tempfile;

 use curl::easy::Easy;
@@ -198,15 +199,17 @@ mod tests {
 mod cxxffi {
     use super::*;
     use std::os::unix::io::IntoRawFd;
+    use glib::translate::*;
     use glib::ObjectType;

     pub fn download_to_fd(url: &str) -> Result<i32> {
         Ok(download_url_to_tmpfile(url)?.into_raw_fd())
     }

-    pub fn passthrough_repo_test(repo: &crate::ffi::OstreeRepo) {
-        dbg!(repo.0.as_ptr());
-        dbg!(repo.0.get_mode());
+    pub fn passthrough_repo_test(repo: Pin<&mut crate::ffi::OstreeRepo>) {
+        let repo: ostree::Repo = unsafe { from_glib_none(&mut repo.get_mut().0 as *mut _) };
+        dbg!(repo.as_ptr());
+        dbg!(repo.get_mode());
     }
 }
 pub use cxxffi::*;

But it leaves me with needing unsafe{} in each Rust function which is the main thing I'd hoped to avoid. On the other hand...if cxx-rs gives me safe wrappers for other types like strings maybe it'll be worth it versus hand-rolling bindgen.

Thanks for replying and your help by the way!

[cgwalters]

But it leaves me with needing unsafe{} in each Rust function which is the main thing I'd hoped to avoid.

OK I can add a little helper like

pub(crate) fn ffi_ostreerepo(repo: std::pin::Pin<&mut crate::ffi::OstreeRepo>) -> ostree::Repo {
    unsafe { from_glib_none(&mut repo.get_mut().0 as *mut _) }
}

Arguably perhaps the glib-rs bindings should have a safe way to convert from a &mut reference of the -sys version to the binding version (e.g. ostree_sys::OstreeRepo to ostree::Repo) instead of just a raw *mut pointer.

For now the main thing being passed back and forth will be just a few types like this...should be viable. I'm going to experiment more. Feel free to close this if you don't think there's anything else to do here on the cxx-rs side. Thanks again for your help!