[OE-core] [PATCH 3/3] systemd: split modules into packages

Anders Darander anders at chargestorm.se
Wed Apr 22 17:27:52 UTC 2015


I'm repeating the comments from my review of the earlier version.

I'm slightly worried as we're changing some of the default
configurations here. And we're suddenly starting to build almost
everything in systemd...

* Bruno Bottazzini <bruno.bottazzini at intel.com> [150422 19:02]:

> if one wants to launch a simple deamon, most modules are not
> required.
> He will be able to save space and exclude unwanted packages
> from the final image.

Missing S-o-B: line. (The same is true for patch 1/3).

> ---
>  meta/recipes-core/systemd/systemd_219.bb | 1075 ++++++++++++++++++++++++++----
>  1 file changed, 943 insertions(+), 132 deletions(-)

> diff --git a/meta/recipes-core/systemd/systemd_219.bb b/meta/recipes-core/systemd/systemd_219.bb
> index d1303f7..d86068c 100644
> --- a/meta/recipes-core/systemd/systemd_219.bb
> +++ b/meta/recipes-core/systemd/systemd_219.bb
> @@ -19,7 +19,7 @@ PROVIDES = "udev"

>  PE = "1"

> -DEPENDS = "kmod docbook-sgml-dtd-4.1-native intltool-native gperf-native acl readline dbus libcap libcgroup glib-2.0 qemu-native util-linux"
> +DEPENDS = "util-linux intltool-native gperf-native readline libcap libcgroup qemu-native"

>  SECTION = "base/shell"

> @@ -58,23 +58,45 @@ LDFLAGS_append_libc-uclibc = " -lrt"

>  GTKDOC_DOCDIR = "${S}/docs/"

> -PACKAGECONFIG ??= "xz ldconfig \
> +PACKAGECONFIG ??= " \
> +                   gcrypt \
> +                   kmod \
> +                   ldconfig \

I guess we could debate whether to remove xz or not from the default set
of PACKAGECONFIG. Though, if it's removed, it should be mentioned in the
commit log at least, as it's a change of the default configuration.

> +                   ${@bb.utils.contains('DISTRO_FEATURES', 'blkid', 'blkid', '', d)} \
> +                   ${@bb.utils.contains('DISTRO_FEATURES', 'efi', 'efi', '', d)} \
> +                   ${@bb.utils.contains('DISTRO_FEATURES', 'lz4', 'lz4', '', d)} \
> +                   ${@bb.utils.contains('DISTRO_FEATURES', 'xz', 'xz', '', d)} \
> +                   ${@bb.utils.contains('DISTRO_FEATURES', 'libidn', 'libidn', '', d)} \
> +                   ${@bb.utils.contains('DISTRO_FEATURES', 'acl', 'acl', '', d)} \
>                     ${@bb.utils.contains('DISTRO_FEATURES', 'pam', 'pam', '', d)} \
>                     ${@bb.utils.contains('DISTRO_FEATURES', 'x11', 'xkbcommon', '', d)}"

> +PACKAGECONFIG[glib] = "--enable-gudev,--disable-gudev,glib-2.0"
> +PACKAGECONFIG[acl] = "--enable-acl,--disable-acl,acl"
> +PACKAGECONFIG[blkid] = "--enable-blkid,--disable-blkid,util-linux"
> +PACKAGECONFIG[efi] = "--enable-efi,--disable-efi"
> +PACKAGECONFIG[kmod] = "--enable-kmod,--disable-kmod,kmod"
> +PACKAGECONFIG[polkit] = "--enable-polkit,--disable-polkit,,polkit"
> +PACKAGECONFIG[selinux] = "--enable-selinux,--disable-selinux,libselinux"
> +PACKAGECONFIG[smack] = "--enable-smack,--disable-smack"
> +PACKAGECONFIG[ima] = "--enable-ima,--disable-ima"
> +PACKAGECONFIG[apparmor] = "--enable-apparmor,--disable-apparmor,libapparmor"
> +PACKAGECONFIG[seccomp] = "--enable-seccomp,--disable-seccomp,libseccomp"
> +PACKAGECONFIG[gcrypt] = "--enable-gcrypt,--disable-gcrypt,libgcrypt"
>  PACKAGECONFIG[journal-upload] = "--enable-libcurl,--disable-libcurl,curl"
>  # Sign the journal for anti-tampering
>  PACKAGECONFIG[gcrypt] = "--enable-gcrypt,--disable-gcrypt,libgcrypt"
>  # regardless of PACKAGECONFIG, libgcrypt is always required to expand
>  # the AM_PATH_LIBGCRYPT autoconf macro
> -DEPENDS += "libgcrypt"

Have you verified that it builds OK if you remove the line above, and
also removes gcrypt from PACKAGECONFIG? If so, the comment above should
also be removed, otherwise the DEPENDS line above should be kept.

>  # Compress the journal
> +PACKAGECONFIG[lz4] = "--enable-lz4,--disable-lz4,lz4"
>  PACKAGECONFIG[xz] = "--enable-xz,--disable-xz,xz"
> +PACKAGECONFIG[qrencode] = "--enable-qrencode,--disable-qrencode,libqrencode"
> +PACKAGECONFIG[libidn] = "--enable-libidn,--disable-libidn,libidn"
>  PACKAGECONFIG[cryptsetup] = "--enable-libcryptsetup,--disable-libcryptsetup,cryptsetup"
>  PACKAGECONFIG[microhttpd] = "--enable-microhttpd,--disable-microhttpd,libmicrohttpd"
>  PACKAGECONFIG[elfutils] = "--enable-elfutils,--disable-elfutils,elfutils"
>  PACKAGECONFIG[resolved] = "--enable-resolved,--disable-resolved"
> -PACKAGECONFIG[networkd] = "--enable-networkd,--disable-networkd"

Why are you removing the PACKAGECONFIG definition for networkd?

>  PACKAGECONFIG[libidn] = "--enable-libidn,--disable-libidn,libidn"
>  PACKAGECONFIG[audit] = "--enable-audit,--disable-audit,audit"
>  PACKAGECONFIG[manpages] = "--enable-manpages,--disable-manpages,libxslt-native xmlto-native docbook-xml-dtd4-native docbook-xsl-stylesheets-native"
> @@ -97,17 +119,33 @@ rootprefix ?= "${base_prefix}"
>  rootlibdir ?= "${base_libdir}"
>  rootlibexecdir = "${rootprefix}/lib"

> -# The gtk+ tools should get built as a separate recipe e.g. systemd-tools
>  EXTRA_OECONF = " --with-rootprefix=${rootprefix} \
>                   --with-rootlibdir=${rootlibdir} \
> -                 --with-roothomedir=${ROOT_HOME} \
> -                 --disable-coredump \
> +                 --enable-coredump \
>                   --disable-introspection \
> -                 --disable-kdbus \
> +                 --enable-kdbus \

This should probably be a PACKAGECONFIG. I haven't looked at the code,
but is it safe to build kdbus-support into systemd if you're running on
kernels without kdbus support? And the same if you're running on a
kernel with kdbus support, but where the rest of user-space lacks kdbus
support?

As I haven't had time to play with kdbus, this makes me nervous.

> +                 --disable-manpages \
>                   --enable-split-usr \
>                   --without-python \
>                   --with-sysvrcnd-path=${sysconfdir} \
>                   --with-firmware-path=/lib/firmware \
> +                 --enable-libcurl \

Do you have a DEPEND on curl? Otherwise you shouldn't enable it.
(Non-deterministic builds). And instead of enabling it, it should be a
PACKAGECONFIG.

> +                 --enable-ldconfig \
> +                 --enable-backlight \
> +                 --enable-binfmt \
> +                 --enable-bootchart \
> +                 --enable-firstboot \
> +                 --enable-hostnamed \
> +                 --enable-localed \
> +                 --enable-logind \
> +                 --enable-machined \
> +                 --enable-networkd \

Why are we suddenly enabling all modules above? E.g. networkd was a
PACKAGECONFIG earlier. Most (all?) of these ought to be PACKAGECONFIGs
instead. I know that you're splitting these out into separate packages
for installation, but they might increas build time both to being built
themselves, as well as having to build dependencies of them.

> +                 --enable-quotacheck \
> +                 --enable-randomseed \
> +                 --enable-resolved \
> +                 --enable-rfkill \
> +                 --enable-sysusers \
> +                 --enable-vconsole \
>                 "

Maybe some of these also should be PACKAGECONFIGS?


I haven't looked at all of your alternatives configuration yet, to see
if they make sense or not.

Cheers,
Anders

-- 
Anders Darander
ChargeStorm AB / eStorm AB



More information about the Openembedded-core mailing list