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: warn on missing stateVersion #39447

Merged
merged 3 commits into from May 12, 2018

Conversation

oxij
Copy link
Member

@oxij oxij commented Apr 24, 2018

Motivation for this change

The first patch is OCD. The last patch is the part of #35366 everyone agreed upon. The middle one is plumbing.

Things done

@matthewbauer
Copy link
Member

Can you verify that this would be set in the vms? I belivenit was missing on those configs.

@oxij
Copy link
Member Author

oxij commented May 12, 2018 via email

@matthewbauer
Copy link
Member

So some VMs don’t generate their config with the perl script. I just want to make sure these have it set correctly (it may be in the profile but i can’t remember).

@oxij
Copy link
Member Author

oxij commented May 12, 2018 via email

@GrahamcOfBorg
Copy link

Failure on aarch64-linux (full log)

Partial log (click to expand)

Merge failed

@GrahamcOfBorg
Copy link

Failure on x86_64-linux (full log)

Partial log (click to expand)

Merge failed

@oxij oxij force-pushed the nixos/warn-missing-stateversion branch from 754dfee to 1f0b692 Compare May 12, 2018 19:28
@oxij
Copy link
Member Author

oxij commented May 12, 2018 via email

@GrahamcOfBorg
Copy link

Success on x86_64-linux

Attempted: tests.boot

No partial log is available.

@GrahamcOfBorg
Copy link

Success on aarch64-linux

Attempted: tests.boot

No partial log is available.

@matthewbauer
Copy link
Member

matthewbauer commented May 12, 2018

Ok sounds good as a warning. Look out for issues regarding this though. I think the fixes should be easy it’s just generated somewhere I can’t find now

@matthewbauer matthewbauer merged commit f799042 into NixOS:master May 12, 2018
@oxij
Copy link
Member Author

oxij commented May 12, 2018 via email

@danbst
Copy link
Contributor

danbst commented May 24, 2018

The intent is that without stateVersion, you get the behaviour of the latest release, which presumably is not broken.

When doing release upgrades with nixos-rebuild, it is untrue that you get unbroken behavior without stateVersion.

To make warning more justified, we can do like this:

  1. Each module exports whether it depends on system.nixos.stateVersion
system.nixos.dependsOnStateVersion = mkIf_isNotOverriden cfg.package [ "services.postgresql.enable" ];

(I don't think we can check whether cfg.package evaluation depends on system.nixos.stateVersion, but if it is overriden in user configuration, then user probably pins it to some concrete version, so he doesn't require setting system.nixos.stateVersion)
2. Here we check if system.nixos.dependsOnStateVersion is empty. If it is, then no warning - user is aware of all his stateful options or doesn't have stateful options enabled. Otherwise we display list of options that would case disruption on release upgrades and abort evaluation.

@oxij
Copy link
Member Author

oxij commented May 25, 2018 via email

@oxij oxij deleted the nixos/warn-missing-stateversion branch September 8, 2018 22:19
@oxij oxij mentioned this pull request Aug 2, 2019
8 tasks
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

5 participants