libdnf-sys: Drop C API, replace with cxx.rs bridge

Right now, we're using libdnf APIs from Rust via hand-crafted `extern C`
interfaces, which is extra dangerous because there is no signature
checking that happens at compile-time.

Until either we can automate libdnf bindings or use its C++ API directly
via cxx.rs, let's do some basic wrapping in C++ ourselves and use libdnf
through that API only instead. That gives us a lot more confidence and
makes the libdnf API feel more natural to use in Rust.
This commit is contained in:
Jonathan Lebon 2021-02-09 18:07:37 -05:00 committed by OpenShift Merge Robot
parent 5cd9e8f5e8
commit b640892f04
7 changed files with 189 additions and 22 deletions

52
Cargo.lock generated
View File

@ -140,6 +140,16 @@ dependencies = [
"cc", "cc",
] ]
[[package]]
name = "codespan-reporting"
version = "0.11.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c6ce42b8998a383572e0a802d859b1f00c79b7b7474e62fff88ee5c2845d9c13"
dependencies = [
"termcolor",
"unicode-width",
]
[[package]] [[package]]
name = "console" name = "console"
version = "0.14.0" version = "0.14.0"
@ -259,6 +269,21 @@ dependencies = [
"link-cplusplus", "link-cplusplus",
] ]
[[package]]
name = "cxx-build"
version = "1.0.31"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "69c0ba9a6c36216f1cd3cb6aedc605ca22feb9fc64d9a1b76099a9e368fa553d"
dependencies = [
"cc",
"codespan-reporting",
"lazy_static",
"proc-macro2",
"quote",
"scratch",
"syn",
]
[[package]] [[package]]
name = "cxxbridge-flags" name = "cxxbridge-flags"
version = "1.0.30" version = "1.0.30"
@ -640,7 +665,8 @@ version = "0.1.0"
dependencies = [ dependencies = [
"anyhow", "anyhow",
"cmake", "cmake",
"libc", "cxx",
"cxx-build",
"system-deps 2.0.3", "system-deps 2.0.3",
] ]
@ -1246,6 +1272,12 @@ version = "1.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd" checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd"
[[package]]
name = "scratch"
version = "1.0.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7e114536316b51a5aa7a0e59fc49661fd263c5507dd08bd28de052e57626ce69"
[[package]] [[package]]
name = "serde" name = "serde"
version = "1.0.123" version = "1.0.123"
@ -1458,6 +1490,15 @@ dependencies = [
"winapi", "winapi",
] ]
[[package]]
name = "termcolor"
version = "1.1.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2dfed899f0eb03f32ee8c6a0aabdb8a7949659e3466561fc0adf54e26d88c5f4"
dependencies = [
"winapi-util",
]
[[package]] [[package]]
name = "terminal_size" name = "terminal_size"
version = "0.1.16" version = "0.1.16"
@ -1605,6 +1646,15 @@ version = "0.4.0"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6"
[[package]]
name = "winapi-util"
version = "0.1.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "70ec6ce85bb158151cae5e5c87f95a8e97d2c0c4b001223f33a334e3ce5de178"
dependencies = [
"winapi",
]
[[package]] [[package]]
name = "winapi-x86_64-pc-windows-gnu" name = "winapi-x86_64-pc-windows-gnu"
version = "0.4.0" version = "0.4.0"

View File

@ -6,7 +6,7 @@ edition = "2018"
links = "dnf" links = "dnf"
[dependencies] [dependencies]
libc = "0.2" cxx = "1.0.30"
[lib] [lib]
name = "libdnf_sys" name = "libdnf_sys"
@ -16,6 +16,7 @@ path = "lib.rs"
cmake = "0.1.45" cmake = "0.1.45"
system-deps = "2.0" system-deps = "2.0"
anyhow = "1.0" anyhow = "1.0"
cxx-build = "1.0.30"
# This currently needs to duplicate the libraries from libdnf # This currently needs to duplicate the libraries from libdnf
[package.metadata.system-deps] [package.metadata.system-deps]
@ -30,3 +31,4 @@ libcurl = "7"
sqlite3 = "3" sqlite3 = "3"
modulemd = { name = "modulemd-2.0", version = "2" } modulemd = { name = "modulemd-2.0", version = "2" }
jsonc = { name = "json-c", version = "0" } jsonc = { name = "json-c", version = "0" }
glib = { name = "glib-2.0", version = "2" }

View File

@ -1,9 +1,10 @@
use anyhow::Result; use anyhow::Result;
fn main() -> Result<()> { fn main() -> Result<()> {
system_deps::Config::new().probe()?; let libs = system_deps::Config::new().probe()?;
use cmake::Config;
let libdnf = Config::new("../../libdnf") // first, the submodule proper
let libdnf = cmake::Config::new("../../libdnf")
// Needed for hardened builds // Needed for hardened builds
.cxxflag("-fPIC") .cxxflag("-fPIC")
// I picked /usr/libexec/rpm-ostree just because we need an // I picked /usr/libexec/rpm-ostree just because we need an
@ -34,5 +35,17 @@ fn main() -> Result<()> {
libdnf.display() libdnf.display()
); );
println!("cargo:rustc-link-lib=static=dnf"); println!("cargo:rustc-link-lib=static=dnf");
// now, our thin cxx.rs bridge wrapper
let mut libdnfcxx = cxx_build::bridge("lib.rs");
libdnfcxx
.file("cxx/libdnf.cxx")
.flag("-std=c++17")
.include("cxx") // this is needed for cxx.rs' `include!("libdnf.hxx")` to work
.include("../../libdnf");
// until https://github.com/gdesmott/system-deps/pull/32
libdnfcxx.includes(libs.iter().flat_map(|lib| lib.1.include_paths.iter()));
libdnfcxx.compile("libdnfcxx.a");
Ok(()) Ok(())
} }

View File

@ -0,0 +1,44 @@
/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*-
*
* Copyright (C) 2021 Red Hat, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published
* by the Free Software Foundation; either version 2 of the licence or (at
* your option) any later version.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General
* Public License along with this library; if not, write to the
* Free Software Foundation, Inc., 59 Temple Place, Suite 330,
* Boston, MA 02111-1307, USA.
*/
#include "libdnf.hxx"
namespace dnfcxx {
// XXX: use macro to dedupe
rust::String dnf_package_get_nevra(DnfPackage &pkg) {
return rust::String(::dnf_package_get_nevra(&pkg));
}
rust::String dnf_package_get_name(DnfPackage &pkg) {
return rust::String(::dnf_package_get_name(&pkg));
}
rust::String dnf_package_get_evr(DnfPackage &pkg) {
return rust::String(::dnf_package_get_evr(&pkg));
}
rust::String dnf_package_get_arch(DnfPackage &pkg) {
return rust::String(::dnf_package_get_arch(&pkg));
}
rust::String dnf_repo_get_id(DnfRepo &repo) {
return rust::String(::dnf_repo_get_id(&repo));
}
guint64 dnf_repo_get_timestamp_generated(DnfRepo &repo) {
return ::dnf_repo_get_timestamp_generated(&repo);
}
}

View File

@ -0,0 +1,37 @@
/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*-
*
* Copyright (C) 2021 Red Hat, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published
* by the Free Software Foundation; either version 2 of the licence or (at
* your option) any later version.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General
* Public License along with this library; if not, write to the
* Free Software Foundation, Inc., 59 Temple Place, Suite 330,
* Boston, MA 02111-1307, USA.
*/
#pragma once
#include <libdnf/libdnf.h>
#include "rust/cxx.h"
namespace dnfcxx {
typedef ::DnfPackage DnfPackage;
rust::String dnf_package_get_nevra(DnfPackage &pkg);
rust::String dnf_package_get_name(DnfPackage &pkg);
rust::String dnf_package_get_evr(DnfPackage &pkg);
rust::String dnf_package_get_arch(DnfPackage &pkg);
typedef ::DnfRepo DnfRepo;
rust::String dnf_repo_get_id(DnfRepo &repo);
guint64 dnf_repo_get_timestamp_generated(DnfRepo &repo);
}

View File

@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0 OR MIT * SPDX-License-Identifier: Apache-2.0 OR MIT
*/ */
use libc; use cxx::{type_id, ExternType};
/* This is an experiment in calling libdnf functions from Rust. Really, it'd be more sustainable to /* This is an experiment in calling libdnf functions from Rust. Really, it'd be more sustainable to
* generate a "libdnf-sys" from SWIG (though it doesn't look like that's supported yet * generate a "libdnf-sys" from SWIG (though it doesn't look like that's supported yet
@ -13,15 +13,34 @@ use libc;
// This technique for an uninstantiable/opaque type is used by libgit2-sys at least: // This technique for an uninstantiable/opaque type is used by libgit2-sys at least:
// https://github.com/rust-lang/git2-rs/blob/master/libgit2-sys/lib.rs#L51 // https://github.com/rust-lang/git2-rs/blob/master/libgit2-sys/lib.rs#L51
// XXX: dedupe with macro
pub enum DnfPackage {} pub enum DnfPackage {}
pub enum DnfRepo {} unsafe impl ExternType for DnfPackage {
type Id = type_id!(dnfcxx::DnfPackage);
extern "C" { type Kind = cxx::kind::Trivial;
pub fn dnf_package_get_nevra(package: *mut DnfPackage) -> *const libc::c_char;
pub fn dnf_package_get_name(package: *mut DnfPackage) -> *const libc::c_char;
pub fn dnf_package_get_evr(package: *mut DnfPackage) -> *const libc::c_char;
pub fn dnf_package_get_arch(package: *mut DnfPackage) -> *const libc::c_char;
pub fn dnf_repo_get_id(repo: *mut DnfRepo) -> *const libc::c_char;
pub fn dnf_repo_get_timestamp_generated(repo: *mut DnfRepo) -> u64;
} }
pub enum DnfRepo {}
unsafe impl ExternType for DnfRepo {
type Id = type_id!(dnfcxx::DnfRepo);
type Kind = cxx::kind::Trivial;
}
#[cxx::bridge(namespace = "dnfcxx")]
pub mod ffi {
unsafe extern "C++" {
include!("libdnf.hxx");
type DnfPackage = crate::DnfPackage;
fn dnf_package_get_nevra(pkg: &mut DnfPackage) -> Result<String>;
fn dnf_package_get_name(pkg: &mut DnfPackage) -> Result<String>;
fn dnf_package_get_evr(pkg: &mut DnfPackage) -> Result<String>;
fn dnf_package_get_arch(pkg: &mut DnfPackage) -> Result<String>;
type DnfRepo = crate::DnfRepo;
fn dnf_repo_get_id(repo: &mut DnfRepo) -> Result<String>;
fn dnf_repo_get_timestamp_generated(repo: &mut DnfRepo) -> Result<u64>;
}
}
pub use ffi::*;

View File

@ -193,7 +193,6 @@ mod ffi {
use crate::ffiutil::*; use crate::ffiutil::*;
use crate::includes::*; use crate::includes::*;
use glib::translate::*; use glib::translate::*;
use glib::GString;
use libdnf_sys::*; use libdnf_sys::*;
use std::ptr; use std::ptr;
@ -251,9 +250,11 @@ mod ffi {
}; };
for pkg in packages { for pkg in packages {
let name: Borrowed<GString> = unsafe { from_glib_borrow(dnf_package_get_name(pkg)) }; let pkg_ref = unsafe { &mut *pkg };
let evr: Borrowed<GString> = unsafe { from_glib_borrow(dnf_package_get_evr(pkg)) }; // XXX: remove unwraps when we move to cxx.rs
let arch: Borrowed<GString> = unsafe { from_glib_borrow(dnf_package_get_arch(pkg)) }; let name = dnf_package_get_name(pkg_ref).unwrap();
let evr = dnf_package_get_evr(pkg_ref).unwrap();
let arch = dnf_package_get_arch(pkg_ref).unwrap();
let mut chksum: *mut libc::c_char = ptr::null_mut(); let mut chksum: *mut libc::c_char = ptr::null_mut();
let r = unsafe { rpmostree_get_repodata_chksum_repr(pkg, &mut chksum, gerror) }; let r = unsafe { rpmostree_get_repodata_chksum_repr(pkg, &mut chksum, gerror) };
@ -281,8 +282,9 @@ mod ffi {
.unwrap(); .unwrap();
for rpmmd_repo in rpmmd_repos { for rpmmd_repo in rpmmd_repos {
let id: String = unsafe { from_glib_none(dnf_repo_get_id(rpmmd_repo)) }; let repo_ref = unsafe { &mut *rpmmd_repo };
let generated = unsafe { dnf_repo_get_timestamp_generated(rpmmd_repo) }; let id = dnf_repo_get_id(repo_ref).unwrap();
let generated = dnf_repo_get_timestamp_generated(repo_ref).unwrap();
lockfile_repos.insert( lockfile_repos.insert(
id, id,
LockfileRepoMetadata { LockfileRepoMetadata {