Skip to content

nixos: force the user to set system.stateVersion#149877

Closed
zimbatm wants to merge 1 commit intoNixOS:masterfrom
zimbatm:nixos-force-stateVersion
Closed

nixos: force the user to set system.stateVersion#149877
zimbatm wants to merge 1 commit intoNixOS:masterfrom
zimbatm:nixos-force-stateVersion

Conversation

@zimbatm
Copy link
Member

@zimbatm zimbatm commented Dec 9, 2021 •
edited
Loading

The argument for introducing system.stateVersion was that some data
that lives on disk, like the postgres database, would change shape
depending on which release of NixOS the user was using.

If the default is always the current version, it can lead to unwanted
breakages. It's better to fail the evaluation and force the user to set
something.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@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 Dec 9, 2021
@aanderse aanderse requested review from edolstra and grahamc December 9, 2021 21:07
@rycee
Copy link
Member

rycee commented Dec 9, 2021 •
edited
Loading

FWIW I'm in favor. I've been wanting to do the same change in Home Manager. 👍

@zimbatm zimbatm force-pushed the nixos-force-stateVersion branch from f3f2d37 to a7595b4 Compare December 11, 2021 21:56
@zimbatm
Copy link
Member Author

zimbatm commented Dec 11, 2021

Hmm, any ideas on how I can avoid setting a default value?

@zimbatm
nixos: force the user to set system.stateVersion
5343839 
The argument for introducing `system.stateVersion` was that some data
that lives on disk, like the postgres database, would change shape
depending on which release of NixOS the user was using.

If the default is always the current version, it can lead to unwanted
breakages. It's better to fail the evaluation and force the user to set
something.
@zimbatm zimbatm force-pushed the nixos-force-stateVersion branch from a7595b4 to 5343839 Compare December 11, 2021 22:19
@zimbatm zimbatm mentioned this pull request Dec 11, 2021
Open
@pennae
Copy link
Contributor

pennae commented Dec 12, 2021

Hmm, any ideas on how I can avoid setting a default value?

if we're understanding this correctly: don't give a default, and add an assertion for options.system.stateVersion.isDefined with your chosen message

vcunat
vcunat reviewed Jan 11, 2022
View reviewed changes
nixos/release.nix
{
system.nixos.versionSuffix = versionSuffix;
system.nixos.revision = nixpkgs.rev or nixpkgs.shortRev;
system.stateVersion = mkForce "21.04";
Copy link
Member

@vcunat vcunat Jan 11, 2022

Choose a reason for hiding this comment

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

21.04?

@rycee rycee mentioned this pull request Jan 20, 2022
Closed
7 tasks
@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 10, 2022
@infinisil infinisil added the 1.severity: significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc. label Apr 19, 2023
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 19, 2023
@Enzime Enzime mentioned this pull request Oct 22, 2023
Closed
8 tasks
@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Dec 20, 2023

@zimbatm do you think we should continue with this?

@infinisil
Copy link
Member

infinisil commented Dec 20, 2023

I don't think we should do this because stateVersion is overall kind of bad and should be replaced with something better, see #50112 (comment)

@infinisil
Copy link
Member

infinisil commented Dec 20, 2023 •
edited
Loading

Actually never mind. While stateVersion is overall kind of bad, we should make it mandatory to prevent automatic breakages. This is orthogonal to a redesign of it.

@zimbatm
Copy link
Member Author

zimbatm commented Dec 20, 2023

I'm not sure. Forcing this would be the right thing to do. But it's also going to cause a lot of pain to existing users that only have warnings so far. Maybe it's best to bunch the change with Infinisil's proposal, so it's one change instead of two.

@ScottFreeCode
Copy link

ScottFreeCode commented Dec 21, 2023

Is it possible to require it only if the configuration includes some software/package/service that is affected by the stateVersion? Then we don't require it for most users, but prevent the risk of error.

@infinisil
Copy link
Member

infinisil commented Dec 22, 2023

@ScottFreeCode Nice idea, I think that should work!

@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict This PR has merge conflicts with the target branch labels Mar 19, 2024
@wegank wegank marked this pull request as draft March 20, 2024 13:33
@zimbatm
Copy link
Member Author

zimbatm commented Aug 7, 2024

it's time to close some old PRs

@zimbatm zimbatm closed this Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vcunat vcunat vcunat left review comments

@edolstra edolstra Awaiting requested review from edolstra

@grahamc grahamc Awaiting requested review from grahamc

Assignees

No one assigned

Labels

1.severity: significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc. 2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 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/`

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@zimbatm @rycee @pennae @SuperSandro2000 @infinisil @ScottFreeCode @vcunat @wegank