Skip to content

nixpkgs module: Fix defaulting of localSystem and system#46341

Merged
grahamc merged 1 commit intoNixOS:masterfrom
obsidiansystems:fix-46320
Sep 8, 2018
Merged

nixpkgs module: Fix defaulting of localSystem and system#46341
grahamc merged 1 commit intoNixOS:masterfrom
obsidiansystems:fix-46320

Conversation

@Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Sep 7, 2018

Take two of #40708 (4fe2898).

That PR attempted to bidirectionally default config.nixpkgs.system and config.nixpkgs.localSystem.system to each be updated by the other. But this is not possible with the way the module system works. Divergence in certain cases in inevitable.

This PR is more conservative and just has system default localSystem and localSystem make the final call as-is. This solves a number of issues.

  • localSystem completely overrides system, just like with nixpkgs proper. There is no need to specify localSystem.system to clobber the old system.

  • config.nixpkgs.localSystem is exactly what is passed to nixpkgs. No spooky steps.

  • config.nixpkgs.localSystem is elaborated just as nixpkgs would so that all attributes are available, not just the ones the user specified.

The remaining issue is just that config.nixpkgs.system doesn't update based on config.nixpkgs.localSystem.system. It should never be referred to lest it is a bogus stale value because config.nixpkgs.localSystem overwrites it.

Fixes #46320


Backport: #46343

Take two of NixOS#40708 (4fe2898).

That PR attempted to bidirectionally default `config.nixpkgs.system` and
`config.nixpkgs.localSystem.system` to each be updated by the other. But
this is not possible with the way the module system works. Divergence in
certain cases in inevitable.

This PR is more conservative and just has `system` default `localSystem`
and `localSystem` make the final call as-is. This solves a number of
issues.

 - `localSystem` completely overrides `system`, just like with nixpkgs
 proper. There is no need to specify `localSystem.system` to clobber the
 old system.

 - `config.nixpkgs.localSystem` is exactly what is passed to nixpkgs. No
 spooky steps.

 - `config.nixpkgs.localSystem` is elaborated just as nixpkgs would so
 that all attributes are available, not just the ones the user
 specified.

The remaining issue is just that `config.nixpkgs.system` doesn't update
based on `config.nixpkgs.localSystem.system`. It should never be
referred to lest it is a bogus stale value because
`config.nixpkgs.localSystem` overwrites it.

Fixes NixOS#46320
@Ericson2314 Ericson2314 requested a review from nbp as a code owner September 7, 2018 20:46
@GrahamcOfBorg GrahamcOfBorg 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/` 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 Sep 7, 2018
@Ericson2314 Ericson2314 added the 8.has: port to stable This PR already has a backport to the stable release. label Sep 7, 2018
@Ericson2314 Ericson2314 requested a review from dtzWill September 7, 2018 21:23
@samueldr
Copy link
Member

samueldr commented Sep 7, 2018

I'd like a confirmation: is this affecting 18.09 too, thus needing a backport? Since #38485 was merged April 19th, and pointed as being at fault, I assume so. I additionally assume so since #46320 was marked as blocking 18.09.

(So yeah, this basically means: please backport or ask appropriately on resolution. :))

Copy link
Member

@grahamc grahamc left a comment

Choose a reason for hiding this comment

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

It works! Than you so much!!

@grahamc grahamc merged commit ca7391d into NixOS:master Sep 8, 2018
@Ericson2314 Ericson2314 deleted the fix-46320 branch September 8, 2018 18:14
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/` 8.has: port to stable This PR already has a backport to the stable release. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants