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

Check if libpng = libpng12 overrides in all-packages.nix are still necessary #259879

Closed
7 of 9 tasks
Artturin opened this issue Oct 9, 2023 · 9 comments
Closed
7 of 9 tasks
Labels
3.skill: sprintable A larger issue which is split into distinct actionable tasks

Comments

@Artturin
Copy link
Member

Artturin commented Oct 9, 2023

If the override is still necessary then move it directly to the default.nix with a comment about why.

@Artturin Artturin added the 3.skill: sprintable A larger issue which is split into distinct actionable tasks label Oct 9, 2023
@Artturin Artturin changed the title Check if libpng = libpng12 overrides are necessary anymore Check if libpng = libpng12 overrides in all-packages.nix are necessary anymore Oct 9, 2023
@Artturin Artturin changed the title Check if libpng = libpng12 overrides in all-packages.nix are necessary anymore Check if libpng = libpng12 overrides in all-packages.nix are still necessary Oct 9, 2023
@kirillrdy
Copy link
Member

If the override is still necessary then move it directly to the default.nix with a comment about why.

may I ask why ? I though preferred way is to override in all-packages. This is also preferred way in pkgs/by-name
https://github.com/NixOS/nixpkgs/tree/master/pkgs/by-name#changing-implicit-attribute-defaults

@Artturin
Copy link
Member Author

Artturin commented Nov 3, 2023

If the override is still necessary then move it directly to the default.nix with a comment about why.

may I ask why ? I though preferred way is to override in all-packages. This is also preferred way in pkgs/by-name master/pkgs/by-name#changing-implicit-attribute-defaults

Because callPackage overrides are kinda hidden and can result in confusion, the reason I created this issue was because a person on matrix was confused why one package used libpng12

@pbsds
Copy link
Member

pbsds commented Nov 27, 2023

In preparation of the great by-name migration, doing fixes like the ones linked ^ should be encouraged for all packages in all-packages.nix. Is there a general consensus on this?

edit for future readers: i seem to be wrong

@kirillrdy
Copy link
Member

In preparation of the great by-name migration, doing fixes like the ones linked ^ should be encouraged for all packages in all-packages.nix. Is there a general consensus on this?

No idea, i thought that this was https://github.com/NixOS/nixpkgs/tree/master/pkgs/by-name#changing-implicit-attribute-defaults
general consensus.

@pbsds
Copy link
Member

pbsds commented Nov 27, 2023

Would this make sense, or would callPackage ignore the default?

, libpng12
, libpng ? libpng12

CC @infinisil

@infinisil
Copy link
Member

@pbsds If libpng exists in the scope it will be passed, so the default would be ignored. (see also #131271)

@pbsds
Copy link
Member

pbsds commented Dec 30, 2023

@Artturin, the unmerged PRs that are left all need move the libpng12 dependency into the derivation, which goes against the by-name recommendation. Do you want these merged or should we close this issue?

@Stunkymonkey
Copy link
Contributor

ping @Artturin

@Artturin
Copy link
Member Author

Artturin commented Feb 3, 2024

Let's close the unmerged PRs. Thanks everyone for checking if the relevant packages can use a newer libpng!

@Artturin Artturin closed this as completed Feb 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.skill: sprintable A larger issue which is split into distinct actionable tasks
Projects
None yet
Development

No branches or pull requests

5 participants