nixos:nixos-rebuild: allow switching to a specific generation number #105910
Conversation
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
|
I am not sure about having Any opinions @edolstra or others? |
|
Many ideas for the |
|
Maybe this should be its own subcommand? I.e. Or alternatively, |
|
I totally agree with your suggestions and I was a bit unhappy with the doubling of the "switch" as well. The reason I chose it nevertheless is that I took the same flag as |
|
Any opinions about the |
|
I think it's okay to add it to |
|
I prepared a branch with two clean commits instead of this mess and I could use those instead. I guess it would be easier to read that way |
5e37822 to
dc528f0
Compare
dc528f0 to
3073ca2
Compare
|
Anyone wants to take a look and review this? |
1ab9cab to
a0e6d87
Compare
a0e6d87 to
84f3e1f
Compare
| --generation) | ||
| if [ $# -eq 0 ]; then | ||
| log "Must provide a generation number" | ||
| log "Usage: \`$(basename "$0") ${action:-switch} --generation <NUMBER>'" |
There was a problem hiding this comment.
Why do I need to give an action when I just want to switch to a generation?
There was a problem hiding this comment.
We don't know if you want to switch, boot or test said generation, so you have to tell us what you want to do with it...
There was a problem hiding this comment.
but if I want to switch to an older generation, why does it start to build the current config file from disk which might be newer than the current generation?
There was a problem hiding this comment.
That is interesting - I don't think this happened to me before 🤔 Where did you see this behavior? If it is in the tests: the first three tests don't actually do the --generation, but they create the generations, so there it is expected that they build the config files...
I know it isn't a good answer, but: I don't really know what I am doing (at least not 3 years after I first wrote this patch). I just adapted the code for the --rollback since I thought they would be more or less identical... I have forgotten the few things I knew about the nix-env stuff and the rest on how this is activated 🙈 If we just wanted to activate the generation and don't care about the action I guess we could also just run /nix/var/nix/profiles/system-$generation-link/activate...
I tried to add a test for the boot --generation, but something is strange (maybe the QEMU runs some kind of tempfs - at least after the machine.reboot() and it doesn't have nix-channels or actually the old generations any more on the disk) and so I could only check that the current generation was still active...
There was a problem hiding this comment.
I could not reproduce the building of the current configuration.nix in the tests, as this passes, which I think is what you mean:
diff --git a/nixos/tests/nixos-rebuild-switch.nix b/nixos/tests/nixos-rebuild-switch.nix
index b9ed2954299a..e5f3e378ad73 100644
--- a/nixos/tests/nixos-rebuild-switch.nix
+++ b/nixos/tests/nixos-rebuild-switch.nix
@@ -84,12 +84,15 @@ import ./make-test-python.nix ({ pkgs, ... }: {
with subtest("Create generation 3"):
${createConfig "3"}
- machine.succeed("nixos-rebuild switch")
+ out = machine.succeed("nixos-rebuild switch 2>&1")
+ assert "building the system configuration..." in out
${testForBootGeneration "3"}
${testForActiveGeneration "3"}
with subtest("must switch to `--generation 1`"):
- machine.succeed("nixos-rebuild switch --generation 1")
+ ${createConfig "4"}
+ out = machine.succeed("nixos-rebuild switch --generation 1 2>&1")
+ assert "building the system configuration..." not in out
${testForBootGeneration "1"}
${testForActiveGeneration "1"}Did I misunderstand you or do you have a reproduction by any chance?
There was a problem hiding this comment.
well the script (re)executes itself https://github.com/NixOS/nixpkgs/blob/1860f2ab371659d417f30e9f32e242c6ce5928a5/pkgs/os-specific/linux/nixos-rebuild/nixos-rebuild.sh#L405C68-L405C75, so maybe that is why it starts building?
There was a problem hiding this comment.
If we just wanted to activate the generation and don't care about the action I guess we could also just run
/nix/var/nix/profiles/system-$generation-link/activate...
Yes, this is basically what I wanted to do.
Did I misunderstand you or do you have a reproduction by any chance?
I think this is what I did, yes.
well the script (re)executes itself
I am usually passing --fast to it.
There was a problem hiding this comment.
If we just wanted to activate the generation and don't care about the action I guess we could also just run
/nix/var/nix/profiles/system-$generation-link/activate...Yes, this is basically what I wanted to do.
I just implemented it with "feature parity" to the --rollback. I personally mostly use switch as an action, but maybe e.g. someone could want to set a "known good generation" as a default after a reboot when testing a server update and on error they can just do a reboot and everything should be back to normal. Could something like this be a use case (and if not, just out of curiosity: why would it be one for rollback)?
Did I misunderstand you or do you have a reproduction by any chance?
I think this is what I did, yes.
Very interesting... This should not happen (https://github.com/NixOS/nixpkgs/pull/105910/files#diff-fc23c722779993fcffc932965cd143b265aa196d577ca10173b63e5bdcbc0001R608-R609) and also the test that I wrote above passes... 🤔 I really don't have a clue how this could have happened... Anyone has any idea (or even better can add a failing test case)?
This comment was marked as off-topic.
This comment was marked as off-topic.
1860f2a to
9e37703
Compare
|
I rebased it and fixed the merge conflicts. Also added a tiny improvement to the test so that it is checked if a new genertion is built, or if it is just a simple switch |
another merge conflict :P |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
9e37703 to
79c579c
Compare
|
Are there any updates on this? |
There was a problem hiding this comment.
The most important parts of this pull request are the changes to nixos/doc/manual/administration/rollback.section.md, nixos/doc/manual/release-notes/rl-2411.section.md and pkgs/os-specific/linux/nixos-rebuild/nixos-rebuild.sh. Those changes are good. There are some minor improvements that I would make to those files, but for the most part they’re good.
That being said, nixos/tests/nixos-rebuild-switch.nix needs work. I tried following the instructions in the NixOS manual for running NixOS tests, but I wasn’t able to run the test by following those instructions.
I created a branch named nixos-rebuild/switch-generation-proposals. Here’s what that branch contains:
- A rebased version of the commit from this pull request. I rebased it on a newer revision of the
masterbranch (3a16031). - A bunch of fixup commits that bring
nixos/tests/nixos-rebuild-switch.nixinto working order. - A few fixup commits that make minor improvements to the other parts of this pull request.
Each fixup commit contains a detailed explanation for why I think that the change from the fixup commit should be made. Here’s what you can do to review my fixup commits and incorporate some or all of them into this pull request:
Click here to show or hide instructions.
-
Change directory into a clone of the Nixpkgs repository by running this command:
cd <path to nixpkgs repo>
-
Make sure that your local copy of the Nixpkgs’s
masterbranch is sufficiently up-to-date by running this command:git switch master && git pull -
Create a local copy of my
nixos-rebuild/switch-generation-proposalsbranch by running this command:git remote add codeberg-JasonYundt https://codeberg.org/JasonYundt/nixpkgs-backup.git && git fetch codeberg-JasonYundt && git switch --track codeberg-JasonYundt/nixos-rebuild/switch-generation-proposals
-
Read through the fixup commits and the code that they change. I recommend using
git log --reverse --patch master..to read through the commits, but it doesn’t really matter how you do it. -
If there are any fixup commits that you don’t like, then use
git rebase --interactiveto drop those fixup commits. -
If there are any fixup commits that you do like, then incorporate them into this pull request by following these instructions:
-
Figure out the hash for the first commit in the
nixos-rebuild/switch-generation-proposalsbranch by running this command:git cherry -v master HEAD | head --lines=1 -
Squash the commits in your local copy of the
nixos-rebuild/switch-generation-proposalsbranch by running this command:git rebase --autosquash <hash>^
-
Switch to your local
nixos-rebuild/switch-generationbranch by running this command:git switch nixos-rebuild/switch-generation
-
Make your local
nixos-rebuild/switch-generationbranch the same as your localnixos-rebuild/switch-generation-proposalsbranch by running this command:git reset --hard nixos-rebuild/switch-generation-proposals
-
Push a new version of this pull request by running this command:
git push --force
-
nixpkgs-review result
Generated using nixpkgs-review.
Command: nixpkgs-review pr 105910
Commit: 79c579c37c119892647875fb53bb69f98e7d39b4
x86_64-linux
⏩ 2 packages blacklisted:
- nixos-install-tools
- tests.nixos-functions.nixos-test
✅ 1 package built:
- nixos-rebuild
|
I'm wondering if it makes more sense to invest time in improving the -ng script since that's really where other improvements are being made. Given that this PR is 5 years old it doesn't seem like people want to touch the old script. |
Run `nixos-rebuild switch --generation 12345` to roll back to generation `12345`. This makes rolling back to an older generation easier Also updated the release notes and add a test for the new feature Co-Authored-By: Jason Yundt <jason@jasonyundt.email>
79c579c to
487a3e9
Compare
|
@Jayman2000, thank you for your suggestions! Your detailed commit messages clearly explained your changes, and you even created a POC repo for a potential bug. It is an absolute joy to see a changelog like this! This PR was one of my first contributions to nixpkgs when I was just starting to learn software engineering. I made multiple mistakes, the biggest being that I combined two features into one PR because both were bothering me. After getting feedback, I split one feature into a separate PR for listing generations. However, this made the discussion on this PR too long for reviewers to read. I am tempted to create a new PR for this remaining feature as well to simplify reviews, but for now, I just pushed the combined changes from @Jayman2000 to this PR. I opted to directly squash them. How to check differencesTo see the difference between the suggestions of @Jayman2000 and this PR, run git remote add github-miallo https://github.com/miallo/nixpkgs.git && git fetch github-miallo && \
git remote add codeberg-JasonYundt https://codeberg.org/JasonYundt/nixpkgs-backup.git && git fetch codeberg-JasonYundt && \
git diff codeberg-JasonYundt/nixos-rebuild/switch-generation-proposals github-miallo/nixos-rebuild/switch-generationOnly difference to his final suggestions is that this now is changing the correct next release notes instead of the 24.11 ones. @eclairevoyant, I’m also looking into the -ng script and will try to add this feature there as well. Maybe this would be another reason to consider a merge then? |
|
As the maintainer of both nixos-rebuild and nixos-rebuild-ng, I prefer that we don't invest any more time in nixos-rebuild, especially now that the former is officially deprecated. |
Motivation for this change
If the user wants to
rollbackto any of the system generations but the last, this is currently not easily doable (see #24374).In order to unify the interface with
nix-enva bit more, I propose to add a flag ``nixos-rebuild --generation` to easily go to a specific generation.I had an issue, where I didn't notice a package breaking ~10 generations ago and while fixing it, I had to go back and forth multiple times. Since I didn't know about
I repeated the rollback
ntimes.This implementation would make it possible to run
Maintainers
@nbp added
--rollback@worldofpeace did the last edits
And I guess @edolstra ?
Things done