nixos/systemd-boot-builder: use pathlib.Path where possible#434767
Merged
JulienMalka merged 1 commit intoNixOS:masterfrom Aug 22, 2025
Merged
nixos/systemd-boot-builder: use pathlib.Path where possible#434767JulienMalka merged 1 commit intoNixOS:masterfrom
JulienMalka merged 1 commit intoNixOS:masterfrom
Conversation
Member
Author
|
@ofborg test installer.simpleUefiSystemdBoot systemd-boot.extraFiles systemd-boot.specialisations |
Member
|
Thank you! Running the tests locally |
nixpkgs-ci
bot
added
10.rebuild-linux: 1-10
Member
Author
|
I'd also appreciate some input on whether we should backport this; it would be nice to have the bug fixed on stable as well, but it is a fairly big refactor that comes with some risk and I'm not sure the tests cover all the ground we should be covering.
|
Member
Author
|
This version of this change doesn't actually fix the bug it's supposed to fix, because it's now comparing relative paths to absolute ones |
`switch-to-configuration boot` was taking suspiciously long on a machine of mine where the boot partition is on a slow SD card. Some tracing led me to discover that it was in fact deleting all the kernels and initrds every time, only to rewrite them. This turned out to be because of the naive (non-path-normalising) string concatenation used to construct paths in `known_paths`, so all the files were recognised as obsolete and deleted: known_paths=['/EFI/nixos/5jz3m9df1cbxn4hzjjs3aaz8lb9vvimc-linux-6.15.7-Image.efi', '/EFI/nixos/xri8qzfvzclf89x7nfwgq248miw7jbp0-initrd-linux-6.15.7-initrd.efi', '/EFI/nixos/b18llskzrcdgw2nbib58qqcaabiik6yc-linux-6.16-Image.efi', '/EFI/nixos/mdj53j746bii1vw227dfhkyd8ajwab2w-initrd-linux-6.16-initrd.efi', '/EFI/nixos/b18llskzrcdgw2nbib58qqcaabiik6yc-linux-6.16-Image.efi', '/EFI/nixos/mdj53j746bii1vw227dfhkyd8ajwab2w-initrd-linux-6.16-initrd.efi', '/EFI/nixos/b18llskzrcdgw2nbib58qqcaabiik6yc-linux-6.16-Image.efi', '/EFI/nixos/mdj53j746bii1vw227dfhkyd8ajwab2w-initrd-linux-6.16-initrd.efi', '/EFI/nixos/5jz3m9df1cbxn4hzjjs3aaz8lb9vvimc-linux-6.15.7-Image.efi', '/EFI/nixos/1ihk03c1i5518hlgm5mnhrig2hy3hq24-initrd-linux-6.15.7-initrd.efi', '/EFI/nixos/5jz3m9df1cbxn4hzjjs3aaz8lb9vvimc-linux-6.15.7-Image.efi', '/EFI/nixos/1ihk03c1i5518hlgm5mnhrig2hy3hq24-initrd-linux-6.15.7-initrd.efi', '/EFI/nixos/5jz3m9df1cbxn4hzjjs3aaz8lb9vvimc-linux-6.15.7-Image.efi', '/EFI/nixos/1ihk03c1i5518hlgm5mnhrig2hy3hq24-initrd-linux-6.15.7-initrd.efi'] path='/boot//EFI/nixos/5jz3m9df1cbxn4hzjjs3aaz8lb9vvimc-linux-6.15.7-Image.efi' path='/boot//EFI/nixos/xri8qzfvzclf89x7nfwgq248miw7jbp0-initrd-linux-6.15.7-initrd.efi' path='/boot//EFI/nixos/b18llskzrcdgw2nbib58qqcaabiik6yc-linux-6.16-Image.efi' path='/boot//EFI/nixos/mdj53j746bii1vw227dfhkyd8ajwab2w-initrd-linux-6.16-initrd.efi' path='/boot//EFI/nixos/1ihk03c1i5518hlgm5mnhrig2hy3hq24-initrd-linux-6.15.7-initrd.efi' This can be avoided by using pathlib.Path, which normalises paths and generally provides a more consistent and convenient API. I therefore went ahead and replaced all use of `str` for path handling with `Path` in the builder. This may fix some other, similar bugs, as well, but I haven't checked in detail.
lheckemann
force-pushed
the
push-nrzsquvplksx
branch
from
d90a7b0 to
f2ca990
Compare
Member
|
I would not take the risk to backport this, I have seen changes to the systemd-boot backend failing in ways that we have difficulty to predict even with careful review, so I'd leave this in unstable. |
Member
|
Otherwise code changes look good and all tests are passing. |
JulienMalka
approved these changes
Aug 19, 2025
nixpkgs-ci
bot
added
the
12.approvals: 1
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
switch-to-configuration bootwas taking suspiciously long on a machine of mine where the boot partition is on a slow SD card. Some tracing led me to discover that it was in fact deleting all the kernels and initrds every time, only to rewrite them.This turned out to be because of the naive (non-path-normalising) string concatenation used to construct paths in
known_paths, so all the files were recognised as obsolete and deleted:This can be avoided by using pathlib.Path, which normalises paths and generally provides a more consistent and convenient API. I therefore went ahead and replaced all use of
strfor path handling withPathin the builder. This may fix some other, similar bugs, as well, but I haven't checked in detail.Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.
cc maintainer @JulienMalka