Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to specify expand for the current crate? #72

Closed
kirbysayshi opened this issue Oct 19, 2017 · 8 comments
Closed

How to specify expand for the current crate? #72

kirbysayshi opened this issue Oct 19, 2017 · 8 comments

Comments

@kirbysayshi
Copy link

I'm using cbindgen via a build.rs script within a root project. Let's call it ffi_playground. I have many macros within ffi_playground that help with making the ffi, including generating structs and functions.

My cbindgen.toml looks like this:

[parse]
parse_deps = false

With this configuration, my macros within ffi_playground (aka my root lib) are not expanded and my header is missing symbols.

If I attempt to force ffi_playground to be expanded:

[parse]
parse_deps = false
expand = ["ffi_playground"]

Now my build never finishes! I get very little output, and watching activity monitor implies an infinite loop somewhere (lots of rustc processes). I have to CTRL+C cargo.

Curiously, if I remove build.rs, and instead use the cbindgen CLI (still using cbindgen.toml), then with the first config I get the same result (incomplete header + warnings about missing symbols), and with the second config I get a complete header but the process takes ~30 seconds (which is a loooong time!).

Am I doing something wrong? My assumption was that macros within the root crate would be expanded automatically, but that seems incorrect with the behavior I'm seeing.

@kirbysayshi
Copy link
Author

While I don't have my own code as an example, this publicly available code displays the same issue: https://github.com/getsentry/symbolic/tree/master/cabi.

To reproduce:

$ git clone git@github.com:getsentry/symbolic.git
$ cd symbolic/cabi
$ cargo build # this will be normal, there is no build.rs
$ time make
RUSTUP_TOOLCHAIN=nightly cbindgen -c cbindgen.toml . -o include/symbolic.h
WARN: Skip symbolic-cabi::LAST_ERROR - (Cannot have a non primitive const definition.)
WARN: Skip symbolic-cabi::LAST_PANIC - (Tuples are not supported types.)

real	1m17.777s
user	2m56.657s
sys	0m15.213s

If you add a build.rs file with the contents from the README from cbindgen, and add cbindgen as a [build-dependency], a subsequent cargo build will never finish.

@eqrion
Copy link
Collaborator

eqrion commented Oct 24, 2017

No I don't think you're doing anything wrong. Specifying expand = ["ffi_playground"] should do the trick.

Curiously, if I remove build.rs, and instead use the cbindgen CLI (still using cbindgen.toml), then with the first config I get the same result (incomplete header + warnings about missing symbols), and with the second config I get a complete header but the process takes ~30 seconds (which is a loooong time!).

Interesting, there might be a bug here with recursive calls to cargo for compiling the root crate. cbindgen expands macro's by essentially calling cargo rustc --pretty=expanded.

Maybe if this is done in the build.rs for the root crate (which is already being compiled by cargo) it trips up and it never finishes? Or maybe will try and run the build.rs again? Would be interesting to try and confirm that.

It seems like this works for you when you use the cbindgen CLI which would give some support to that theory.

The long compile times with the CLI are unfortunately normal, as cargo rustc --pretty=expanded can take a while.

@kirbysayshi
Copy link
Author

Maybe if this is done in the build.rs for the root crate (which is already being compiled by cargo) it trips up and it never finishes? Or maybe will try and run the build.rs again? Would be interesting to try and confirm that.

This seems likely! Perhaps cbindgen is compiling each crate independently when in expand mode?

The long compile times with the CLI are unfortunately normal, as cargo rustc --pretty=expanded can take a while.

It seems like cbindgen is doing more work than it needs to. Does it use a separate target/build dir for each crate that expand is specified? Or perhaps just a separate target/build dir for the overall expand task? A clean build for my project takes about 30 seconds, which is exactly the same amount of time cbindgen takes as well... and is also the same amount of time a clean cargo expand takes. Of course a subsequent cargo expand is less than a second.

Is there a way to force cbindgen to reuse the same build artifacts as cargo build / cargo expand?

@josefalcon
Copy link
Contributor

https://github.com/eqrion/cbindgen/blob/master/src/bindgen/cargo/cargo_expand.rs#L22

It seems like using TempDir is causing this to happen, as the directory is deleted after it goes out of scope. I built a version using a consistent directory in /tmp that was not destroyed, and subsequent invocations of cbindgen are fast.

You can simulate this by running:

CARGO_TARGET_DIR=/tmp/foo cargo rustc -- --pretty=expanded -Z unstable-options

a few times in your project. The first invocation will be slow as it compiles all the necessary artifacts, but the subsequent calls will be fast.

Then run,

CARGO_TARGET_DIR=/tmp/bar cargo rustc -- --pretty=expanded -Z unstable-options

and you'll see it's like running it for the first time again (notice the target directory is /tmp/bar and not /tmp/foo).

@eqrion
Copy link
Collaborator

eqrion commented Nov 9, 2017

I initially added use of tempdir as webrender in Firefox/Gecko uses a custom build system, so the default target dir for cargo rustc was causing problems.

I think this can be revisited and either be controlled by a flag or figure out how to use the correct target dir in Firefox/Gecko.

@eqrion
Copy link
Collaborator

eqrion commented Nov 14, 2017

@kirbysayshi Is this still an issue or has this been resolved?

@kirbysayshi
Copy link
Author

I haven't retested with a build.rs yet, but setting CARGO_EXPAND_TARGET_DIR from #81 definitely speeds up subsequent runs!

@eqrion
Copy link
Collaborator

eqrion commented Dec 14, 2017

Okay I will close this for now, feel free to file a new issue if this problem reoccurs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants