nixpkgs-vet should throw error when package attribute name in by-name is prefixed with a number

[Aleksanaa]

In NixOS/nixpkgs#333281 the package attribute name is prefixed with 1 but nixpkgs-vet agreed with that. This may cause problems for us later, as nix treats "1a", 1a and _1a differently. I edited the title to rerun the check and confirmed that it still didn't throw an error.

[philiptaron]

I see that this does actually work through both nix build .#1fps and nix-build -A 1fps, though, and also works if the attrset name is stringified.

nix-repl> pkgs."1fps"
«derivation /nix/store/b8d1br1rjq4i1yw8w5l71b5c26cqacrz-1fps-0.1.10.drv»

nix-repl> pkgs.1fps
error: undefined variable 'fps'

       at «string»:1:7:

            1| pkgs.1fps
             |       ^

Is the core reason it should be discouraged that a package like this can't be expressed as a dependency in callPackage argument lists?

nix-repl> { 1fps }: null
error: syntax error, unexpected INT

       at «string»:1:3:

            1| { 1
             |   ^

[Aleksanaa]

Is the core reason it should be discouraged that a package like this can't be expressed as a dependency in callPackage argument lists?

...Yes, in other situations we just need quotes.

[infinisil]

Oh this would be nice, and should be simple to implement

[infinisil]

I'll refrain from implementing this, I think this is a great first issue for perhaps a new contributor

[Aleksanaa]

https://github.com/NixOS/nixpkgs-vet/blob/main/src/structure.rs#L16-L19

The correct regex would be:

shard: ^((_[0-9])|([a-z][a-z0-9_-]?))$
package: ^((_[0-9])|[a-zA-Z])[a-zA-Z0-9_-]*$

[Ben-PH]

Feel free to assign me. I'll get started now.

As this will be the first time navigating the code base, any onboarding info would be handy (Structure of code, which parts are most relevant to this, etc).

[philiptaron]

Feel free to assign me. I'll get started now.

Done! Please feel free to open a draft PR as a WIP rather than polishing until "done" -- rough drafts are just fine in these parts. 🙂

Any on-boarding info would be handy, like structure of the code, which parts are most relevant to this, and so forth.

1. Make a Problem or two (see the src/problems directory) for this issue. @Aleksanaa has highlighted a couple of possible problems, one with shards and one with packages.
2. Find the place in the code that checks package names (usually by looking for other Problems that have to do with package names)
3. Write a test for the package (see the tests/ directory and look for a test that is testing the Problem you are patterning after). See that the test fails before your change 🙂
4. Make the change, enabling the warning.
5. Run nix-build -A nixpkgsCheck to check against a recent snapshot of nixpkgs. If there are failures, the check needs to be a ratchet check, not a pass/fail check. If that's the case, come back here and report the failure, or report it on the draft PR and we'll talk next steps.
6. Assuming the nixpkgsCheck is successful, we'll commit the change and do a release, updating nixpkgs CI afterwards.
7. Bask in the glory of a successfully completed GitHub Issue that helps check all of nixpkgs. 🎉

[willbush]

@Ben-PH also consider joining us on matrix https://matrix.to/#/#nixpkgs-architecture:nixos.org ! The room hasn't been too active lately, but I'm going to have more time the next couple months so who knows :)

Also, here is some past context of Nixpkgs Architecture Team https://discourse.nixos.org/t/nixpkgs-architecture-team-conclusion-and-prospective/41020/1