Skip to content

various: remove by-name overrides (part 4)#483859

Merged
jopejoe1 merged 4 commits intoNixOS:masterfrom
mdaniels5757:mdaniels5757-remove-by-name-overrides-part-4
Jan 27, 2026
Merged

various: remove by-name overrides (part 4)#483859
jopejoe1 merged 4 commits intoNixOS:masterfrom
mdaniels5757:mdaniels5757-remove-by-name-overrides-part-4

Conversation

@mdaniels5757
Copy link
Member

@mdaniels5757 mdaniels5757 commented Jan 26, 2026

Split from #474456.

This is a step towards #454525, which will help enable checking for additional by-name directories (e.g. Python) in nixpkgs-vet.

Things done

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Jan 26, 2026
@mdaniels5757 mdaniels5757 marked this pull request as ready for review January 26, 2026 03:36
@diniamo
Copy link
Contributor

diniamo commented Jan 26, 2026

I'm not sure I undersrand. What is the point of this?

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jan 26, 2026
@paperdigits
Copy link
Contributor

paperdigits commented Jan 26, 2026 via email

@mdaniels5757
Copy link
Member Author

I'm not sure I undersrand. What is the point of this?

Currently, packages defined in pkgs/by-name can be overridden, with the same name, in pkgs/top-level/all-packages.nix. So pkgs/by-name/fo/foo can have an all-packages.nix entry like:

  foo = callPackage ../by-name/fo/foo {
    bar = bar_2_0;
  };

This is problematic: both because it makes the package definition a bit harder to read (because it's not all in one place), and (more importantly IMO) it massively complicates checking the by-name directory in continuous integration. This is making it much much harder to, e.g., add a pkgs/development/python-modules/by-name directory, which would allow the merge bot to be used for those packages (which would be a huge win for maintainers (less waiting!) and committers (less work!)).

So we are moving to disallow overriding packages defined in by-name with the same name. This PR is a step towards removing the existing overrides.

@diniamo
Copy link
Contributor

diniamo commented Jan 26, 2026

So in the case of Odin, llvmPackages is an override from all-packages? Or?

@mdaniels5757
Copy link
Member Author

mdaniels5757 commented Jan 26, 2026

In pkgs/by-name/od/odin/package.nix before this PR's change, the value of the "llvmPackages" argument passed in is actually that of the package llvmPackages_18.

The thing we are trying to eliminate is everything of the form foo = callPackage ../by-name/fo/foo { ... } in all-packages.nix, for the reason above.

Note that e.g. foo_1_0 = ... is fine (as long as there is no package in by-name called foo_1_0). The problem is the override with the same name.

Copy link
Contributor

@diniamo diniamo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I missed the all-packages changes somehow. I get it now.

Looks good to me then.

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Jan 27, 2026
Copy link
Member

@jopejoe1 jopejoe1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this clean up.

@jopejoe1 jopejoe1 added this pull request to the merge queue Jan 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants