Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

systemd: 253.5 -> 254.3 #243242

Merged
merged 16 commits into from
Sep 13, 2023
Merged

systemd: 253.5 -> 254.3 #243242

merged 16 commits into from
Sep 13, 2023

Conversation

RaitoBezarius
Copy link
Member

@RaitoBezarius RaitoBezarius commented Jul 13, 2023

Description of changes
  • brings systemd to v254
  • multiple fixes as always
  • pkgsMusl.systemdMinimal builds, the full one, doesn't
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/)
  • 23.11 Release Notes (or backporting 23.05 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
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot requested a review from flokli July 15, 2023 11:30
@RaitoBezarius RaitoBezarius changed the title systemd: 253.5 -> 254-rc1 systemd: 253.5 -> 254-rc2 Jul 19, 2023
flokli
flokli previously requested changes Jul 19, 2023
Copy link
Contributor

@flokli flokli left a comment

Choose a reason for hiding this comment

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

Please reflow the patches with the process described right above the patches = line:

https://github.com/NixOS/nixpkgs/pull/243242/files#diff-a284772b505bd573f834b58591d380128545257d9e02601dbe112548526c8710R170-R174

This does also take care of some tiny details like not adding the git version pointers to the patches.

@RaitoBezarius RaitoBezarius changed the title systemd: 253.5 -> 254-rc2 systemd: 253.5 -> 254-rc3 Jul 24, 2023
@RaitoBezarius RaitoBezarius changed the base branch from master to staging July 24, 2023 22:20
@RaitoBezarius RaitoBezarius changed the title systemd: 253.5 -> 254-rc3 systemd: 253.5 -> 254 Jul 28, 2023
@ofborg ofborg bot requested a review from flokli July 28, 2023 18:00
@RaitoBezarius
Copy link
Member Author

RaitoBezarius commented Jul 28, 2023

TODO:

  • investigate hibernate test
  • run it on actual machine:
    • laptop
    • server
  • reflow patches properly
  • more tests?

@ElvishJerricco
Copy link
Contributor

We will also need to mention in the changelog that boot.resumeDevice now must be set to use hibernation when not EFI booting.

@ElvishJerricco
Copy link
Contributor

ElvishJerricco commented Jul 29, 2023

These changes are also required:

diff --git a/nixos/modules/system/boot/systemd/initrd.nix b/nixos/modules/system/boot/systemd/initrd.nix
index 3f40a5b2dfa0..7476a4f9040a 100644
--- a/nixos/modules/system/boot/systemd/initrd.nix
+++ b/nixos/modules/system/boot/systemd/initrd.nix
@@ -57,7 +57,6 @@ let
     "systemd-ask-password-console.service"
     "systemd-fsck@.service"
     "systemd-halt.service"
-    "systemd-hibernate-resume@.service"
     "systemd-journald-audit.socket"
     "systemd-journald-dev-log.socket"
     "systemd-journald.service"
diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix
index 19226904e0e0..6ca73349ccb4 100644
--- a/pkgs/top-level/all-packages.nix
+++ b/pkgs/top-level/all-packages.nix
@@ -28515,6 +28515,7 @@ with pkgs;
     withTpm2Tss = false;
     withUserDb = false;
     withUkify = false;
+    withBootloader = false;
   };
   systemdStage1 = systemdMinimal.override {
     pname = "systemd-stage-1";

@RaitoBezarius RaitoBezarius force-pushed the systemd-254 branch 2 times, most recently from 0f1870a to 1402699 Compare August 2, 2023 17:08
@RaitoBezarius RaitoBezarius merged commit a314291 into NixOS:staging Sep 13, 2023
24 checks passed
@RaitoBezarius RaitoBezarius deleted the systemd-254 branch September 13, 2023 13:23
@@ -371,7 +368,7 @@ stdenv.mkDerivation (finalAttrs: {
# when cross-compiling.
+ ''
shopt -s extglob
patchShebangs tools test src/!(rpm|kernel-install|ukify) src/kernel-install/test-kernel-install.sh
patchShebangs tools test src/!(rpm|ukify) src/kernel-install/test-kernel-install.sh
Copy link
Member

Choose a reason for hiding this comment

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

What was the reason for this change? It broke cross compilation (because these scripts end up with a build bash shebang), and isn't mentioned in the commit message.

Copy link
Contributor

Choose a reason for hiding this comment

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

My bad for not making this explicit. But for all I know, kernel-install is not a script anymore. It was rewritten in C. What am I missing?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I am seeing

error: build of '/nix/store/llnwcn2bgri0m9cn58h8ym6wj42s1jyv-systemd-minimal-aarch64-unknown-linux-gnu-254.3.drv' on 'ssh://build01-x86-newtype' failed: output '/nix/store/7p5kh7smhwni9pwihs3r4mnzm5sxr022-systemd-minimal-aarch64-unknown-linux-gnu-254.3' is not allowed to refer to the following paths:
         /nix/store/vqvj60h076bhqj6977caz0pfxs6543nb-bash-5.2-p15

Copy link
Contributor

Choose a reason for hiding this comment

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

Could it be related to this: #252874 ?

Copy link
Member

@lilyinstarlight lilyinstarlight Sep 14, 2023

Choose a reason for hiding this comment

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

No, this line just needs to be put back as it was before since the .install scripts get installed, as Alyssa mentioned, and need to use the host bash, not the build bash (which when excluded here like it was before, is what will automatically happen)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll open a PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you @alyssais for the ping and sorry for the random drop

Copy link
Member Author

Choose a reason for hiding this comment

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

(and thank you @nikstur for fast turnaround in an ICE)

@misuzu
Copy link
Contributor

misuzu commented Sep 18, 2023

systemd on staging-next is broken on native armv7l-linux https://hydra.armv7l.xyz/build/17881/nixlog/2

[662/1780] Generating src/boot/efi/addonarm.efi.stub with a custom command
FAILED: src/boot/efi/addonarm.efi.stub 
/build/source/tools/elf2efi.py --version-major=254 --version-minor=0 --efi-major=1 --efi-minor=1 --subsystem=10 --minimum-sections=15 src/boot/efi/addonarm.elf.stub src/boot/efi/addonarm.efi.stub
Traceback (most recent call last):
  File "/build/source/tools/elf2efi.py", line 601, in <module>
    main()
  File "/build/source/tools/elf2efi.py", line 597, in main
    elf2efi(parser.parse_args())
  File "/build/source/tools/elf2efi.py", line 499, in elf2efi
    pe_reloc_s = convert_elf_relocations(elf, opt, sections)
  File "/build/source/tools/elf2efi.py", line 388, in convert_elf_relocations
    raise RuntimeError("ELF .dynamic section is missing.")
RuntimeError: ELF .dynamic section is missing.
[663/1780] Linking target src/boot/efi/linuxarm.elf.stub
[664/1780] Compiling C object src/import/libimport-common.a.p/import-common.c.o
[665/1780] Compiling C object src/boot/efi/systemd-bootarm.elf.p/boot.c.o
ninja: build stopped: subcommand failed.

Bisected this issue to fe6e299

@alyssais
Copy link
Member

Cross to riscv64 is broken for the same reason:

FAILED: src/boot/efi/addonriscv64.efi.stub
/build/source/tools/elf2efi.py --version-major=254 --version-minor=0 --efi-major=1 --efi-minor=1 --subsystem=10 --minimum-sections=15 src/boot/efi/addonriscv64.elf.stub src/boot/efi/addonriscv64.efi.stub
Traceback (most recent call last):
  File "/build/source/tools/elf2efi.py", line 601, in <module>
    main()
  File "/build/source/tools/elf2efi.py", line 597, in main
    elf2efi(parser.parse_args())
  File "/build/source/tools/elf2efi.py", line 499, in elf2efi
    pe_reloc_s = convert_elf_relocations(elf, opt, sections)
  File "/build/source/tools/elf2efi.py", line 388, in convert_elf_relocations
    raise RuntimeError("ELF .dynamic section is missing.")
RuntimeError: ELF .dynamic section is missing.

@nikstur
Copy link
Contributor

nikstur commented Sep 27, 2023

Is this an upstream error or something that we have messed up?

@misuzu
Copy link
Contributor

misuzu commented Sep 27, 2023

Is this an upstream error or something that we have messed up?

I think it's an upstream issue. Likely related: systemd/systemd#26641

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/breaking-changes-announcement-for-unstable/17574/33

@lopsided98
Copy link
Contributor

Arch Linux ARM builds addonarm.efi.stub successfully so I don't think this is an upstream issue.

@lopsided98
Copy link
Contributor

lopsided98 commented Sep 30, 2023

The root cause of this bug appears to be that gcc doesn't support the -static-pie argument on 32-bit ARM or RISC-V. The argument is accepted, but doesn't actually do anything. On x86_64, -static-pie causes -static -pie --no-dynamic-linker -z text to be passed to the linker (with slight variations on the other supported architectures).

In systemd, they noticed something wasn't quite right with -static-pie, and therefore they manually pass most of these arguments to the linker, see:
https://github.com/systemd/systemd/blob/cb88da825460c410139a9a3041af849ee2d01d5b/src/boot/efi/meson.build#L156-L158

Notably though, they don't pass -pie. The reason for this is that pretty much every distribution except for nixpkgs builds gcc with --enable-default-pie, so -pie is passed to the linker even without -static-pie. (edit: systemd does pass -pie, but it comes before -static-pie so it doesn't work unless PIE is enabled by default)

This raises several questions about how this should be fixed:

  • If manually passing -pie --no-dynamic-linker -z text is enough to make static PIE work, why doesn't gcc support -static-pie on more architectures?
  • Should systemd pass -Wl,-pie to work around this gcc issue?
  • Why don't we build gcc with --enable-default-pie in nixpkgs? We have the pie hardening option that is disabled by default (except on musl) because it apparently caused problems. Would these same problems occur with --enable-default-pie? If so, how do other distributions not run into these problems?

@lopsided98
Copy link
Contributor

It looks like -static-pie support on the gcc side is as simple as passing those flags to the linker. Some more work is probably needed in glibc to support it, but that is not relevant to systemd.

In fact, someone submitted a patch to gcc to add -static-pie support on ARM, but it was ignored: https://patchwork.ozlabs.org/project/gcc/patch/c7dc02a4-5fc1-956b-6d89-6c09a1a48b5a@gmail.com/

@@ -146,17 +149,21 @@ assert withCoredump -> withCompression;
assert withHomed -> withCryptsetup;
assert withHomed -> withPam;
assert withUkify -> withEfi;
assert withRepart -> withCryptsetup;
assert withBootloader -> withEfi;
Copy link
Member

Choose a reason for hiding this comment

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

This breaks cross compilation for powerpc:

> nix-build --arg crossSystem '{ config = "powerpc64le-unknown-linux-gnu"; }' -A systemd
error:
       … while evaluating a branch condition

         at /home/sascha/git/nixpkgs/lib/customisation.nix:86:7:

           85|     in
           86|       if builtins.isAttrs result then
             |       ^
           87|         result // {

       … while calling the 'isAttrs' builtin

         at /home/sascha/git/nixpkgs/lib/customisation.nix:86:10:

           85|     in
           86|       if builtins.isAttrs result then
             |          ^
           87|         result // {

       error: assertion '(withBootloader -> withEfi)' failed

       at /home/sascha/git/nixpkgs/pkgs/os-specific/linux/systemd/default.nix:153:1:

          152| assert withRepart -> withCryptsetup;
          153| assert withBootloader -> withEfi;
             | ^
          154| # passwdqc is not packaged in nixpkgs yet, if you want to fix this, please submit a PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is stdenv.hostPlatform.isEfi true for PowerPC? If not, I think we should disable the bootloader for PowerPC.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same problem on armv5tel

Copy link
Contributor

Choose a reason for hiding this comment

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

These are all the platforms where isEfi is true:

isEfi = [
{ cpu = { family = "arm"; version = "6"; }; }
{ cpu = { family = "arm"; version = "7"; }; }
{ cpu = { family = "arm"; version = "8"; }; }
{ cpu = { family = "riscv"; }; }
{ cpu = { family = "x86"; }; }
];

It seems like withBootloader should default to the value of withEfi.

Copy link
Contributor

Choose a reason for hiding this comment

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

jerith666 added a commit to jerith666/nixpkgs that referenced this pull request Dec 13, 2023
per lorri's readme:

  lorri creates an indirect garbage collection root for each .drv in
  $XDG_CACHE_HOME/lorri (~/.cache/lorri/ by default) each time it
  evaluates your project.

... so it doesn't make sense to have ProtectHome enabled for
lorri.service.

fixes:

lorri: ERRO IO error binding to socket: Read-only file system (os error 30)

bisecting this error leads to a range of unbuildable commits including
'a31429165204 Merge pull request NixOS#243242 from
RaitoBezarius/systemd-254', so it's likely that systemd update changed
the behaviour of ProtectHome somehow (though the release notes don't
have any obvious culprits).
jerith666 added a commit to jerith666/nixpkgs that referenced this pull request Dec 17, 2023
per lorri's readme:

  lorri creates an indirect garbage collection root for each .drv in
  $XDG_CACHE_HOME/lorri (~/.cache/lorri/ by default) each time it
  evaluates your project.

... so it doesn't make sense to have ProtectHome enabled for
lorri.service.  lorri also needs to be able to modify
/nix/var/nix/gcroots/per-user/, so ProtectSystem can't be 'strict';
'full' is the next strongest.

fixes:

lorri: ERRO IO error binding to socket: Read-only file system (os error 30)

bisecting this error leads to a range of unbuildable commits including
'a31429165204 Merge pull request NixOS#243242 from
RaitoBezarius/systemd-254', so it's likely that systemd update changed
the behaviour of ProtectHome somehow (though the release notes don't
have any obvious culprits).
jerith666 added a commit to jerith666/nixpkgs that referenced this pull request Dec 17, 2023
per lorri's readme:

  lorri creates an indirect garbage collection root for each .drv in
  $XDG_CACHE_HOME/lorri (~/.cache/lorri/ by default) each time it
  evaluates your project.

... so it doesn't make sense to have ProtectHome enabled for
lorri.service.  lorri also needs to be able to modify
/nix/var/nix/gcroots/per-user/, so ProtectSystem can't be 'strict';
'full' is the next strongest.

fixes:

lorri: ERRO IO error binding to socket: Read-only file system (os error 30)

bisecting this error leads to a range of unbuildable commits including
'a31429165204 Merge pull request NixOS#243242 from
RaitoBezarius/systemd-254', so it's likely that systemd update changed
the behaviour of ProtectHome somehow (though the release notes don't
have any obvious culprits).
SuperSandro2000 pushed a commit to SuperSandro2000/nixpkgs that referenced this pull request Feb 24, 2024
per lorri's readme:

  lorri creates an indirect garbage collection root for each .drv in
  $XDG_CACHE_HOME/lorri (~/.cache/lorri/ by default) each time it
  evaluates your project.

... so it doesn't make sense to have ProtectHome enabled for
lorri.service.  lorri also needs to be able to modify
/nix/var/nix/gcroots/per-user/, so ProtectSystem can't be 'strict';
'full' is the next strongest.

fixes:

lorri: ERRO IO error binding to socket: Read-only file system (os error 30)

bisecting this error leads to a range of unbuildable commits including
'a31429165204 Merge pull request NixOS#243242 from
RaitoBezarius/systemd-254', so it's likely that systemd update changed
the behaviour of ProtectHome somehow (though the release notes don't
have any obvious culprits).
SuperSandro2000 pushed a commit to SuperSandro2000/nixpkgs that referenced this pull request Feb 25, 2024
per lorri's readme:

  lorri creates an indirect garbage collection root for each .drv in
  $XDG_CACHE_HOME/lorri (~/.cache/lorri/ by default) each time it
  evaluates your project.

... so it doesn't make sense to have ProtectHome enabled for
lorri.service.  lorri also needs to be able to modify
/nix/var/nix/gcroots/per-user/, so ProtectSystem can't be 'strict';
'full' is the next strongest.

fixes:

lorri: ERRO IO error binding to socket: Read-only file system (os error 30)

bisecting this error leads to a range of unbuildable commits including
'a31429165204 Merge pull request NixOS#243242 from
RaitoBezarius/systemd-254', so it's likely that systemd update changed
the behaviour of ProtectHome somehow (though the release notes don't
have any obvious culprits).
jerith666 added a commit to jerith666/nixpkgs that referenced this pull request Jun 22, 2024
per lorri's readme:

  lorri creates an indirect garbage collection root for each .drv in
  $XDG_CACHE_HOME/lorri (~/.cache/lorri/ by default) each time it
  evaluates your project.

... so it doesn't make sense to have ProtectHome enabled for
lorri.service.  lorri also needs to be able to modify
/nix/var/nix/gcroots/per-user/, so ProtectSystem can't be 'strict';
'full' is the next strongest.

fixes:

lorri: ERRO IO error binding to socket: Read-only file system (os error 30)

bisecting this error leads to a range of unbuildable commits including
'a31429165204 Merge pull request NixOS#243242 from
RaitoBezarius/systemd-254', so it's likely that systemd update changed
the behaviour of ProtectHome somehow (though the release notes don't
have any obvious culprits).

(cherry picked from commit db64f7f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet