Leverage Cargo workspaces in rustbuild #37016
Conversation
|
r? @brson |
|
(rust_highfive has picked a reviewer for you, use r? to override) |
|
☔ The latest upstream changes (presumably #36945) made this pull request unmergeable. Please resolve the merge conflicts. |
This involves hacking the code used to run cargo test on various packages, because it reads Cargo.lock to determine which packages should be tested. This change implements a blacklist, since that will catch new crates when they are added in the future.
alexcrichton
force-pushed
the
workspaces
branch
from
5b34d96 to
6a4952c
Compare
|
r? @japaric |
| @@ -268,51 +293,64 @@ pub fn krate(build: &Build, | |||
| compiler: &Compiler, | |||
There was a problem hiding this comment.
this function has a doc comment that mentions "Cargo.lock"; that part should be changed to say cargo metadata
| // Right now jemalloc is our only target-specific crate in the sense | ||
| // that it's not present on all platforms. Custom skip it here for now, | ||
| // but if we add more this probably wants to get more generalized. | ||
| if crate_name.contains("jemalloc") { |
There was a problem hiding this comment.
I take that the cargo metadata approach handles more gracefully because jemalloc is not listed as a dep on targets that don't use it?
There was a problem hiding this comment.
Ah oops, definitely needs to be carried over!
| let prefix = "name = \""; | ||
| if !line.starts_with(prefix) { | ||
| let mut visited = HashSet::new(); | ||
| let root_pkg = output.packages.iter().find(|p| p.name == root).unwrap(); |
There was a problem hiding this comment.
I'm not exactly sure how this function integrates with the rest of the testing infrastructure. But it seems that if you "test" std_shim and then "test" test_shim you'll end up testing twice the dependencies are appear in the DAG of both root crates, like core? Perhaps that scenario doesn't happen or perhaps that's the current behavior. Just want to check that the behavior's not changing.
There was a problem hiding this comment.
Ah yeah there are actually no edges between these three crates to test. In a sense they're all disconnected graphs.
(note that we want there to be edges, we're just not quite there yet!
This updates the commit to use workspaces to use `cargo metadata` instead of hardcoded lists about what to test. This should help us be resilient to updates in the future on behalf of the crate DAG and minimize the amount of files that need to be touched.
alexcrichton
force-pushed
the
workspaces
branch
from
6a4952c to
147e2da
Compare
|
@bors: r=japaric |
|
📌 Commit 147e2da has been approved by |
Leverage Cargo workspaces in rustbuild This is a continuation of rust-lang#36032 which implements the change to use `cargo metadata` to learn about the crate graph.
This is a continuation of #36032 which implements the change to use
cargo metadatato learn about the crate graph.