cargo-llvm-cov: fix tests, mark broken as needed

[CobaltCause]

Description of changes

This packages cargo-llvm-cov, a tool for generating code coverage reports using built-in features of recent Rust toolchains.

Supersedes #197453
Alternative to #185183

Notably, this PR has all the tests running and passing, and does not wrap the binary, allowing users to choose their own toolchain versions.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 22.11 Release Notes (or backporting 22.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[winterqt]

So the change I want to try here is using fetchurl and postFetch to not only get the lockfile from crates.io, but collapse it into a single FOD to take advantage of caching.

Essentially, use fetchurl on https://crates.io/api/v1/crates/${pname}/${version}/download, with downloadToTemp set to true, and do something like tar xzf $downloadedFile ${pname}-${version}/Cargo.lock; mv */Cargo.lock $out in postFetch:

fetchurl {
  name = "Cargo.lock";
  url = "https://crates.io/api/v1/crates/${pname}/${version}/download";
  downloadToTemp = true;
  postFetch = ''
    tar xzf $downloadedFile ${pname}-${version}/Cargo.lock
    mv */Cargo.lock $out
  '';
}

Then, copy this into the source in postPatch and cargoUpdateHook, which should then allow us to run tests while not having to include the lockfile.

[winterqt]

Though, now that I say that, I wonder if the compressed source tarball is smaller than the uncompressed Cargo.lock, lol. I'd guess that it's bigger, but who knows with the sourcery that is compression.

[CobaltCause]

Cursed: compress the checked-in Cargo.lock for maximum smallness.

I'll try implementing your suggestion. Is the concern mainly the size of the nixpkgs repository or the size of the stuff required to build the derivation?

[winterqt]

I'd say checking in large autogenerated files is worse than doing this, but at the same time... this is a pretty small lockfile. Hrm.

Unrelated to this, but we currently disable the profiler runtime on non-Linux platforms. Given this, can you set meta.broken = !(stdenv.isLinux && stdenv.targetPlatform.isRedox)? (Maybe there's a better clause to use, not sure if we care about Redox here.)

(If you have Darwin hardware, feel free to try to enable it and test it -- if not, I'll do it sometime this week, I just don't want to have this package have to wait for a staging cycle.)

[CobaltCause]

I'd say checking in large autogenerated files is worse than doing this, but at the same time... this is a pretty small lockfile. Hrm.

Well, good news: your trick worked perfectly. And yeah I agree that checking in autogenerated stuff is suboptimal.

Unrelated to this, but we currently disable the profiler runtime on non-Linux platforms. Given this, can you set meta.broken = !(stdenv.isLinux && stdenv.targetPlatform.isRedox)? (Maybe there's a better clause to use, not sure if we care about Redox here.)

Sure. EDIT: One thing I'm curious about is whether this makes sense/still applies when getting your rust toolchain from somewhere other than nixpkgs, e.g. fenix.

(If you have Darwin hardware, feel free to try to enable it and test it -- if not, I'll do it sometime this week, I just don't want to have this package have to wait for a staging cycle.)

I don't, but I have a friends with x86_64-darwin and aarch64-darwin that I may be able to rope into this.

[winterqt]

@Mic92 @zowoq Thoughts on 09b7abe1a29c4c47982984d8ea092b9c50031048, just to use cargoUpdateHook to copy a Cargo.lock into the derivation?

There's another option we can take here, and that's adding a way to specify a postPatch that gets passed to fetchCargoTarball when using buildRustPackage, but I think this is probably the least impactful (since only one other package even uses cargoUpdateHook) since we already have it (cargoUpdateHook).

[CobaltCause]

Looks like someone else beat us to it: #192813

Do we want to close this, or should I rebase on top of their changes so we can have working tests and a proper meta.broken?

[winterqt]

Let's do that, and adjust the commit message(s?) appropriately, please.

Sorry about forgetting this.

[CobaltCause]

No problem! I can do that, yes. Do we want to leave the changes to fetchCargoTarball or get rid of it since it's not needed anymore?

[winterqt]

Do we want to leave the changes to fetchCargoTarball or get rid of it since it's not needed anymore?

Probably best to move it to a separate PR, so this isn't blocked on any (potential) issues/discussions.

[CobaltCause]

Okay, rebase is done and fetchCargoTarball changes are gone. The diff is pretty big because I just kind of blindly rebased, should I break it into smaller commits?

[figsoda]

oops, merge conflict again

[matthiasbeyer]

I think we can merge this now, can't we?

[CobaltCause]

I mean, I think so, but I'm obviously biased. And it would be nice to get an approval from @winterqt since they've invested a lot of time helping with this and reviewing it though.

Waited for three weeks but no reply.

[matthiasbeyer]

I assume that they won't reply, so I'll merge after some nixpkgs-review call.

[matthiasbeyer]

Result of nixpkgs-review pr 197478 run on x86_64-linux 1

1 package built:

• cargo-llvm-cov