nixos/generations-dir: Improve symlinking in `/boot` by Majiir · Pull Request #198728 · NixOS/nixpkgs

Majiir · 2022-10-30T22:34:36Z

Description of changes

boot.loader.generationsDir is a simple "boot loader" that creates a directory structure in /boot with symlinks to kernel, initrd, init, and system. This is ideal for some boot loaders (e.g. old or anemic builds of U-Boot) that don't otherwise support a menu of system profiles at boot.

f7ccb384323830c3af2b7032b6558a46f4cacc0c fixes an issue where those symlinks are broken when /boot has its own partition.
15d56abf52a9a404c97744fc25b917f1ce72e243 makes it easier to change the default boot profile by re-pointing the default symlink.
9ee1a4b60994514fff2538dc8c6840b232835674 adds an option to suppress the redundant copying of kernel, initrd and init files.
69cdd9fb5e9632f88a8aff5788b03fb7cccc600a changes the default for that option to false.

I tested this on an armv7l-linux system which has a U-Boot build from 2011 that lacks support for extlinux.conf. In particular, I tested that:

U-Boot can follow the double symlinks for the default profile.
NixOS cleanly updates /boot when upgrading to this branch.

Things done

nixos/modules/system/boot/loader/generations-dir/generations-dir-builder.sh

ncfavier

Code LGTM, not sure how to test so I'll wait for others.

nixos-discourse · 2022-11-11T03:27:50Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/1398

nixos/modules/system/boot/loader/generations-dir/generations-dir-builder.sh

nixos/modules/system/boot/loader/generations-dir/generations-dir.nix

nixos-discourse · 2022-12-31T23:18:54Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/1635

Majiir · 2023-07-16T23:44:16Z

Rebased and updated the stateVersion check. Still looking for a review/merge.

RaitoBezarius · 2023-07-17T17:32:44Z

Please wait some days so that I have time to review this properly.

Majiir · 2023-08-16T02:18:30Z

@RaitoBezarius Have you had a chance to look this over? Anything that can be improved? Thanks.

nixos-discourse · 2023-09-17T21:26:14Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-in-distress/3604/70

philiptaron

This PR makes sense to me, given the use case that @Majiir lays out in the description, and certainly seems no worse than the current implementation.

nixos/modules/system/boot/loader/generations-dir/generations-dir-builder.sh

RaitoBezarius · 2024-02-09T16:07:08Z

It slipped my mind… I will review it ASAP.

RaitoBezarius · 2024-02-10T14:04:14Z

nixos/modules/system/boot/loader/generations-dir/generations-dir.nix

Suggested change

default = versionOlder config.system.stateVersion "23.11";

default = versionOlder config.system.stateVersion "24.05";

no?

Fixed the stateVersion

RaitoBezarius

LGTM, sorry for the delay, we can proceed once you fix the state version.

Majiir · 2024-05-23T17:39:53Z

Rebased on latest master.
Removed mdDoc usages.

philiptaron · 2024-08-13T14:12:15Z

I'm planning on merging this after retesting it later today. Any objections, please speak now.

FliegendeWurst · 2024-12-04T09:20:08Z

nixos/modules/system/boot/loader/generations-dir/generations-dir.nix

This should be 25.05. Looks good otherwise

K900

This is not how stateVersion is meant to be used.

Majiir · 2024-12-05T01:07:39Z

This is not how stateVersion is meant to be used.

nixos/README.md says:

Ensure that option changes are backward compatible.
...

Use lib.versionAtLeast config.system.stateVersion "24.05" on backward incompatible changes which may corrupt, change or update the state stored on existing setups.

I get that "state" was meant for handling things like Postgres upgrades, but there are plenty of stateVersion checks in nixpkgs that are using it to change a default option value without breaking existing users.

Since this change could break boot, we shouldn't quietly change behavior on users. But the existing behavior is not a good default.

If not stateVersion, then what would you suggest?

philiptaron · 2024-12-05T01:20:16Z

I'm planning on merging this after retesting it later today. Any objections, please speak now.

Wow, my timer did not go off.

Majiir · 2024-12-05T02:43:43Z

Wow, my timer did not go off.

🥲 If you look upthread, you'll find you weren't even the first! This is my personal best example of PRs that get nitpicked and delayed. stateVersion went stale three times. But the core change is good, right?

I would remove the stateVersion change to unstick things, but we need @K900 to unblock it in any case, so I think we need an answer first.

K900 · 2024-12-05T10:59:47Z

Keep the old behavior, add a warning that it's deprecated, drop it entirely next release?

Majiir · 2024-12-07T15:17:43Z

Rebased on latest master.
Dropped the last commit (with the stateVersion check) for now.

K900 · 2024-12-11T06:47:13Z

nixos/modules/system/boot/loader/generations-dir/generations-dir-builder.sh

This feels very dangerous. Maybe at least log a warning?

The delete is required. If $defaultdir is not deleted, then the following ln would error out with ln: default: cannot overwrite directory.

Further up, there is an unconditional rm -Rf /boot/system*. The updates are already destructive and not atomic. Improvements in those areas would be nice, but is it a blocker?

Is the concern that users are storing files in /boot/default other than those created by generationsDir? If so, then what kind of warning would help? At this point in the script, we either finish setting up /boot or the user is left with an unbootable system.

It's always hard to tell what is in scope of the bootloader script. IMO if there's an existing directory that contains anything but the files we're about to put there, we should just abort.

When /boot is on a separate partition, absolute symlinks can break boot loaders that assume the partition is mounted at /. For example, U-Boot's ext2load command follows the relative symlinks correctly but fails to follow the absolute symlink.

This makes it easier to quickly change the default boot profile. The contents of the 'default' directory are already symlinks, so anything that can follow symlinks currently should continue to do so.

Adds an option to suppress the second full copy of the default profile files (kernel, initrd, init) made in /boot.

@K900

Looking to @K900 to either say yay or nay or recuse.

Majiir requested a review from dasJ as a code owner October 30, 2022 22:34

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 Oct 30, 2022

Majiir force-pushed the generationsdir-improvements branch 2 times, most recently from 679e0de to 7a97352 Compare October 30, 2022 22:43

ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Oct 30, 2022

bjornfor requested a review from samueldr October 31, 2022 08:52

ncfavier reviewed Oct 31, 2022

View reviewed changes

nixos/modules/system/boot/loader/generations-dir/generations-dir-builder.sh Outdated Show resolved Hide resolved

Majiir force-pushed the generationsdir-improvements branch from 7a97352 to 0015892 Compare November 1, 2022 18:14

Majiir requested a review from ncfavier November 1, 2022 18:15

ncfavier reviewed Nov 1, 2022

View reviewed changes

SuperSandro2000 reviewed Nov 11, 2022

View reviewed changes

nixos/modules/system/boot/loader/generations-dir/generations-dir-builder.sh Outdated Show resolved Hide resolved

nixos/modules/system/boot/loader/generations-dir/generations-dir.nix Outdated Show resolved Hide resolved

Majiir force-pushed the generationsdir-improvements branch from 444ed6a to 69cdd9f Compare November 14, 2022 02:00

Majiir requested a review from SuperSandro2000 November 14, 2022 02:03

Majiir requested a review from ncfavier January 14, 2023 04:39

This was referenced Mar 5, 2023

nixos/systemd/initrd: follow init param symlinks #209964

Merged

systemd-initrd does not follow symlinks in init #219767

Closed

Majiir force-pushed the generationsdir-improvements branch from 69cdd9f to a25c3f6 Compare July 16, 2023 23:40

philiptaron approved these changes Feb 9, 2024

View reviewed changes

nixos/modules/system/boot/loader/generations-dir/generations-dir-builder.sh Outdated Show resolved Hide resolved

RaitoBezarius reviewed Feb 10, 2024

View reviewed changes

RaitoBezarius approved these changes Feb 10, 2024

View reviewed changes

Majiir force-pushed the generationsdir-improvements branch from a25c3f6 to d8a7435 Compare February 18, 2024 17:19

philiptaron approved these changes Feb 19, 2024

View reviewed changes

wegank added the 12.approvals: 2 This PR was reviewed and approved by two persons. label Mar 9, 2024

Majiir force-pushed the generationsdir-improvements branch from d8a7435 to a8ae946 Compare May 23, 2024 17:39

philiptaron approved these changes May 24, 2024

View reviewed changes

FliegendeWurst reviewed Dec 4, 2024

View reviewed changes

K900 requested changes Dec 4, 2024

View reviewed changes

Majiir force-pushed the generationsdir-improvements branch from a8ae946 to dc60840 Compare December 7, 2024 15:17

philiptaron previously approved these changes Dec 7, 2024

View reviewed changes

Majiir requested a review from K900 December 11, 2024 03:49

K900 reviewed Dec 11, 2024

View reviewed changes

wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 15, 2025

Majiir added 3 commits April 21, 2025 21:25

nixos/generations-dir: symlink default to system generation

bb9b8f6

This makes it easier to quickly change the default boot profile. The contents of the 'default' directory are already symlinks, so anything that can follow symlinks currently should continue to do so.

nixos/generations-dir: add copyDefault option

0fd9962

Adds an option to suppress the second full copy of the default profile files (kernel, initrd, init) made in /boot.

Majiir force-pushed the generationsdir-improvements branch from dc60840 to 0fd9962 Compare April 22, 2025 01:30

ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 22, 2025

nixpkgs-ci bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Dec 4, 2025

	default = versionOlder config.system.stateVersion "23.11";
	default = versionOlder config.system.stateVersion "24.05";

Uh oh!

Conversation

Majiir commented Oct 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes

Things done

Uh oh!

Uh oh!

ncfavier left a comment

Choose a reason for hiding this comment

Uh oh!

nixos-discourse commented Nov 11, 2022

Uh oh!

Uh oh!

Uh oh!

nixos-discourse commented Dec 31, 2022

Uh oh!

Majiir commented Jul 16, 2023

Uh oh!

RaitoBezarius commented Jul 17, 2023

Uh oh!

Majiir commented Aug 16, 2023

Uh oh!

nixos-discourse commented Sep 17, 2023

Uh oh!

philiptaron left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RaitoBezarius commented Feb 9, 2024

Uh oh!

RaitoBezarius Feb 10, 2024

Choose a reason for hiding this comment

Uh oh!

Majiir Feb 18, 2024

Choose a reason for hiding this comment

Uh oh!

RaitoBezarius left a comment

Choose a reason for hiding this comment

Uh oh!

Majiir commented May 23, 2024

Uh oh!

philiptaron commented Aug 13, 2024

Uh oh!

FliegendeWurst Dec 4, 2024

Choose a reason for hiding this comment

Uh oh!

K900 left a comment

Choose a reason for hiding this comment

Uh oh!

Majiir commented Dec 5, 2024

Uh oh!

philiptaron commented Dec 5, 2024

Uh oh!

Majiir commented Dec 5, 2024

Uh oh!

K900 commented Dec 5, 2024

Uh oh!

Majiir commented Dec 7, 2024

Uh oh!

K900 Dec 11, 2024

Choose a reason for hiding this comment

Uh oh!

Majiir Dec 12, 2024

Choose a reason for hiding this comment

Uh oh!

K900 Dec 12, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Majiir commented Oct 30, 2022 •

edited

Loading