Make running cbindgen with own create expansion from build.rs #371
Conversation
|
The build failure seems to be env related and not related to this change. May someone please restart the build? |
| @@ -82,6 +82,10 @@ pub fn expand( | |||
| cmd.env("CARGO_TARGET_DIR", out_dir.join("expanded")); | |||
| } | |||
|
|
|||
| // Set this variable so that we don't call it recursively if we expand a crate that is using | |||
| // cbindgen | |||
| cmd.env("_CBINDGEN_IS_RUNNING", "1"); | |||
There was a problem hiding this comment.
This is a bit hacky... Though I don't have better ideas.
There was a problem hiding this comment.
Yes, it's super hacky, but it's also the best idea I had.
src/bindgen/cargo/cargo_expand.rs
Outdated
| } else { | ||
| // Don't use the default dir, else we could end up in a deadlock when cbindgen is run from | ||
| // a build.rs file | ||
| let out_dir = PathBuf::from(&env::var("OUT_DIR").unwrap()); |
There was a problem hiding this comment.
Doesn't this break CLI usage? There's no guarantee that OUT_DIR exists when called from the CLI.
Can we instead check the return value of env::var and only set it if OUT_DIR exists?
Or am I missing something?
There was a problem hiding this comment.
I haven't thought about CLI use, you are right. I'll add this check.
When crates should get expanded, then cargo is run again from within cbindgen. When cbindgen is started from a build.rs file, this means that cargo is starting another cargo run. Cargo locks the directory it is running on. If two cargos spawn by each other run with the same target directory, then they both want to accquire a lock and hence deadlock. This commit fixes the problem with running the spawned cargo at a different directory. Please note that this will always be the same directory, hence any subsequent runs will be faster (as you would exepct it to be). This is part of mozilla#347.
To expand a crate, cbindgen calls cargo on that crate. cbindgen can be called from a build.rs file. But if cargo is called again, it will also call cbindgen again and hence end up in an endless recursion. This commit makes sure that cbindgen isn't called again if it is already running. You can verify this fix with this minimal example https://github.com/vmx/cbindgen-expand-bug Fixes mozilla#347.
|
The build failure seems again env related. I ran |
|
Yeah, this looks good, not sure what's up with Travis... |
|
Ok, I retriggered travis and seems much happier now. |
Changelog:
## 0.9.1
* Various improvements to comment output. mozilla/cbindgen#370 / mozilla/cbindgen#375.
* Fixed expand when ran from build.rs. mozilla/cbindgen#371
* More debugging output for expansion. mozilla/cbindgen#383
* New option to add a default private constructor in C++ tagged enums. mozilla/cbindgen#377
* Syn and related dependencies updated to 1.0. mozilla/cbindgen#379
With the current version of cbindgen it's not possible to have macros in the crate cbindgen is triggered from if you run cbindgen programmatically via build.rs. This issue was reported as #347. This PR fixes that problem.
I've create two commits as it is kind of two issues that need a fix.
Commit 1: Run cargo expand outside the normal target directory
When crates should get expanded, then cargo is run again from within cbindgen.
When cbindgen is started from a build.rs file, this means that cargo is starting
another cargo run.
Cargo locks the directory it is running on. If two cargos spawn by each other
run with the same target directory, then they both want to accquire a lock and
hence deadlock.
This commit fixes the problem with running the spawned cargo at a different
directory. Please note that this will always be the same directory, hence
any subsequent runs will be faster (as you would exepct it to be).
Commit 2: Don't call cbindgen recursively
To expand a crate, cbindgen calls cargo on that crate. cbindgen can be called from a build.rs
file. But if cargo is called again, it will also call cbindgen again and hence end up in an
endless recursion.
This commit makes sure that cbindgen isn't called again if it is already running.
You can verify this fix with this minimal example https://github.com/vmx/cbindgen-expand-bug