Added rule_cbindgen
#392
Added rule_cbindgen
#392
Conversation
|
There are a few TODO:
|
|
Can you put the generated files in a separate commit? |
|
@damienmg Yeah, I'll split up the commit. Also, do you know if documentation is generated or hand written in the /docs directory? |
|
For the documentation, you just have to run https://github.com/bazelbuild/rules_rust/blob/master/docs/update_docs.sh and that should just work (TM). |
|
I tried that but I'll try it again 😅 |
|
Ok, some things I could use some help with:
diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml
index 7ae5fed..2517ac0 100644
--- a/.bazelci/presubmit.yml
+++ b/.bazelci/presubmit.yml
@@ -53,8 +53,6 @@ tasks:
- "-@examples//ffi/rust_calling_c:matrix_dylib_test"
- "-@examples//ffi/rust_calling_c:matrix_dynamically_linked"
- "-@examples//ffi/rust_calling_c/simple/..."
+ # TODO: Remove
+ - "-@examples//cbindgen/..."
build_targets: *windows_targets
test_targets: *windows_targets
examples:
edit: I figured out how to generate docs. There should be a guide for this somewhere in this repo... (I may open a PR later) |
It seems like it select the target |
|
My apologieze to anyone subscribed to this pull request in case all my commits spammed notifications somehow. I fixed what I could for Windows. I wasn't able to get the All that being said though, I think this PR is ready for some review. I'll look into a java example later for |
|
@damienmg Could I ask you to be my reviewer or recommend one? 😄 |
|
Assigning Marc as he has more knowledge than me around C / rust interactions. |
cbindgen/cbindgen.bzl
Outdated
| system_includes = rust_compilation_context.system_includes, | ||
| ) | ||
|
|
||
| # Return all providers given by `rust_library` to ensure compatiblity with other rules |
There was a problem hiding this comment.
Why do we need to do that? Should this say cc_library?
There was a problem hiding this comment.
rust_cbindgen_library should effectively be transparent to other rules, in my opinion. It's job is only to inject a header into the outputs of a rust_library. So I thought the providers returned by the rule should match a rust_library
There was a problem hiding this comment.
I still think it would be weird for another rust_library to depend on this.
It would make more sense if this was generated on-demand when a cc_library consumed a rust_library, but I have no idea how to do that. Maybe @damienmg knows something about that sort of thing?
There was a problem hiding this comment.
That actually sounds more ideal. All I want is to use a rust_library target with other targets that are compatible with cc_library. I do feel it's nicer to forward all the providers in this case though to avoid potential situations where people want the header file generated by this rule as well as other crate info. But yeah, I'd love to discuss a solution where rust_library just has a generated header.
There was a problem hiding this comment.
I actually think this is worth getting more input on. @smklein @damienmg would you guys also be able to chime in on this? I think it would be fantastic if rust_library had a compatible header file such that it could be used in all places a cc_library was expected. I think this could simplify the implementation and be far more impactful and intuitive.
| #[no_mangle] | ||
| pub extern "C" fn struct_into_rust(point: Point) { | ||
| let epsilon = 0.01f32; | ||
| assert!((point.x - 42.0f32).abs() < epsilon); |
There was a problem hiding this comment.
Out of curiosity, shouldn't the result be bitwise identical?
There was a problem hiding this comment.
It is but clippy was complaining about float comparisons so I (likely did the bad thing and) changed the implementation. Any suggestions?
There was a problem hiding this comment.
Looks like you can skip lints, https://stackoverflow.com/questions/55402812/how-to-disable-a-clippy-lint-for-a-single-line-block
There was a problem hiding this comment.
If this is for https://rust-lang.github.io/rust-clippy/master/#float_cmp , there are some examples of how to fix (without the annotation to skip the lint, but that is always an option too).
@UebelAndre , are you still running into issues with clippy beyond the examples directory?
There was a problem hiding this comment.
No, that was the only outstanding issue. I had some complaints about the use of unsafe and the float_cmp. I'll check those out, thanks guys 😄
It looks like the windows build doesn't pick up the rust .so, so potentially something wrong with our CcInfo.. I doubt it has anything to do with cbindgen.
Maybe python would be easier to do, but that's up to you. How does this work with dependencies? eg. If |
I can update the examples to demonstrate that but anything that's doable with
Yeah, I may try that, I don't think java will be too hard but am nervous about swift. I'm curious to know if I there would be a way to limit certain targets to certain platforms such that |
|
@mfarrugi I also feel like the current state of the examples is not the greatest. I tried to reduce code duplication and feel like I ended up over-complicating them. Suggestions are welcome |
I don't see how cbindgen knows how to parse past the current crate, but it seems to work with libc (although that could be special cased and use the rustc private version, or something like that).
The only thing that comes to mind is pasting the .cpp code which might be easier to follow, but kind of obscures that the intent is to test different build parameter permutations. |
|
Adding a separate one, though I could see an argument for adding to it.
…On Sun, Sep 13, 2020 at 1:32 AM UebelAndre ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In cbindgen/cbindgen.bzl
<#392 (comment)>:
> + "lang": attr.string(
+ doc = "Optional target language identifier of the generated header file",
+ values = [
+ "c",
+ "c++",
+ ],
+ default = "c++",
+ ),
+ "config": attr.label(
+ doc = "Optional Cbindgen configuration template",
+ allow_single_file = True,
+ ),
+ "substitutions": attr.string_dict(
+ doc = "Optional substitutions for the cbindgen config template passed to 'config'",
+ ),
+ "cbindgen": attr.label(
Are you suggesting I add to that toolchain or make a new one similar to it?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#392 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAROG6ELM6WFFIKGOPLIK43SFRKPPANCNFSM4QSN2H6Q>
.
|
|
@mfarrugi How do you feel about this rule just being a Part of |
|
I don't like adding (hard) dependencies to the main rules, for many users
they will not need bindgen/cbindgen/protoc etc, and for those that do
configuring them can be kind of painful.
I suspect it's less painful if you're using nixpkgs, but I don't think that
is ubiquitous just yet.
…On Sun, Sep 13, 2020 at 7:58 AM UebelAndre ***@***.***> wrote:
@mfarrugi <https://github.com/mfarrugi> How do you feel about this rule
just being a Part of rust_library? Something that optionally gets done
for libraries of crate type cdylib andstaticlib?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#392 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAROG6FFYUXHESYSBFYXFBLSFSXVNANCNFSM4QSN2H6Q>
.
|
|
Finally got around to working on this again. The thing I'm trying to figure out is how to handle transitive |
|
@mfarrugi Do you have any suggestions on how to run |
| @@ -0,0 +1,7 @@ | |||
| #if os(Linux) | |||
There was a problem hiding this comment.
Also, I may delete the swift examples. They're not currently hooked up because I don't know how to restrict them to mac/linux and I want to make sure bazel build //... and bazel test //... works on all platforms for the example. It's upsetting that that currently does not work . Don't want to grow the issue. If anyone has any suggestions on how to achieve this, I'd love to hear them
|
I think it makes sense to generate the Cargo.toml files for this and other
tooling that expects them. I don't know how well that will work, or if
there's an easier way to communicate with cbindgen at this point, but
that's what I had in mind before.
mfarrugi@4887e9a
is code to generate Cargo.toml in an aspect, I believe I was using it for
local IDE support.
master...mfarrugi:c-bind-gen
is a previous cbindgen attempt, referenced in the original issue. Looks
like it didn't make it because it didn't work sandboxed, and I didn't need
it at the time.
…On Sun, Oct 18, 2020 at 10:39 PM UebelAndre ***@***.***> wrote:
@mfarrugi <https://github.com/mfarrugi> Do you have any suggestions on
how to run cbindgen against a crate defined in Bazel? cbindgen seems to
act either on a crate root file or a Cargo.toml file (for extra context).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#392 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAROG6FWKRWF4IOBBPWCHUTSLORGNANCNFSM4QSN2H6Q>
.
|
|
@mfarrugi would you mind explaining aspects to me? I don't think I ever quite understood them |
|
Aspects are a way of extending rules with additional actions that are not
part of the rule, ie. they run when the aspect is invoked via command line.
The clippy aspect is a good example: [implementation](
https://github.com/bazelbuild/rules_rust/blob/9279938e7aa2afc85b7fe7dd367f54095d1468a4/rust/private/clippy.bzl#L125),
[doc](
https://github.com/bazelbuild/rules_rust/blob/9279938e7aa2afc85b7fe7dd367f54095d1468a4/rust/private/clippy.bzl#L145-L174).
At a high level this is just "For each rust rule, run clippy."
We also have an aspect for protobuf that is more complicated, and is used
as part of the build (ie. it is not only run by command line invocation).
…On Mon, Oct 19, 2020 at 12:08 AM UebelAndre ***@***.***> wrote:
@mfarrugi <https://github.com/mfarrugi> would you mind explaining aspects
to me? I don't think I ever quite understood them
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#392 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAROG6EC4XG5QDXL4JGXQOTSLO3T5ANCNFSM4QSN2H6Q>
.
|
|
I don't know anytthing about `cargo metadata`, but I'm sure anything is
possible. We could generate the Cargo.toml and then call `cargo metadata`,
but that doesn't sound like it helps :)
…On Sat, Oct 31, 2020 at 10:39 PM UebelAndre ***@***.***> wrote:
@mfarrugi <https://github.com/mfarrugi> is there a way for the bazel
rules to generate something similar to the output of cargo metadata? This
can also be used by cbindgen
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#392 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAROG6AUBASTD65CQSCFOV3SNTC6HANCNFSM4QSN2H6Q>
.
|
|
lol that doesn't really help. There needs to be a way to generate cargo metadata in order to use |
|
Just pinging my last question 😅 |
|
You can either generate the Cargo.toml from the regular rules or in an aspect for cbindgen, the implementation should be mostly the same for either approach. |
|
Currently running into a very frustrating issue: Given the current implementation, I don't understand why I'd be receiving this error. @mfarrugi if you have a free moment, I could definitely use a second pair of eyes. |
|
Doesn't mean anything to me, looks like generic sandboxing issues if it
can't open a particular file.
…On Sun, Nov 8, 2020 at 10:18 PM UebelAndre ***@***.***> wrote:
Currently running into a very frustrating issue:
ERROR: /Users/user/Code/rules_rust/examples/cbindgen/examples_cc/BUILD.bazel:116:22: Generating C++ C/C++ bindings for 'cbindgen/examples_cc/transitive_repr_c_cbindgen.h'.. failed (Exit 1): sandbox-exec failed: error executing command
(cd /private/var/tmp/_bazel_user/1426ad6c00f8f7b996ddc75bef70871c/sandbox/darwin-sandbox/2072/execroot/examples && \
exec env - \
PATH='/Users/user/Library/Caches/bazelisk/downloads/bazelbuild/bazel-3.7.0-darwin-x86_64/bin:/Users/user/.cargo/bin:/Users/user/.cargo/bin:/Library/Frameworks/Python.framework/Versions/3.8/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Applications/VMware Fusion.app/Contents/Public:/Library/Apple/usr/bin:/Applications/Wireshark.app/Contents/MacOS' \
TMPDIR=/var/folders/hf/phjl9q7501gb5wt_qr4ltn3m0000gn/T/ \
/usr/bin/sandbox-exec -f /private/var/tmp/_bazel_user/1426ad6c00f8f7b996ddc75bef70871c/sandbox/darwin-sandbox/2072/sandbox.sb /var/tmp/_bazel_user/install/3ac6e4bc67346f686d73708a27e81d33/process-wrapper '--timeout=0' '--kill_delay=15' bazel-out/host/bin/external/rules_rust_cbindgen__cbindgen__0_15_0/cargo_bin_cbindgen --config bazel-out/darwin-fastbuild/bin/cbindgen/examples_cc/transitive_repr_c_cbindgen.cbindgen.toml --output bazel-out/darwin-fastbuild/bin/cbindgen/examples_cc/transitive_repr_c_cbindgen.h bazel-out/darwin-fastbuild/bin/cbindgen/examples_cc/transitive_repr_c_lib.cargo) sandbox-exec failed: error executing command
(cd /private/var/tmp/_bazel_user/1426ad6c00f8f7b996ddc75bef70871c/sandbox/darwin-sandbox/2072/execroot/examples && \
exec env - \
PATH='/Users/user/Library/Caches/bazelisk/downloads/bazelbuild/bazel-3.7.0-darwin-x86_64/bin:/Users/user/.cargo/bin:/Users/user/.cargo/bin:/Library/Frameworks/Python.framework/Versions/3.8/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Applications/VMware Fusion.app/Contents/Public:/Library/Apple/usr/bin:/Applications/Wireshark.app/Contents/MacOS' \
TMPDIR=/var/folders/hf/phjl9q7501gb5wt_qr4ltn3m0000gn/T/ \
/usr/bin/sandbox-exec -f /private/var/tmp/_bazel_user/1426ad6c00f8f7b996ddc75bef70871c/sandbox/darwin-sandbox/2072/sandbox.sb /var/tmp/_bazel_user/install/3ac6e4bc67346f686d73708a27e81d33/process-wrapper '--timeout=0' '--kill_delay=15' bazel-out/host/bin/external/rules_rust_cbindgen__cbindgen__0_15_0/cargo_bin_cbindgen --config bazel-out/darwin-fastbuild/bin/cbindgen/examples_cc/transitive_repr_c_cbindgen.cbindgen.toml --output bazel-out/darwin-fastbuild/bin/cbindgen/examples_cc/transitive_repr_c_cbindgen.h bazel-out/darwin-fastbuild/bin/cbindgen/examples_cc/transitive_repr_c_lib.cargo)
ERROR: Parsing crate `libc`: cannot open file `/private/var/tmp/_bazel_user/1426ad6c00f8f7b996ddc75bef70871c/sandbox/darwin-sandbox/2072/execroot/examples/bazel-out/darwin-fastbuild/bin/external/cbindgen_cc_examples__libc__0_2_79/libc.cargo/../../../../../../external/cbindgen_cc_examples__libc__0_2_79/src/lib.rs`.
ERROR: Couldn't generate bindings for bazel-out/darwin-fastbuild/bin/cbindgen/examples_cc/transitive_repr_c_lib.cargo.
Given the current implementation, I don't understand why I'd be receiving
this error.
@mfarrugi <https://github.com/mfarrugi> if you have a free moment, I
could definitely use a second pair of eyes.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#392 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAROG6BAYHL3KAEWYMJLZODSO5NP3ANCNFSM4QSN2H6Q>
.
|
|
Yeah, I'm not sure why the source from the external crates are not available for this action. If I iterate over and print the |
|
Ok, I think I got something that mostly works but in order to parse transitive dependencies a |
|
I think generating Would also love thoughts from @damienmg on this |
|
Ok, so the thing I'm thinking of doing at this point after #520 is to make a dedicated toolchain for cargo based on what's already a part of |
|
I write an example of this using swift but will not be including it since the rules don't play nicely with Windows. So I'll put it here in case anyone was interested. examples/cbindgen/examples_swift/BUILD.bazel"""Example of `rust_cbindgen_library` rule with a Swift test binary"""
load("@io_bazel_rules_rust//rust:rust.bzl", "rust_library")
load("@io_bazel_rules_rust//cbindgen:cbindgen.bzl", "rust_cbindgen_library")
load("@build_bazel_rules_swift//swift:swift.bzl", "swift_test")
rust_library(
name = "cbindgen_example_lib",
srcs = ["@examples//cbindgen:rust_srcs"],
crate_type = "staticlib",
)
rust_cbindgen_library(
name = "cbindgen_example",
lang = "c",
lib = ":cbindgen_example_lib",
tags = [
"swift_module=CBindgenExample",
],
)
swift_test(
name = "cbindgen_example_test",
srcs = [
"CBindgenExample.swift",
"main.swift",
],
deps = [
":cbindgen_example",
],
)examples/cbindgen/examples_swift/main.swift#if os(Linux)
import XCTest
XCTMain([
testCase(CBindgenExample.allTests),
])
#endifexamples/cbindgen/examples_swift/CBindgenExample.swiftimport XCTest
import CBindgenExample
class CBindgenSwiftExample: XCTestCase {
func testInt() {
XCTAssertEqual(CBindgenExample.int_from_rust(), 1337);
CBindgenExample.int_into_rust(9999);
}
func testString() {
var buffer: UnsafeMutablePointer<Int8>?;
let size = CBindgenExample.string_from_rust(&buffer);
XCTAssert(size > 0);
XCTAssertEqual(String(cString: buffer!), "Hello Fellow Rustacean!");
CBindgenExample.free_rust_string(buffer);
CBindgenExample.string_into_rust("Hello World!")
}
func testStruct() {
let point = CBindgenExample.struct_from_rust();
XCTAssert((point.x - 3.14) < 0.01);
XCTAssert((point.y - 42.0) < 0.01);
var point_swift = CBindgenExample.Point2D();
point_swift.x = 42.0;
point_swift.y = 3.14;
CBindgenExample.struct_into_rust(point_swift);
}
static var allTests = [
("testInt", testInt),
("testString", testString),
("testStruct", testStruct),
]
} |
|
running into one last error on windows: But I don't have windows hardware to debug this 😢 |
| """ | ||
| return target.label.workspace_root.startswith("external") | ||
|
|
||
| def _clone_external_crate_sources(ctx, target): |
There was a problem hiding this comment.
This should ideally be unnecessary but when resolving the generaged Cargo manifest for external crates, the sources could not be found. This copies them so cbindgen (and ideally other rules) can locate those files.
| # file does not work off of a crates extern/use crates | ||
| # declared in source. To include transitive symbols, a | ||
| # custom config must be created until this feature is | ||
| # implemented: https://github.com/eqrion/cbindgen/issues/7 |
There was a problem hiding this comment.
I tried finding some smarter ways to transitively gather symbols based on either how the dependencies were defined and by parsing some rust sources but ultimately mozilla/cbindgen#7 is the correct solution here. If users need symbols defined in other crates, they'll have to use something like the following
[parse]
parse_deps = true
clean = true
extra_bindings = [
# deps go here
]
|
@mfarrugi Ok! This is ready for review now! 🎉 😄 |
|
@smklein @illicitonion @damienmg Also, would any of you be able to review this as well? |
|
Oh, and just to share, there seems to be a pretty neat alternative at https://github.com/dtolnay/cxx/tree/master/tools/bazel Interested in hearing some thoughts on that vs the |
|
Gonna close this. Doesn't look like there's any interest in this rule and I don't have the bandwidth to continue to try to push this forward. I've attached a dump of this branch below in case I decide to delete the branch from my fork. The rules here work and have tests based on the feedback in this PR. I hope someone finds this useful 😄 |
This pull request adds a
rust_cbindgenrule for generating C/C++ bindings to Rust code. (see #381)(I'll update the description of this pull-request once I'm able to figure out how to properly generate documentation)