[Buildroot] [PATCH 2/3] Rework of the init system

[Maxime Ripard <maxime.ripard@free-electrons.com>]

On 20/12/2011 09:03, Arnout Vandecappelle wrote:
> On Wednesday 23 November 2011 12:30:10 Maxime Ripard wrote:
>> Since we have now two uncompatible init systems, and we want only one of
>> them at the same time in the rootfs, we need to select a particular init
>> system. This patch also adds $(PKG)_INIT_SYSV and $(PKG)_INIT_SYSTEMD
>> variables, that basically moves the init script installation in the
>> package infrastructure, and installs them only when needed.
>>
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
>> ---
>>  package/Makefile.package.in  |    8 ++++++++
>>  package/busybox/busybox.mk   |   11 +++++++++++
>>  package/systemd/Config.in    |    1 +
>>  package/sysvinit/Config.in   |    1 +
>>  package/sysvinit/sysvinit.mk |    5 -----
>>  target/generic/Config.in     |   19 +++++++++++++++++++
>>  6 files changed, 40 insertions(+), 5 deletions(-)
>>
>> diff --git a/package/Makefile.package.in b/package/Makefile.package.in
>> index bd9ceb5..c0684d0 100644
>> --- a/package/Makefile.package.in
>> +++ b/package/Makefile.package.in
>> @@ -367,6 +367,10 @@ $(BUILD_DIR)/%/.stamp_images_installed:
>>  # Install to target dir
>>  $(BUILD_DIR)/%/.stamp_target_installed:
>>  	@$(call MESSAGE,"Installing to target")
>> +	$(if $(and $(BR2_INIT_SYSTEMD),$($(PKG)_INIT_SYSTEMD)),\
>> +		$(foreach init,$($(PKG)_INIT_SYSTEMD),$(INSTALL) -m 0755 -D $(init) $(TARGET_DIR)/etc/init.d/$(notdir $(init))))
>> +	$(if $(and $(or $(BR2_INIT_SYSV),$(BR2_INIT_BUSYBOX)),$($(PKG)_INIT_SYSV)),\
>> +		$(foreach init,$($(PKG)_INIT_SYSV),$(INSTALL) -m 0755 -D $(init) $(TARGET_DIR)/etc/init.d/$(notdir $(init))))
>  Since this is used twice, it makes sense to either define a function for
> it or to go over SYSTEMD and SYSV in a loop.  Also to simplify things, I
> would rename BR2_INIT_SYSV to BR2_INIT_SYSVINIT and define an additional 
> BR2_INIT_SYSV that is selected by both INIT_BUSYBOX and INIT_SYSV (see 
> below).  So this becomes (untested!):
> 	$(foreach style,SYSTEMD SYSV,\
> 	  $(if $(and $(BR2_INIT_$(style)),$($(PKG)_INIT_$(style))),\
> 	    $(foreach init,$($(PKG)_INIT_$(style)),\
> 	      $(INSTALL) -m 0755 -D $(init) $(TARGET_DIR)/etc/init.d/$(notdir $(init)))))

This seem to be a way better solution :)

>>  	$($(PKG)_INSTALL_TARGET_CMDS)
>>  	$(foreach hook,$($(PKG)_POST_INSTALL_TARGET_HOOKS),$(call $(hook))$(sep))
>>  	$(Q)touch $@
>> @@ -384,6 +388,8 @@ $(BUILD_DIR)/%/.stamp_uninstalled:
>>  	rm -f $($(PKG)_TARGET_INSTALL_STAGING)
>>  	$($(PKG)_UNINSTALL_TARGET_CMDS)
>>  	rm -f $($(PKG)_TARGET_INSTALL_TARGET)
>> +	$(foreach init, $($(PKG)_INIT_SYSV), rm -f $(TARGET_DIR)/etc/init.d/$(notdir $(init)))
>> +	$(foreach init, $($(PKG)_INIT_SYSTEMD), rm -f $(TARGET_DIR)/etc/init.d/$(notdir $(init)))
>>  
>>  # Remove package sources
>>  $(BUILD_DIR)/%/.stamp_dircleaned:
>> @@ -479,6 +485,8 @@ endif
>>  endif
>>  
>>  $(2)_DEPENDENCIES		?=
>> +$(2)_INIT_SYSTEMD		?=
>> +$(2)_INIT_SYSV			?=
>>  $(2)_INSTALL_STAGING		?= NO
>>  $(2)_INSTALL_IMAGES		?= NO
>>  $(2)_INSTALL_TARGET		?= YES
>> diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
>> index 9e91136..73c4969 100644
>> --- a/package/busybox/busybox.mk
>> +++ b/package/busybox/busybox.mk
>> @@ -129,6 +129,16 @@ define BUSYBOX_DISABLE_MMU_APPLETS
>>  endef
>>  endif
>>  
>> +ifeq ($(BR2_INIT_BUSYBOX),y)
>> +define BUSYBOX_SET_INIT
>> +	$(call KCONFIG_ENABLE_OPT,CONFIG_INIT,$(BUSYBOX_BUILD_CONFIG))
>> +endef
>> +else
>> +define BUSYBOX_SET_INIT
>> +	$(call KCONFIG_DISABLE_OPT,CONFIG_INIT,$(BUSYBOX_BUILD_CONFIG))
>> +endef
>> +endif
>> +
>  I'm not sure if I'd want to have the disable there if BR2_INIT_BUSYBOX
> is not selected.  There's nothing against a parallel install of
> systemd and busybox-init, selectable by 'init=...' on the kernel
> command line.  (Well, except that systemd's symlinks destroy busybox init.)

Well mutually exclusive init systems is the more obvious solution.
Having an sysv init and systemd should be ok at the same time should be
ok indeed, at least from a filesystem and boot point of view, but it
will probably be more problematic for the init scripts.

From what I understood of it, if init scripts are presents, systemd will
check the name to see if it has a unit for this, and will prefer the
unit if it has one. But I'm not sure how it will behave with our way of
naming the init scripts.

For example, if there is S10udev installed, I'm afraid we will try to
launch udev twice, as systemd won't match S10udev to it udev unit.

I need to check that.

>>  define BUSYBOX_INSTALL_LOGGING_SCRIPT
>>  	if grep -q CONFIG_SYSLOGD=y $(@D)/.config; then \
>>  		[ -f $(TARGET_DIR)/etc/init.d/S01logging ] || \
>> @@ -145,6 +155,7 @@ define BUSYBOX_CONFIGURE_CMDS
>>  	$(BUSYBOX_SET_BB_PWD)
>>  	$(BUSYBOX_SET_LARGEFILE)
>>  	$(BUSYBOX_SET_IPV6)
>> +	$(BUSYBOX_SET_INIT)
>>  	$(BUSYBOX_SET_RPC)
>>  	$(BUSYBOX_PREFER_STATIC)
>>  	$(BUSYBOX_SET_MDEV)
>> diff --git a/package/systemd/Config.in b/package/systemd/Config.in
>> index 09cedb9..33d5ccf 100644
>> --- a/package/systemd/Config.in
>> +++ b/package/systemd/Config.in
>> @@ -1,5 +1,6 @@
>>  config BR2_PACKAGE_SYSTEMD
>>  	bool "systemd"
>> +	depends on BR2_INIT_SYSTEMD
>  Again, there is no reason why you can't have sysvinit and systemd in
> parallel (except for the symlinks).  Also, if you really want them to
> be mutually exclusive, I'd prefer the more explicit
> 	depends on !BR2_PACKAGE_SYSVINIT

Hmmm, you mean BR2_INIT_SYSV ?

>>  	depends on BR2_PACKAGE_UDEV
>>  	depends on BR2_PACKAGE_DBUS
>>  	select BR2_PACKAGE_LIBCAP
>> diff --git a/package/sysvinit/Config.in b/package/sysvinit/Config.in
>> index 34ec391..d91c643 100644
>> --- a/package/sysvinit/Config.in
>> +++ b/package/sysvinit/Config.in
>> @@ -1,5 +1,6 @@
>>  config BR2_PACKAGE_SYSVINIT
>>  	bool "sysvinit"
>> +	depends on BR2_INIT_SYSV
>>  	help
>>  	  /sbin/init - parent of all processes
>>  
>> diff --git a/package/sysvinit/sysvinit.mk b/package/sysvinit/sysvinit.mk
>> index 0d65c43..163ba9b 100644
>> --- a/package/sysvinit/sysvinit.mk
>> +++ b/package/sysvinit/sysvinit.mk
>> @@ -8,11 +8,6 @@ SYSVINIT_SOURCE  = sysvinit_$(SYSVINIT_VERSION)dsf.orig.tar.gz
>>  SYSVINIT_PATCH   = sysvinit_$(SYSVINIT_VERSION)dsf-13.1.diff.gz
>>  SYSVINIT_SITE    = $(BR2_DEBIAN_MIRROR)/debian/pool/main/s/sysvinit
>>  
>> -# Override Busybox implementations if Busybox is enabled.
>> -ifeq ($(BR2_PACKAGE_BUSYBOX),y)
>> -SYSVINIT_DEPENDENCIES = busybox
>> -endif
>> -
>>  define SYSVINIT_DEBIAN_PATCHES
>>  	if [ -d $(@D)/debian/patches ]; then \
>>  		support/scripts/apply-patches.sh $(@D) $(@D)/debian/patches \*.patch; \
>> diff --git a/target/generic/Config.in b/target/generic/Config.in
>> index a91de32..d9b93af 100644
>> --- a/target/generic/Config.in
>> +++ b/target/generic/Config.in
>> @@ -32,6 +32,25 @@ config BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_UDEV
>>  
>>  endchoice
>>  
>> +choice
>> +	prompt "Init system"
>> +	default BR2_INIT_BUSYBOX
>> +
>> +config BR2_INIT_BUSYBOX
>> +	bool "Busybox init"
>> +	select BR2_PACKAGE_BUSYBOX
>> +
>> +config BR2_INIT_SYSTEMD
>> +	bool "Use systemd"
>> +	depends on BR2_PACKAGE_UDEV
>  Missing:
> 	depends on BR2_PACKAGE_DBUS
>  Also missing the comment in case the dependencies are not met.

Indeed

>> +	select BR2_PACKAGE_SYSTEMD
>> +
>> +config BR2_INIT_SYSV
>> +	bool "Use systemV init"
>> +	select BR2_PACKAGE_SYSVINIT
>> +
>> +endchoice
>> +
>  I'd prefer the ordering to be busybox, sysvinit, systemd, because:
> - systemd is newer
> - systemd has more dependencies

Ok

-- 
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com