Skip to content

various: remove by-name overrides (part 2)#483689

Merged
doronbehar merged 8 commits intoNixOS:masterfrom
mdaniels5757:mdaniels5757-remove-by-name-overrides-part-2
Jan 25, 2026
Merged

various: remove by-name overrides (part 2)#483689
doronbehar merged 8 commits intoNixOS:masterfrom
mdaniels5757:mdaniels5757-remove-by-name-overrides-part-2

Conversation

@mdaniels5757
Copy link
Copy Markdown
Member

@mdaniels5757 mdaniels5757 commented Jan 25, 2026

These are all taken from #474456. My hope is that this PR is easier to review, and contains fewer (or no!) objectionable changes.

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 25, 2026
@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jan 25, 2026
@doronbehar
Copy link
Copy Markdown
Contributor

I once heard an objection to this kind of changes that sounds similarly to this: "If someone has (e.g in the case of sile), a user that has an override such as sile.override { lua = lua5_4; }; will all of a sudden experience an error that will be unclear and force them to read the expression in Nixpkgs."

I think the above is a small price to pay, but is this a consensus?

@mdaniels5757
Copy link
Copy Markdown
Member Author

mdaniels5757 commented Jan 25, 2026

I'm aware that that objection has been made in the past. (See also #469771, where Aleksanaa noted that others have made that objection, but where no objection was made.)

My opinion is:

  • Yeah, it could be taken as a breaking change, that's why I'm not backporting it. :)
  • The breaking change (if it even counts as one) is absolutely worth it: at a minimum, it will enable substantial simplification of nixpkgs-vet (Remove support for by-name overrides nixpkgs-vet#181), which will allow us to add support for other by-name directories in a clean and easy manner.

@mdaniels5757
Copy link
Copy Markdown
Member Author

mdaniels5757 commented Jan 25, 2026

Also the error is actually pretty decent:

> nix-build -I nixpkgs=. --expr 'with (import <nixpkgs> {}); sile.override { lua = lua5_4; }'
error:
       … while calling a functor (an attribute set with a '__functor' attribute)
         at «string»:1:29:
            1| with (import <nixpkgs> {}); sile.override { lua = lua5_4; }
             |                             ^

       … while calling a functor (an attribute set with a '__functor' attribute)
         at /home/m/nixpkgs/lib/customisation.nix:194:20:
          193|           */
          194|           newArgs: makeOverridable f (overrideWith newArgs)
             |                    ^
          195|         );

       (stack trace truncated; use '--show-trace' to show the full, detailed trace)

       error: function 'anonymous lambda' called with unexpected argument 'lua'
       at /home/m/nixpkgs/pkgs/by-name/si/sile/package.nix:1:1:
            1| {
             | ^
            2|   lib,
       Did you mean lib?

Copy link
Copy Markdown
Contributor

@doronbehar doronbehar left a comment

Choose a reason for hiding this comment

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

Changes LGTM.

@doronbehar
Copy link
Copy Markdown
Contributor

Good enough IMO.

@doronbehar doronbehar added this pull request to the merge queue Jan 25, 2026
@7c6f434c
Copy link
Copy Markdown
Member

7c6f434c commented Jan 25, 2026

A bigger problem with Lua is that the interface stops making sense in some cases (LuaJIT is not a Lua 5.4 implementation, but a package might accept either as Lua). Although for many pins the pin is there exactly because you need an implementation of a specific version's behaviour.

(Here, for example, RenPy change is absolutely a sacrifice of semantics in the name of a minor refactoring)

@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 25, 2026
Merged via the queue into NixOS:master with commit 7b1a07e Jan 25, 2026
33 checks passed
@mdaniels5757 mdaniels5757 deleted the mdaniels5757-remove-by-name-overrides-part-2 branch January 25, 2026 19:52
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.

4 participants