Allow yk to be consumed like a normal Rust dependency. #227
Conversation
At the moment I don't seem to require |
Afraid we need to specify this again. |
README.md
Outdated
| This repo uses xtask instead of regular cargo, so instead of running `cargo | ||
| <target>`, you will instead need to execute `cargo xtask <target>`. | ||
| This repo contains two separately-compiled workspaces, and thus the build | ||
| system is a little non-standard. To act recursively on both workspaces, instead |
There was a problem hiding this comment.
All this talk about "recursion" seems to me like an implementation detail that isn't of much importance to whoever indents to use/build this. Should we simplify these descriptions by removing any mention to recursion and just say something like: "To build both workspaces, use cargo xtask target instead of cargo target."
There was a problem hiding this comment.
I did start with something like that, but I thought people would ask "so how comes it works when I consume yk? Dependencies can't run xtask!".
There was a problem hiding this comment.
I don't know why anyone would ask this. The README says this is how you build it so that's what you have to do. ;-) And if they are still interested they can go read the code and its comment. My worry is that it's more likely to confuse people reading the README about what any of this means and why they should know about it, if all they want to do is build the thing.
|
I think this addresses all review comments. |
| This repo contains two separately-compiled workspaces, and thus the build | ||
| system is a little non-standard. To act recursively on both workspaces, instead | ||
| of running `cargo <target>`, instead execute `cargo xtask <target>`. | ||
| To work with this repository, instead of running `cargo <target>`, instead |
There was a problem hiding this comment.
Didn't you say offline that you need to use cargo build without xtask if you just want to build? Or can I just always use xtask in either case?
There was a problem hiding this comment.
You can do either, but it's simpler to say "always use xtask".
| system is a little non-standard. To act recursively on both workspaces, instead | ||
| of running `cargo <target>`, instead execute `cargo xtask <target>`. | ||
| To work with this repository, instead of running `cargo <target>`, instead | ||
| execute `cargo xtask <target>`. | ||
|
|
||
| You will also need to add the `-C tracer=<kind>` flag to `RUSTFLAGS` to tell | ||
| the build system what kind of tracer you want to use (`hw` or `sw`, although |
There was a problem hiding this comment.
There's still a mention of recursively below.
|
Try this. |
|
If @bjorn3 is happy, please squash. |
For the `cargo build` target, we can make `build.rs` recursively run `cargo build` in the internal workspace and then symlink `libykshim.so` into the consumer's target directory. This avoids the need for consumers to have special build logic. We have to keep xtask around for running other targets (`test`/`fmt`/...) recursively on both workspaces. I had hoped to have `build.rs` do this automatically too, but we would need to know which target has been requested and this information isn't exposed to `build.rs`. Tested with `ykbf`, which can now be built with simply: ``` RUSTFLAGS="-C tracer=hw" cargo build ``` Whereas previous attempts had tried using complicated wrapper scripts, which each consumer in turn would need to duplicate.
vext01
force-pushed
the
build-sys-again
branch
from
be3a5b5 to
1aa3948
Compare
|
Splat. |
|
bors r+ |
|
Build succeeded: |
For the
cargo buildtarget, we can makebuild.rsrecursively runcargo buildin the internal workspace and then symlinklibykshim.sointo the consumer's target directory. This avoids the need for consumers
to have special build logic.
We have to keep xtask around for running other targets
(
test/fmt/...) recursively on both workspaces. I had hoped to havebuild.rsdo this automatically too, but we would need to know whichtarget has been requested and this information isn't exposed to
build.rs.Tested with
ykbf, which can now be built with simply:Whereas previous attempts had tried using complicated wrapper scripts,
which each consumer in turn would need to duplicate.