Skip to content

Commit

Permalink
build-sys: Switch to committing cxx.rs generated code
Browse files Browse the repository at this point in the history
Closes: #3252

This avoids creating a magical, manual error prone step in the
release process, at the cost of dealing with generated code in git.
  • Loading branch information
cgwalters committed Jul 29, 2022
1 parent 6d25ccb commit 9c6320e
Show file tree
Hide file tree
Showing 15 changed files with 9,822 additions and 36 deletions.
2 changes: 0 additions & 2 deletions .copr/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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

2 changes: 2 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
rpmostree-cxxrs.{h,cxx} linguist-generated=true
rust/cxx.h linguist-generated=true
12 changes: 12 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 1 addition & 14 deletions Makefile-rpm-ostree.am
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 4 additions & 2 deletions Makefile.bindings
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
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 >$@.tmp && mv $@.tmp $@

rpmostree-cxxrs.h: $(binding_rust_sources) ./target/cxxbridge/bin/cxxbridge
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 > $@.tmp; then \
clang-format -i $@.tmp && \
if test -f $@ && cmp $@.tmp $@ 2>/dev/null; then rm -f $@.tmp; else \
mv $@.tmp $@; \
fi; \
Expand All @@ -15,6 +16,7 @@ rpmostree-cxxrs.h: $(binding_rust_sources) ./target/cxxbridge/bin/cxxbridge
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 \
clang-format -i $@.tmp && \
if test -f $@ && cmp $@.tmp $@ 2>/dev/null; then rm -f $@.tmp; else \
mv $@.tmp $@; \
fi; \
Expand Down
7 changes: 7 additions & 0 deletions ci/install-cxx.sh
Original file line number Diff line number Diff line change
@@ -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}"
13 changes: 13 additions & 0 deletions ci/install-dev-deps.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/usr/bin/bash
# Install developer build dependencies

# This installs tools used by local development only.

set -xeuo pipefail

# 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}"
11 changes: 11 additions & 0 deletions ci/verify-cxx.sh
Original file line number Diff line number Diff line change
@@ -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"
8 changes: 4 additions & 4 deletions docs/HACKING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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-dev-deps.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.


Expand Down
4 changes: 1 addition & 3 deletions docs/RELEASE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
8 changes: 0 additions & 8 deletions packaging/make-git-snapshot.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Loading

0 comments on commit 9c6320e

Please sign in to comment.