nixos/version: Warn about using the default of system.stateVersion

[dasJ]

Motivation for this change

Using the default which switches every release is dangerous and potentially breaking when upgrading to a new release.
This should be required in the future but it will only be a warning for now.

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Added a release notes entry if the change is major or breaking
• Fits CONTRIBUTING.md.

[mweinelt]

I think this is worth backporting, since it prevents unexpected issues and otherwise does not have a negative impact.

[Synthetica9]

This also throws a warning for all(?) NixOS tests.

I see a two of solutions:

• Explicitly set stateVersion to the old default in the test builder
• Add an option to specify that the system is only short-lived and should therefore not get this warning.

(+1 for the idea tho 👍🏻)

[dasJ]

@mweinelt Thanks, force pushed

@Synthetica9 Can you point me in the right direction on where to set this in the test runner? I really have no idea where to look. Edit: Do I just add it to build-vms.nix into buildVm?

@NixOS/nixos-release-managers Do you think this should be backported to 21.05? (assuming I can get rid of the warnings in the tests)

[Synthetica9]

@Synthetica9 Can you point me in the right direction on where to set this in the test runner? I really have no idea where to look. Edit: Do I just add it to build-vms.nix into buildVm?

I think adding a new module to baseModules where you override the default should work, but I haven't tested that.

[Synthetica9]

Actually, you should probably add it to https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/testing/test-instrumentation.nix:

diff --git a/nixos/modules/testing/test-instrumentation.nix b/nixos/modules/testing/test-instrumentation.nix
index be5fa88b8ad..774738d0be1 100644
--- a/nixos/modules/testing/test-instrumentation.nix
+++ b/nixos/modules/testing/test-instrumentation.nix
@@ -129,6 +129,9 @@ with import ../../lib/qemu-flags.nix { inherit pkgs; };
     # Make sure we use the Guest Agent from the QEMU package for testing
     # to reduce the closure size required for the tests.
     services.qemuGuest.package = pkgs.qemu_test.ga;
+
+    # Squelch warning about unset system.stateVersion
+    system.stateVersion = lib.trivial.release;
   };
 
 }

[dasJ]

Fixed and rebased

[Synthetica9]

Good change, reducing the amount of footguns in NixOS

[dotlambda]

@NixOS/nixos-release-managers Do you think this should be backported to 21.05? (assuming I can get rid of the warnings in the tests)

I would say yes. This has only positive affects as long as ofborg is happy.

[dasJ]

@grahamc Any idea about the eval error? That would probably affect other PRs as well once this is merged

[dotlambda]

@dasJ If you temporarily replace warnings with assertions ofborg should print the trace.

[dasJ]

Nope, adding an assertion fixes the eval

[dasJ]

Weird, running PAGER=true NIX_PATH= nix-env -qaP --no-name --out-path -f $PWD --arg checkMeta true locally (which is the closest I can get to ofborg I guess), the warning does not get logged.

[dotlambda]

Another idea: How about we temporarily drop the default and see if ofborg shows us a trace?

[dasJ]

I'm out of ideas now

[mohe2015]

@dasJ for https://github.com/NixOS/ofborg#running-meta-checks-locally I get the warning logged but no non-zero exit code (which may mean nothing as ofborg maybe parses that output somehow). I have no idea what that command does though so maybe that doesn't matter.

It eats all your RAM btw (~32 GB or so).

Edit: I'm pretty sure that's the offending command.

[dasJ]

Finally, ofborg succeeded 🎉

[roberth]

This one was redundant because of the default in test-instrumentation.nix. Removing in #171647.