build-sys: Switch to committing cxx.rs generated code

.copr/Makefile

@@ -7,8 +7,6 @@ srpm:
 	git submodule update --init --recursive
 	# Our primary CI build goes via RPM rather than direct to binaries
 	# to better test that path, including our vendored spec file, etc.
-	# The RPM build expects pre-generated bindings, so do that now.
-	make -f Makefile.bindings bindings
 	make -C packaging -f Makefile.dist-packaging srpm
 	if test -n "$$outdir"; then mv packaging/*.src.rpm $$outdir; fi
 

.gitattributes

@@ -0,0 +1,2 @@
+rpmostree-cxxrs.{h,cxx} linguist-generated=true
+rust/cxx.h linguist-generated=true

.github/workflows/ci.yaml

@@ -30,6 +30,18 @@ jobs:
         with:
           name: install.tar
           path: install.tar
+  cxx-verify:
+    name: "Verify CXX generation"
+    runs-on: ubuntu-latest
+    container: registry.ci.openshift.org/coreos/fcos-buildroot:testing-devel
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v2
+      # https://github.com/actions/checkout/issues/760
+      - name: Mark git checkout as safe
+        run: git config --global --add safe.directory "$GITHUB_WORKSPACE"
+      - name: Codestyle
+        run: ./ci/verify-cxx.sh
   build-clang:
     name: "Build (clang)"
     runs-on: ubuntu-latest

.gitignore

@@ -73,9 +73,6 @@
 /rpm-ostreed-generated.c
 /rpm-ostreed-generated.h
 /rpm-ostreed.conf.5
-/rpmostree-cxxrs.cxx
-/rpmostree-cxxrs.h
-/rust/cxx.h
 /so_locations
 /src/daemon/org.projectatomic.rpmostree1.service
 /src/daemon/rpm-ostree-bootstatus.service

Makefile-rpm-ostree.am

@@ -115,27 +115,14 @@ CLEANFILES += rpm-ostree
 .PHONY: cargo-build
 ALL_LOCAL_HOOKS += cargo-build
 
-# cxx.rs
+# cxx.rs - to regenerate this, run `make -f Makefile.bindings bindings`
 binding_generated_sources = rpmostree-cxxrs.h rpmostree-cxxrs.cxx rust/cxx.h
-if BUILDOPT_PREBUILT_BINDINGS
-# In this case we shipped prebuilt versions in the source tarball
-rpmostree-cxxrs.h:
-	ln -sfr rpmostree-cxxrs-prebuilt.h $@
-rpmostree-cxxrs.cxx:
-	ln -sfr rpmostree-cxxrs-prebuilt.cxx $@
-rust/cxx.h:
-	ln -sfr rust/cxx-prebuilt.h $@
-else
-include Makefile.bindings
-CLEANFILES += rpmostree-cxxrs.h rpmostree-cxxrs.cxx rust/cxx.h
-endif
 
 noinst_LTLIBRARIES += librpmostreecxxrs.la
 librpmostreecxxrs_la_SOURCES = rpmostree-cxxrs.h rpmostree-cxxrs.cxx
 # Suppress missing-declarations because https://github.com/dtolnay/cxx/issues/590
 librpmostreecxxrs_la_CXXFLAGS = $(AM_CXXFLAGS) $(SANITIZER_FLAGS) $(rpmostree_common_cflags) -Wno-missing-declarations
 librpmostreecxxrs_la_LIBADD = -lstdc++
-BUILT_SOURCES += $(binding_generated_sources)
 
 ostreeextdir = $(DESTDIR)$(libexecdir)/libostree/ext
 

Makefile.bindings

@@ -3,18 +3,18 @@
 binding_rust_sources = $(shell find rust/src/ -name '*.rs') Cargo.toml Cargo.lock
 
 rust/cxx.h: Makefile.bindings
-	./target/cxxbridge/bin/cxxbridge --header >$@.tmp && mv $@.tmp $@
+	./target/cxxbridge/bin/cxxbridge --header | clang-format --assume-filename=$@ >$@.tmp && mv $@.tmp $@
 
-rpmostree-cxxrs.h: $(binding_rust_sources) ./target/cxxbridge/bin/cxxbridge
-	$(AM_V_GEN) if ./target/cxxbridge/bin/cxxbridge rust/src/lib.rs --header > $@.tmp; then \
+rpmostree-cxxrs.h: $(binding_rust_sources) rust/cxx.h ./target/cxxbridge/bin/cxxbridge
+	$(AM_V_GEN) if ./target/cxxbridge/bin/cxxbridge rust/src/lib.rs --header | clang-format --assume-filename=$@ > $@.tmp; then \
 	  if test -f $@ && cmp $@.tmp $@ 2>/dev/null; then rm -f $@.tmp; else \
 	    mv $@.tmp $@; \
 	  fi; \
 	else \
 	  echo cxxbridge failed; exit 1; \
 	fi
 rpmostree-cxxrs.cxx: $(binding_rust_sources) rpmostree-cxxrs.h
-	$(AM_V_GEN) if ./target/cxxbridge/bin/cxxbridge --include rpmostree-cxxrs.h rust/src/lib.rs > $@.tmp; then \
+	$(AM_V_GEN) if ./target/cxxbridge/bin/cxxbridge --include rpmostree-cxxrs.h rust/src/lib.rs | clang-format --assume-filename=$@ > $@.tmp; then \
 	  if test -f $@ && cmp $@.tmp $@ 2>/dev/null; then rm -f $@.tmp; else \
 	    mv $@.tmp $@; \
 	  fi; \

ci/install-cxx.sh

@@ -0,0 +1,7 @@
+#!/usr/bin/bash
+# We use https://cxx.rs to generate C++ and Rust bridge code.  If you change
+# rust/src/lib.rs, you will need to install the tool.
+set -xeuo pipefail
+CXX_VER=$(cargo metadata --format-version 1 | jq -r '.packages[]|select(.name == "cxx").version')
+mkdir -p target
+time cargo install --root=target/cxxbridge cxxbridge-cmd --version "${CXX_VER}"

ci/installdeps.sh

@@ -13,10 +13,5 @@ if [ -z "${SKIP_INSTALLDEPS:-}" ] && test $(id -u) -eq 0; then
     time dnf builddep --spec -y packaging/rpm-ostree.spec.in --allowerasing
 fi
 
-# cxx.rs (cxxbridge) isn't packaged in Fedora today.  It generates
-# source code, which we vendor along with our dependent crates into release
-# tarballs.
-CXX_VER=$(cargo metadata --format-version 1 | jq -r '.packages[]|select(.name == "cxx").version')
 mkdir -p target
-time cargo install --root=target/cxxbridge cxxbridge-cmd --version "${CXX_VER}"
 time cargo install --root=target/cargo-vendor-filterer cargo-vendor-filterer --version ^0.5

ci/verify-cxx.sh

@@ -0,0 +1,11 @@
+#!/usr/bin/bash
+# Verify that the cxx-generated C++ code is in sync
+set -xeuo pipefail
+dn=$(dirname $0)
+$dn/install-cxx.sh
+make -f Makefile.bindings bindings
+if ! git diff; then
+    echo "Found diff in cxx-generated code; please run: make -f Makefile.bindings bindings" 1>&2
+    exit 1
+fi
+echo "ok: cxx generated code matches"

docs/HACKING.md

@@ -24,17 +24,17 @@ When developing either in a toolbx container or natively on your system, you
 must install all the required dependencies. In the `ci/` subfolder, there are
 scripts that will do this for you:
 
-- **To install the build dependencies**: `./ci/installdeps.sh`
+- **To install the build dependencies**: `./ci/installdeps.sh` and `ci/install-cxx.sh`
     - **Note**: This command must be rerun after the dependencies in
       `Cargo.lock` change. This will eventually be fixed.
 - **To install the test dependencies**:
     - For `make check`: `./ci/install-test-deps.sh`
     - For `make vmcheck`: Follow the instructions for [installing cosa inside
       an existing container][3] from the [cosa GitHub repository][4]
 
-Today rpm-ostree uses [cxx.rs](https://cxx.rs/) - the CLI tools for that aren't
-packaged in e.g. Fedora; we ship the pre-generated source in the releases. It
-is installed as part of the `installdeps.sh` script. Most importantly, it must
+Today rpm-ostree uses [cxx.rs](https://cxx.rs/), and as of https://github.com/coreos/rpm-ostree/pull/3864
+we commit the generated code to git.  If you want to regenerate it (particularly
+when changing it), use `ci/install-cxx.sh`.  Most importantly, it currently must
 be reinstalled after you run `make clean` on the project.
 
 

docs/RELEASE.md

@@ -19,8 +19,6 @@ nav_order: 3
    tag with the release notes as its content. Make the first line be the name of
    the tag itself.
 8. Push the tag using `git push $upstream v202X.XX`.
-9. Create the xz tarball:
-   * `make -f Makefile.bindings bindings`
-   * `make -C packaging -f Makefile.dist-packaging dist-snapshot`
+9. Create the xz tarball: `make -C packaging -f Makefile.dist-packaging dist-snapshot`
 10. Create a GitHub release for the new release tag using its contents and
     attach the tarball.

packaging/make-git-snapshot.sh

@@ -52,12 +52,4 @@ fi
  tar --owner=0 --group=0 --transform="s,^,${PKG_VER}/," -rf ${TARFILE_TMP} * .cargo/
  )
 
-# And finally, vendor generated code.  See installdeps.sh
-# and Makefile-rpm-ostree.am for more.
-(cd ${srcdir}
- cp rpmostree-cxxrs{,-prebuilt}.h
- cp rpmostree-cxxrs{,-prebuilt}.cxx
- cp rust/cxx.h rust/cxx-prebuilt.h
- tar --owner=0 --group=0 --transform "s,^,${PKG_VER}/," -rf ${TARFILE_TMP} rpmostree-cxxrs-prebuilt.h rpmostree-cxxrs-prebuilt.cxx rust/cxx-prebuilt.h)
-
 mv ${TARFILE_TMP} ${TARFILE}