Add a cargo_build_script_run rule #320
Conversation
|
cc @smklein I finally got time to update my build script PR. Would that works for you? We could also add an additional rule to provide the various file directly. If we do so I would be up to deprecate also the out_dir attribute for rules_rust, as they would conflict (and the logic there is bit hairy to be honest). |
damienmg
force-pushed
the
cargo_build_script
branch
2 times, most recently
from
9322fbb
to
0cac7ee
Compare
|
A possible alternative: println! is a macro; can we just redefine it to write to a file in OUT_DIR? (I don't know Rust very well so it seems like it should be possible, but I'm not sure how to do it.) |
There is a bit more than that to do. To fix the data access, we have to change also the root directory, and adjust all the path to be absolute. I would expect also to do some wrapping code to redefine the println output, so basically the same as what it is doing (in addition to the need to parse the output nevertheless). |
damienmg
force-pushed
the
cargo_build_script
branch
from
cfad5dd
to
2715e9e
Compare
damienmg
requested review from
smklein
and removed request for
acmcarther and
mfarrugi
damienmg
force-pushed
the
cargo_build_script
branch
from
9f8af17
to
4e81711
Compare
This rule allows to run a build.rs file (cf. https://doc.rust-lang.org/cargo/reference/build-scripts.html)a get its output (environment variable, rust flags and generated flags) and pass it to `rust_binary` and `rust_library` in order to build crate with build script.
This is not logical to allow multiple of those and create a hairy logic.
The binary will now run from the workspace root of the workspace they belong too which is also the root of the cargo package when importing with cargo raze.
This create a simple API for executing a build_script by just exposing `cargo_build_script` which behaves like a rust_binary except creating a This creates an intermediate target `<name>_script_` that compiles the binary and the final target is for executing the binary. This could be joined as one rule by calling the `_rust_binary_impl` in addition to the `_cargo_build_script_run`.
fs::canonalize convert a relative path to an absolute one so there is no need for a custom method. Addressing review comment.
This might be a bit verbose for directive that are voluntarely ignored but will be better for catching not yet supported directive. Note that directive that are currently ignored on purposely are still silently ignored.
Also removed the unused popd argument (code review).
It seems like RBE is not necessarily creating the output directory, this should fix it.
This reverts commit 4e81711. RBE succeed now.
damienmg
force-pushed
the
cargo_build_script
branch
from
80bea43
to
91f5b13
Compare
|
Ok this is now passing all the tests having not received any more comment I'll go ahead and merge it. |
|
This is super exciting, thanks for putting it together! (I found this after writing something very similar and preparing to make a PR for it, but yours is much better than mine was going to be!) Do you have any plans to modify |
|
This is the plan. The actual plan I had originally was to totally replace cargo raze but at the time there was no correct way to handle feature propagation so I kind of side-track this effort until @smklein sent out a change that was in that direction. If you want to go ahead and send a PR to cargo-raze I would be more than willing to let you do it ;-) |
I'm familiar with the cargo-raze codebase; happy to take this on! |
Instead of a custom genrule. See bazelbuild/rules_rust#320
I've actually just put this together - see google/cargo-raze#166 :) |
This rule allows to run a build.rs file (cf. https://doc.rust-lang.org/cargo/reference/build-scripts.html)
get its output (environment variable, rust flags and generated flags) and pass it to
rust_binaryandrust_libraryin order to build crate with build script.