Generate a C header from Rust with rust_cbindgen - #54
Merged
Conversation
A crate declared at two versions gets one bare label and one versioned one, and the bare label went to the newest. So a new major arriving as an indirect dependency of something unrelated takes the label, and every first-party rule naming //third_party/crates:<crate> moves a major version with nobody saying so. Adding cbindgen, which wants toml 0.9, did exactly that: please_rust asks for toml 0.8, the bare toml label became 0.9.12, and the tool linked against it. The symptom was one unrelated test failing to parse a lockfile, pointing nowhere near the cause. A root is what somebody declared and what rules depend on by name, so it keeps the bare label. Newest still wins between two roots, and when nothing is a root.
rust_bindgen makes Rust bindings from a C header. The reverse had no
rule, so C calling into a staticlib hand-wrote the declarations and
nothing checked them against the Rust definitions.
The same shape as rust_bindgen: cbindgen is a published crate, declared
by rust_repo and aliased through CbindgenTool, so the supply chain stays
in the graph. cbindgen parses the source rather than compiling it, so
the rule needs no toolchain and no dependencies.
test/cc_interop generates its header now rather than carrying one, and
main.c includes it instead of declaring ffi_add by hand. Adding a
parameter on the Rust side turns into
main.c:6:30: error: too few arguments to function 'ffi_add'
where before it compiled and crashed at run time.
The table claimed rules_rust supported Rust to C headers. It does not. Its extensions are bindgen, mdbook, prost, pyo3 and wasm_bindgen, and a code search over the repository returns no hits for cbindgen at all, against 23 for rust_bindgen and 41 for wasm_bindgen, so the search works and the absence is real. Cargo's row is right as it stands: cbindgen is called from build.rs as a library, generated by the crate being built rather than as an artifact another rule can depend on.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #25.
The gap
rust_bindgenmakes Rust bindings from a C header. The reverse had no rule, so C calling into astaticlibhand-wrote the declarations:The rule
The same shape as
rust_bindgen, which is what #25 asked for: cbindgen is a published crate, declared byrust_repoand aliased through aCbindgenToolconfig knob, so the supply chain stays in the graph.cbindgen parses the source rather than compiling it, so the rule needs no toolchain and no dependencies: a crate's
extern "C"surface is decided by its own text. That makes this rule considerably simpler thanrust_bindgen, which needs libclang.Supports
lang(c / c++ / cython, picking the extension), acbindgen.toml, extra flags, andsrcsfor a crate whose root declares modules.How you know it works
test/cc_interopgenerates its header now rather than carrying one, andmain.cincludes it instead of declaringffi_addby hand.plz run //test/cc_interop:uses_ruststill printsrust says 42.Adding a parameter on the Rust side:
Before, that compiled and crashed at run time. That is the property #25 asked for.
A bug this surfaced, fixed in its own commit
lock --add cbindgenbroke an unrelated test with a confusing TOML parse error. cbindgen wants toml 0.9;please_rustasks for toml 0.8. Both were declared correctly as separate buckets, but:The bare label went to whatever was newest. So
//third_party/crates:tomlbecame 0.9.12, andplease_rust, which names that label, silently moved a major version. Any first-party rule depending on a crate by its bare name is exposed to this the moment a new major arrives as an indirect dependency of something unrelated.A root is what somebody declared and what rules depend on by name, so it now keeps the bare label. Newest still wins between two roots and when nothing is a root. Covered by a test.
192 tests pass, clippy clean, fmt clean.
Next: #26 wasm-bindgen, then both exercised in rust-corpus.