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

More flexible systemd expression for minimal builds #72802

Closed
wants to merge 10 commits into from

Conversation

kirelagin
Copy link
Member

@kirelagin kirelagin commented Nov 4, 2019

Motivation for this change

The primary motivation is #72401. The essence of this PR is exposing more configuration options and making sure that optional dependencies are actually treated as optional. This way a minimalistic build for embedded systems or starge1 initramfs can disable everything it doesn’t need to ensure small closure size.

The first two commits are changed to the dependencies of systemd. The third commit of the series is just a refactoring: some parameters and configuration options are merely reordered for readability. Then follows the actual meat of the PR, which I tried to organise into logical commits to make reviewing them more enjoyable.

My hope is that without any overrides the only difference is that utillinuxMinimal no longer depends on PAM and uses shadow that does not depend on it. The default configuration of systemd, I hope, will remain the same as before.

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 nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc maintainers @edolstra @andir @Mic92
cc mobile-nixos crowd @lheckemann @samueldr
cc a random subset of people I saw in git log @worldofpeace @FRidh @abbradar

@kirelagin
Copy link
Member Author

(NixOS tests still compiling, I’ll keep you updated.)

@flokli
Copy link
Contributor

flokli commented Nov 4, 2019

This looks really nice, but we should replace some of these optionals with multiple outputs, like debian has separate packages for the container management, remote journal and coredump functionality.

In NixOS, we don't really want to override systemd system-wide, because it'd rebuild a lot of things.

Multiple outputs are much nicer, because we can teach the NixOS systemd module on how to compose these outputs. Regular systems could still ship the full feature set, while smaller things like the minimal installer could ship a reduced feature set.

@andir
Copy link
Member

andir commented Nov 4, 2019

Thank you very much for that work! After a short sweep it looks fine.

Can you elaborate how the intended use case for initramfs systemd is? Would the daemon remain after switching to stage2 or is it just to have the systemd tooling around in stage1?

If the daemon would stay alive I'd rather have just one build for both stage1 & stage2 or we must do a re-exec aka "upgrade" when entering stage2.

(I am aware that using it in stage1 is just one of the use cases this enables and that it is still out of scope for this PR.)

@kirelagin
Copy link
Member Author

@andir My plan is to build the most bare-bones version of systemd that I can without abusing their build system, in order to get the smallest possible closure size, throw it into initramfs and use to boot the system. When switching root for stage2 it will also re-exec into a fully-featured version of itself, hopefully correctly passing on all the accumulated state (I haven’t tried this yet, but I looked at the code, and I believe it should work.)

@kirelagin
Copy link
Member Author

kirelagin commented Nov 4, 2019

@flokli I am all in favour of splitting outputs, my only concern is that it falls into the “abusing their build system” category. I am not saying it is bad, I am just saying that it might require some work initially and then will incur maintenance costs.

But even assuming that we will be able to get more outputs, I think that a) Exposing parameters as in this PR might still be useful; b) I would still like to get the re-exec thing working because it’s fun it can be useful for things like removing the functionality from the systemd binary itself if reducing the closure size is the goal (and I am targeting not just regular NixOS stage1, but phones, and embedded devices as well).

@kirelagin
Copy link
Member Author

Hmmmmmm

[ 92%] Building C object tests/CMakeFiles/libgit2_clar.dir/status/submodules.c.o
    [LN] libdevmapper-event.so.1.02
gcc -O2  -fPIC -L.  -L../../lib -L../../libdaemon/client -L../../daemons/dmeventd -Wl,-z,relro,-z,now -pie -fPIE -Wl,--export-dynamic dmeventd.o \
        -o dmeventd -ldl -ldevmapper-event ../../device_mapper/libdevice-mapper.a ../../base/libbase.a   -L/nix/store/s52hf1lh104dhr4nvqdyp9dypww2rpbs-systemd-243-lib/lib -ludev -L/nix/store/cybq1magn5ij8r9nvcb6sqy3i7lpgwcs-util-linux-2.33.2/lib -lblkid  -lm -lpthread -lm
gcc: error: ../../device_mapper/libdevice-mapper.a: No such file or directory
make[2]: *** [Makefile:63: dmeventd] Error 1
make[2]: Leaving directory '/build/lvm2/daemons/dmeventd'
make[1]: *** [../make.tmpl:363: dmeventd.device-mapper] Error 2
make[1]: Leaving directory '/build/lvm2/daemons'
make: *** [make.tmpl:363: daemons.device-mapper] Error 2
gcc -DHAVE_CONFIG_H -I. -I..  -DLOCALEDIR=\"/nix/store/l4j3g1rfb2lqdnjz724fcwzfbbgrcgs7-gnupg-2.2.17/share/locale\" -DGNUPG_BINDIR="\"/nix/store/l4j3g1rfb2lqdnjz724fcwzfbbgrcgs7-gnupg-2.2.17/bin\"" -DGNUPG_LIBEXECDIR="\"/nix/store/l4j3g1rfb2lqdnjz724fcwzfbbgrcgs7-gnupg-2.2.17/libexec\"" -DGNUPG_LIBDIR="\"/nix/store/l4j3g1rfb2lqdnjz724fcwzfbbgrcgs7-gnupg-2.2.17/lib/gnupg\"" -DGNUPG_DATADIR="\"/nix/store/l4j3g1rfb2lqdnjz724fcwzfbbgrcgs7-gnupg-2.2.17/share/gnupg\"" -DGNUPG_SYSCONFDIR="\"/nix/store/l4j3g1rfb2lqdnjz724fcwzfbbgrcgs7-gnupg-2.2.17/etc/gnupg\"" -DGNUPG_LOCALSTATEDIR="\"/nix/store/l4j3g1rfb2lqdnjz724fcwzfbbgrcgs7-gnupg-2.2.17/var\""        -I/nix/store/9ppnzpfnfjrv63b6qn2bsb9pcbsglika-sqlite-3.30.0-dev/include -I/nix/store/yn9mjh7imln7kisk2gxv7sj633m4zwz6-libgcrypt-1.8.5-dev/include -I/nix/store/2d48r8cjw1gd700g6nacil7nw4yvjimq-libgpg-error-1.36-dev/include -I/nix/store/dszc2czm7bnawnjkjc6jhvsv8alwf1rw-libassuan-2.5.3-dev/include -I/nix/store/2d48r8cjw1gd700g6nacil7nw4yvjimq-libgpg-error-1.36-dev/include -I/nix/store/2d48r8cjw1gd700g6nacil7nw4yvjimq-libgpg-error-1.36-dev/include -Wall -Wno-pointer-sign -Wpointer-arith -g -O2 -c -o progress.o progress.c
builder for '/nix/store/dnyr2yvracmhi2k5vvj37p9s9firafrl-lvm2-2.03.01.drv' failed with exit code 2

That’s weird. Did I break it? 😕
Anyway, I’ll look into it tomorrow.

@kirelagin
Copy link
Member Author

kirelagin commented Nov 4, 2019

Ugggghh, ok, so it’s the very last commit. I have no clue how. Any ideas welcome.
Wait, no, I have no idea what is going on.

@arianvp
Copy link
Member

arianvp commented Nov 5, 2019

Can you elaborate how the intended use case for initramfs systemd is? Would the daemon remain after switching to stage2 or is it just to have the systemd tooling around in stage1?

systemd-based initramfs usually uses systemctl switch-root to switch to the non-minimal systemd daemon inside stage-2. It will then replace pid-1 with the new systemd system manager.

https://www.freedesktop.org/wiki/Software/systemd/InitrdInterface/

this manpage explains what a systemd-based initrd should look like:
https://www.freedesktop.org/software/systemd/man/bootup.html#Bootup%20in%20the%20Initial%20RAM%20Disk%20(initrd)

@kirelagin
Copy link
Member Author

Ok, very funny 494d2de

@kirelagin
Copy link
Member Author

#72823

@lheckemann
Copy link
Member

This should be targeted against staging since it's a mass-rebuild. I'd suggest rebasing the branch on the last common commit of master and staging in order to avoid pinging everyone when retargeting:

git fetch origin
git rebase origin/master --onto $(git merge-base origin/master origin/staging)
git push --force-with-lease mine systemd-minimal

and only then retarget.

@kirelagin
Copy link
Member Author

Alright, an unscientifically chosen subset of tests ({boot,login,systemd*}.nix) passes.

@kirelagin kirelagin changed the base branch from master to staging November 5, 2019 11:01
The problem with vlock is that it pulls in PAM, and PAM is quite large,
while kbd is a pretty basic package that might be very useful for basic
console-related activities, such as setting font during early boot.
util-linux minial is meant to be minimal and PAM is not minimal.
@kirelagin
Copy link
Member Author

Alrighty, there is a conflict with #70352, should not be too hard to resolve, but I’ll need some time to make sure the minimal closure is not affected.

This is mostly a readability commit.

It essentially doesn’t change anything in the expression, except that
it splits the expression parameters into logical groups and reorders
the configuration options to split them into logical groups as well
and to have them in the order that more closely resembles that of
the upstream documentation and configuration scripts.

The only two changes to the semantics of the expression are:

* `withSelinux` is removed and replaced with `selinux != null`.
* `lz4` is not forced-enabled, instead the build system will detect
  whether it is available or not.
This allowd for a minimalistic build of systemd for embedded
applications or stage1 initramfs.
Make sure the following optional dependencies are truly optional:

* pam
* gnu-efi
* libidn2
* curl
* gnutar
* gnupg

Add a missing importd dependency (also optional):

* zlib
@kirelagin
Copy link
Member Author

Resolving these conflicts was no fun at all, but I hope I managed to. Let’s merge this pls before anyone decided to make other changes to systemd :).

(Can’t test right now, I’ll wait until Hydra has at least some cache.)

@flokli
Copy link
Contributor

flokli commented Nov 8, 2019

@kirelagin thanks for following up, and sorry this made a rebase necessary.

I don't see the minimal systemd being used somewhere, so I don't think it'll be built by hydra. Did you only meant to test the regular systemd?
Also, we should probably track closure size somehow, so we notice regressions.

Depending on how we want to intend to use that minimal systemd, it might make a lot of sense to run some of the basis systemd tests with the minimal systemd aswell - WDYT?

It might make sense to align these changes with the "port tests to the python test runner" efforts: #71684

@ninegua
Copy link
Contributor

ninegua commented Nov 9, 2019

This looks very interesting! I might have a good use case for this.

My goal is to build an encrypted bootable USB for secure key management. I was trying to use nix to build an image with encrypted partition, but ran into some problem. Although I could try to use nix-plugins to solve it, I think a better solution is to let the USB drive encrypts itself on first boot.

So now what I want is on first boot, it should ask for a passphrase, and then encrypt the root partition, and reboot. Next boot grub will prompt for passphrase to decrypt the USB drive before continuing.

However, in order to encrypt the disk, I cannot have root partition already mounted. So I'm thinking either using two partitions, or let the first boot only use initramfs, so the actual root partition an be encrypted using cryptsetup.

So what is the size of an initramfs with systemd that this PR will enable me to build?

P.S. I'm not exactly sure if I need systemd for my purpose, but something close to a fully functioning system will save me quite some effort.

@kirelagin
Copy link
Member Author

@flokli

I don't see the minimal systemd being used somewhere, so I don't think it'll be built by hydra. Did you only meant to test the regular systemd?

Yes, I’m only testing regular systemd. There is nothing to test about the minimal systemd yet, since, you are right, it is not used anywhere :). However, testing that it build probably makes sense indeed, so, do you think, I should add a top-level expression for it?

Also, we should probably track closure size somehow, so we notice regressions.

That would be amazing, but I have no idea how to approach this. I couldn’t find anything like this done in nixpkgs before, maybe I wasn’t looking good enough. Anyway, I’ll look into it later.

Depending on how we want to intend to use that minimal systemd, it might make a lot of sense to run some of the basis systemd tests with the minimal systemd aswell - WDYT?

So, as I mentioned, by ultimate goal is replacing current stage1 with one based on minimal systemd. Once this is done, the boot.nix test will get us covered. For now I am not sure what can be done... in theory, we can try to boot a regular systemd with minimal systemd and run some of the already existing systemd*.nix tests, but I am somewhat sceptic that it will work 🤔.

@kirelagin
Copy link
Member Author

@ninegua I don’t think you really need systemd for this, if your initramfs will be as simple as encrypting a partition, a bash script that runs cryptsetup should be enough.

If you still decide to go the systemd-in-initramfs route, you might be interested in watching #72401. As to the sizes, current initramfs that I have is 107M uncompressed, 24M compressed. But you have to keep in mind that it a) includes systemd linked with glibc rather than musl; b) has no luks support; c) doesn’t work.

@ofborg ofborg bot requested a review from Mic92 November 9, 2019 12:21
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.

Is this really about optimized build, or about cross compilation?

How does gcc-ar differ from ${stdenv.cc.targetPrefix}gcc-ar?

@kirelagin
Copy link
Member Author

That’s a fixup commit for the “Do optimised build” commit. The line itself, as the comment, indicates, is for LTO. The fixup is for cross-compilation.

@flokli
Copy link
Contributor

flokli commented Nov 17, 2019

@kirelagin can you squash a80a719 into 62593e7, and while doing so, rebase on latest staging?

I think this is in general pretty useful, but wonder if we should make sure a minimal systemd gets built somewhere, too (so we see if it suddenly fails to build or dramatically increases in size).

I'd be fine with merging it in as-is, but with using it, we'd decrease the risk of bitrot.

@lovesegfault
Copy link
Member

@kirelagin What's the status on this?

@FlorianFranzen FlorianFranzen requested a review from flokli May 20, 2020 09:45
@flokli
Copy link
Contributor

flokli commented May 22, 2020

I think this is in general very useful. I'm a bit concerned about a more minimal version of systemd breaking without us noticing it, however.

@kirelagin This needs a rebase on latest staging (since #85334, we don't use a custom fork, but the upstream repo and NixOS-specific patches on top) and a squash as described in #72802 (comment).

Also, do you have any thoughts on how we could test a more minimal systemd in a VM test?

@arianvp
Copy link
Member

arianvp commented May 22, 2020

I think this is in general very useful. I'm a bit concerned about a more minimal version of systemd breaking without us noticing it, however.

This PR just gives you the ability to override some meson flags; it doesnt introduce a minimal systemd package yet; just gives people to ability to make one through .override. I think this is fine!

Personally I just would expose all systemd flags as nix flags instead of a now a curated set

At least we should also exposed systemd-homed and systemd-portabled flags so I can opt in to them when testing them out easily (they're currently disabled)

What do you think?

@flokli
Copy link
Contributor

flokli commented May 22, 2020

IMHO, we should only introduce options that are somewhat working (and by some test coverage, ensured to be not silently breaking).

If someone wants to tackle homed or portabled, adding these options isn't much of work, but having those already now added (in a broken fashion) causes unnecesary pain, especially during refactorings.

@kirelagin
Copy link
Member Author

kirelagin commented May 24, 2020

Hi everyone. The status is that there were conflicts with other work going on on the systemd expression at that time, so it didn’t get merged at that time, and then I kinda forgot about the project (systemd-based initramfs) that I was doing this for. I am sure I am going to get back to it, I am just not sure when. Hopefully, soon. If someone needs this and has some specific use-cases in mind, I can prioritise it on my todo list.

Also, do you have any thoughts on how we could test a more minimal systemd in a VM test?

Basically, once I have my initramfs ready, I’m sure I’ll be able to extract some basic code from it that will be a reasonable VM test. But it will be just stage1, I think it is good enough, let me know if you think otherwise.

@flokli
Copy link
Contributor

flokli commented May 24, 2020

Thanks for the feedback! I mostly just want to avoid having untested combinations in the systemd expression that might silently break.

If you're not currently actively working on this, let's get back to this after #66856 - which is needed to natively handle crypto volumes with systemd in initrd anyways.

@FRidh FRidh mentioned this pull request Jun 21, 2020
@kloenk
Copy link
Member

kloenk commented Sep 19, 2020

@flokli

If someone wants to tackle homed or portabled, adding these options isn't much of work, but having those already now added (in a broken fashion) causes unnecesary pain, especially during refactorings.

Would that not mean that a system closure contains to 2 systemd, one with and one without homed (the one without to build dbus, for the one with homed). Should we disable everything, except the features used for dbus?

Can we maybe link dbus's systemd dependency to /run/current-system? So we only need one systemd per generation?

@kloenk kloenk mentioned this pull request Sep 19, 2020
10 tasks
@flokli
Copy link
Contributor

flokli commented Sep 20, 2020

Would that not mean that a system closure contains to 2 systemd, one with and one without homed (the one without to build dbus, for the one with homed). Should we disable everything, except the features used for dbus?

Can we maybe link dbus's systemd dependency to /run/current-system? So we only need one systemd per generation?

Let's do that discussion in #98094.


Also, is most of this PR included in #98299?

@kloenk
Copy link
Member

kloenk commented Sep 20, 2020

Yes, I build my systemd part after looking into this pr. So this is the code I used to build #98299

@flokli
Copy link
Contributor

flokli commented Sep 21, 2020

Then let's close this in favor of #98299. Thanks for the work so far!

@flokli flokli closed this Sep 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

10 participants