Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lint on broken recursion structure #312345

Open
lf- opened this issue May 17, 2024 · 2 comments
Open

Lint on broken recursion structure #312345

lf- opened this issue May 17, 2024 · 2 comments
Labels
5. scope: tracking Long-lived issue tracking long-term fixes or multiple sub-problems 6.topic: architecture Relating to code and API architecture of Nixpkgs 6.topic: best practices significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc.

Comments

@lf-
Copy link
Member

lf- commented May 17, 2024

Issue description

It's possible to write something like:

{somePackageName, stdenv}:
stdenv.mkDerivation {
  pname = "some-package-name";
  # ...
  passthru.someOtherThing = pkgs.callPackage ./something.nix { inherit somePackageName; };
}

where something in passthru doesn't follow the containing derivation's overrideAttrs properly due to broken recursion structure. The correct code here would be:

{somePackageName, stdenv}:
stdenv.mkDerivation (finalAttrs: {
  pname = "some-package-name";
  # ...
  passthru.someOtherThing = pkgs.callPackage ./something.nix { somePackageName = finalAttrs.finalPackage; };
})

I would like to have an automated lint that rejects code that exhibits this broken recursion structure: passthru referring to the parent derivation but the passthru attrs get the parent derivation from other means than finalAttrs.finalPackage.

@lf- lf- added 6.topic: best practices significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc. labels May 17, 2024
@eclairevoyant
Copy link
Contributor

eclairevoyant commented May 17, 2024

Why? EDIT: makes sense as the first construct won't respect overrides.

Also, shouldn't we ensure all the builders support the finalAttrs pattern in the first place?

@lf-
Copy link
Member Author

lf- commented May 17, 2024

Also, shouldn't we ensure all the builders support the finalAttrs pattern in the first place?

yeah, there's a lot of very broken builders that have very broken overrides in general. we should at least fix the standard mkDerivation though.

@lf- lf- added the 6.topic: architecture Relating to code and API architecture of Nixpkgs label May 20, 2024
@tomodachi94 tomodachi94 added the 5. scope: tracking Long-lived issue tracking long-term fixes or multiple sub-problems label Jun 1, 2024
getchoo added a commit to getchoo-contrib/nixpkgs that referenced this issue Jun 15, 2024
getchoo added a commit to getchoo-contrib/nixpkgs that referenced this issue Jul 12, 2024
getchoo added a commit to getchoo-contrib/nixpkgs that referenced this issue Jul 16, 2024
This should help

- Simplify expressions
- Reduce (some) repition/workarounds
- Improve automated testing by making these checks a requirement to
  build
- Maintain consistency between which of these version check
  implementations are used across pkgs/by-name
- Provide more real-world examples of this newly added hook
- Avoid NixOS#312345 in many places
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5. scope: tracking Long-lived issue tracking long-term fixes or multiple sub-problems 6.topic: architecture Relating to code and API architecture of Nixpkgs 6.topic: best practices significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc.
Projects
None yet
Development

No branches or pull requests

3 participants