Decrease src/rust/engine/run-all-tests.sh overhead. #4908
Conversation
|
|
||
| exit_code=0 | ||
|
|
||
| for crate in $(find ${here} -name Cargo.toml); do | ||
| ( | ||
| echo >&2 "Running tests for ${crate}:" | ||
| export RUST_BACKTRACE=1 | ||
| "${CARGO_HOME}/bin/cargo" test --manifest-path=${crate} | ||
| RUST_BACKTRACE=1 PANTS_SRCPATH="${REPO_ROOT}/src/python" run_cargo test ${MODE_FLAG} \ |
There was a problem hiding this comment.
Did not mention this in the description, but specifying the same MODE_FLAG as ./pants does indirectly through bootstrap.sh (ie: --release vs nothing, aka debug), was a part of this puzzle, even after which the build.rs rename and removal of the Cargo.toml [project] build were both necessary to achieve ~0s noop test times.
jsirois
force-pushed
the
jsirois/rust/improve-test-time
branch
from
de5c8ca
to
78bfb09
Compare
|
CI legit failing because the native_engine_version file is wrong Otherwise LGTM I'm kind of unhappy that the rust toolchain exhibits this behaviour :( Being explicit about what a build is going to do should not change its performance characteristics! |
src/rust/engine/Cargo.toml
Outdated
| @@ -9,7 +9,7 @@ authors = [ "Pants Build <pantsbuild@gmail.com>" ] | |||
| # incremntalism is engaged; without it is not and O(10s) is sent no-op recompiling for | |||
| # `cargo test`. | |||
| # | |||
| # build = "src/build.rs" | |||
| # build = "build.rs" | |||
There was a problem hiding this comment.
Why drop the src/? It seemed to be working before...
src/rust/engine/Cargo.toml
Outdated
| # NB: We use the custom build script below, but omit the entry and count on Cargo's default | ||
| # behavior of looking for `build.rs` in the package root and using it (see: | ||
| # http://doc.crates.io/build-script.html). By allowing the default scan proper cargo build | ||
| # incremntalism is engaged; without it is not and O(10s) is sent no-op recompiling for |
There was a problem hiding this comment.
"incrementalism"
Shall I file a ticket about this one upstream? Or is this one close enough? rust-lang/cargo#3076
There was a problem hiding this comment.
It may in fact explain what we're doing wrong. Gimme more time here.
This required both ensuring generated cffi code has a stable mtime for stable contents and that the build script tell cargo which files to change detect against. Additionally, refactor the bootstrap.sh a bit to encapsulate calling a bootstrapped `cargo`.
jsirois
force-pushed
the
jsirois/rust/improve-test-time
branch
from
78bfb09
to
ea962f5
Compare
|
Alright - we're now doing things a bit differently to work with Cargo's change detection. PTAL. Also note the revelations here may have impact on #4910 |
|
Looks good :) I've reflected the cargo tracking into #4910 too :) |
This required both ensuring generated cffi code has a stable mtime for
stable contents and that the build script tell cargo which files to
change detect against.
Additionally, refactor the bootstrap.sh a bit to encapsulate calling a
bootstrapped
cargo.