Skip to content

Commit

Permalink
Split cbindgen to separate build, support external version
Browse files Browse the repository at this point in the history
The problem is building bindgen as part of our single run
locks serde to way old versions, and I want to use newer versions.

Since Fedora will now again ship a `cbindgen` package, let's
also support using it if we find it, saving ourselves
the cost of building it.

For distros that don't ship it (e.g. CentOS) for CI purposes
we build it.  For downstream builds that are offline, rather
than vendor the cbindgen sources like we do with our main Rust,
let's just vendor the `rpmostree-rust.h` file as was suggested
in https://bugzilla.redhat.com/show_bug.cgi?id=1608670

Closes: #1557

Closes: #1573
Approved by: jlebon
  • Loading branch information
cgwalters authored and rh-atomic-bot committed Sep 25, 2018
1 parent 71588f9 commit 03b5f6b
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 14 deletions.
22 changes: 19 additions & 3 deletions Makefile-rpm-ostree.am
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,16 @@ if BUILDOPT_NEW_NAME
INSTALL_DATA_HOOKS += install-bin-hook
endif

if !HAVE_PREBUILT_CBINDGEN
if !HAVE_EXTERNAL_CBINDGEN
rpmostree-bindgen: bindgen/Cargo.toml bindgen/src/main.rs
cd $(top_srcdir)/bindgen && \
export CARGO_TARGET_DIR=@abs_top_builddir@/bindgen-target && \
$(cargo) build --verbose && \
ln -sf $${CARGO_TARGET_DIR}/debug/rpmostree-bindgen $(abs_top_builddir)/rpmostree-bindgen
endif
endif

librpmostree_rust_path = @abs_top_builddir@/target/@RUST_TARGET_SUBDIR@/librpmostree_rust.a
# If the target directory exists, and isn't owned by our uid,
# we use --frozen this happens with the `make && sudo make install`
Expand All @@ -98,11 +108,17 @@ $(librpmostree_rust_path): Makefile $(LIBRPMOSTREE_RUST_SRCS)
$(cargo) build --verbose $${frozen} $(CARGO_RELEASE_ARGS)
EXTRA_DIST += $(LIBRPMOSTREE_RUST_SRCS) rust/Cargo.lock

# See rust/build.rs - it uses cbindgen as part of the Rust build
# Generate bindings from Rust to C
if !HAVE_PREBUILT_CBINDGEN
if HAVE_EXTERNAL_CBINDGEN
rpmostree-rust.h: $(librpmostree_rust_path)
$(AM_V_GEN) src=$$(find @abs_top_builddir@/target/@RUST_TARGET_SUBDIR@/ -name 'rpmostree-rust.h') && \
test -n "$${src}" && ln -sfr "$${src}" rpmostree-rust.h
$(AM_V_GEN) cbindgen -c rust/cbindgen.toml -o $@ $(top_srcdir)/rust
else
rpmostree-rust.h: $(librpmostree_rust_path) rpmostree-bindgen
$(AM_V_GEN) ./rpmostree-bindgen $(top_srcdir)/rust
endif
BUILT_SOURCES += rpmostree-rust.h
endif

rpm_ostree_CFLAGS += $(PKGDEP_RPMOSTREE_RS_CFLAGS)
rpm_ostree_LDADD += $(librpmostree_rust_path) $(PKGDEP_RPMOSTREE_RS_LIBS)
Expand Down
2 changes: 2 additions & 0 deletions bindgen/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Cargo.lock
target
12 changes: 12 additions & 0 deletions bindgen/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[package]
name = "rpmostree-bindgen"
version = "0.1.0"
authors = ["Colin Walters <[email protected]>"]

[dependencies]
cbindgen = "0.6.3"

# Might as well keep these from the main Rust source
[profile.release]
panic = "abort"
debug = true
12 changes: 5 additions & 7 deletions rust/build.rs → bindgen/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,14 @@
extern crate cbindgen;

fn run() -> Result<(), String> {
let out_dir_v = std::env::var("OUT_DIR").expect("OUT_DIR is unset");
let out_dir = std::path::Path::new(&out_dir_v);
let args: Vec<String> = std::env::args().collect();
let rustdir = std::path::Path::new(&args[1]);
let out = std::path::Path::new("rpmostree-rust.h");

// First, output our dependencies https://doc.rust-lang.org/cargo/reference/build-scripts.html
println!("cargo:rerun-if-changed=cbindgen.toml");

let bindings = cbindgen::generate(std::path::Path::new(".")).map_err(|e| e.to_string())?;
let bindings = cbindgen::generate(rustdir).map_err(|e| e.to_string())?;
// This uses unwraps internally; it'd be good to submit a patch
// to add a Result-based API.
bindings.write_to_file(out_dir.join("rpmostree-rust.h"));
bindings.write_to_file(out);
Ok(())
}

Expand Down
12 changes: 12 additions & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,17 @@ AS_IF([test -z "$cargo"], [AC_MSG_ERROR([cargo is required for --enable-rust])])
AC_PATH_PROG([rustc], [rustc])
AS_IF([test -z "$rustc"], [AC_MSG_ERROR([rustc is required for --enable-rust])])

dnl https://github.com/projectatomic/rpm-ostree/pull/1573
dnl We support 3 modes for bindgen:
dnl - External /usr/bin/cbindgen (Fedora + CI)
dnl - Downloading it from crates.io (CentOS + CI builds)
dnl - Not running it at all, and using a pre-built rpmostree-rust.h (Koji)
AS_IF([test -d "${srcdir}"/rust/vendor],
[cbindgen=dist],
[AC_PATH_PROG([cbindgen], [cbindgen])])
AM_CONDITIONAL(HAVE_PREBUILT_CBINDGEN, [test "${cbindgen}" = "dist"])
AM_CONDITIONAL(HAVE_EXTERNAL_CBINDGEN, [test -n "${cbindgen}" && test "${cbindgen}" != dist])

AC_MSG_CHECKING(whether to build in debug mode)
debug_release=release
if $(echo $CFLAGS |grep -q -E "(-O0|-Og)"); then
Expand Down Expand Up @@ -271,4 +282,5 @@ echo "
bubblewrap: $with_bubblewrap
gtk-doc: $enable_gtk_doc
rust: $rust_debug_release
cbindgen: ${cbindgen:-internal}
"
7 changes: 7 additions & 0 deletions packaging/make-git-snapshot.sh
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,11 @@ j["files"] = {f:c for f, c in j["files"].items() if not f.startswith("curl/")}
open(sys.argv[1], "w").write(json.dumps(j))' vendor/curl-sys/.cargo-checksum.json
tar --transform="s,^,${PKG_VER}/rust/," -rf ${TARFILE_TMP} * .cargo/
)

# And finally, vendor rpmostree-rust.h; it's generated by cbindgen,
# and it's easier than vendoring all of the source for that too.
# This is currently the *only* generated file we treat this way. See also
# https://github.com/projectatomic/rpm-ostree/pull/1573
(cd ${srcdir} && tar --transform "s,^,${PKG_VER}/," -rf ${TARFILE_TMP} rpmostree-rust.h)

mv ${TARFILE_TMP} ${TARFILE}
4 changes: 0 additions & 4 deletions rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
name = "rpmostree-rust"
version = "0.1.0"
authors = ["Colin Walters <[email protected]>"]
build = "build.rs"

[dependencies]
serde = "1.0"
Expand All @@ -17,9 +16,6 @@ tempfile = "3.0.3"
openat = "0.1.15"
curl = "0.4.14"

[build-dependencies]
cbindgen = "0.6.3"

[lib]
name = "rpmostree_rust"
path = "src/lib.rs"
Expand Down

0 comments on commit 03b5f6b

Please sign in to comment.