[Buildroot] [PATCH 0/3] init: Add s6 as init system

[Buildroot] [PATCH 0/3] init: Add s6 as init system

[Vadim Kochan]

Add new system init support (BR2_INIT_S6) based on s6 suite. This is very-very
basic support which does not have any setup automation logic (but only getty)
but it allows to install custom s6-rc services in a bit easier way by the
board. All s6-rc services which were copied by rootfs-overlay or post-build
will be compiled as s6-rc db into /etc/s6/rc/compiled-initial, which also has
/etc/s6/rc/compiled link to easy replace it in runtime.

In case if such kind of change will be merged, then may be it would be good to
think about of some packages integration into s6-rc system.

v1:
    1) Changed s6-rc compiled path to /etc/s6/rc/compiled-init, added patch which
       allows to use this path as default.

    2) Select s6-rc package if s6 init is choosen.

Vadim Kochan (3):
  package/s6-linux-init: Build also for the host
  package/s6-linux-init: Allow to install as init system
  package/s6-rc: Allow to integrate s6-rc services

 package/s6-linux-init/s6-linux-init.mk             | 69 +++++++++++++++++++-
 ...figure-Allow-to-specify-compiled-base-dir.patch | 76 ++++++++++++++++++++++
 package/s6-rc/rc.init                              |  4 ++
 package/s6-rc/rc.shutdown                          |  3 +
 package/s6-rc/s6-rc.mk                             | 33 ++++++++++
 system/Config.in                                   |  9 ++-
 6 files changed, 191 insertions(+), 3 deletions(-)
 create mode 100644 package/s6-rc/0001-configure-Allow-to-specify-compiled-base-dir.patch
 create mode 100644 package/s6-rc/rc.init
 create mode 100644 package/s6-rc/rc.shutdown

-- 
2.14.1

[Buildroot] [PATCH 1/3] package/s6-linux-init: Build also for the host

[Vadim Kochan]

Add strict dependency for host-s6-linux-init to use s6-linux-init-maker
on the host during the rootfs generation.

Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
 package/s6-linux-init/s6-linux-init.mk | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/package/s6-linux-init/s6-linux-init.mk b/package/s6-linux-init/s6-linux-init.mk
index d25504ae5a..913f837c1f 100644
--- a/package/s6-linux-init/s6-linux-init.mk
+++ b/package/s6-linux-init/s6-linux-init.mk
@@ -8,7 +8,7 @@ S6_LINUX_INIT_VERSION = 0.4.0.0
 S6_LINUX_INIT_SITE = http://skarnet.org/software/s6-linux-init
 S6_LINUX_INIT_LICENSE = ISC
 S6_LINUX_INIT_LICENSE_FILES = COPYING
-S6_LINUX_INIT_DEPENDENCIES = s6 s6-linux-utils s6-portable-utils
+S6_LINUX_INIT_DEPENDENCIES = s6 s6-linux-utils s6-portable-utils host-s6-linux-init
 
 S6_LINUX_INIT_CONF_OPTS = \
 	--prefix=/usr \
@@ -33,4 +33,31 @@ define S6_LINUX_INIT_INSTALL_TARGET_CMDS
 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) install
 endef
 
+HOST_S6_LINUX_INIT_DEPENDENCIES = host-s6
+
+HOST_S6_LINUX_INIT_CONF_OPTS = \
+	--prefix=$(HOST_DIR) \
+	--with-sysdeps=$(HOST_DIR)/usr/lib/skalibs/sysdeps \
+	--with-include=$(HOST_DIR)/usr/include \
+	--with-dynlib=$(HOST_DIR)/usr/lib \
+	--with-lib=$(HOST_DIR)/usr/lib/execline \
+	--with-lib=$(HOST_DIR)/usr/lib/s6 \
+	--with-lib=$(HOST_DIR)/usr/lib/skalibs \
+	--disable-static \
+	--enable-shared \
+	--disable-allstatic
+
+define HOST_S6_LINUX_INIT_CONFIGURE_CMDS
+	(cd $(@D); $(HOST_CONFIGURE_OPTS) ./configure $(HOST_S6_LINUX_INIT_CONF_OPTS))
+endef
+
+define HOST_S6_LINUX_INIT_BUILD_CMDS
+	$(HOST_MAKE_ENV) $(MAKE) -C $(@D)
+endef
+
+define HOST_S6_LINUX_INIT_INSTALL_CMDS
+	$(HOST_MAKE_ENV) $(MAKE) -C $(@D) install
+endef
+
 $(eval $(generic-package))
+$(eval $(host-generic-package))
-- 
2.14.1

[Buildroot] [PATCH 2/3] package/s6-linux-init: Allow to install as init system

[Vadim Kochan]

Add BR2_INIT_S6 option which allows to install minimal init support
by s6-linux-init-maker into /etc/s6 and link /sbin/init to it.

Also 2 additional files /etc/rc.init and /etc/rc.shutdown are generated as
dummy "execline'd" scripts which are triggered by /etc/s6/init.

Consider generic tty option support and generate getty service if
needed by s6-linux-init-maker.

Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
 package/s6-linux-init/s6-linux-init.mk | 40 ++++++++++++++++++++++++++++++++++
 system/Config.in                       |  9 ++++++--
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/package/s6-linux-init/s6-linux-init.mk b/package/s6-linux-init/s6-linux-init.mk
index 913f837c1f..c298d62477 100644
--- a/package/s6-linux-init/s6-linux-init.mk
+++ b/package/s6-linux-init/s6-linux-init.mk
@@ -33,6 +33,46 @@ define S6_LINUX_INIT_INSTALL_TARGET_CMDS
 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) install
 endef
 
+ifeq ($(BR2_INIT_S6),y)
+
+# Don't let Busybox to install it's own tools like poweroff, reboot, halt, etc
+S6_LINUX_INIT_DEPENDENCIES += $(if $(BR2_PACKAGE_BUSYBOX),busybox)
+
+S6_LINUX_INIT_MAKER_OPTS = -b /usr/bin -c /etc/s6
+
+ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
+S6_LINUX_INIT_MAKER_OPTS += -d 0
+else
+S6_LINUX_INIT_MAKER_OPTS += $(if $(BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_DEVTMPFS),,-d 1)
+endif
+
+ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
+S6_LINUX_INIT_MAKER_OPTS += -G "/sbin/getty -L \
+			  $(SYSTEM_GETTY_OPTIONS) \
+			  $(SYSTEM_GETTY_PORT) \
+			  $(SYSTEM_GETTY_BAUDRATE) \
+			  $(SYSTEM_GETTY_TERM)"
+endif
+
+define S6_LINUX_INIT_INSTALL_INIT
+	ln -sf ../etc/s6/init $(TARGET_DIR)/sbin/init
+
+	ln -sf /usr/bin/s6-reboot $(TARGET_DIR)/sbin/reboot
+	ln -sf /usr/bin/s6-poweroff $(TARGET_DIR)/sbin/poweroff
+	ln -sf /usr/bin/s6-halt $(TARGET_DIR)/sbin/halt
+
+	echo "#! /usr/bin/execlineb -P" > $(@D)/rc.init
+	$(INSTALL) -m 0755 $(@D)/rc.init $(TARGET_DIR)/etc/rc.init
+	echo "#! /usr/bin/execlineb -P" > $(@D)/rc.shutdown
+	$(INSTALL) -m 0755 $(@D)/rc.shutdown $(TARGET_DIR)/etc/rc.shutdown
+
+	rm -rf $(TARGET_DIR)/etc/s6
+	$(HOST_DIR)/bin/s6-linux-init-maker $(S6_LINUX_INIT_MAKER_OPTS) $(TARGET_DIR)/etc/s6
+endef
+S6_LINUX_INIT_POST_INSTALL_TARGET_HOOKS += S6_LINUX_INIT_INSTALL_INIT
+
+endif # BR2_INIT_S6
+
 HOST_S6_LINUX_INIT_DEPENDENCIES = host-s6
 
 HOST_S6_LINUX_INIT_CONF_OPTS = \
diff --git a/system/Config.in b/system/Config.in
index 498b56e222..7b99424040 100644
--- a/system/Config.in
+++ b/system/Config.in
@@ -12,6 +12,7 @@ config BR2_ROOTFS_SKELETON_DEFAULT
 	select BR2_PACKAGE_SKELETON_INIT_SYSV if BR2_INIT_SYSV
 	select BR2_PACKAGE_SKELETON_INIT_SYSV if BR2_INIT_BUSYBOX
 	select BR2_PACKAGE_SKELETON_INIT_SYSTEMD if BR2_INIT_SYSTEMD
+	select BR2_PACKAGE_SKELETON_INIT_COMMON if BR2_INIT_S6
 	select BR2_PACKAGE_SKELETON_INIT_NONE if BR2_INIT_NONE
 	help
 	  Use default target skeleton
@@ -120,6 +121,10 @@ comment "systemd needs a glibc toolchain w/ SSP, headers >= 3.10"
 		!BR2_TOOLCHAIN_HAS_SSP || \
 		!BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_10
 
+config BR2_INIT_S6
+	bool "s6"
+	select BR2_PACKAGE_S6_LINUX_INIT
+
 config BR2_INIT_NONE
 	bool "None"
 	help
@@ -338,7 +343,7 @@ config BR2_TARGET_GENERIC_GETTY_TERM
 	string "TERM environment variable"
 	default "vt100"
 	# currently observed only by busybox and sysvinit
-	depends on BR2_INIT_BUSYBOX || BR2_INIT_SYSV
+	depends on BR2_INIT_BUSYBOX || BR2_INIT_SYSV || BR2_INIT_S6
 	help
 	  Specify a TERM type.
 
@@ -346,7 +351,7 @@ config BR2_TARGET_GENERIC_GETTY_OPTIONS
 	string "other options to pass to getty"
 	default ""
 	# currently observed only by busybox and sysvinit
-	depends on BR2_INIT_BUSYBOX || BR2_INIT_SYSV
+	depends on BR2_INIT_BUSYBOX || BR2_INIT_SYSV || BR2_INIT_S6
 	help
 	  Any other flags you want to pass to getty,
 	  Refer to getty --help for details.
-- 
2.14.1

[Buildroot] [PATCH 3/3] package/s6-rc: Allow to integrate s6-rc services

[Vadim Kochan]

It allows to install s6-rc services by post-build, rootfs-overlay or some
package into /etc/s6/rc/service which will be compiled as s6-rc db as
/etc/s6/rc/compiled-initial. Services are compiled on stage when rootfs overlay
& post-build already performed.

Added rc.init & rc.shutdown scripts which are needed to run s6-rc
services.

This is very basic s6-rc system support which even does not have the
basic stuff like /proc /sys /dev setup, but this might be added in
future work.

All above is only possible if BR2_INIT_S6 is selected.

Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
 ...figure-Allow-to-specify-compiled-base-dir.patch | 76 ++++++++++++++++++++++
 package/s6-rc/rc.init                              |  4 ++
 package/s6-rc/rc.shutdown                          |  3 +
 package/s6-rc/s6-rc.mk                             | 33 ++++++++++
 system/Config.in                                   |  1 +
 5 files changed, 117 insertions(+)
 create mode 100644 package/s6-rc/0001-configure-Allow-to-specify-compiled-base-dir.patch
 create mode 100644 package/s6-rc/rc.init
 create mode 100644 package/s6-rc/rc.shutdown

diff --git a/package/s6-rc/0001-configure-Allow-to-specify-compiled-base-dir.patch b/package/s6-rc/0001-configure-Allow-to-specify-compiled-base-dir.patch
new file mode 100644
index 0000000000..baf1706bb3
--- /dev/null
+++ b/package/s6-rc/0001-configure-Allow-to-specify-compiled-base-dir.patch
@@ -0,0 +1,76 @@
+From cb89562fcec582bbb5bc9a3bf5faf33338e6b535 Mon Sep 17 00:00:00 2001
+From: Vadim Kochan <vadim4j@gmail.com>
+Date: Wed, 13 Feb 2019 03:36:03 +0200
+Subject: [PATCH] configure: Allow to specify compiled base dir
+
+It allows to specify different s6-rc compiled dir (for example
+/etc/s6/rc/compiled), without specifying it as command line argument
+for s6-rc* tools.
+
+Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
+---
+ configure                          | 6 +++++-
+ src/include/s6-rc/s6rc-constants.h | 2 --
+ 2 files changed, 5 insertions(+), 3 deletions(-)
+
+diff --git a/configure b/configure
+index e3ff39e..562cf05 100755
+--- a/configure
++++ b/configure
+@@ -23,6 +23,7 @@ Fine tuning of the installation directories:
+   --libdir=DIR                  static library files [PREFIX/lib/$package]
+   --includedir=DIR              C header files [PREFIX/include]
+   --livedir=DIR                 default live directory [/run/s6-rc]
++  --compiledir=DIR              default compiled directory [/etc/s6-rc/compiled]
+ 
+  If no --prefix option is given, by default libdir (but not dynlibdir) will be
+  /usr/lib/$package, and includedir will be /usr/include.
+@@ -141,6 +142,7 @@ bindir='$exec_prefix/bin'
+ libdir='$prefix/lib/$package'
+ includedir='$prefix/include'
+ livedir=/run/s6-rc
++compiledir=/etc/s6-rc/compiled
+ sysdeps='$prefix/lib/skalibs/sysdeps'
+ manualsysdeps=false
+ shared=false
+@@ -171,6 +173,7 @@ for arg ; do
+     --libdir=*) libdir=${arg#*=} ;;
+     --includedir=*) includedir=${arg#*=} ;;
+     --livedir=*) livedir=${arg#*=} ;;
++    --compiledir=*) compiledir=${arg#*=} ;;
+     --with-sysdeps=*) sysdeps=${arg#*=} manualsysdeps=true ;;
+     --with-include=*) var=${arg#*=} ; stripdir var ; addincpath="$addincpath -I$var" ;;
+     --with-lib=*) var=${arg#*=} ; stripdir var ; addlibspath="$addlibspath -L$var" ; vpaths="$vpaths $var" ;;
+@@ -214,7 +217,7 @@ fi
+ 
+ # Expand installation directories
+ stripdir prefix
+-for i in exec_prefix dynlibdir libexecdir bindir libdir includedir sysdeps sproot livedir ; do
++for i in exec_prefix dynlibdir libexecdir bindir libdir includedir sysdeps sproot livedir compiledir ; do
+   eval tmp=\${$i}
+   eval $i=$tmp
+   stripdir $i
+@@ -454,6 +457,7 @@ cat <<EOF
+ 
+ #define ${package_macro_name}_VERSION "$version"
+ #define ${package_macro_name}_LIVE_BASE "$livedir"
++#define ${package_macro_name}_COMPILED_BASE "$compiledir"
+ EOF
+ if $slashpackage ; then
+   echo "#define ${package_macro_name}_BINPREFIX \"$bindir/\""
+diff --git a/src/include/s6-rc/s6rc-constants.h b/src/include/s6-rc/s6rc-constants.h
+index 168dac8..833e0a7 100644
+--- a/src/include/s6-rc/s6rc-constants.h
++++ b/src/include/s6-rc/s6rc-constants.h
+@@ -3,8 +3,6 @@
+ #ifndef S6RC_CONSTANTS_H
+ #define S6RC_CONSTANTS_H
+ 
+-#define S6RC_COMPILED_BASE "/etc/s6-rc/compiled"
+-
+ #define S6RC_ONESHOT_RUNNER "s6rc-oneshot-runner"
+ #define S6RC_ONESHOT_RUNNER_LEN (sizeof S6RC_ONESHOT_RUNNER - 1)
+ 
+-- 
+2.14.1
+
diff --git a/package/s6-rc/rc.init b/package/s6-rc/rc.init
new file mode 100644
index 0000000000..6709c6488b
--- /dev/null
+++ b/package/s6-rc/rc.init
@@ -0,0 +1,4 @@
+#! /usr/bin/execlineb -P
+
+if { s6-rc-init /run/service }
+if { s6-rc -t 600000 -- change default }
diff --git a/package/s6-rc/rc.shutdown b/package/s6-rc/rc.shutdown
new file mode 100644
index 0000000000..58d49b5cfd
--- /dev/null
+++ b/package/s6-rc/rc.shutdown
@@ -0,0 +1,3 @@
+#! /usr/bin/execlineb -P
+
+s6-rc -da change
diff --git a/package/s6-rc/s6-rc.mk b/package/s6-rc/s6-rc.mk
index 222ba139c6..6bba54a28a 100644
--- a/package/s6-rc/s6-rc.mk
+++ b/package/s6-rc/s6-rc.mk
@@ -11,6 +11,13 @@ S6_RC_LICENSE_FILES = COPYING
 S6_RC_INSTALL_STAGING = YES
 S6_RC_DEPENDENCIES = s6
 
+ifeq ($(BR2_INIT_S6),y)
+# Needs s6-rc-compile to create initial rc db, also
+# build after s6-linux-init to rewrite rc.init for run
+# s6-rc services.
+S6_RC_DEPENDENCIES += host-s6-rc s6-linux-init
+endif
+
 S6_RC_CONF_OPTS = \
 	--prefix=/usr \
 	--with-sysdeps=$(STAGING_DIR)/usr/lib/skalibs/sysdeps \
@@ -20,6 +27,7 @@ S6_RC_CONF_OPTS = \
 	--with-lib=$(STAGING_DIR)/usr/lib/s6 \
 	--with-lib=$(STAGING_DIR)/usr/lib/skalibs \
 	$(if $(BR2_STATIC_LIBS),,--disable-allstatic) \
+	$(if $(BR2_INIT_S6),--compiledir=/etc/s6/rc/compiled,) \
 	$(SHARED_STATIC_LIBS_OPTS)
 
 define S6_RC_CONFIGURE_CMDS
@@ -44,6 +52,31 @@ define S6_RC_INSTALL_STAGING_CMDS
 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(STAGING_DIR) install
 endef
 
+ifeq ($(BR2_INIT_S6),y)
+
+define S6_RC_PREPARE_INIT_RC
+	mkdir -p $(TARGET_DIR)/etc/s6/rc/service/default
+	echo bundle > $(TARGET_DIR)/etc/s6/rc/service/default/type
+	touch $(TARGET_DIR)/etc/s6/rc/service/default/contents
+
+	mkdir -p $(TARGET_DIR)/etc/s6/rc/compiled-initial
+	ln -sf compiled-initial $(TARGET_DIR)/etc/s6/rc/compiled
+
+	$(INSTALL) -m 0755 $(S6_RC_PKGDIR)/rc.init $(TARGET_DIR)/etc/rc.init
+	$(INSTALL) -m 0755 $(S6_RC_PKGDIR)/rc.shutdown $(TARGET_DIR)/etc/rc.shutdown
+endef
+S6_RC_POST_INSTALL_TARGET_HOOKS += S6_RC_PREPARE_INIT_RC
+
+define S6_RC_FINALIZE_INIT_RC
+	rm -rf $(TARGET_DIR)/etc/s6/rc/compiled-initial
+	$(HOST_DIR)/bin/s6-rc-compile -v 1 \
+		$(TARGET_DIR)/etc/s6/rc/compiled-initial \
+		$(TARGET_DIR)/etc/s6/rc/service
+endef
+S6_RC_ROOTFS_PRE_CMD_HOOKS += S6_RC_FINALIZE_INIT_RC
+
+endif # BR2_INIT_S6
+
 HOST_S6_RC_DEPENDENCIES = host-s6
 
 HOST_S6_RC_CONF_OPTS = \
diff --git a/system/Config.in b/system/Config.in
index 7b99424040..9e4d0783f2 100644
--- a/system/Config.in
+++ b/system/Config.in
@@ -124,6 +124,7 @@ comment "systemd needs a glibc toolchain w/ SSP, headers >= 3.10"
 config BR2_INIT_S6
 	bool "s6"
 	select BR2_PACKAGE_S6_LINUX_INIT
+	select BR2_PACKAGE_S6_RC
 
 config BR2_INIT_NONE
 	bool "None"
-- 
2.14.1

[Buildroot] [PATCH 1/3] package/s6-linux-init: Build also for the host

[Thomas Petazzoni]

Hello Vadim,

I am not familiar with s6, so sorry if I'm asking silly questions in
this review.

On Sat, 16 Feb 2019 23:28:33 +0200
Vadim Kochan <vadim4j@gmail.com> wrote:

> Add strict dependency for host-s6-linux-init to use s6-linux-init-maker
> on the host during the rootfs generation.

I think the wording is not great here: "strict dependency" does not
mean anything in the typical Buildroot speak. I think you wanted to say
"We add host-s6-linux-init as a dependency of the target s6-linux-init,
so that the latter can use the s6-linux-init-maker program as part of
its build process".

However, I think adding this dependency should belong to PATCH 2/3,
where s6-linux-init is actually changed to use s6-linux-init-maker.
Indeed, as I understand it, it's only starting with PATCH 2/3 that
s6-linux-init will need this program. Is this correct ?

If it is, then PATCH 1/3 should only change s6-linux-init.mk to add the
host variant of s6-linux-init.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 2/3] package/s6-linux-init: Allow to install as init system

[Thomas Petazzoni]

Hello Vadim,

On Sat, 16 Feb 2019 23:28:34 +0200
Vadim Kochan <vadim4j@gmail.com> wrote:

> +ifeq ($(BR2_INIT_S6),y)
> +
> +# Don't let Busybox to install it's own tools like poweroff, reboot, halt, etc
> +S6_LINUX_INIT_DEPENDENCIES += $(if $(BR2_PACKAGE_BUSYBOX),busybox)

The comment is not good: i doesn't prevent Busybox from installing
these programs, but it ensures Busybox is built before, and therefore
we have the ability to "win" over Busybox as we can overwrite those
programs.

However, this is not how we handle this situation. In fact, we handle
it in exactly the opposite way. In package/busybox/busybox.mk, we add
to BUSYBOX_DEPENDENCIES all packages that install programs that
"conflict" with Busybox applets. This way, we are sure they are built
before Busybox. And the installation process of Busybox has been
carefully modified to not overwrite an existing program by a Busybox
applet if this program already exists.

So, you should use the same model for s6-linux-init.

> +
> +S6_LINUX_INIT_MAKER_OPTS = -b /usr/bin -c /etc/s6
> +
> +ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
> +S6_LINUX_INIT_MAKER_OPTS += -d 0
> +else
> +S6_LINUX_INIT_MAKER_OPTS += $(if $(BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_DEVTMPFS),,-d 1)

So you need -d1 in which cases ? Where udev is used ?

What about:

ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
S6_LINUX_INIT_MAKER_OPTS += -d 0
else ifeq ($(BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_MDEV)$(BR2_PACKAGE_HAS_UDEV),y)
S6_LINUX_INIT_MAKER_OPTS += -d 1
endif

or something like that ?

> +endif
> +
> +ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
> +S6_LINUX_INIT_MAKER_OPTS += -G "/sbin/getty -L \
> +			  $(SYSTEM_GETTY_OPTIONS) \
> +			  $(SYSTEM_GETTY_PORT) \
> +			  $(SYSTEM_GETTY_BAUDRATE) \
> +			  $(SYSTEM_GETTY_TERM)"

I'd say keep this on one single line.

> +endif
> +
> +define S6_LINUX_INIT_INSTALL_INIT
> +	ln -sf ../etc/s6/init $(TARGET_DIR)/sbin/init
> +
> +	ln -sf /usr/bin/s6-reboot $(TARGET_DIR)/sbin/reboot
> +	ln -sf /usr/bin/s6-poweroff $(TARGET_DIR)/sbin/poweroff
> +	ln -sf /usr/bin/s6-halt $(TARGET_DIR)/sbin/halt
> +
> +	echo "#! /usr/bin/execlineb -P" > $(@D)/rc.init

What is this empty script doing ? Perhaps we should have it as a file
in package/s6-linux-init/ instead of generating it.

> +	$(INSTALL) -m 0755 $(@D)/rc.init $(TARGET_DIR)/etc/rc.init
> +	echo "#! /usr/bin/execlineb -P" > $(@D)/rc.shutdown
> +	$(INSTALL) -m 0755 $(@D)/rc.shutdown $(TARGET_DIR)/etc/rc.shutdown

Same question about what this empty script is doing.

> +	rm -rf $(TARGET_DIR)/etc/s6

Any reason to have this rm -rf ?

> +	$(HOST_DIR)/bin/s6-linux-init-maker $(S6_LINUX_INIT_MAKER_OPTS) $(TARGET_DIR)/etc/s6
> +endef
> +S6_LINUX_INIT_POST_INSTALL_TARGET_HOOKS += S6_LINUX_INIT_INSTALL_INIT
> +
> +endif # BR2_INIT_S6

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 3/3] package/s6-rc: Allow to integrate s6-rc services

[Thomas Petazzoni]

Hello Vadim,

On Sat, 16 Feb 2019 23:28:35 +0200
Vadim Kochan <vadim4j@gmail.com> wrote:

> It allows to install s6-rc services by post-build, rootfs-overlay or some
> package into /etc/s6/rc/service which will be compiled as s6-rc db as
> /etc/s6/rc/compiled-initial. Services are compiled on stage when rootfs overlay
> & post-build already performed.

This last sentence is confusing. What about "Services at compiled after
the rootfs overlay have been copied and post-build scripts have been
executed".

> Added rc.init & rc.shutdown scripts which are needed to run s6-rc
> services.

Isn't this already done in PATCH 2/3 ?

> This is very basic s6-rc system support which even does not have the
> basic stuff like /proc /sys /dev setup, but this might be added in
> future work.
> 
> All above is only possible if BR2_INIT_S6 is selected.

"possible" ? "enabled" perhaps ?


> diff --git a/package/s6-rc/0001-configure-Allow-to-specify-compiled-base-dir.patch b/package/s6-rc/0001-configure-Allow-to-specify-compiled-base-dir.patch
> new file mode 100644
> index 0000000000..baf1706bb3
> --- /dev/null
> +++ b/package/s6-rc/0001-configure-Allow-to-specify-compiled-base-dir.patch
> @@ -0,0 +1,76 @@
> +From cb89562fcec582bbb5bc9a3bf5faf33338e6b535 Mon Sep 17 00:00:00 2001
> +From: Vadim Kochan <vadim4j@gmail.com>
> +Date: Wed, 13 Feb 2019 03:36:03 +0200
> +Subject: [PATCH] configure: Allow to specify compiled base dir
> +
> +It allows to specify different s6-rc compiled dir (for example
> +/etc/s6/rc/compiled), without specifying it as command line argument
> +for s6-rc* tools.
> +
> +Signed-off-by: Vadim Kochan <vadim4j@gmail.com>

Has this patch been submitted upstream ? What does they say about it ?
Why can't we use the command line argument and avoid the patch ?


> diff --git a/package/s6-rc/rc.init b/package/s6-rc/rc.init
> new file mode 100644
> index 0000000000..6709c6488b
> --- /dev/null
> +++ b/package/s6-rc/rc.init
> @@ -0,0 +1,4 @@
> +#! /usr/bin/execlineb -P
> +
> +if { s6-rc-init /run/service }
> +if { s6-rc -t 600000 -- change default }

So this will replace the dummy rc.init script created in PATCH 2/3. So
why is PATCH 2/3 creating a rc.init file ?

> diff --git a/package/s6-rc/rc.shutdown b/package/s6-rc/rc.shutdown
> new file mode 100644
> index 0000000000..58d49b5cfd
> --- /dev/null
> +++ b/package/s6-rc/rc.shutdown
> @@ -0,0 +1,3 @@
> +#! /usr/bin/execlineb -P
> +
> +s6-rc -da change

Same question.

> diff --git a/package/s6-rc/s6-rc.mk b/package/s6-rc/s6-rc.mk
> index 222ba139c6..6bba54a28a 100644
> --- a/package/s6-rc/s6-rc.mk
> +++ b/package/s6-rc/s6-rc.mk
> @@ -11,6 +11,13 @@ S6_RC_LICENSE_FILES = COPYING
>  S6_RC_INSTALL_STAGING = YES
>  S6_RC_DEPENDENCIES = s6
>  
> +ifeq ($(BR2_INIT_S6),y)
> +# Needs s6-rc-compile to create initial rc db, also
> +# build after s6-linux-init to rewrite rc.init for run
> +# s6-rc services.
> +S6_RC_DEPENDENCIES += host-s6-rc s6-linux-init

Why do we rewrite rc.init ?

> +endif
> +
>  S6_RC_CONF_OPTS = \
>  	--prefix=/usr \
>  	--with-sysdeps=$(STAGING_DIR)/usr/lib/skalibs/sysdeps \
> @@ -20,6 +27,7 @@ S6_RC_CONF_OPTS = \
>  	--with-lib=$(STAGING_DIR)/usr/lib/s6 \
>  	--with-lib=$(STAGING_DIR)/usr/lib/skalibs \
>  	$(if $(BR2_STATIC_LIBS),,--disable-allstatic) \
> +	$(if $(BR2_INIT_S6),--compiledir=/etc/s6/rc/compiled,) \

You can drop the last comma, it is not necessary since you don't have a
"else" part in the condition.

>  	$(SHARED_STATIC_LIBS_OPTS)
>  
>  define S6_RC_CONFIGURE_CMDS
> @@ -44,6 +52,31 @@ define S6_RC_INSTALL_STAGING_CMDS
>  	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(STAGING_DIR) install
>  endef
>  
> +ifeq ($(BR2_INIT_S6),y)
> +
> +define S6_RC_PREPARE_INIT_RC
> +	mkdir -p $(TARGET_DIR)/etc/s6/rc/service/default
> +	echo bundle > $(TARGET_DIR)/etc/s6/rc/service/default/type
> +	touch $(TARGET_DIR)/etc/s6/rc/service/default/contents
> +
> +	mkdir -p $(TARGET_DIR)/etc/s6/rc/compiled-initial
> +	ln -sf compiled-initial $(TARGET_DIR)/etc/s6/rc/compiled

Why do we have this "compiled-initial" thing and a symlink to it ?

> +
> +	$(INSTALL) -m 0755 $(S6_RC_PKGDIR)/rc.init $(TARGET_DIR)/etc/rc.init
> +	$(INSTALL) -m 0755 $(S6_RC_PKGDIR)/rc.shutdown $(TARGET_DIR)/etc/rc.shutdown
> +endef
> +S6_RC_POST_INSTALL_TARGET_HOOKS += S6_RC_PREPARE_INIT_RC
> +
> +define S6_RC_FINALIZE_INIT_RC
> +	rm -rf $(TARGET_DIR)/etc/s6/rc/compiled-initial
> +	$(HOST_DIR)/bin/s6-rc-compile -v 1 \
> +		$(TARGET_DIR)/etc/s6/rc/compiled-initial \
> +		$(TARGET_DIR)/etc/s6/rc/service
> +endef
> +S6_RC_ROOTFS_PRE_CMD_HOOKS += S6_RC_FINALIZE_INIT_RC

Meh, I really don't like that this gets executed as a "rootfs pre
command" hook, but I understand the reasoning behind doing this after
rootfs overlay / post-build scripts have been handled. Do we need to
add another sort of hook at the end of the target-finalize target ?
Should we move the place where TARGET_FINALIZE_HOOKS are executed ? I'm
not sure.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 3/3] package/s6-rc: Allow to integrate s6-rc services

[Arnout Vandecappelle]

On 27/03/2019 19:54, Thomas Petazzoni wrote:
>> +define S6_RC_FINALIZE_INIT_RC
>> +	rm -rf $(TARGET_DIR)/etc/s6/rc/compiled-initial
>> +	$(HOST_DIR)/bin/s6-rc-compile -v 1 \
>> +		$(TARGET_DIR)/etc/s6/rc/compiled-initial \
>> +		$(TARGET_DIR)/etc/s6/rc/service
>> +endef
>> +S6_RC_ROOTFS_PRE_CMD_HOOKS += S6_RC_FINALIZE_INIT_RC
> Meh, I really don't like that this gets executed as a "rootfs pre
> command" hook, but I understand the reasoning behind doing this after
> rootfs overlay / post-build scripts have been handled. Do we need to
> add another sort of hook at the end of the target-finalize target ?
> Should we move the place where TARGET_FINALIZE_HOOKS are executed ? I'm
> not sure.

 At the moment, we have the same problem for Python: the pyc files are generated
in a finalize-hook, so any .py file from the overlay doesn't get compiled.

 On the other hand, a post-build script might want to make changes to something
that was done by Buildroot... Although without a concrete example I don't think
we need to try supporting such a use case.

 So, I've taken a quick look at all the finalize hooks we have, and there's
nothing that jumps out as "this you really want to do before the
overlay/post-build". I'm not sure though how it would interfere with the other
(non-hook) finalizations we're doing (i.e. removing static libs etc.). Every
hook would need to be reviewed in detail to be sure it's OK.

 If such a review is done, I wouldn't mind moving the hooks after the
overlay/post-build. I've checked the history, and I think the order of things is
mostly accidental. When post-build script was introduced (commit eed7d8737ad) it
was put at the end of target-finalize, without specific explanation. However, at
that time, purgelocales was still done after target-finalize (now it has moved
to a TARGET_FINALIZE_HOOK so it's done in the beginning). So I think there was
no real consideration at the time. When the overlay was added, it was put just
before the post-build script, which makes sense. And around that time,
everything had moved before the post-build script, so when the
TARGET_FINALIZE_HOOKS were introduced it made sense to keep that before the
post-build script as well.

 We do have to take into account, however, that this may break existing
overlays/post-build scripts in surprising ways... So it should definitely be
mentioned in the release notes!

 Regards,
 Arnout

[Buildroot] [PATCH 3/3] package/s6-rc: Allow to integrate s6-rc services

[Arnout Vandecappelle]

On 27/03/2019 21:35, Arnout Vandecappelle wrote:
>  If such a review is done, I wouldn't mind moving the hooks after the
> overlay/post-build.

 Something essential I forgot to mention: for this s6 patch, the same approach
as python should be followed: do it as a TARGET_FINALIZE_HOOK. Too bad that it
doesn't help for init scripts added in the overlay, but such is life.

 Regards,
 Arnout

[Buildroot] [PATCH 3/3] package/s6-rc: Allow to integrate s6-rc services

[Thomas Petazzoni]

On Wed, 27 Mar 2019 21:35:51 +0100
Arnout Vandecappelle <arnout@mind.be> wrote:

>  So, I've taken a quick look at all the finalize hooks we have, and there's
> nothing that jumps out as "this you really want to do before the
> overlay/post-build". I'm not sure though how it would interfere with the other
> (non-hook) finalizations we're doing (i.e. removing static libs etc.). Every
> hook would need to be reviewed in detail to be sure it's OK.
> 
>  If such a review is done, I wouldn't mind moving the hooks after the
> overlay/post-build. I've checked the history, and I think the order of things is
> mostly accidental. When post-build script was introduced (commit eed7d8737ad) it
> was put at the end of target-finalize, without specific explanation. However, at
> that time, purgelocales was still done after target-finalize (now it has moved
> to a TARGET_FINALIZE_HOOK so it's done in the beginning). So I think there was
> no real consideration at the time. When the overlay was added, it was put just
> before the post-build script, which makes sense. And around that time,
> everything had moved before the post-build script, so when the
> TARGET_FINALIZE_HOOKS were introduced it made sense to keep that before the
> post-build script as well.

I agree that the order of the steps is definitely not the result of a
lot of thinking, but is mostly accidental.

>  We do have to take into account, however, that this may break existing
> overlays/post-build scripts in surprising ways... So it should definitely be
> mentioned in the release notes!

Indeed, and that makes the change potentially a bit annoying :/

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 3/3] package/s6-rc: Allow to integrate s6-rc services

[Thomas Petazzoni]

Hello,

On Wed, 27 Mar 2019 21:37:38 +0100
Arnout Vandecappelle <arnout@mind.be> wrote:

> On 27/03/2019 21:35, Arnout Vandecappelle wrote:
> >  If such a review is done, I wouldn't mind moving the hooks after the
> > overlay/post-build.  
> 
>  Something essential I forgot to mention: for this s6 patch, the same approach
> as python should be followed: do it as a TARGET_FINALIZE_HOOK. Too bad that it
> doesn't help for init scripts added in the overlay, but such is life.

The downside is that for custom sysv init scripts or systemd unit
files, you can put them in an overlay/post-build, but for s6 "service"
files, you cannot do that: they must be installed by a package.

But perhaps that is an acceptable thing, at least in a first step.

Vadim: what do you think ? Would you be OK with a solution where only
packages can install s6 service files ?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/3] package/s6-linux-init: Build also for the host

[Vadym Kochan]

Hi Thomas,

On Wed, Mar 27, 2019 at 05:55:03PM +0100, Thomas Petazzoni wrote:
> Hello Vadim,
> 
> I am not familiar with s6, so sorry if I'm asking silly questions in
> this review.
> 
> On Sat, 16 Feb 2019 23:28:33 +0200
> Vadim Kochan <vadim4j@gmail.com> wrote:
> 
> > Add strict dependency for host-s6-linux-init to use s6-linux-init-maker
> > on the host during the rootfs generation.
> 
> I think the wording is not great here: "strict dependency" does not
> mean anything in the typical Buildroot speak. I think you wanted to say
> "We add host-s6-linux-init as a dependency of the target s6-linux-init,
> so that the latter can use the s6-linux-init-maker program as part of
> its build process".
> 
> However, I think adding this dependency should belong to PATCH 2/3,
> where s6-linux-init is actually changed to use s6-linux-init-maker.
> Indeed, as I understand it, it's only starting with PATCH 2/3 that
> s6-linux-init will need this program. Is this correct ?

Yes, will move it to the PATCH 2/3.
> 
> If it is, then PATCH 1/3 should only change s6-linux-init.mk to add the
> host variant of s6-linux-init.

Regards,
Vadim Kochan

[Buildroot] [PATCH 2/3] package/s6-linux-init: Allow to install as init system

[Vadym Kochan]

Hi Thomas,

On Wed, Mar 27, 2019 at 07:43:12PM +0100, Thomas Petazzoni wrote:
> Hello Vadim,
> 
> On Sat, 16 Feb 2019 23:28:34 +0200
> Vadim Kochan <vadim4j@gmail.com> wrote:
> 
> > +ifeq ($(BR2_INIT_S6),y)
> > +
> > +# Don't let Busybox to install it's own tools like poweroff, reboot, halt, etc
> > +S6_LINUX_INIT_DEPENDENCIES += $(if $(BR2_PACKAGE_BUSYBOX),busybox)
> 
> The comment is not good: i doesn't prevent Busybox from installing
> these programs, but it ensures Busybox is built before, and therefore
> we have the ability to "win" over Busybox as we can overwrite those
> programs.
> 
> However, this is not how we handle this situation. In fact, we handle
> it in exactly the opposite way. In package/busybox/busybox.mk, we add
> to BUSYBOX_DEPENDENCIES all packages that install programs that
> "conflict" with Busybox applets. This way, we are sure they are built
> before Busybox. And the installation process of Busybox has been
> carefully modified to not overwrite an existing program by a Busybox
> applet if this program already exists.
> 
> So, you should use the same model for s6-linux-init.
> 
> > +
> > +S6_LINUX_INIT_MAKER_OPTS = -b /usr/bin -c /etc/s6
> > +
> > +ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
> > +S6_LINUX_INIT_MAKER_OPTS += -d 0
> > +else
> > +S6_LINUX_INIT_MAKER_OPTS += $(if $(BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_DEVTMPFS),,-d 1)
> 
> So you need -d1 in which cases ? Where udev is used ?
> 
> What about:
> 
> ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
> S6_LINUX_INIT_MAKER_OPTS += -d 0
> else ifeq ($(BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_MDEV)$(BR2_PACKAGE_HAS_UDEV),y)
> S6_LINUX_INIT_MAKER_OPTS += -d 1
> endif
> 
> or something like that ?
> 
> > +endif
> > +
> > +ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
> > +S6_LINUX_INIT_MAKER_OPTS += -G "/sbin/getty -L \
> > +			  $(SYSTEM_GETTY_OPTIONS) \
> > +			  $(SYSTEM_GETTY_PORT) \
> > +			  $(SYSTEM_GETTY_BAUDRATE) \
> > +			  $(SYSTEM_GETTY_TERM)"
> 
> I'd say keep this on one single line.
> 
> > +endif
> > +
> > +define S6_LINUX_INIT_INSTALL_INIT
> > +	ln -sf ../etc/s6/init $(TARGET_DIR)/sbin/init
> > +
> > +	ln -sf /usr/bin/s6-reboot $(TARGET_DIR)/sbin/reboot
> > +	ln -sf /usr/bin/s6-poweroff $(TARGET_DIR)/sbin/poweroff
> > +	ln -sf /usr/bin/s6-halt $(TARGET_DIR)/sbin/halt
> > +
> > +	echo "#! /usr/bin/execlineb -P" > $(@D)/rc.init
> 
> What is this empty script doing ? Perhaps we should have it as a file
> in package/s6-linux-init/ instead of generating it.
> 
> > +	$(INSTALL) -m 0755 $(@D)/rc.init $(TARGET_DIR)/etc/rc.init
> > +	echo "#! /usr/bin/execlineb -P" > $(@D)/rc.shutdown
> > +	$(INSTALL) -m 0755 $(@D)/rc.shutdown $(TARGET_DIR)/etc/rc.shutdown
> 
> Same question about what this empty script is doing.

These scripts are required by s6-linux-init to execute them on
start/shutdown, I decided to do not add them as separae files to
package but rather generate them as dummy scripts.
> 
> > +	rm -rf $(TARGET_DIR)/etc/s6
> 
> Any reason to have this rm -rf ?

Do not remember for sure, but I think that s6-linux-init generation tool
was not happy that files were existed already on destination if to
re-build package on 2nd time, of course if it will be not needed then it
should be removed.

> 
> > +	$(HOST_DIR)/bin/s6-linux-init-maker $(S6_LINUX_INIT_MAKER_OPTS) $(TARGET_DIR)/etc/s6
> > +endef
> > +S6_LINUX_INIT_POST_INSTALL_TARGET_HOOKS += S6_LINUX_INIT_INSTALL_INIT
> > +
> > +endif # BR2_INIT_S6

Regards,
Vadim Kochan

[Buildroot] [PATCH 3/3] package/s6-rc: Allow to integrate s6-rc services

[Vadym Kochan]

Hi Arnout, Thomas, Eric, All

On Thu, Mar 28, 2019 at 03:23:46PM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Wed, 27 Mar 2019 21:37:38 +0100
> Arnout Vandecappelle <arnout@mind.be> wrote:
> 
> > On 27/03/2019 21:35, Arnout Vandecappelle wrote:
> > >  If such a review is done, I wouldn't mind moving the hooks after the
> > > overlay/post-build.  
> > 
> >  Something essential I forgot to mention: for this s6 patch, the same approach
> > as python should be followed: do it as a TARGET_FINALIZE_HOOK. Too bad that it
> > doesn't help for init scripts added in the overlay, but such is life.
> 
> The downside is that for custom sysv init scripts or systemd unit
> files, you can put them in an overlay/post-build, but for s6 "service"
> files, you cannot do that: they must be installed by a package.
> 
> But perhaps that is an acceptable thing, at least in a first step.
> 
> Vadim: what do you think ? Would you be OK with a solution where only
> packages can install s6 service files ?
> 

I think it might be enough for the first step, then to add custom s6
scripts it will be needed to add special custom package in custom exernal tree
which will install the needed s6 scripts. BTW, to manually re-install the
services by buildroot from command line it will be needed to call re-installation
of all packages to re-generate s6-init db again.

Regards,
Vadim Kochan

[Buildroot] [PATCH 3/3] package/s6-rc: Allow to integrate s6-rc services

[Vadim Kochan]

Hi Thomas,

On Wed, Mar 27, 2019 at 07:54:09PM +0100, Thomas Petazzoni wrote:
> Hello Vadim,
> 
> On Sat, 16 Feb 2019 23:28:35 +0200
> Vadim Kochan <vadim4j@gmail.com> wrote:
> 
> > It allows to install s6-rc services by post-build, rootfs-overlay or some
> > package into /etc/s6/rc/service which will be compiled as s6-rc db as
> > /etc/s6/rc/compiled-initial. Services are compiled on stage when rootfs overlay
> > & post-build already performed.
> 
> This last sentence is confusing. What about "Services at compiled after
> the rootfs overlay have been copied and post-build scripts have been
> executed".
> 
> > Added rc.init & rc.shutdown scripts which are needed to run s6-rc
> > services.
> 
> Isn't this already done in PATCH 2/3 ?

Yeah, I messed things up a bit... Initially I was thinking to have ability
to have 2 s6 init options:

    1) s6-linut-init only - with customized everything by user

    2) s6-rc - this is s6-linux-init + s6 solution, so thats why I
    was rewritting what created by s6-linux-init

but now I think s6-rc should be enough, and I think that PATCH 2/3 & 3/3
might be squashed.

> 
> > This is very basic s6-rc system support which even does not have the
> > basic stuff like /proc /sys /dev setup, but this might be added in
> > future work.
> > 
> > All above is only possible if BR2_INIT_S6 is selected.
> 
> "possible" ? "enabled" perhaps ?
> 
> 
> > diff --git a/package/s6-rc/0001-configure-Allow-to-specify-compiled-base-dir.patch b/package/s6-rc/0001-configure-Allow-to-specify-compiled-base-dir.patch
> > new file mode 100644
> > index 0000000000..baf1706bb3
> > --- /dev/null
> > +++ b/package/s6-rc/0001-configure-Allow-to-specify-compiled-base-dir.patch
> > @@ -0,0 +1,76 @@
> > +From cb89562fcec582bbb5bc9a3bf5faf33338e6b535 Mon Sep 17 00:00:00 2001
> > +From: Vadim Kochan <vadim4j@gmail.com>
> > +Date: Wed, 13 Feb 2019 03:36:03 +0200
> > +Subject: [PATCH] configure: Allow to specify compiled base dir
> > +
> > +It allows to specify different s6-rc compiled dir (for example
> > +/etc/s6/rc/compiled), without specifying it as command line argument
> > +for s6-rc* tools.
> > +
> > +Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
> 
> Has this patch been submitted upstream ? What does they say about it ?
> Why can't we use the command line argument and avoid the patch ?

Without this patch then it needs to manually type this path /etc/s6/rc/compiled,
in case if to run s6-rc from command-line.

> 
> 
> > diff --git a/package/s6-rc/rc.init b/package/s6-rc/rc.init
> > new file mode 100644
> > index 0000000000..6709c6488b
> > --- /dev/null
> > +++ b/package/s6-rc/rc.init
> > @@ -0,0 +1,4 @@
> > +#! /usr/bin/execlineb -P
> > +
> > +if { s6-rc-init /run/service }
> > +if { s6-rc -t 600000 -- change default }
> 
> So this will replace the dummy rc.init script created in PATCH 2/3. So
> why is PATCH 2/3 creating a rc.init file ?

As I wrote above - these patches might be squashed.

> 
> > diff --git a/package/s6-rc/rc.shutdown b/package/s6-rc/rc.shutdown
> > new file mode 100644
> > index 0000000000..58d49b5cfd
> > --- /dev/null
> > +++ b/package/s6-rc/rc.shutdown
> > @@ -0,0 +1,3 @@
> > +#! /usr/bin/execlineb -P
> > +
> > +s6-rc -da change
> 
> Same question.
> 
> > diff --git a/package/s6-rc/s6-rc.mk b/package/s6-rc/s6-rc.mk
> > index 222ba139c6..6bba54a28a 100644
> > --- a/package/s6-rc/s6-rc.mk
> > +++ b/package/s6-rc/s6-rc.mk
> > @@ -11,6 +11,13 @@ S6_RC_LICENSE_FILES = COPYING
> >  S6_RC_INSTALL_STAGING = YES
> >  S6_RC_DEPENDENCIES = s6
> >  
> > +ifeq ($(BR2_INIT_S6),y)
> > +# Needs s6-rc-compile to create initial rc db, also
> > +# build after s6-linux-init to rewrite rc.init for run
> > +# s6-rc services.
> > +S6_RC_DEPENDENCIES += host-s6-rc s6-linux-init
> 
> Why do we rewrite rc.init ?
> 
> > +endif
> > +
> >  S6_RC_CONF_OPTS = \
> >  	--prefix=/usr \
> >  	--with-sysdeps=$(STAGING_DIR)/usr/lib/skalibs/sysdeps \
> > @@ -20,6 +27,7 @@ S6_RC_CONF_OPTS = \
> >  	--with-lib=$(STAGING_DIR)/usr/lib/s6 \
> >  	--with-lib=$(STAGING_DIR)/usr/lib/skalibs \
> >  	$(if $(BR2_STATIC_LIBS),,--disable-allstatic) \
> > +	$(if $(BR2_INIT_S6),--compiledir=/etc/s6/rc/compiled,) \
> 
> You can drop the last comma, it is not necessary since you don't have a
> "else" part in the condition.
> 
> >  	$(SHARED_STATIC_LIBS_OPTS)
> >  
> >  define S6_RC_CONFIGURE_CMDS
> > @@ -44,6 +52,31 @@ define S6_RC_INSTALL_STAGING_CMDS
> >  	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(STAGING_DIR) install
> >  endef
> >  
> > +ifeq ($(BR2_INIT_S6),y)
> > +
> > +define S6_RC_PREPARE_INIT_RC
> > +	mkdir -p $(TARGET_DIR)/etc/s6/rc/service/default
> > +	echo bundle > $(TARGET_DIR)/etc/s6/rc/service/default/type
> > +	touch $(TARGET_DIR)/etc/s6/rc/service/default/contents
> > +
> > +	mkdir -p $(TARGET_DIR)/etc/s6/rc/compiled-initial
> > +	ln -sf compiled-initial $(TARGET_DIR)/etc/s6/rc/compiled
> 
> Why do we have this "compiled-initial" thing and a symlink to it ?

Hm, this is for case when upgrade to new s6-rc db on the target, so
it will just need to point link on newer db.

> 
> > +
> > +	$(INSTALL) -m 0755 $(S6_RC_PKGDIR)/rc.init $(TARGET_DIR)/etc/rc.init
> > +	$(INSTALL) -m 0755 $(S6_RC_PKGDIR)/rc.shutdown $(TARGET_DIR)/etc/rc.shutdown
> > +endef
> > +S6_RC_POST_INSTALL_TARGET_HOOKS += S6_RC_PREPARE_INIT_RC
> > +
> > +define S6_RC_FINALIZE_INIT_RC
> > +	rm -rf $(TARGET_DIR)/etc/s6/rc/compiled-initial
> > +	$(HOST_DIR)/bin/s6-rc-compile -v 1 \
> > +		$(TARGET_DIR)/etc/s6/rc/compiled-initial \
> > +		$(TARGET_DIR)/etc/s6/rc/service
> > +endef
> > +S6_RC_ROOTFS_PRE_CMD_HOOKS += S6_RC_FINALIZE_INIT_RC
> 
> Meh, I really don't like that this gets executed as a "rootfs pre
> command" hook, but I understand the reasoning behind doing this after
> rootfs overlay / post-build scripts have been handled. Do we need to
> add another sort of hook at the end of the target-finalize target ?
> Should we move the place where TARGET_FINALIZE_HOOKS are executed ? I'm
> not sure.
> 

So, then use TARGET_FINALIZE_HOOKS for the 1st step ?

Regards,
Vadim Kochan

[Buildroot] [PATCH 3/3] package/s6-rc: Allow to integrate s6-rc services

[Vadim Kochan]

Hi Thomas,

[SNIP]

> > > diff --git a/package/s6-rc/0001-configure-Allow-to-specify-compiled-base-dir.patch b/package/s6-rc/0001-configure-Allow-to-specify-compiled-base-dir.patch
> > > new file mode 100644
> > > index 0000000000..baf1706bb3
> > > --- /dev/null
> > > +++ b/package/s6-rc/0001-configure-Allow-to-specify-compiled-base-dir.patch
> > > @@ -0,0 +1,76 @@
> > > +From cb89562fcec582bbb5bc9a3bf5faf33338e6b535 Mon Sep 17 00:00:00 2001
> > > +From: Vadim Kochan <vadim4j@gmail.com>
> > > +Date: Wed, 13 Feb 2019 03:36:03 +0200
> > > +Subject: [PATCH] configure: Allow to specify compiled base dir
> > > +
> > > +It allows to specify different s6-rc compiled dir (for example
> > > +/etc/s6/rc/compiled), without specifying it as command line argument
> > > +for s6-rc* tools.
> > > +
> > > +Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
> > 
> > Has this patch been submitted upstream ? What does they say about it ?
> > Why can't we use the command line argument and avoid the patch ?
> 
> Without this patch then it needs to manually type this path /etc/s6/rc/compiled,
> in case if to run s6-rc from command-line.

Sorry, this is not a good explanation. By default s6-linux-init is
installed to /etc/s6-init and s6-rc into /etc/s6-rc, but I decided to
put them together into /etc/s6, thats why it needs patch which allows to
define new default db's base dir in /etc/s6.
 
Regards,
Vadim Kochan