update pkgs/build-support/coq/default.nix to match new best practices

[lolbinarycat]

this file blatantly disregards all best practices, it starts with a top-level nested with, it adds functions into the lib namespace, it messes with derivation meta attributes in strange ways, and not a single line is commented.

cc @vbgl @Zimmi48 @CohenCyril

---

Add a 👍 reaction to issues you find important.

[CohenCyril]

@NixOS/moderation while I believe the contents of this issue is perfectly legit, I'm afraid the phrasing is not "fostering an open and welcoming environment".

I will gladly replace the "top-level nested with" by appropriate inherit (relevant-namespace) relevant-contents, if I get confirmation that this is the desired style, and of course document the file when time allows, unless someone beats me to it. As for the rest I would appreciate pointers to said best practices and constructive feedback on how to improve the file w.r.t. meta and lib while preserving the same features.

[CohenCyril]

Also, please note that the documentation on how to use this, is there: https://nixos.org/manual/nixpkgs/stable/#coq-packages-attribute-sets-coqpackages, so I would advocate for making a reference to this and documenting essentially the implementation details inside pkgs/build-support/coq/default.nix itself.

[mweinelt]

@lolbinarycat Please always remember that there is a person on the receiving end, when creating an issue. We don't require you to sugarcoat issues, but they certainly shouldn't come across as aggressive.

[lolbinarycat]

i am sorry, i did not mean to come of as aggressive. i was merely trying to point out that this file was written before most of these best practices were put in place, and it needs to be updated to match the much stricter code standard nixpkgs has these days.

[mweinelt]

Can you reference where the best practices can be found?

[lolbinarycat]

currently the most concrete guide is a page on nix.dev. i've been working on adding a more concrete guide to nixpkgs itself, but that hasn't been approved yet.

[Zimmi48]

FWIW, the extensions to lib could be useful beyond this context, but we got some backlash when proposing them more widely (#113661). In the end, since there was not enough support to add them globally, they stayed where they currently are, since for managing coqPackages we couldn't live without them (see #113661 (comment)).

[CohenCyril]

currently the most concrete guide is a page on nix.dev. i've been working on adding a more concrete guide to nixpkgs itself, but that hasn't been approved yet.

Thanks for providing this link. I have two questions about this: a general one and a technical one.

1. How and why is this the new best practice guide for nixpkgs? Who designed it and by whom is it endorsed? I would expect to see a reference in the main nixpkgs CONTRIBUTING.md if it were the official best practice.
2. I see in this guide that the only given example of toplevel with is the one for nixpkgs, which makes a lot of sense because it is a moving target, and may be overriden extensively by local derivations with the same names. However with the stability and complementarity between lib and builtins, it looks sensible to use a general with as a way of importing library functions from either (intentionally blurring the difference between the two because functions might jump from one to the other and it looks sensible not to break script just because they do). So if there were an official channel for best practices, I would put this in the balance.

[lolbinarycat]

functions might jump from one to the other and it looks sensible not to break script just because they do

1. if a function moves from lib to builtins, nixpkgs will almost certainly provide an alias
2. a function moving out of builtins could only happen during major nix versions
3. it is entirly possible for two functions to have the same name and different semantics, see pkgs.fetchurl and builtins.fetchurl