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

nixos: use uniq in the type of system.build #155963

Merged
merged 1 commit into from
Jan 20, 2022

Conversation

ncfavier
Copy link
Member

@ncfavier ncfavier commented Jan 20, 2022

unspecified will happily concatenate strings together from two unrelated modules, causing spurious errors (see #155925).

Ideally we could make system.build a submodule with a few distinguished options like installBootLoader, but I don't think this is necessary right now.

I've tested this by running a vm with build-vm-with-bootloader.

@ncfavier ncfavier requested a review from dasJ as a code owner January 20, 2022 22:13
@ncfavier ncfavier requested review from roberth and removed request for dasJ January 20, 2022 22:13
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Jan 20, 2022
@ncfavier ncfavier requested a review from dasJ January 20, 2022 22:14
@roberth
Copy link
Member

roberth commented Jan 20, 2022

I didn't know that it did that, but anything also merges, deeply even. How about uniq unspecified?

Regarding the submodule, see blocked pr #155883.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Jan 20, 2022
@ncfavier
Copy link
Member Author

Sounds good, I looked at all definitions of system.build.* and couldn't find anything that relies on merging.

`unspecified` will happily concatenate strings together from two
unrelated modules, causing spurious errors (see NixOS#155925).
@ncfavier ncfavier changed the title nixos: use anything instead of unspecified in the type of system.build nixos: use uniq in the type of system.build Jan 20, 2022
@ncfavier
Copy link
Member Author

@ofborg test grub systemd-boot login

@roberth
Copy link
Member

roberth commented Jan 20, 2022

Before attrsOf unspecified, it was attrs, which would replace attributes arbitrarily in case of duplicates; arbitrary being determined by the module import order and module system internals. So uniq will only find pre-existing problems.

@roberth
Copy link
Member

roberth commented Jan 20, 2022

OfBorg evaluates fine. The error on arm is caused by a build failure after eval, caused by a full disk.

I hope not too many collisions exist out there, because they're about to be discovered -- well, except for the fraction of users that turned away because they couldn't figure out why their config or module didn't apply because of the silent collisions. Sad.

Let's fix this problem for current and future configs. Merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants