check-meta.nix: Add package warnings

[piegamesde]

Implements RFC 127 NixOS/rfcs#127

Description of changes

See the RFC

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.

[infinisil]

Don't use types.unspecified, also because it has poor merge semantics. types.raw or types.anything instead. Or better: Specify the full type

[piegamesde]

This is simply copied out of the NixOS module and is only temporary, the full type should be specified in #207187 (comment)

[ConnorBaker]

@piegamesde thank you so much for leading this effort! Are there any blockers preventing further progress on this PR?

I've been doing a somewhat ad-hoc version of this to allow for easier diagnoses of build failures, like so:

nixpkgs/pkgs/development/python-modules/torch/default.nix

Lines 118 to 124 in 49cd2f2

	brokenConditions = attrsets.filterAttrs (_: cond: cond) {
	"CUDA and ROCm are mutually exclusive" = cudaSupport && rocmSupport;
	"CUDA is not targeting Linux" = cudaSupport && !stdenv.isLinux;
	"Unsupported CUDA version" = cudaSupport && !(builtins.elem cudaPackages.cudaMajorVersion [ "11" "12" ]);
	"MPI cudatoolkit does not match cudaPackages.cudatoolkit" = MPISupport && cudaSupport && (mpi.cudatoolkit != cudaPackages.cudatoolkit);
	"Magma cudaPackages does not match cudaPackages" = cudaSupport && (effectiveMagma.cudaPackages != cudaPackages);
	};

nixpkgs/pkgs/development/python-modules/torch/default.nix

Line 503 in 49cd2f2

broken = builtins.any trivial.id (builtins.attrValues brokenConditions);

I would love to see this merged -- let me know if there's any way I can help!

[infinisil]

Me and @piegamesde sat together at NixCon to discuss this, but my conclusion there was that mkDerivation's code is very messy, and this PR makes the mess worse. It would take considerable effort to make it not messy, but I think that's what we need if we want to have any chance of maintaining this over time. And I think currently @piegamesde's priorities are with NixOS/rfcs#166

[piegamesde]

@ConnorBaker This code does a minimal implementation, which I have hopes of getting merged eventually. The current bigger task to do here is QA, especially writing unit tests for the different conditions. That's why I factored out the business logic into its own file, as Infinisil requested back then. Feel free to take on that task, I won't be working on it for a bit probably. Apart from that this change has been heavily review-stalled for a lot of time, so simply watching here and giving review as necessary would be a big help too

[infinisil]

It seems like I'm unintentionally blocking this from progressing. I'm just taking another look, and unless I'm just missing it, the PR changes itself really aren't bad. What I think needs to be done before merging:

• Very important: Have decent tests, there's currently none. Somewhere in pkgs/test would fit.
• Double-Check that this PR actually implements the RFC correctly.
• Try to make reasonably sure that CI (ofborg and Hydra) doesn't break. Can still be reverted if it does end up breaking CI

I'd also really like for somebody to think about how we could clean up check-meta.nix over time, because while this RFC introduces a more consistent mechanism, the old mechanism is also still there for now. I think that's what I was thinking of in the previous comment.

Unfortunately I'm pretty spread out with my availability, so I don't have time to do this myself, I'll dismiss my review and don't intend to interfere with this anymore.

It's okay

[infinisil]

I was asked by @AkechiShiro to help a bit with how to write tests for this, so here's some proof of concept for how it could work: tweag@9b55daf

[AkechiShiro]

Hello all,

Here are the tests cases, I've though of so far, this matrix needs to be made 3 times to cover for each handlers (ignore,warn,error), in total 39 tests would be needed for full coverage for all handlers, in case I'm missing anything feel free to let me know.

So far, I have almost full coverage on the error handler and partial coverage on the warn handler.

Problem kinds	Case 1	Case 2	Case 3	Case 4	Case 5	Case 6	Case 7	Case 8	Case 9	Case 10	Case 11	Case 12	Case 13
Removal	X	X	X		XX			XX	XX		XX	(X)
Maintainerless	X	X		X	XX		XX			XX	XX	(X)
Deprecated	X		X	X	XX	XX			XX	XX		(X)

Note

Case 12 : is each of the problems alone on their own

Thanks a lot for your help for establishing the ground work for the tests @infinisil

[AkechiShiro]

The new PR is ready for review, thanks to all involved