Rust packages are built with --release by default, but the check phase just runs cargo test, which will build tests in debug. This means that all the dependencies of the crate have to be recompiled. We should probably switch that to cargo test --${buildType} so that way it will build release (if the full package is built as release), which should allow it to reuse the already-built dependencies and only recompile the top-level crate. Tests are normally built as debug to make it faster to do the code-build-test cycle during development but there should be no benefit to compiling them as debug during distribution.
I’d submit a PR for this right now but I’m not sure offhand how to go about verifying that all existing test-enabled Rust packages still compile this way; while I wouldn’t expect this to break anything, it’s conceivable that some package could have written its tests in a manner that only works when compiled as debug (which I would consider to be a bug in the package).
I suppose we could just go ahead and make this change and then see if we get any unexpected build failures for rust packages in Hydra, though I don’t know how to actually watch for that.
3 Likes
Doesn’t release disable bounds checks and a few other things? Your tests would be less likely to catch problems if run in release mode.
That’s a good point. It doesn’t disable bounds checks, but it does disable integer overflow/underflow checks, as well as debug_assert!(). Unfortunately Cargo has no way of saying "build all the dependencies with release but build the top-level crate as debug, not that this would work anyway if the package is actually a workspace that splits the top-level crate into multiple local dependencies).
It seems rather unfortunate that we have to significantly slow down compilation of all Rust packages just in case the test suite relies on testing integer overflow or debug_assert!() (especially since Rust packages frequently don’t have substitutes available on macOS). I’m inclined to believe that the vast majority of Rust packages are not testing integer overflow or debug_assert!() in their test suites anyway, so it’s unclear what benefit we’re actually getting from this.
Personally, I feel the pain because I’m maintaining multiple Rust packages and my NixOS machine is rather underpowered (it’s a low-tier Linode box), so I’m frequently waiting for a significant amount of time when updating packages just to verify that it compiles properly on NixOS.