Use cxx-rs for core.rs

This is much better than bindgen because it's fully safe.  It's
much more ergonomic too:

 - Invoke Rust methods-on-structs just like C++ methods-on-structs
 - Rust `Result<>` is translated automatically to exceptions

See https://cxx.rs/context.html for more.

Cargo.lock

@@ -215,6 +215,35 @@ dependencies = [
  "winapi",
 ]
 
+[[package]]
+name = "cxx"
+version = "1.0.18"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c76e80fb6d39f044d141223d7b56c468beaab53e7698a45cc535b780c2382b3c"
+dependencies = [
+ "cc",
+ "cxxbridge-flags",
+ "cxxbridge-macro",
+ "link-cplusplus",
+]
+
+[[package]]
+name = "cxxbridge-flags"
+version = "1.0.18"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "5f3bfe71a2767344b8ca945162df2445ed5a0a8244992723b9c68dd98a98ab32"
+
+[[package]]
+name = "cxxbridge-macro"
+version = "1.0.18"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b6a66e61d3db394b82ffa7e7849732d57e38e1bc25509527a5d30bfc2c39b5e6"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
 [[package]]
 name = "drop_bomb"
 version = "0.1.5"
@@ -564,6 +593,15 @@ dependencies = [
  "vcpkg",
 ]
 
+[[package]]
+name = "link-cplusplus"
+version = "1.0.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f96aa785c87218ec773df6c510af203872b34e2df2cf47d6e908e5f36231e354"
+dependencies = [
+ "cc",
+]
+
 [[package]]
 name = "linked-hash-map"
 version = "0.5.3"
@@ -949,6 +987,7 @@ dependencies = [
  "chrono",
  "clap",
  "curl",
+ "cxx",
  "envsubst",
  "gio",
  "gio-sys",
@@ -1115,9 +1154,9 @@ dependencies = [
 
 [[package]]
 name = "syn"
-version = "1.0.45"
+version = "1.0.54"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ea9c5432ff16d6152371f808fb5a871cd67368171b09bb21b43df8e4a47a3556"
+checksum = "9a2af957a63d6bd42255c359c93d9bfdb97076bd3b820897ce55ffbfbf107f44"
 dependencies = [
  "proc-macro2",
  "quote",

Cargo.toml

@@ -11,6 +11,7 @@ serde_derive = "1.0.118"
 serde_json = "1.0.60"
 serde_yaml = "0.8.14"
 libc = "0.2.81"
+cxx = "1.0.18"
 nix = "0.19.1"
 glib-sys = "0.10.1"
 glib = "0.10.3"

Makefile-libpriv.am

@@ -87,3 +87,13 @@ AM_V_GPERF_0 = @echo "  GPERF   " $@;
 src/%.c: src/%.gperf Makefile
 	$(AM_V_at)$(MKDIR_P) $(dir $@)
 	$(AM_V_GPERF)$(GPERF) < $< > $@.tmp && mv $@.tmp $@
+
+# Also we now use cxx.rs
+rpmostree-cxxrs.h: rust/src/lib.rs
+	$(AM_V_GEN) cxxbridge rust/src/lib.rs --header > $@
+rpmostree-cxxrs.cxx: rust/src/lib.rs
+	$(AM_V_GEN) cxxbridge --include rpmostree-cxxrs.h rust/src/lib.rs > $@
+cxxrs_sources = rpmostree-cxxrs.h rpmostree-cxxrs.cxx
+librpmostreepriv_sources += $(cxxrs_sources)
+BUILT_SOURCES += $(cxxrs_sources)
+GITIGNOREFILES += $(cxxrs_sources)

ci/install-extra-builddeps.sh

@@ -2,5 +2,6 @@
 # Install build dependencies not in the cosa buildroot already
 set -xeuo pipefail
 if ! command -v cxxbridge; then
-    cargo install --root=/usr cxxbridge-cmd
+    ver=$(cargo metadata --format-version 1 | jq -r '.packages[]|select(.name == "cxx").version')
+    cargo install --root=/usr cxxbridge-cmd --version "${ver}"
 fi

configure.ac

@@ -41,7 +41,7 @@ CC_CHECK_FLAGS_APPEND([WARN_CFLAGS], [CFLAGS], [\
   -Werror=parenthesis \
   -Werror=undef \
   -Werror=misleading-indentation \
-  -Werror=missing-include-dirs -Werror=aggregate-return \
+  -Werror=missing-include-dirs \
   -Wstrict-aliasing=2 \
   -Werror=unused-result \
 ])])

rust/src/core.rs

@@ -1,5 +1,6 @@
-pub use self::ffi::*;
 use crate::ffiutil;
+use anyhow::Result;
+use ffiutil::ffi_view_openat_dir;
 use openat_ext::OpenatDirExt;
 
 /// Guard for running logic in a context with temporary /etc.
@@ -13,27 +14,27 @@ pub struct TempEtcGuard {
     renamed_etc: bool,
 }
 
-impl TempEtcGuard {
-    /// Create a context with a temporary /etc, and return a guard to it.
-    pub fn undo_usretc(rootfs: openat::Dir) -> anyhow::Result<Self> {
-        let has_usretc = rootfs.exists("usr/etc")?;
-        if has_usretc {
-            // In general now, we place contents in /etc when running scripts
-            rootfs.local_rename("usr/etc", "etc")?;
-            // But leave a compat symlink, as we used to bind mount, so scripts
-            // could still use that too.
-            rootfs.symlink("usr/etc", "../etc")?;
-        }
-
-        let guard = Self {
-            rootfs,
-            renamed_etc: has_usretc,
-        };
-        Ok(guard)
+pub fn prepare_tempetc_guard(rootfs: i32) -> Result<Box<TempEtcGuard>> {
+    let rootfs = ffi_view_openat_dir(rootfs);
+    let has_usretc = rootfs.exists("usr/etc")?;
+    let mut renamed_etc = false;
+    if has_usretc {
+        // In general now, we place contents in /etc when running scripts
+        rootfs.local_rename("usr/etc", "etc")?;
+        // But leave a compat symlink, as we used to bind mount, so scripts
+        // could still use that too.
+        rootfs.symlink("usr/etc", "../etc")?;
+        renamed_etc = true;
     }
+    Ok(Box::new(TempEtcGuard {
+        rootfs,
+        renamed_etc,
+    }))
+}
 
+impl TempEtcGuard {
     /// Remove the temporary /etc, and destroy the guard.
-    pub fn redo_usretc(self) -> anyhow::Result<()> {
+    pub fn undo(&self) -> anyhow::Result<()> {
         if self.renamed_etc {
             /* Remove the symlink and swap back */
             self.rootfs.remove_file("usr/etc")?;
@@ -43,28 +44,24 @@ impl TempEtcGuard {
     }
 }
 
-mod ffi {
+#[cfg(test)]
+mod test {
     use super::*;
-    use glib_sys::GError;
+    use std::os::unix::prelude::*;
 
-    #[no_mangle]
-    pub extern "C" fn ror_tempetc_undo_usretc(
-        rootfs: libc::c_int,
-        gerror: *mut *mut GError,
-    ) -> *mut TempEtcGuard {
-        let fd = ffiutil::ffi_view_openat_dir(rootfs);
-        let res = TempEtcGuard::undo_usretc(fd).map(Box::new);
-        ffiutil::ptr_glib_error(res, gerror)
-    }
-
-    #[no_mangle]
-    pub extern "C" fn ror_tempetc_redo_usretc(
-        guard_ptr: *mut TempEtcGuard,
-        gerror: *mut *mut GError,
-    ) -> libc::c_int {
-        assert!(!guard_ptr.is_null());
-        let guard = unsafe { Box::from_raw(guard_ptr) };
-        let res = guard.redo_usretc();
-        ffiutil::int_glib_error(res, gerror)
+    #[test]
+    fn basic() -> Result<()> {
+        let td = tempfile::tempdir()?;
+        let d = openat::Dir::open(td.path())?;
+        let g = super::prepare_tempetc_guard(d.as_raw_fd())?;
+        g.undo()?;
+        d.ensure_dir_all("usr/etc/foo", 0o755)?;
+        assert!(!d.exists("etc/foo")?);
+        let g = super::prepare_tempetc_guard(d.as_raw_fd())?;
+        assert!(d.exists("etc/foo")?);
+        g.undo()?;
+        assert!(!d.exists("etc")?);
+        assert!(d.exists("usr/etc/foo")?);
+        Ok(())
     }
 }

rust/src/lib.rs

@@ -10,11 +10,23 @@
 mod ffiutil;
 mod includes;
 
+#[cxx::bridge(namespace = "rpmostreecxx")]
+mod ffi {
+    // core.rs
+    extern "Rust" {
+        type TempEtcGuard;
+
+        fn prepare_tempetc_guard(rootfs: i32) -> Result<Box<TempEtcGuard>>;
+        fn undo(self: &TempEtcGuard) -> Result<()>;
+    }
+}
+
 mod cliwrap;
 pub use cliwrap::*;
 mod composepost;
 pub use self::composepost::*;
 mod core;
+use crate::core::*;
 mod history;
 pub use self::history::*;
 mod journal;

src/libpriv/rpmostree-core.cxx

@@ -43,6 +43,7 @@
 #include "rpmostree-importer.h"
 #include "rpmostree-output.h"
 #include "rpmostree-rust.h"
+#include "rpmostree-cxxrs.h"
 
 #define RPMOSTREE_MESSAGE_COMMIT_STATS SD_ID128_MAKE(e6,37,2e,38,41,21,42,a9,bc,13,b6,32,b3,f8,93,44)
 #define RPMOSTREE_MESSAGE_SELINUX_RELABEL SD_ID128_MAKE(5a,e0,56,34,f2,d7,49,3b,b1,58,79,b7,0c,02,e6,5d)
@@ -4322,9 +4323,7 @@ rpmostree_context_assemble (RpmOstreeContext      *self,
   gboolean skip_sanity_check = FALSE;
   g_variant_dict_lookup (self->spec->dict, "skip-sanity-check", "b", &skip_sanity_check);
 
-  RORTempEtcGuard * etc_guard = ror_tempetc_undo_usretc (tmprootfs_dfd, error);
-  if (etc_guard == NULL)
-    return FALSE;
+  auto etc_guard = rpmostreecxx::prepare_tempetc_guard (tmprootfs_dfd);
 
   /* NB: we're not running scripts right now for removals, so this is only for overlays and
    * replacements */
@@ -4576,8 +4575,7 @@ rpmostree_context_assemble (RpmOstreeContext      *self,
     }
 
   /* Undo the /etc move above */
-  if (!ror_tempetc_redo_usretc (etc_guard, error))
-    return FALSE;
+  etc_guard->undo();
 
   /* And clean up var/tmp, we don't want it in commits */
   if (!glnx_shutil_rm_rf_at (tmprootfs_dfd, "var/tmp", cancellable, error))

src/libpriv/rpmostree-kernel.cxx

@@ -39,6 +39,7 @@
 #include "rpmostree-kernel.h"
 #include "rpmostree-bwrap.h"
 #include "rpmostree-rust.h"
+#include "rpmostree-cxxrs.h"
 #include "rpmostree-util.h"
 
 static const char usrlib_ostreeboot[] = "usr/lib/ostree-boot";
@@ -530,9 +531,8 @@ rpmostree_run_dracut (int     rootfs_dfd,
    * today.  Though maybe in the future we should add it, but
    * in the end we want to use systemd-sysusers of course.
    **/
-  RORTempEtcGuard * etc_guard = ror_tempetc_undo_usretc (rootfs_dfd, error);
-  if (etc_guard == NULL)
-    return FALSE;
+  auto etc_guard = rpmostreecxx::prepare_tempetc_guard (rootfs_dfd);
+
   gboolean have_passwd = FALSE;
   if (!rpmostree_passwd_prepare_rpm_layering (rootfs_dfd,
                                               NULL,
@@ -645,8 +645,7 @@ rpmostree_run_dracut (int     rootfs_dfd,
   if (have_passwd && !rpmostree_passwd_complete_rpm_layering (rootfs_dfd, error))
     return FALSE;
 
-  if (!ror_tempetc_redo_usretc (etc_guard, error))
-    return FALSE;
+  etc_guard->undo();
 
   *out_initramfs_tmpf = tmpf; tmpf.initialized = FALSE; /* Transfer */
   return TRUE;