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

nixpkgs module: Fix defaulting of localSystem and system #46341

Merged
merged 1 commit into from Sep 8, 2018

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
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants