Since you note that this needs testing, I tried it on the recently-added zoxide package. (If it's useful to know: I run an "other Linux distribution", Arch, as opposed to NixOS.)

After building cargo, it proceeded to build the project, followed by tests. With this patch, they completed instantly (because there are none to run). I cannot comfortably speak for the Nix code, but the functionality works fine in this case.

running tests
Running cargo cargo test --release --target x86_64-unknown-linux-gnu --frozen --
   Compiling zoxide v0.2.1 (/build/source)
    Finished release [optimized] target(s) in 0.50s
     Running target/x86_64-unknown-linux-gnu/release/deps/zoxide-6631378e48db285a

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

EDIT: If there's anything else I can help with, let me know -- excited to see this land to reduce unnecessary compilation.

@cole-h thanks for taking a look at this!

If there's anything else I can help with, let me know -- excited to see this land to reduce unnecessary compilation.

Well, this might be pretty time-consuming, but in case someone finds a crate (preferably something used in a NixOS-package) that breaks with this patch it would be fairly helpful (I didn't find any, but I don't consider myself an expert in the rust ecosystem though :) )

Looks like a great change in general. Build performance optimizations aside, this is also arguably superior on its own right: we're shipping optimizied binaries built against optimized libraries, but previously we were running the test suite against debug libs. This gets us closer to testing what we're deploying. The fact that it basically halves our compilations is nice too :)

That said, after trying several successfully, I think I found a package that has issues. (It builds on master, and still fails when this PR is rebased onto master.)

@GrahamcOfBorg build zcash

Fix the build issue and also changed the base to staging as there's quite much to rebuild :)

While I was messing with #82982, I noticed that, when combined with this PR, ripgrep would fail to compile because of its preFixup -- it wants to cd $releaseDir/build/ripgrep-*/out, but there are multiple directories that match that glob (most likely due to the tests being compiled in release mode)!

I have no idea how this should be worked around, but it's something that should be dealt with before this merges. This worked in the past (I assume) because there was only 1 possible match in the $releaseDir due to the tests being run in debug mode.

(I guess I didn't properly test this with ripgrep earlier, because it fails similarly on 11.0.2 -- that's a failing on my part and I will try to be more careful about these kinds of assertions in the future.)

EDIT: Originally, I was thinking we could use the --target-dir flag, but this changes where it outputs to as well as where it reads from. This might require some magic.

EDIT2: Or we could just change the cps to cp -n -- the outputs should all be the same, so it shouldn't matter which one gets copied. Maybe this is another instance of something that should be changed to improve robustness.

EDIT4: The real solution (IMHO) is to use the installShellCompletion and installManPage helpers on that globbed path instead of cding to it or cping the files, e.g. installShellCompletions --zsh $releaseDir/build/ripgrep-*/out/_rg. No problems with that solution, so far.

EDIT3: Probably the most annoying side-effect of this change is rg, rg-edd1cb3b1163a7ff, and integration-3cd6ef3798b37d0e get installed to bin/ -- is there any way to reliably detect these hash-appended executables and not copy them?

EDIT3: Probably the most annoying side-effect of this change is rg, rg-edd1cb3b1163a7ff, and integration-3cd6ef3798b37d0e get installed to bin/ -- is there any way to reliably detect these hash-appended executables and not copy them?

Not sure yet. In case I have sufficient time, I will start investigating tomorrow :)

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/speeding-up-rust-app-packaging/6384/6

I wonder if this is actually a good idea, debug doesn’t just compile with fewer optimizations, it also turns on integer overflow checks and debug assertions (which are used throughout the standard library and crates to do more expensive checks). Doing tests with release builds may miss some bugs which are captured by regular testing using debug builds.

<opinion>

In general I see nix tests as a quick check to ensure that you didn't compile things terribly wrong. In this case I think that integer overflow checks should be caught by the application developers. Debug assertions are more difficult to categorize as catching build/configuration issues but I think that in general it is worth the time savings to turn them off. The job of nix isn't to check that the code is perfect, but mostly to ensure that we built it correctly.

Docs have been fixed, thanks @cole-h and @asymmetric !

I'd love to see a final review, after that I'd consider this mergable (will leave a comment, then). /cc @FRidh @vcunat

Sadly I don't have time for a full review/test, but I'll at least comment to say I'm morally in favor of this change.

@cole-h fixed :)

@vcunat @FRidh I'd consider this PR good to go now. Is there anything else to be done from your PoV before merging this to staging?