| Message ID | 20191212075400.1499536-2-jeremy.rosen@smile.fr |
|---|---|
| State | Superseded |
| Headers | show |
| Series | use host-systemd to enable units | expand |
Hello, Thanks for this work, very useful! On Thu, 12 Dec 2019 08:53:48 +0100 Jérémy Rosen <jeremy.rosen@smile.fr> wrote: > diff --git a/package/systemd/Config.in b/package/systemd/Config.in > index 8f1d6fc0c0..81761b33a4 100644 > --- a/package/systemd/Config.in > +++ b/package/systemd/Config.in > @@ -199,7 +199,6 @@ config BR2_PACKAGE_SYSTEMD_HOSTNAMED > config BR2_PACKAGE_SYSTEMD_HWDB > bool "enable hwdb installation" > default y > - select BR2_PACKAGE_HOST_EUDEV # for udevadm, during target-finalize > help > Enables hardware database installation to /usr/lib/udev/hwdb.d > > diff --git a/package/systemd/Config.in.host b/package/systemd/Config.in.host > new file mode 100644 > index 0000000000..219f24239e > --- /dev/null > +++ b/package/systemd/Config.in.host > @@ -0,0 +1,3 @@ > +# Select this if you need host systemd tools (e.g. systemctl) > +config BR2_PACKAGE_HOST_SYSTEMD > + bool > diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk > index a292a7512b..ad16b58ceb 100644 > --- a/package/systemd/systemd.mk > +++ b/package/systemd/systemd.mk > @@ -204,6 +204,11 @@ endif > > ifeq ($(BR2_PACKAGE_SYSTEMD_HWDB),y) > SYSTEMD_CONF_OPTS += -Dhwdb=true > +define SYSTEMD_BUILD_HWDB > + $(HOST_DIR)/bin/udevadm hwdb --update --root $(TARGET_DIR) > +endef I think this change could be a separate patch, no? Or is the udevadm binary installed by host-systemd conflicting with the one installed by host-eudev ? Should be explained in the commit log. > +SYSTEMD_TARGET_FINALIZE_HOOKS += SYSTEMD_BUILD_HWDB > + > else > SYSTEMD_CONF_OPTS += -Dhwdb=false > endif > @@ -551,3 +556,96 @@ SYSTEMD_CONF_ENV = $(HOST_UTF8_LOCALE_ENV) > SYSTEMD_NINJA_ENV = $(HOST_UTF8_LOCALE_ENV) > > $(eval $(meson-package)) We normally have both meson-package and host-meson-package macro invocations at the end of .mk files. > +# > +# Host-systemd configuration > +# > +#Options tweaked for buildroot > +HOST_SYSTEMD_CONF_OPTS= \ > + -Dsplit-bin=true \ > + -Dsplit-usr=false \ > + --prefix=/usr \ This should really be: --prefix=$(HOST_DIR) > + --libdir=lib \ > + --sysconfdir=/etc \ > + --localstatedir=/var But in fact, all these options are already passed by the host-meson-package infrastructure. Why are you overriding them ? > +#disable everything else Space after # > +HOST_SYSTEMD_CONF_OPTS+= \ Space before += > +HOST_SYSTEMD_DEPENDENCIES = \ > + host-util-linux \ > + host-patchelf \ > + host-libcap \ > + host-gperf > + > +# Fix RPATH After installation > +# * systemd provides a install_rpath instruction to meson because the binaries need to link with > +# libsystemd which is not in a standard path > +# * meson can only replace the RPATH, not append to it > +# * the original rpatch is thus lost. rpatch -> rpath > +# * the original path had been tweaked by buildroot vial LD_FLAGS to add $(HOST_DIR)/lib vial -> via LD_FLAGS -> LDFLAGS > +# * thus re-tweak rpath after the installation for all binaries that need it I must admit I don't really understand what's going on here. Can't you simply do something like this: HOST_SYSTEMD_CONF_ENV = \ LDFLAGS="$(HOST_LDFLAGS) -Wl,-rpath,$(HOST_DIR)/lib/systemd" and that's it ? > +#buildroot detects incorrect RPATH, so adding new binaries should be safe (it won't compile > +#unless properly integrated). Space after #, and lines are too long. > +HOST_SYSTEMD_HOST_TOOLS = \ > + systemd-analyze systemd-mount systemctl udevadm > + > +define HOST_SYSTEMD_FIX_RPATH > + $(foreach f,$(HOST_SYSTEMD_HOST_TOOLS), \ > + $(HOST_DIR)/bin/patchelf --set-rpath $(HOST_DIR)/lib:$(HOST_DIR)/lib/systemd $(HOST_DIR)/bin/$(f) > + ) > +endef > + > +HOST_SYSTEMD_POST_INSTALL_HOOKS += HOST_SYSTEMD_FIX_RPATH > +HOST_SYSTEMD_NINJA_ENV = DESTDIR=$(HOST_DIR) With the correct --prefix=$(HOST_DIR) passed by the default host-meson-package infrastructure, this DESTDIR=$(HOST_DIR) should no longer be necessary. Thanks! Thomas
Le jeu. 12 déc. 2019 à 11:19, Thomas Petazzoni <thomas.petazzoni@bootlin.com> a écrit : > > > ifeq ($(BR2_PACKAGE_SYSTEMD_HWDB),y) > > SYSTEMD_CONF_OPTS += -Dhwdb=true > > +define SYSTEMD_BUILD_HWDB > > + $(HOST_DIR)/bin/udevadm hwdb --update --root $(TARGET_DIR) > > +endef > > I think this change could be a separate patch, no? Or is the udevadm > binary installed by host-systemd conflicting with the one installed by > host-eudev ? > > Should be explained in the commit log. > > That's a matter of taste, I guess No it doesn't conflict, i'll split in v5 > > +SYSTEMD_TARGET_FINALIZE_HOOKS += SYSTEMD_BUILD_HWDB > > + > > else > > SYSTEMD_CONF_OPTS += -Dhwdb=false > > endif > > @@ -551,3 +556,96 @@ SYSTEMD_CONF_ENV = $(HOST_UTF8_LOCALE_ENV) > > SYSTEMD_NINJA_ENV = $(HOST_UTF8_LOCALE_ENV) > > > > $(eval $(meson-package)) > > We normally have both meson-package and host-meson-package macro > invocations at the end of .mk files. > > Ok, I wanted to separate the host and target part clearly, but i'll follow proper styling > > +# > > +# Host-systemd configuration > > +# > > +#Options tweaked for buildroot > > +HOST_SYSTEMD_CONF_OPTS= \ > > + -Dsplit-bin=true \ > > + -Dsplit-usr=false \ > > + --prefix=/usr \ > > This should really be: > > --prefix=$(HOST_DIR) > > No... upstream does not user --prefix properly and we are force to set it to /usr even if HOST_DIR would make more sense. I'll discuss this more in depth in the commit log > > + --libdir=lib \ > > + --sysconfdir=/etc \ > > + --localstatedir=/var > > But in fact, all these options are already passed by the > host-meson-package infrastructure. Why are you overriding them ? > > it's probably a bit overkill, but these need to be set to the same value as the target, not the ones provided by the host It's a bit complex to follow because upstream does not understand the concept of cross-tool correctly so we might build a systemctl that does the right thing for the host instead of doing the right thing for the target > > +#disable everything else > > Space after # > > ok > > +HOST_SYSTEMD_CONF_OPTS+= \ > > Space before += > > ok > > > +HOST_SYSTEMD_DEPENDENCIES = \ > > + host-util-linux \ > > + host-patchelf \ > > + host-libcap \ > > + host-gperf > > + > > +# Fix RPATH After installation > > +# * systemd provides a install_rpath instruction to meson because the > binaries need to link with > > +# libsystemd which is not in a standard path > > +# * meson can only replace the RPATH, not append to it > > +# * the original rpatch is thus lost. > > rpatch -> rpath > > darn me typing to fast > > +# * the original path had been tweaked by buildroot vial LD_FLAGS to > add $(HOST_DIR)/lib > > vial -> via > > ditto > LD_FLAGS -> LDFLAGS > > > +# * thus re-tweak rpath after the installation for all binaries that > need it > > I must admit I don't really understand what's going on here. Can't you > simply do something like this: > > HOST_SYSTEMD_CONF_ENV = \ > LDFLAGS="$(HOST_LDFLAGS) -Wl,-rpath,$(HOST_DIR)/lib/systemd" > > and that's it ? > > No.. that's already what is done by host-package-meson but meson+systemd breaks that. Meson does not support using -rpath in LDFLAGS. It will rewrite the rpath at the end of the build using the information it has, but that does not include parsing the ENV var and directly understanding the linker flags. Normally, everything should be alright since systemd tells meson about libsystemd's location * through meson conf files. meson would user systemd's instructions and combine it with the --prefix option to rewrite rpath correctly. However systemd does not handle --prefix correctly (it hardcodes lots of stuff in its build system based on --prefix) so --prefix can either be empty or point to /usr. (this is heavily linked to the split-usr problem) Because of that, meson will erase our LDFLAGS provided rpath but won't rewrite them correctly. I have spent quite some time with Yann at Lyon looking for a way around, and this seems the best one until upstream deals correctly with --prefix. I tried to be both concise and clear in my comment in the .mk (since i'm pretty sure this is where future contributors will stumble) but apparently i'm not there yet :) > > +#buildroot detects incorrect RPATH, so adding new binaries should be > safe (it won't compile > > +#unless properly integrated). > > Space after #, and lines are too long. > > > +HOST_SYSTEMD_HOST_TOOLS = \ > > + systemd-analyze systemd-mount systemctl udevadm > > + > > +define HOST_SYSTEMD_FIX_RPATH > > + $(foreach f,$(HOST_SYSTEMD_HOST_TOOLS), \ > > + $(HOST_DIR)/bin/patchelf --set-rpath > $(HOST_DIR)/lib:$(HOST_DIR)/lib/systemd $(HOST_DIR)/bin/$(f) > > + ) > > +endef > > + > > +HOST_SYSTEMD_POST_INSTALL_HOOKS += HOST_SYSTEMD_FIX_RPATH > > +HOST_SYSTEMD_NINJA_ENV = DESTDIR=$(HOST_DIR) > > With the correct --prefix=$(HOST_DIR) passed by the default > host-meson-package infrastructure, this DESTDIR=$(HOST_DIR) should no > longer be necessary. > > see my comment above Thx for the review... I'll wait a bit and send a v5 integrating all your remarks > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com >
Jérémy, Thomas,All, On 2019-12-12 11:19 +0100, Thomas Petazzoni spake thusly: > On Thu, 12 Dec 2019 08:53:48 +0100 > Jérémy Rosen <jeremy.rosen@smile.fr> wrote: [--SNIP--] > > diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk > > index a292a7512b..ad16b58ceb 100644 > > --- a/package/systemd/systemd.mk > > +++ b/package/systemd/systemd.mk > > @@ -204,6 +204,11 @@ endif > > > > ifeq ($(BR2_PACKAGE_SYSTEMD_HWDB),y) > > SYSTEMD_CONF_OPTS += -Dhwdb=true > > +define SYSTEMD_BUILD_HWDB > > + $(HOST_DIR)/bin/udevadm hwdb --update --root $(TARGET_DIR) > > +endef > > I think this change could be a separate patch, no? Or is the udevadm > binary installed by host-systemd conflicting with the one installed by > host-eudev ? > > Should be explained in the commit log. I think this change should go in current patch 2, where systemd is made dependent on host-systemd, which indeed installs its own udevadm which would conflict with the one from host-eudev. [--SNIP--] > > +# Host-systemd configuration > > +# > > +#Options tweaked for buildroot > > +HOST_SYSTEMD_CONF_OPTS= \ > > + -Dsplit-bin=true \ > > + -Dsplit-usr=false \ > > + --prefix=/usr \ > > This should really be: > > --prefix=$(HOST_DIR) > > > + --libdir=lib \ > > + --sysconfdir=/etc \ > > + --localstatedir=/var > > But in fact, all these options are already passed by the > host-meson-package infrastructure. Why are you overriding them ? [--SNIP--] > > +# * thus re-tweak rpath after the installation for all binaries that need it > I must admit I don't really understand what's going on here. Can't you > simply do something like this: > > HOST_SYSTEMD_CONF_ENV = \ > LDFLAGS="$(HOST_LDFLAGS) -Wl,-rpath,$(HOST_DIR)/lib/systemd" > > and that's it ? For the records, here are the explanations I provided on IRC. The first question is: why do we use --prefix=/usr ? systemd will store its --prefix in all the executables it generates. As such, systemctl will have a hardcoded 'prefix', where it will manipulate and create files/symlinks in. When called natively, this is nice and shinny. However, for cross-setup, that does not work obviously. So, systemd has its tools know about the 'root' directory where this prefix should be related to. We can call systemctl --root=$(TARGET_DIR) and systemctl wil do the links and such in there. However, it does so by appending its known prefix to it. So, if we were to configure host-systemd as we usually do, with --prefix=$(HOST_DIR), then when we would call host systemctl --root=$(TARGET_DIR) it would look for files in $(TARGET_DIR)/$(HOST_DIR), which is wrong. Calling the host systemctl without --root is also wrong, as it would look for files in $(HOST_DIR) So, there is no satisfying official support for this case. The trick then, is to configure systemd with the prefix it would expect at runtime (on the target!), that is with /usr, but install out-of-tree. That was it for the first part of the question: why do we use --prefix. Now, the second question is: why do we need to muck up with the rpath after installation? Well, this boils down to meson (and not systemd itself). When it installs executables, meson will handily insert whatever rpath the package meson.build would tell it to use. systemd installs libs in $(prefix)/lib/systemd and as a NEEDED to those libs, so it uses an RPATH to find those libs, and meson does introduce that RPATH. However, we Buildroot also want to insert our own RPATH, because systemd uses util-linux' libs and libcap, so it needs our RPATH. However, meson can not extend the RPATH from the LDFLAGS in the environment; meson can only set the RPATH from what it knows about from the package's meson.build. That, in addition to the --prefix=/usr issue above, means that the executables installed by host-systemd have an RPATH set to /usr/lib/systemd. when we would want it to be set to $(HOST_DIR)/lib:$(HOST_DIR)/lib/systemd That;s what is done in the post-install hook: set the RPATH to the appropriate values. [--SNIP--] > > +HOST_SYSTEMD_POST_INSTALL_HOOKS += HOST_SYSTEMD_FIX_RPATH > > +HOST_SYSTEMD_NINJA_ENV = DESTDIR=$(HOST_DIR) > With the correct --prefix=$(HOST_DIR) passed by the default > host-meson-package infrastructure, this DESTDIR=$(HOST_DIR) should no > longer be necessary. Alas, this is not possible... :-( Regards, Yann E. MORIN.
diff --git a/package/Config.in.host b/package/Config.in.host index 758c268e00..62e860d7c3 100644 --- a/package/Config.in.host +++ b/package/Config.in.host @@ -70,6 +70,7 @@ menu "Host utilities" source "package/squashfs/Config.in.host" source "package/sunxi-tools/Config.in.host" source "package/swig/Config.in.host" + source "package/systemd/Config.in.host" source "package/tegrarcm/Config.in.host" source "package/ti-cgt-pru/Config.in.host" source "package/uboot-tools/Config.in.host" diff --git a/package/systemd/Config.in b/package/systemd/Config.in index 8f1d6fc0c0..81761b33a4 100644 --- a/package/systemd/Config.in +++ b/package/systemd/Config.in @@ -199,7 +199,6 @@ config BR2_PACKAGE_SYSTEMD_HOSTNAMED config BR2_PACKAGE_SYSTEMD_HWDB bool "enable hwdb installation" default y - select BR2_PACKAGE_HOST_EUDEV # for udevadm, during target-finalize help Enables hardware database installation to /usr/lib/udev/hwdb.d diff --git a/package/systemd/Config.in.host b/package/systemd/Config.in.host new file mode 100644 index 0000000000..219f24239e --- /dev/null +++ b/package/systemd/Config.in.host @@ -0,0 +1,3 @@ +# Select this if you need host systemd tools (e.g. systemctl) +config BR2_PACKAGE_HOST_SYSTEMD + bool diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk index a292a7512b..ad16b58ceb 100644 --- a/package/systemd/systemd.mk +++ b/package/systemd/systemd.mk @@ -204,6 +204,11 @@ endif ifeq ($(BR2_PACKAGE_SYSTEMD_HWDB),y) SYSTEMD_CONF_OPTS += -Dhwdb=true +define SYSTEMD_BUILD_HWDB + $(HOST_DIR)/bin/udevadm hwdb --update --root $(TARGET_DIR) +endef +SYSTEMD_TARGET_FINALIZE_HOOKS += SYSTEMD_BUILD_HWDB + else SYSTEMD_CONF_OPTS += -Dhwdb=false endif @@ -551,3 +556,96 @@ SYSTEMD_CONF_ENV = $(HOST_UTF8_LOCALE_ENV) SYSTEMD_NINJA_ENV = $(HOST_UTF8_LOCALE_ENV) $(eval $(meson-package)) + +# +# Host-systemd configuration +# +#Options tweaked for buildroot +HOST_SYSTEMD_CONF_OPTS= \ + -Dsplit-bin=true \ + -Dsplit-usr=false \ + --prefix=/usr \ + --libdir=lib \ + --sysconfdir=/etc \ + --localstatedir=/var + +#disable everything else +HOST_SYSTEMD_CONF_OPTS+= \ + -Dutmp=false \ + -Dhibernate=false \ + -Dldconfig=false \ + -Dresolve=false \ + -Defi=false \ + -Dtpm=false \ + -Denvironment-d=false \ + -Dbinfmt=false \ + -Dcoredump=false \ + -Dpstore=false \ + -Dlogind=false \ + -Dhostnamed=false \ + -Dlocaled=false \ + -Dmachined=false \ + -Dportabled=false \ + -Dnetworkd=false \ + -Dtimedated=false \ + -Dtimesyncd=false \ + -Dremote=false \ + -Dcreate-log-dirs=false \ + -Dnss-myhostname=false \ + -Dnss-mymachines=false \ + -Dnss-resolve=false \ + -Dnss-systemd=false \ + -Dfirstboot=false \ + -Drandomseed=false \ + -Dbacklight=false \ + -Dvconsole=false \ + -Dquotacheck=false \ + -Dsysusers=false \ + -Dtmpfiles=false \ + -Dimportd=false \ + -Dhwdb=false \ + -Drfkill=false \ + -Dman=false \ + -Dhtml=false \ + -Dsmack=false \ + -Dpolkit=false \ + -Dblkid=false \ + -Didn=false \ + -Dadm-group=false \ + -Dwheel-group=false \ + -Dzlib=false \ + -Dgshadow=false \ + -Dima=false \ + -Dtests=false \ + -Dglib=false \ + -Dacl=false \ + -Dsysvinit-path='' + +HOST_SYSTEMD_DEPENDENCIES = \ + host-util-linux \ + host-patchelf \ + host-libcap \ + host-gperf + +# Fix RPATH After installation +# * systemd provides a install_rpath instruction to meson because the binaries need to link with +# libsystemd which is not in a standard path +# * meson can only replace the RPATH, not append to it +# * the original rpatch is thus lost. +# * the original path had been tweaked by buildroot vial LD_FLAGS to add $(HOST_DIR)/lib +# * thus re-tweak rpath after the installation for all binaries that need it +#buildroot detects incorrect RPATH, so adding new binaries should be safe (it won't compile +#unless properly integrated). +HOST_SYSTEMD_HOST_TOOLS = \ + systemd-analyze systemd-mount systemctl udevadm + +define HOST_SYSTEMD_FIX_RPATH + $(foreach f,$(HOST_SYSTEMD_HOST_TOOLS), \ + $(HOST_DIR)/bin/patchelf --set-rpath $(HOST_DIR)/lib:$(HOST_DIR)/lib/systemd $(HOST_DIR)/bin/$(f) + ) +endef + +HOST_SYSTEMD_POST_INSTALL_HOOKS += HOST_SYSTEMD_FIX_RPATH +HOST_SYSTEMD_NINJA_ENV = DESTDIR=$(HOST_DIR) + +$(eval $(host-meson-package))
Add the infrastructure to build the host version of systemd * disable all optional features, they can be re-added when needed * systemd has creative way of dealing with cross compile we build a "normal" host systemd, but install it in $HOST_DIR we use systemctl --root to correctly act on TARGET_DIR * we need to adjust RPATH using patchelf because meson can't do it correctly by itsel Signed-off-by: Jérémy Rosen <jeremy.rosen@smile.fr> --- package/Config.in.host | 1 + package/systemd/Config.in | 1 - package/systemd/Config.in.host | 3 ++ package/systemd/systemd.mk | 98 ++++++++++++++++++++++++++++++++++ 4 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 package/systemd/Config.in.host