All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] package/openrc: new init package OpenRC (0.38.3)
@ 2019-02-28 19:44 Michał Łyszczek
  2019-03-04 18:59 ` Arnout Vandecappelle
  0 siblings, 1 reply; 12+ messages in thread
From: Michał Łyszczek @ 2019-02-28 19:44 UTC (permalink / raw
  To: buildroot

OpenRC is a dependency based init system that works with the system
provided init program, normally located at /sbin/init. It is not a
replacement for /sbin/init.

Signed-off-by: Micha? ?yszczek <michal.lyszczek@bofc.pl>
---
OpenRC uses own set of scripts for sysinit thus
BR2_PACKAGE_INITSCRIPTS is disabled. I decided to add new init
choice and choice which program to use as PID1 since OpenRC uses
its own scripts for services so S** scripts cannot be used. It is
possible to add simple script to local.d that would start all S**
scripts, but that would diminish OpenRC's features and usefullness,
so additional startup scripts should be written and added to
buildroot. If OpenRC init is accepted into Buildroot, I will commit
some time to port existing sysvinit startup scripts to OpenRC.

OpenRC works with default config - a single change is needed (which
init system to use, with optional PID1 change - default BusyBox). I
tested it with both BusyBox and sysvinit as PID1. Also tested with
BusyBox programs enabled and disabled (standalone apps used, none
of which was from BusyBox package). I tested it using
qemu-x86_64_defconfig and beaglebone_defconfig.


 DEVELOPERS                                    |  1 +
 package/Config.in                             |  1 +
 ...ecking-for-lib-dir-while-cross-compi.patch | 25 ++++++++
 package/openrc/Config.in                      | 19 ++++++
 package/openrc/inittab                        | 22 +++++++
 package/openrc/openrc.hash                    |  2 +
 package/openrc/openrc.mk                      | 63 +++++++++++++++++++
 package/sysvinit/Config.in                    |  2 +-
 system/Config.in                              | 47 ++++++++++++--
 9 files changed, 177 insertions(+), 5 deletions(-)
 create mode 100644 package/openrc/0001-mk-sys.mk-fix-checking-for-lib-dir-while-cross-compi.patch
 create mode 100644 package/openrc/Config.in
 create mode 100644 package/openrc/inittab
 create mode 100644 package/openrc/openrc.hash
 create mode 100644 package/openrc/openrc.mk

diff --git a/DEVELOPERS b/DEVELOPERS
index f9b6a0e7d7..7beb582808 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -1555,6 +1555,7 @@ F:	board/altera/socrates_cyclone5/
 F:	board/pine64/rock64
 F:	configs/rock64_defconfig
 F:	configs/socrates_cyclone5_defconfig
+F:	package/openrc
 
 N:	Mike Harmony <mike.harmony@snapav.com>
 F:	board/sinovoip/m2-plus/
diff --git a/package/Config.in b/package/Config.in
index cc232b9fba..9a40003192 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -2159,6 +2159,7 @@ menu "System tools"
 	source "package/ncdu/Config.in"
 	source "package/numactl/Config.in"
 	source "package/nut/Config.in"
+	source "package/openrc/Config.in"
 	source "package/openvmtools/Config.in"
 	source "package/pamtester/Config.in"
 	source "package/polkit/Config.in"
diff --git a/package/openrc/0001-mk-sys.mk-fix-checking-for-lib-dir-while-cross-compi.patch b/package/openrc/0001-mk-sys.mk-fix-checking-for-lib-dir-while-cross-compi.patch
new file mode 100644
index 0000000000..645411dd40
--- /dev/null
+++ b/package/openrc/0001-mk-sys.mk-fix-checking-for-lib-dir-while-cross-compi.patch
@@ -0,0 +1,25 @@
+From 5e0e0d488c775a921db5c131d3763430833d8d50 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Micha=C5=82=20=C5=81yszczek?= <michal.lyszczek@bofc.pl>
+Date: Wed, 27 Feb 2019 16:12:02 +0100
+Subject: [PATCH] mk/sys.mk: fix checking for lib dir while cross compiling
+
+---
+ mk/sys.mk | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/mk/sys.mk b/mk/sys.mk
+index 92bb55ea..c01f3efb 100644
+--- a/mk/sys.mk
++++ b/mk/sys.mk
+@@ -47,7 +47,7 @@ SBINMODE?=		0755
+ INCDIR?=		${UPREFIX}/include
+ INCMODE?=		0644
+ 
+-_LIBNAME_SH=		case `readlink /lib` in /lib64|lib64) echo "lib64";; *) echo "lib";; esac
++_LIBNAME_SH=		case `readlink $(TARGETDIR)/lib` in /lib64|lib64) echo "lib64";; *) echo "lib";; esac
+ _LIBNAME:=		$(shell ${_LIBNAME_SH})
+ LIBNAME?=		${_LIBNAME}
+ LIBDIR?=		${UPREFIX}/${LIBNAME}
+-- 
+2.18.1
+
diff --git a/package/openrc/Config.in b/package/openrc/Config.in
new file mode 100644
index 0000000000..4f9eac6452
--- /dev/null
+++ b/package/openrc/Config.in
@@ -0,0 +1,19 @@
+config BR2_PACKAGE_OPENRC
+	bool "OpenRC"
+	select BR2_PACKAGE_UTIL_LINUX
+	select BR2_PACKAGE_UTIL_LINUX_AGETTY
+	select BR2_PACKAGE_NCURSES
+	select BR2_PACKAGE_BUSYBOX if BR2_OPENRC_PID1_BUSYBOX
+	select BR2_PACKAGE_SYSVINIT if BR2_OPENRC_PID1_SYSVINIT
+	help
+	  Init that works on top of pid 1 (for example sysvinit). By default
+	  it does quite a lot on startup (like setting hwclock, mounting
+	  directories, configuring interfaces and so on). So for this init to
+	  properly work you need at least these tools (most of which come with
+	  BusyBox) on the root filesystem
+
+	  swapon, fsck, hwclock, agetty, login, grep, mount, coreutils, procps,
+	  modprobe (kmod), net-tools
+
+	  OpenRC will work without them, but some services won't start.
+	  Number of tools may be decreased by removing services that use them.
diff --git a/package/openrc/inittab b/package/openrc/inittab
new file mode 100644
index 0000000000..5540fb5fbb
--- /dev/null
+++ b/package/openrc/inittab
@@ -0,0 +1,22 @@
+# /etc/inittab
+#
+# systemv inittab that gives control to OpenRC
+id:3:initdefault:
+
+# sysinit, mounts /dev and /sys, initializes cgroup etc.
+si::sysinit:/sbin/openrc sysinit
+
+# configure network, local mounts, time, swap, loads modules etc.
+rc::bootwait:/sbin/openrc boot
+
+# runlevels
+l0:0:wait:/sbin/openrc shutdown
+l1:1:wait:/sbin/openrc single
+l2:2:wait:/sbin/openrc nonetwork
+l3:3:wait:/sbin/openrc default
+l4:4:wait:/sbin/openrc default
+l5:5:wait:/sbin/openrc default
+l6:6:wait:/sbin/openrc reboot
+
+# Stuff to do for the 3-finger salute
+#ca::ctrlaltdel:/sbin/reboot
diff --git a/package/openrc/openrc.hash b/package/openrc/openrc.hash
new file mode 100644
index 0000000000..d18aa5f680
--- /dev/null
+++ b/package/openrc/openrc.hash
@@ -0,0 +1,2 @@
+# Calculated manually
+sha256	ebd0d0462ab8eb375a232c1d9c1ddac11957ac93fae4935441353dd2c1fb01ec	openrc-0.38.3.tar.gz
diff --git a/package/openrc/openrc.mk b/package/openrc/openrc.mk
new file mode 100644
index 0000000000..1ac256764d
--- /dev/null
+++ b/package/openrc/openrc.mk
@@ -0,0 +1,63 @@
+OPENRC_VERSION = 0.38.3
+OPENRC_SOURCE = openrc-$(OPENRC_VERSION).tar.gz
+OPENRC_SITE = $(call github,OpenRC,openrc,$(OPENRC_VERSION))
+OPENRC_LICENSE = BSD-2-Clause
+OPENRC_LICENSE_FILES = LICENSE
+OPENRC_DEPENDENCIES = ncurses
+
+ifeq ($(BR2_OPENRC_PID1_SYSVINIT),y)
+	OPENRC_DEPENDENCIES += sysvinit
+endif
+
+ifeq ($(BR2_OPENRC_PID1_BUSYBOX),y)
+	OPENRC_DEPENDENCIES += busybox
+endif
+
+ifeq ($(BR2_PACKAGE_BASH),y)
+	OPENRC_MAKE_OPTS = MKBASHCOMP=yes
+endif
+
+ifeq ($(BR2_PACKAGE_ZSH),y)
+	OPENRC_MAKE_OPTS += MKZSHCOMP=yes
+endif
+
+ifeq ($(BR2_SHARED_LIBS),y)
+	OPENRC_MAKE_OPTS += MKSTATICLIBS=no
+else
+	OPENRC_MAKE_OPTS += MKSTATICLIBS=yes
+endif
+
+OPENRC_MAKE_OPTS += TARGETDIR=$(TARGET_DIR)
+OPENRC_MAKE_OPTS += MKNET=yes
+OPENRC_MAKE_OPTS += MKPKGCONFIG=no
+OPENRC_MAKE_OPTS += MKSELINUX=no
+OPENRC_MAKE_OPTS += MKSYSVINIT=yes
+OPENRC_MAKE_OPTS += BRANDING=$(BR2_OPENRC_BRANDING)
+OPENRC_MAKE_OPTS += CC=$(TARGET_CC)
+
+define OPENRC_BUILD_CMDS
+	$(MAKE) $(OPENRC_MAKE_OPTS) -C $(@D)
+endef
+
+define OPENRC_INSTALL_TARGET_CMDS
+	DESTDIR=$(TARGET_DIR) $(MAKE) $(OPENRC_MAKE_OPTS) -C $(@D) install
+	$(INSTALL) -D -m 0644 package/openrc/inittab $(TARGET_DIR)/etc/inittab
+endef
+
+ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
+define OPENRC_SET_GETTY
+	ln -sf agetty $(TARGET_DIR)/etc/init.d/agetty.$(SYSTEM_GETTY_PORT)
+	ln -sf /etc/init.d/agetty.$(SYSTEM_GETTY_PORT) $(TARGET_DIR)/etc/runlevels/default
+	$(SED) '/#term_type=/c\term_type="$(SYSTEM_GETTY_TERM)"' $(TARGET_DIR)/etc/conf.d/agetty
+	$(SED) '/#agetty_options=/c\agetty_options="$(SYSTEM_GETTY_OPTIONS)"' $(TARGET_DIR)/etc/conf.d/agetty
+	if [ $(SYSTEM_GETTY_BAUDRATE) != 0 ]; then \
+		$(SED) '/#baud=/c\baud="$(SYSTEM_GETTY_BAUDRATE)"' $(TARGET_DIR)/etc/conf.d/agetty; \
+	fi
+	if [ x$(BR2_PACKAGE_UTIL_LINUX_AGETTY) = x ]; then \
+		ln -sf getty $(TARGET_DIR)/sbin/agetty; \
+	fi
+endef
+OPENRC_TARGET_FINALIZE_HOOKS += OPENRC_SET_GETTY
+endif # BR2_TARGET_GENERIC_GETTY
+
+$(eval $(generic-package))
diff --git a/package/sysvinit/Config.in b/package/sysvinit/Config.in
index 7f27a70fcc..0ea5221ad8 100644
--- a/package/sysvinit/Config.in
+++ b/package/sysvinit/Config.in
@@ -1,7 +1,7 @@
 config BR2_PACKAGE_SYSVINIT
 	bool "sysvinit"
 	depends on BR2_USE_MMU # fork()
-	depends on BR2_INIT_SYSV
+	depends on BR2_INIT_SYSV || BR2_INIT_OPENRC
 	depends on BR2_PACKAGE_BUSYBOX_SHOW_OTHERS
 	help
 	  System V style implementation of /sbin/init, parent of all
diff --git a/system/Config.in b/system/Config.in
index 498b56e222..12a59ed7d6 100644
--- a/system/Config.in
+++ b/system/Config.in
@@ -10,6 +10,7 @@ choice
 config BR2_ROOTFS_SKELETON_DEFAULT
 	bool "default target skeleton"
 	select BR2_PACKAGE_SKELETON_INIT_SYSV if BR2_INIT_SYSV
+	select BR2_PACKAGE_SKELETON_INIT_SYSV if BR2_INIT_OPENRC
 	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_NONE if BR2_INIT_NONE
@@ -120,6 +121,11 @@ 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_OPENRC
+	bool "OpenRC"
+	select BR2_PACKAGE_OPENRC
+	# select BR2_PACKAGE_INITSCRIPTS # OpenRC has its own scripts
+
 config BR2_INIT_NONE
 	bool "None"
 	help
@@ -129,6 +135,39 @@ config BR2_INIT_NONE
 
 endchoice
 
+if BR2_INIT_OPENRC
+
+config BR2_OPENRC_BRANDING
+	string "OpenRC branding"
+	default "Buildroot"
+	depends on BR2_INIT_OPENRC
+	help
+	  Custom string that will show up when OpenRC starts like:
+
+	  OpenRC 0.38.3.8ee04a5 is starting up <branding-name> (armv7l)
+
+choice
+	prompt "PID1 to use with OpenRC"
+	default BR2_OPENRC_PID1_BUSYBOX
+	help
+	  Since OpenRC is not replacement for /sbin/init but works with
+	  it, you can select which PID1 service you want to run
+
+config BR2_OPENRC_PID1_BUSYBOX
+	bool "BusyBox"
+	# select BR2_PACKAGE_INITSCRIPTS # OpenRC has its own scripts
+
+config BR2_OPENRC_PID1_SYSVINIT
+	bool "systemV"
+	depends on BR2_USE_MMU # sysvinit
+	select BR2_PACKAGE_BUSYBOX_SHOW_OTHERS # sysvinit
+	select BR2_PACKAGE_SYSVINIT
+	# select BR2_PACKAGE_INITSCRIPTS # OpenRC has its own scripts
+
+endchoice
+
+endif # BR2_INIT_OPENRC
+
 choice
 	prompt "/dev management" if !BR2_INIT_SYSTEMD
 	default BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_DEVTMPFS
@@ -337,16 +376,16 @@ config BR2_TARGET_GENERIC_GETTY_BAUDRATE
 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
+	# currently observed only by busybox, sysvinit and openrc
+	depends on BR2_INIT_BUSYBOX || BR2_INIT_SYSV || BR2_INIT_OPENRC
 	help
 	  Specify a TERM type.
 
 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
+	# currently observed only by busybox, sysvinit and openrc
+	depends on BR2_INIT_BUSYBOX || BR2_INIT_SYSV || BR2_INIT_OPENRC
 	help
 	  Any other flags you want to pass to getty,
 	  Refer to getty --help for details.
-- 
2.18.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH] package/openrc: new init package OpenRC (0.38.3)
  2019-02-28 19:44 [Buildroot] [PATCH] package/openrc: new init package OpenRC (0.38.3) Michał Łyszczek
@ 2019-03-04 18:59 ` Arnout Vandecappelle
  2019-03-05 21:38   ` Yann E. MORIN
  0 siblings, 1 reply; 12+ messages in thread
From: Arnout Vandecappelle @ 2019-03-04 18:59 UTC (permalink / raw
  To: buildroot

 Hi Micha?,

 I can tell you, we don't add something like a new init system easily, so this
may take some time to get merged...

 First of all, the patch that adds the openrc package as an ordinary package
(not an init system) should be separate. That one has to be standalone, obviously.

On 28/02/2019 20:44, Micha? ?yszczek wrote:
> OpenRC is a dependency based init system that works with the system
> provided init program, normally located at /sbin/init. It is not a
> replacement for /sbin/init.
> 
> Signed-off-by: Micha? ?yszczek <michal.lyszczek@bofc.pl>
> ---
> OpenRC uses own set of scripts for sysinit thus
> BR2_PACKAGE_INITSCRIPTS is disabled. I decided to add new init
> choice and choice which program to use as PID1 since OpenRC uses
> its own scripts for services so S** scripts cannot be used. It is
> possible to add simple script to local.d that would start all S**
> scripts, but that would diminish OpenRC's features and usefullness,
> so additional startup scripts should be written and added to
> buildroot.

 While this is true, I think you should still add that script so that packages
which don't have an OpenRC service description will still work. (We should also
have done that for systemd IMO.) Then you can have something like thisin
pkg-generic.mk:

$(2)_INSTALL_INIT_OPENRC ?= $$($(2)_INSTALL_INIT_SYSV)

so if a package defines its OpenRC file, that one will be used instead of the
sysvinit script.

 With that in mind, I think an OpenRC skeelton would make sense at well. It
would be pretty empty :-).

> If OpenRC init is accepted into Buildroot, I will commit
> some time to port existing sysvinit startup scripts to OpenRC.
> 
> OpenRC works with default config - a single change is needed (which
> init system to use, with optional PID1 change - default BusyBox). I
> tested it with both BusyBox and sysvinit as PID1. Also tested with
> BusyBox programs enabled and disabled (standalone apps used, none
> of which was from BusyBox package). I tested it using
> qemu-x86_64_defconfig and beaglebone_defconfig.

 If this gets added as a full BR2_INIT_ option, you should also add the runtime
tests for it (in separate patches, obviously). Look at the system tests for
inspiration.

[snip]
> diff --git a/package/openrc/0001-mk-sys.mk-fix-checking-for-lib-dir-while-cross-compi.patch b/package/openrc/0001-mk-sys.mk-fix-checking-for-lib-dir-while-cross-compi.patch
> new file mode 100644
> index 0000000000..645411dd40
> --- /dev/null
> +++ b/package/openrc/0001-mk-sys.mk-fix-checking-for-lib-dir-while-cross-compi.patch
> @@ -0,0 +1,25 @@
> +From 5e0e0d488c775a921db5c131d3763430833d8d50 Mon Sep 17 00:00:00 2001
> +From: =?UTF-8?q?Micha=C5=82=20=C5=81yszczek?= <michal.lyszczek@bofc.pl>
> +Date: Wed, 27 Feb 2019 16:12:02 +0100
> +Subject: [PATCH] mk/sys.mk: fix checking for lib dir while cross compiling
> +
> +---
> + mk/sys.mk | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/mk/sys.mk b/mk/sys.mk
> +index 92bb55ea..c01f3efb 100644
> +--- a/mk/sys.mk
> ++++ b/mk/sys.mk
> +@@ -47,7 +47,7 @@ SBINMODE?=		0755
> + INCDIR?=		${UPREFIX}/include
> + INCMODE?=		0644
> + 
> +-_LIBNAME_SH=		case `readlink /lib` in /lib64|lib64) echo "lib64";; *) echo "lib";; esac
> ++_LIBNAME_SH=		case `readlink $(TARGETDIR)/lib` in /lib64|lib64) echo "lib64";; *) echo "lib";; esac

 Is TARGETDIR something you introduced yourself? It doesn't look right...
Typically you'd instead use $(DESTDIR)$(PREFIX)/lib.

 If this gets used during build as well, it's not so good but not much that can
be done about it, so you'd have to pass DESTDIR=$(TARGET_DIR) during build as well.

> + _LIBNAME:=		$(shell ${_LIBNAME_SH})
> + LIBNAME?=		${_LIBNAME}
> + LIBDIR?=		${UPREFIX}/${LIBNAME}
> +-- 
> +2.18.1
> +
> diff --git a/package/openrc/Config.in b/package/openrc/Config.in
> new file mode 100644
> index 0000000000..4f9eac6452
> --- /dev/null
> +++ b/package/openrc/Config.in
> @@ -0,0 +1,19 @@
> +config BR2_PACKAGE_OPENRC
> +	bool "OpenRC"
> +	select BR2_PACKAGE_UTIL_LINUX
> +	select BR2_PACKAGE_UTIL_LINUX_AGETTY

 Does it really depend on agetty? Seems strange...

 That said, if additional tweaks are needed to support busybox getty, that can
be done in a follow-up patch.

> +	select BR2_PACKAGE_NCURSES
> +	select BR2_PACKAGE_BUSYBOX if BR2_OPENRC_PID1_BUSYBOX
> +	select BR2_PACKAGE_SYSVINIT if BR2_OPENRC_PID1_SYSVINIT
> +	help
> +	  Init that works on top of pid 1 (for example sysvinit). By default
> +	  it does quite a lot on startup (like setting hwclock, mounting
> +	  directories, configuring interfaces and so on). So for this init to
> +	  properly work you need at least these tools (most of which come with
> +	  BusyBox) on the root filesystem
> +
> +	  swapon, fsck, hwclock, agetty, login, grep, mount, coreutils, procps,
> +	  modprobe (kmod), net-tools> +
> +	  OpenRC will work without them, but some services won't start.
> +	  Number of tools may be decreased by removing services that use them.
> diff --git a/package/openrc/inittab b/package/openrc/inittab
> new file mode 100644
> index 0000000000..5540fb5fbb
> --- /dev/null
> +++ b/package/openrc/inittab
> @@ -0,0 +1,22 @@
> +# /etc/inittab
> +#
> +# systemv inittab that gives control to OpenRC
> +id:3:initdefault:
> +
> +# sysinit, mounts /dev and /sys, initializes cgroup etc.
> +si::sysinit:/sbin/openrc sysinit
> +
> +# configure network, local mounts, time, swap, loads modules etc.
> +rc::bootwait:/sbin/openrc boot
> +
> +# runlevels
> +l0:0:wait:/sbin/openrc shutdown
> +l1:1:wait:/sbin/openrc single
> +l2:2:wait:/sbin/openrc nonetwork
> +l3:3:wait:/sbin/openrc default
> +l4:4:wait:/sbin/openrc default
> +l5:5:wait:/sbin/openrc default

 So... This works with both busybox and full sysvinit? I thought busybox didn't
look at runlevel so would execute all of these?

> +l6:6:wait:/sbin/openrc reboot
> +
> +# Stuff to do for the 3-finger salute
> +#ca::ctrlaltdel:/sbin/reboot
> diff --git a/package/openrc/openrc.hash b/package/openrc/openrc.hash
> new file mode 100644
> index 0000000000..d18aa5f680
> --- /dev/null
> +++ b/package/openrc/openrc.hash
> @@ -0,0 +1,2 @@
> +# Calculated manually
> +sha256	ebd0d0462ab8eb375a232c1d9c1ddac11957ac93fae4935441353dd2c1fb01ec	openrc-0.38.3.tar.gz

 Please add a hash for the license file as well.

> diff --git a/package/openrc/openrc.mk b/package/openrc/openrc.mk
> new file mode 100644
> index 0000000000..1ac256764d
> --- /dev/null
> +++ b/package/openrc/openrc.mk
> @@ -0,0 +1,63 @@
> +OPENRC_VERSION = 0.38.3
> +OPENRC_SOURCE = openrc-$(OPENRC_VERSION).tar.gz

 This is the default, no need to add it.

> +OPENRC_SITE = $(call github,OpenRC,openrc,$(OPENRC_VERSION))
> +OPENRC_LICENSE = BSD-2-Clause
> +OPENRC_LICENSE_FILES = LICENSE
> +OPENRC_DEPENDENCIES = ncurses
> +
> +ifeq ($(BR2_OPENRC_PID1_SYSVINIT),y)
> +	OPENRC_DEPENDENCIES += sysvinit

 Don't indent (same for all the following).

 Is it really a build-time dependency?

> +endif
> +
> +ifeq ($(BR2_OPENRC_PID1_BUSYBOX),y)
> +	OPENRC_DEPENDENCIES += busybox
> +endif
> +
> +ifeq ($(BR2_PACKAGE_BASH),y)
> +	OPENRC_MAKE_OPTS = MKBASHCOMP=yes

 We normally put the mandatory ones before the optional ones. Also, use += when
it's inside a condition, even if it is the first/only one.

> +endif
> +
> +ifeq ($(BR2_PACKAGE_ZSH),y)
> +	OPENRC_MAKE_OPTS += MKZSHCOMP=yes
> +endif
> +
> +ifeq ($(BR2_SHARED_LIBS),y)
> +	OPENRC_MAKE_OPTS += MKSTATICLIBS=no
> +else
> +	OPENRC_MAKE_OPTS += MKSTATICLIBS=yes
> +endif
> +
> +OPENRC_MAKE_OPTS += TARGETDIR=$(TARGET_DIR)
> +OPENRC_MAKE_OPTS += MKNET=yes
> +OPENRC_MAKE_OPTS += MKPKGCONFIG=no
> +OPENRC_MAKE_OPTS += MKSELINUX=no
> +OPENRC_MAKE_OPTS += MKSYSVINIT=yes
> +OPENRC_MAKE_OPTS += BRANDING=$(BR2_OPENRC_BRANDING)
 We typically use an explicit qstrip and adding quotes here. That way, it works
correctly even if you override it from the command line or in local.mk.

> +OPENRC_MAKE_OPTS += CC=$(TARGET_CC)

 Please use full TARGET_CONFIGURE_OPTS, preferably passed in the environment
rather than as arguments.

> +
> +define OPENRC_BUILD_CMDS
> +	$(MAKE) $(OPENRC_MAKE_OPTS) -C $(@D)

 Please add TARGET_MAKE_ENV as well.

> +endef
> +
> +define OPENRC_INSTALL_TARGET_CMDS
> +	DESTDIR=$(TARGET_DIR) $(MAKE) $(OPENRC_MAKE_OPTS) -C $(@D) install

 DESTDIR is typically specified as an argument rather than environment.

> +	$(INSTALL) -D -m 0644 package/openrc/inittab $(TARGET_DIR)/etc/inittab
> +endef
> +
> +ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
> +define OPENRC_SET_GETTY
> +	ln -sf agetty $(TARGET_DIR)/etc/init.d/agetty.$(SYSTEM_GETTY_PORT)

 Is it really useful/necessary to have this additional symlink instead of
symlinking directly from /etc/runlevels?

> +	ln -sf /etc/init.d/agetty.$(SYSTEM_GETTY_PORT) $(TARGET_DIR)/etc/runlevels/default
> +	$(SED) '/#term_type=/c\term_type="$(SYSTEM_GETTY_TERM)"' $(TARGET_DIR)/etc/conf.d/agetty
> +	$(SED) '/#agetty_options=/c\agetty_options="$(SYSTEM_GETTY_OPTIONS)"' $(TARGET_DIR)/etc/conf.d/agetty

 We usually do this kind of thing in a single SED invocation, with -e to specify
each command.

> +	if [ $(SYSTEM_GETTY_BAUDRATE) != 0 ]; then \

 We generally prefer 'make' conditions, so something like (with the single SED
command)

		$(if $(BR2_TARGET_GENERIC_GETTY_BAUDRATE_KEEP),,\
			-e '/#baud=/c\baud="$(SYSTEM_GETTY_BAUDRATE)"')

> +		$(SED) '/#baud=/c\baud="$(SYSTEM_GETTY_BAUDRATE)"' $(TARGET_DIR)/etc/conf.d/agetty; \
> +	fi
> +	if [ x$(BR2_PACKAGE_UTIL_LINUX_AGETTY) = x ]; then \
> +		ln -sf getty $(TARGET_DIR)/sbin/agetty; \
> +	fi
> +endef
> +OPENRC_TARGET_FINALIZE_HOOKS += OPENRC_SET_GETTY

 Why is this a finalize hook and not a post-install hook?

> +endif # BR2_TARGET_GENERIC_GETTY
> +
> +$(eval $(generic-package))
> diff --git a/package/sysvinit/Config.in b/package/sysvinit/Config.in
> index 7f27a70fcc..0ea5221ad8 100644
> --- a/package/sysvinit/Config.in
> +++ b/package/sysvinit/Config.in
> @@ -1,7 +1,7 @@
>  config BR2_PACKAGE_SYSVINIT
>  	bool "sysvinit"
>  	depends on BR2_USE_MMU # fork()
> -	depends on BR2_INIT_SYSV
> +	depends on BR2_INIT_SYSV || BR2_INIT_OPENRC
>  	depends on BR2_PACKAGE_BUSYBOX_SHOW_OTHERS
>  	help
>  	  System V style implementation of /sbin/init, parent of all
> diff --git a/system/Config.in b/system/Config.in
> index 498b56e222..12a59ed7d6 100644
> --- a/system/Config.in
> +++ b/system/Config.in
> @@ -10,6 +10,7 @@ choice
>  config BR2_ROOTFS_SKELETON_DEFAULT
>  	bool "default target skeleton"
>  	select BR2_PACKAGE_SKELETON_INIT_SYSV if BR2_INIT_SYSV
> +	select BR2_PACKAGE_SKELETON_INIT_SYSV if BR2_INIT_OPENRC
>  	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_NONE if BR2_INIT_NONE
> @@ -120,6 +121,11 @@ 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_OPENRC
> +	bool "OpenRC"
> +	select BR2_PACKAGE_OPENRC
> +	# select BR2_PACKAGE_INITSCRIPTS # OpenRC has its own scripts

 Don't leave commented-out lines like this, the commit message provides enough
documentation why you don't select initscripts.

 That said, of course, I think you *should* select initscripts...

> +
>  config BR2_INIT_NONE
>  	bool "None"
>  	help
> @@ -129,6 +135,39 @@ config BR2_INIT_NONE
>  
>  endchoice
>  
> +if BR2_INIT_OPENRC
> +
> +config BR2_OPENRC_BRANDING
> +	string "OpenRC branding"
> +	default "Buildroot"
> +	depends on BR2_INIT_OPENRC
> +	help
> +	  Custom string that will show up when OpenRC starts like:
> +
> +	  OpenRC 0.38.3.8ee04a5 is starting up <branding-name> (armv7l)

 I really don't think it's worth to add a config option for this. Would it be
possible to reuse one of the existing options, e.g. BR2_TARGET_GENERIC_HOSTNAME
or BR2_TARGET_GENERIC_ISSUE? Or alternatively, just leave branding empty?

 If it is a config option, I think it is more appropriate as a suboption of the
openrc package. We do that for the systemd suboptions as well.

> +
> +choice
> +	prompt "PID1 to use with OpenRC"
> +	default BR2_OPENRC_PID1_BUSYBOX
> +	help
> +	  Since OpenRC is not replacement for /sbin/init but works with
> +	  it, you can select which PID1 service you want to run
> +
> +config BR2_OPENRC_PID1_BUSYBOX

 Since this is a suboption of BR2_INIT_OPENRC, it should be called
BR2_INIT_OPENRC_PID1_BUSYBOX. But I think the PID1 bit is a bit redundant.

> +	bool "BusyBox"
> +	# select BR2_PACKAGE_INITSCRIPTS # OpenRC has its own scripts

 Here the commented-out line is really redundant, it shouldn't be there to begin
with!

> +
> +config BR2_OPENRC_PID1_SYSVINIT
> +	bool "systemV"
> +	depends on BR2_USE_MMU # sysvinit
> +	select BR2_PACKAGE_BUSYBOX_SHOW_OTHERS # sysvinit
> +	select BR2_PACKAGE_SYSVINIT
> +	# select BR2_PACKAGE_INITSCRIPTS # OpenRC has its own scripts
> +
> +endchoice
> +
> +endif # BR2_INIT_OPENRC
> +
>  choice
>  	prompt "/dev management" if !BR2_INIT_SYSTEMD
>  	default BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_DEVTMPFS
> @@ -337,16 +376,16 @@ config BR2_TARGET_GENERIC_GETTY_BAUDRATE
>  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
> +	# currently observed only by busybox, sysvinit and openrc
> +	depends on BR2_INIT_BUSYBOX || BR2_INIT_SYSV || BR2_INIT_OPENRC

 Perhaps this should be changed into !BR2_INIT_SYSTEMD - that's actually what it
is at the moment.

 Come to think of it, BR2_TARGET_GENERIC_GETTY should depend on !BR2_INIT_NONE
as well.

 The above two changes should be separate patches, obviously. And only if you
feel up to it.

 Regards,
 Arnout

>  	help
>  	  Specify a TERM type.
>  
>  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
> +	# currently observed only by busybox, sysvinit and openrc
> +	depends on BR2_INIT_BUSYBOX || BR2_INIT_SYSV || BR2_INIT_OPENRC
>  	help
>  	  Any other flags you want to pass to getty,
>  	  Refer to getty --help for details.
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH] package/openrc: new init package OpenRC (0.38.3)
  2019-03-04 18:59 ` Arnout Vandecappelle
@ 2019-03-05 21:38   ` Yann E. MORIN
  2019-03-05 23:20     ` michal.lyszczek at bofc.pl
  0 siblings, 1 reply; 12+ messages in thread
From: Yann E. MORIN @ 2019-03-05 21:38 UTC (permalink / raw
  To: buildroot

Micha?, Arnout,

On 2019-03-04 19:59 +0100, Arnout Vandecappelle spake thusly:
>  I can tell you, we don't add something like a new init system easily, so this
> may take some time to get merged...
> 
>  First of all, the patch that adds the openrc package as an ordinary package
> (not an init system) should be separate. That one has to be standalone, obviously.

Agreed.

> On 28/02/2019 20:44, Micha? ?yszczek wrote:
> > OpenRC is a dependency based init system that works with the system
> > provided init program, normally located at /sbin/init. It is not a
> > replacement for /sbin/init.
> > 
> > Signed-off-by: Micha? ?yszczek <michal.lyszczek@bofc.pl>
> > ---
> > OpenRC uses own set of scripts for sysinit thus
> > BR2_PACKAGE_INITSCRIPTS is disabled. I decided to add new init
> > choice and choice which program to use as PID1 since OpenRC uses
> > its own scripts for services so S** scripts cannot be used. It is
> > possible to add simple script to local.d that would start all S**
> > scripts, but that would diminish OpenRC's features and usefullness,
> > so additional startup scripts should be written and added to
> > buildroot.
> 
>  While this is true, I think you should still add that script so that packages
> which don't have an OpenRC service description will still work. (We should also
> have done that for systemd IMO.)

Not needed, as it is my understanding that systemd autoimatically
creates units for scripts in /etc/init.d/ when there is no corresponding
native unit. From http://man7.org/linux/man-pages/man1/systemd.1.html :

    systemd is compatible with the SysV init system to a large degree:
    SysV init scripts are supported and simply read as an alternative
    (though limited) configuration file format. The SysV /dev/initctl
    interface is provided, and compatibility implementations of the
    various SysV client tools are available. In addition to that, various
    established Unix functionality such as /etc/fstab or the utmp
    database are supported.

> Then you can have something like thisin
> pkg-generic.mk:
> 
> $(2)_INSTALL_INIT_OPENRC ?= $$($(2)_INSTALL_INIT_SYSV)
> 
> so if a package defines its OpenRC file, that one will be used instead of the
> sysvinit script.

Is that works so easily, then this is indeed a good idea.

>  With that in mind, I think an OpenRC skeelton would make sense at well. It
> would be pretty empty :-).

Except maybe for that one script that does the sysv-init fallback.

> > If OpenRC init is accepted into Buildroot, I will commit
> > some time to port existing sysvinit startup scripts to OpenRC.
> > 
> > OpenRC works with default config - a single change is needed (which
> > init system to use, with optional PID1 change - default BusyBox). I
> > tested it with both BusyBox and sysvinit as PID1. Also tested with
> > BusyBox programs enabled and disabled (standalone apps used, none
> > of which was from BusyBox package). I tested it using
> > qemu-x86_64_defconfig and beaglebone_defconfig.
> 
>  If this gets added as a full BR2_INIT_ option, you should also add the runtime
> tests for it (in separate patches, obviously). Look at the system tests for
> inspiration.

... located in support/testing/tests/init/.

[--SNIP--]
> > +++ b/package/openrc/Config.in
> > @@ -0,0 +1,19 @@
> > +config BR2_PACKAGE_OPENRC
> > +	bool "OpenRC"
> > +	select BR2_PACKAGE_UTIL_LINUX
> > +	select BR2_PACKAGE_UTIL_LINUX_AGETTY
> 
>  Does it really depend on agetty? Seems strange...
> 
>  That said, if additional tweaks are needed to support busybox getty, that can
> be done in a follow-up patch.

We have a similar workaround for systemd, but I just sent a patch to
actually remove it:
    https://patchwork.ozlabs.org/patch/1051821/

If OpenRC does not require util-linux, it would be acceptable to have a
workaround to be able to use busybox' getty instead. But if OpenRC
otherwise requires util-linux (e.g. for lib"stuff"), then there is no
point for such a workaround.

> > +	OPENRC_MAKE_OPTS += MKZSHCOMP=yes
> > +endif
> > +
> > +ifeq ($(BR2_SHARED_LIBS),y)
> > +	OPENRC_MAKE_OPTS += MKSTATICLIBS=no
> > +else
> > +	OPENRC_MAKE_OPTS += MKSTATICLIBS=yes
> > +endif
> > +
> > +OPENRC_MAKE_OPTS += TARGETDIR=$(TARGET_DIR)

Arnout, TARGETDIR comes from here. Although I admit it is confusing...

> > +OPENRC_MAKE_OPTS += MKNET=yes
> > +OPENRC_MAKE_OPTS += MKPKGCONFIG=no
> > +OPENRC_MAKE_OPTS += MKSELINUX=no
> > +OPENRC_MAKE_OPTS += MKSYSVINIT=yes

When doing a sequence, I's nicer to write:

    OPENRC_MAKE_OPTS += \
        TARGETDIR=$(TARGET_DIR) \
        MKNET=yes \
        MKPKGCONFIG=no \
        [...]

> > +OPENRC_MAKE_OPTS += BRANDING=$(BR2_OPENRC_BRANDING)
>  We typically use an explicit qstrip and adding quotes here. That way, it works
> correctly even if you override it from the command line or in local.mk.

To be homest, I don;t thionk overriding frm the command-line should be
suported that nicely, because it is not reproducible. It may work for a
quicky, but how much more complex is it to go to menuconfig, or edit
.config ?

> > +	ln -sf /etc/init.d/agetty.$(SYSTEM_GETTY_PORT) $(TARGET_DIR)/etc/runlevels/default
> > +	$(SED) '/#term_type=/c\term_type="$(SYSTEM_GETTY_TERM)"' $(TARGET_DIR)/etc/conf.d/agetty
> > +	$(SED) '/#agetty_options=/c\agetty_options="$(SYSTEM_GETTY_OPTIONS)"' $(TARGET_DIR)/etc/conf.d/agetty
> 
>  We usually do this kind of thing in a single SED invocation, with -e to specify
> each command.

... except for the first, as $(SED) already ends with -e.

(and I find it ugly indeed, because it makes the first pattern different
from the others... Bleark...)

> > +config BR2_OPENRC_BRANDING
> > +	string "OpenRC branding"
> > +	default "Buildroot"
> > +	depends on BR2_INIT_OPENRC
> > +	help
> > +	  Custom string that will show up when OpenRC starts like:
> > +
> > +	  OpenRC 0.38.3.8ee04a5 is starting up <branding-name> (armv7l)
> 
>  I really don't think it's worth to add a config option for this. Would it be
> possible to reuse one of the existing options, e.g. BR2_TARGET_GENERIC_HOSTNAME
> or BR2_TARGET_GENERIC_ISSUE? Or alternatively, just leave branding empty?

Agreed.

>  If it is a config option, I think it is more appropriate as a suboption of the
> openrc package. We do that for the systemd suboptions as well.

I don't think it has to be a new option, BR2_TARGET_GENERIC_ISSUE is
exactly there for this kind of usage.

> > +choice
> > +	prompt "PID1 to use with OpenRC"
> > +	default BR2_OPENRC_PID1_BUSYBOX
> > +	help
> > +	  Since OpenRC is not replacement for /sbin/init but works with
> > +	  it, you can select which PID1 service you want to run
> > +
> > +config BR2_OPENRC_PID1_BUSYBOX
> 
>  Since this is a suboption of BR2_INIT_OPENRC, it should be called
> BR2_INIT_OPENRC_PID1_BUSYBOX. But I think the PID1 bit is a bit redundant.

I think this should be a sub-option of the openrc package, not the 'init'
part.

> > +endif # BR2_INIT_OPENRC
> > +
> >  choice
> >  	prompt "/dev management" if !BR2_INIT_SYSTEMD
> >  	default BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_DEVTMPFS
> > @@ -337,16 +376,16 @@ config BR2_TARGET_GENERIC_GETTY_BAUDRATE
> >  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
> > +	# currently observed only by busybox, sysvinit and openrc
> > +	depends on BR2_INIT_BUSYBOX || BR2_INIT_SYSV || BR2_INIT_OPENRC
> 
>  Perhaps this should be changed into !BR2_INIT_SYSTEMD - that's actually what it
> is at the moment.
> 
>  Come to think of it, BR2_TARGET_GENERIC_GETTY should depend on !BR2_INIT_NONE
> as well.

Probably, indeed... I missed that one when doing my big overhaul of that
section.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH] package/openrc: new init package OpenRC (0.38.3)
  2019-03-05 21:38   ` Yann E. MORIN
@ 2019-03-05 23:20     ` michal.lyszczek at bofc.pl
  2019-03-06  9:58       ` Arnout Vandecappelle
  0 siblings, 1 reply; 12+ messages in thread
From: michal.lyszczek at bofc.pl @ 2019-03-05 23:20 UTC (permalink / raw
  To: buildroot

Yann, Arnout

First, thank you for your extensive review. Now to the point.

On 2019-03-05 22:38:58, Yann E. MORIN wrote:
> Micha?, Arnout,
>
> On 2019-03-04 19:59 +0100, Arnout Vandecappelle spake thusly:
> >  I can tell you, we don't add something like a new init system easily, so this
> > may take some time to get merged...

Totally understandable. I myself don't want to commit crap under my name ;-)

> >  First of all, the patch that adds the openrc package as an ordinary package
> > (not an init system) should be separate. That one has to be standalone, obviously.
>
> Agreed.

Will do

> > On 28/02/2019 20:44, Micha? ?yszczek wrote:
> > > OpenRC is a dependency based init system that works with the system
> > > provided init program, normally located at /sbin/init. It is not a
> > > replacement for /sbin/init.
> > >
> > > Signed-off-by: Micha? ?yszczek <michal.lyszczek@bofc.pl>
> > > ---
> > > OpenRC uses own set of scripts for sysinit thus
> > > BR2_PACKAGE_INITSCRIPTS is disabled. I decided to add new init
> > > choice and choice which program to use as PID1 since OpenRC uses
> > > its own scripts for services so S** scripts cannot be used. It is
> > > possible to add simple script to local.d that would start all S**
> > > scripts, but that would diminish OpenRC's features and usefullness,
> > > so additional startup scripts should be written and added to
> > > buildroot.
> >
> >  While this is true, I think you should still add that script so that packages
> > which don't have an OpenRC service description will still work. (We should also
> > have done that for systemd IMO.)
>
> Not needed, as it is my understanding that systemd autoimatically
> creates units for scripts in /etc/init.d/ when there is no corresponding
> native unit. From http://man7.org/linux/man-pages/man1/systemd.1.html :
>
>     systemd is compatible with the SysV init system to a large degree:
>     SysV init scripts are supported and simply read as an alternative
>     (though limited) configuration file format. The SysV /dev/initctl
>     interface is provided, and compatibility implementations of the
>     various SysV client tools are available. In addition to that, various
>     established Unix functionality such as /etc/fstab or the utmp
>     database are supported.
>
> > Then you can have something like thisin
> > pkg-generic.mk:
> >
> > $(2)_INSTALL_INIT_OPENRC ?= $$($(2)_INSTALL_INIT_SYSV)
> >
> > so if a package defines its OpenRC file, that one will be used instead of the
> > sysvinit script.
>
> Is that works so easily, then this is indeed a good idea.
>
> >  With that in mind, I think an OpenRC skeelton would make sense at well. It
> > would be pretty empty :-).
>
> Except maybe for that one script that does the sysv-init fallback.

Ok, how about this: by default OpenRC installs and enables many services that
are not necessarily needed, like setting hwclock, configuring network
interfaces, activating swap, setting keymaps. Some of these services fails or
may fail due to missing network interfaces, or due to missing additional
programs like loadkeys for setting keymaps. Also I notices that sysctl fails
with BusyBox version since it is missing "--system" option. These are not
critical and OpenRC still boots up and spawns tty with login - but it prints
error and thus is ugly and may confuse users.

So my proposition would be to not perform default 'make install' but install
only bare minimum set of init scrips, that are sure not to fail. And scripts
that are not compatible with default buildroot installation could land in
skeleton.

One notable example is "agetty". It's not like it's some hardcore dependency for
OpenRC, but it uses it by default to spawn shell, and agetty is not compatible
with busybox due to one change in order:

agetty [options] port [baud_rate...] [term]
getty [OPTIONS] BAUD_RATE TTY [TERMTYPE]

See, baud rate is swaped with port. So I could write custom getty service for
OpenRC and put it into skeleton. That would lift agetty dependency.


What might be best way to install only chosed files?

- Do 'make install' to temp directory, remove what is not needed than copy to
target?
- Invoke "install" for each file? There are quite of them.
- Do normal 'make install' then call 'rm' on each uneeded files in target_dir?
- or maybe there is some easy way of doing it I didn't think about?

> > > If OpenRC init is accepted into Buildroot, I will commit
> > > some time to port existing sysvinit startup scripts to OpenRC.
> > >
> > > OpenRC works with default config - a single change is needed (which
> > > init system to use, with optional PID1 change - default BusyBox). I
> > > tested it with both BusyBox and sysvinit as PID1. Also tested with
> > > BusyBox programs enabled and disabled (standalone apps used, none
> > > of which was from BusyBox package). I tested it using
> > > qemu-x86_64_defconfig and beaglebone_defconfig.
> >
> >  If this gets added as a full BR2_INIT_ option, you should also add the runtime
> > tests for it (in separate patches, obviously). Look at the system tests for
> > inspiration.
>
> ... located in support/testing/tests/init/.
>
> [--SNIP--]
> > > +++ b/package/openrc/Config.in
> > > @@ -0,0 +1,19 @@
> > > +config BR2_PACKAGE_OPENRC
> > > +	bool "OpenRC"
> > > +	select BR2_PACKAGE_UTIL_LINUX
> > > +	select BR2_PACKAGE_UTIL_LINUX_AGETTY
> >
> >  Does it really depend on agetty? Seems strange...

It does not, answer is above. I wanted to preserve as much upstream stuff as
possible - but maybe that is not the way?

> >  That said, if additional tweaks are needed to support busybox getty, that can
> > be done in a follow-up patch.
>
> We have a similar workaround for systemd, but I just sent a patch to
> actually remove it:
>     https://patchwork.ozlabs.org/patch/1051821/

As stated above, I could create simple service that would use getty instead of
agetty and put it in skeleon.

> If OpenRC does not require util-linux, it would be acceptable to have a
> workaround to be able to use busybox' getty instead. But if OpenRC
> otherwise requires util-linux (e.g. for lib"stuff"), then there is no
> point for such a workaround.

OpenRC does not need util-linux.

> > > +	OPENRC_MAKE_OPTS += MKZSHCOMP=yes
> > > +endif
> > > +
> > > +ifeq ($(BR2_SHARED_LIBS),y)
> > > +	OPENRC_MAKE_OPTS += MKSTATICLIBS=no
> > > +else
> > > +	OPENRC_MAKE_OPTS += MKSTATICLIBS=yes
> > > +endif
> > > +
> > > +OPENRC_MAKE_OPTS += TARGETDIR=$(TARGET_DIR)
>
> Arnout, TARGETDIR comes from here. Although I admit it is confusing...

This patch was introduced because OpenRC determines location for its libs,
wheter to put them in /lib or /lib64 directory, by reading link of /lib.
Unfortunately it has "/lib" path hardcoded and buildroot is reading that
link from host os, so libs generated for cross x86 could land in lib64 on amd64
hosts. I will rise this issue to OpenRC's author - maybe there is already
solution for this and I've just invented wheel again.

> > > +OPENRC_MAKE_OPTS += MKNET=yes
> > > +OPENRC_MAKE_OPTS += MKPKGCONFIG=no
> > > +OPENRC_MAKE_OPTS += MKSELINUX=no
> > > +OPENRC_MAKE_OPTS += MKSYSVINIT=yes
>
> When doing a sequence, I's nicer to write:
>
>     OPENRC_MAKE_OPTS += \
>         TARGETDIR=$(TARGET_DIR) \
>         MKNET=yes \
>         MKPKGCONFIG=no \
>         [...]
>
> > > +OPENRC_MAKE_OPTS += BRANDING=$(BR2_OPENRC_BRANDING)
> >  We typically use an explicit qstrip and adding quotes here. That way, it works
> > correctly even if you override it from the command line or in local.mk.
>
> To be homest, I don;t thionk overriding frm the command-line should be
> suported that nicely, because it is not reproducible. It may work for a
> quicky, but how much more complex is it to go to menuconfig, or edit
> .config ?
> > > +	ln -sf /etc/init.d/agetty.$(SYSTEM_GETTY_PORT) $(TARGET_DIR)/etc/runlevels/default
> > > +	$(SED) '/#term_type=/c\term_type="$(SYSTEM_GETTY_TERM)"' $(TARGET_DIR)/etc/conf.d/agetty
> > > +	$(SED) '/#agetty_options=/c\agetty_options="$(SYSTEM_GETTY_OPTIONS)"' $(TARGET_DIR)/etc/conf.d/agetty
> >
> >  We usually do this kind of thing in a single SED invocation, with -e to specify
> > each command.
>
> ... except for the first, as $(SED) already ends with -e.
>
> (and I find it ugly indeed, because it makes the first pattern different
> from the others... Bleark...)
>
> > > +config BR2_OPENRC_BRANDING
> > > +	string "OpenRC branding"
> > > +	default "Buildroot"
> > > +	depends on BR2_INIT_OPENRC
> > > +	help
> > > +	  Custom string that will show up when OpenRC starts like:
> > > +
> > > +	  OpenRC 0.38.3.8ee04a5 is starting up <branding-name> (armv7l)
> >
> >  I really don't think it's worth to add a config option for this. Would it be
> > possible to reuse one of the existing options, e.g. BR2_TARGET_GENERIC_HOSTNAME
> > or BR2_TARGET_GENERIC_ISSUE? Or alternatively, just leave branding empty?
>
> Agreed.
>

That field was not designed to place hostname there but os branding. For
example, "GNU/Linux Debian" would be good example of branding, since multiple
hardware with different hostnames can use single OS. Of course it's just for
eye-candy but I would prefer to hardcode it to "Buildroot" (or empty) than put
hostname or issue here.

> >  If it is a config option, I think it is more appropriate as a suboption of the
> > openrc package. We do that for the systemd suboptions as well.
>
> I don't think it has to be a new option, BR2_TARGET_GENERIC_ISSUE is
> exactly there for this kind of usage.
>
> > > +choice
> > > +	prompt "PID1 to use with OpenRC"
> > > +	default BR2_OPENRC_PID1_BUSYBOX
> > > +	help
> > > +	  Since OpenRC is not replacement for /sbin/init but works with
> > > +	  it, you can select which PID1 service you want to run
> > > +
> > > +config BR2_OPENRC_PID1_BUSYBOX
> >
> >  Since this is a suboption of BR2_INIT_OPENRC, it should be called
> > BR2_INIT_OPENRC_PID1_BUSYBOX. But I think the PID1 bit is a bit redundant.
>
> I think this should be a sub-option of the openrc package, not the 'init'
> part.
>
> > > +endif # BR2_INIT_OPENRC
> > > +
> > >  choice
> > >  	prompt "/dev management" if !BR2_INIT_SYSTEMD
> > >  	default BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_DEVTMPFS
> > > @@ -337,16 +376,16 @@ config BR2_TARGET_GENERIC_GETTY_BAUDRATE
> > >  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
> > > +	# currently observed only by busybox, sysvinit and openrc
> > > +	depends on BR2_INIT_BUSYBOX || BR2_INIT_SYSV || BR2_INIT_OPENRC
> >
> >  Perhaps this should be changed into !BR2_INIT_SYSTEMD - that's actually what it
> > is at the moment.
> >
> >  Come to think of it, BR2_TARGET_GENERIC_GETTY should depend on !BR2_INIT_NONE
> > as well.
>
> Probably, indeed... I missed that one when doing my big overhaul of that
> section.

I agree with statements where I didn't leave comment and will apply proposed
fixes.

Thanks again for review.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20190306/1563c814/attachment.asc>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH] package/openrc: new init package OpenRC (0.38.3)
  2019-03-05 23:20     ` michal.lyszczek at bofc.pl
@ 2019-03-06  9:58       ` Arnout Vandecappelle
  2019-03-06 17:36         ` Yann E. MORIN
  2019-03-07  8:36         ` michal.lyszczek at bofc.pl
  0 siblings, 2 replies; 12+ messages in thread
From: Arnout Vandecappelle @ 2019-03-06  9:58 UTC (permalink / raw
  To: buildroot



On 06/03/2019 00:20, michal.lyszczek at bofc.pl wrote:
> Yann, Arnout
> 
> First, thank you for your extensive review. Now to the point.
> 
> On 2019-03-05 22:38:58, Yann E. MORIN wrote:
>> Micha?, Arnout,
>>
>> On 2019-03-04 19:59 +0100, Arnout Vandecappelle spake thusly:
[snip]
>>> On 28/02/2019 20:44, Micha? ?yszczek wrote:
>>>> OpenRC is a dependency based init system that works with the system
>>>> provided init program, normally located at /sbin/init. It is not a
>>>> replacement for /sbin/init.
>>>>
>>>> Signed-off-by: Micha? ?yszczek <michal.lyszczek@bofc.pl>
>>>> ---
>>>> OpenRC uses own set of scripts for sysinit thus
>>>> BR2_PACKAGE_INITSCRIPTS is disabled. I decided to add new init
>>>> choice and choice which program to use as PID1 since OpenRC uses
>>>> its own scripts for services so S** scripts cannot be used. It is
>>>> possible to add simple script to local.d that would start all S**
>>>> scripts, but that would diminish OpenRC's features and usefullness,
>>>> so additional startup scripts should be written and added to
>>>> buildroot.
>>>
>>>  While this is true, I think you should still add that script so that packages
>>> which don't have an OpenRC service description will still work. (We should also
>>> have done that for systemd IMO.)
>>
>> Not needed, as it is my understanding that systemd autoimatically
>> creates units for scripts in /etc/init.d/ when there is no corresponding
>> native unit. From http://man7.org/linux/man-pages/man1/systemd.1.html :
>>
>>     systemd is compatible with the SysV init system to a large degree:
>>     SysV init scripts are supported and simply read as an alternative
>>     (though limited) configuration file format. The SysV /dev/initctl
>>     interface is provided, and compatibility implementations of the
>>     various SysV client tools are available. In addition to that, various
>>     established Unix functionality such as /etc/fstab or the utmp
>>     database are supported.

 Yes, but at the moment we don't install the sysvinit scripts so systemd is not
going to execute them.

 Ideally we'd also have a runtime test that checks if the sysvinit script indeed
gets called.

>>
>>> Then you can have something like thisin
>>> pkg-generic.mk:
>>>
>>> $(2)_INSTALL_INIT_OPENRC ?= $$($(2)_INSTALL_INIT_SYSV)
>>>
>>> so if a package defines its OpenRC file, that one will be used instead of the
>>> sysvinit script.
>>
>> Is that works so easily, then this is indeed a good idea.
>>
>>>  With that in mind, I think an OpenRC skeelton would make sense at well. It
>>> would be pretty empty :-).
>>
>> Except maybe for that one script that does the sysv-init fallback.
> 
> Ok, how about this:

 Er, the following paragraph is completely unrelated to what we were discussing
above... Did you understand what we were talking about above?


> by default OpenRC installs and enables many services that
> are not necessarily needed, like setting hwclock, configuring network
> interfaces, activating swap, setting keymaps. Some of these services fails or
> may fail due to missing network interfaces, or due to missing additional
> programs like loadkeys for setting keymaps. Also I notices that sysctl fails
> with BusyBox version since it is missing "--system" option. These are not
> critical and OpenRC still boots up and spawns tty with login - but it prints
> error and thus is ugly and may confuse users.

 For services that are provided by busybox, it will be difficult to know if a
custom busybox config enables them or not... Anyway, I think this is something
that is better solved in a follow-up patch (which you can propose in the same
series, of course).

> So my proposition would be to not perform default 'make install' but install
> only bare minimum set of init scrips, that are sure not to fail. 

 Instead of doing a custom install, I prefer to just do the default install and
remove things are not needed / wanted.

> And scripts
> that are not compatible with default buildroot installation could land in
> skeleton.

 Instead you can make the removal conditional.


> One notable example is "agetty". It's not like it's some hardcore dependency for
> OpenRC, but it uses it by default to spawn shell, and agetty is not compatible
> with busybox due to one change in order:
> 
> agetty [options] port [baud_rate...] [term]
> getty [OPTIONS] BAUD_RATE TTY [TERMTYPE]
> 
> See, baud rate is swaped with port. So I could write custom getty service for
> OpenRC and put it into skeleton. That would lift agetty dependency.

 Yeah, again, you can do this conditionally on whether agetty is enabled or not.
But indeed in that case it's better to put it in a skeleton package and not in
the openrc package itself, so you can get rid of it by using a custom skeleton.


> What might be best way to install only chosed files?

 In the .mk file (of the skeleton-init-openrc package), don't use rsync but
instead do conditional $(INSTALL) calls. Something like:

ifeq ($(BR2_PACKAGE_UTIL_LINUX_AGETTY),)
define SKELETON_INIT_OPENRC_INSTALL_GETTY
	$(INSTALL) -D -m 0644 $(@D)/getty $(TARGET_DIR)/etc/conf.d/getty
	$(RM) $(TARGET_DIR)/etc/conf.d/agetty
endef
endif

define SKELETON_INIT_OPENRC_INSTALL_TARGET_CMDS
	...
	$(SKELETON_INIT_OPENRC_INSTALL_GETTY)
endef

 You can also do it with a POST_INSTALL_HOOK if you prefer that.

> 
> - Do 'make install' to temp directory, remove what is not needed than copy to
> target?
> - Invoke "install" for each file? There are quite of them.
> - Do normal 'make install' then call 'rm' on each uneeded files in target_dir?

 Yep, that's the one.

> - or maybe there is some easy way of doing it I didn't think about?
> 
>>>> If OpenRC init is accepted into Buildroot, I will commit
>>>> some time to port existing sysvinit startup scripts to OpenRC.
>>>>
>>>> OpenRC works with default config - a single change is needed (which
>>>> init system to use, with optional PID1 change - default BusyBox). I
>>>> tested it with both BusyBox and sysvinit as PID1. Also tested with
>>>> BusyBox programs enabled and disabled (standalone apps used, none
>>>> of which was from BusyBox package). I tested it using
>>>> qemu-x86_64_defconfig and beaglebone_defconfig.
>>>
>>>  If this gets added as a full BR2_INIT_ option, you should also add the runtime
>>> tests for it (in separate patches, obviously). Look at the system tests for
                                                               ^^^^^^
                                                I meant systemd here.

>>> inspiration.
>>
>> ... located in support/testing/tests/init/.
>>
>> [--SNIP--]
>>>> +++ b/package/openrc/Config.in
>>>> @@ -0,0 +1,19 @@
>>>> +config BR2_PACKAGE_OPENRC
>>>> +	bool "OpenRC"
>>>> +	select BR2_PACKAGE_UTIL_LINUX
>>>> +	select BR2_PACKAGE_UTIL_LINUX_AGETTY
>>>
>>>  Does it really depend on agetty? Seems strange...
> 
> It does not, answer is above. I wanted to preserve as much upstream stuff as
> possible - but maybe that is not the way?

 So we found a way to get rid of the agetty dependency. Still, better do that in
a follow-up patch and keep the initial patch as simple as possible.

 For sure, it should not be the openrc package itself that selects it, because
it is only needed if you actually want to use openrc to start a getty. So it
belongs to a skeleton package instead.

> 
>>>  That said, if additional tweaks are needed to support busybox getty, that can
>>> be done in a follow-up patch.
>>
>> We have a similar workaround for systemd, but I just sent a patch to
>> actually remove it:
>>     https://patchwork.ozlabs.org/patch/1051821/
> 
> As stated above, I could create simple service that would use getty instead of
> agetty and put it in skeleon.
> 
>> If OpenRC does not require util-linux, it would be acceptable to have a
>> workaround to be able to use busybox' getty instead. But if OpenRC
>> otherwise requires util-linux (e.g. for lib"stuff"), then there is no
>> point for such a workaround.
> 
> OpenRC does not need util-linux.
> 
>>>> +	OPENRC_MAKE_OPTS += MKZSHCOMP=yes
>>>> +endif
>>>> +
>>>> +ifeq ($(BR2_SHARED_LIBS),y)
>>>> +	OPENRC_MAKE_OPTS += MKSTATICLIBS=no
>>>> +else
>>>> +	OPENRC_MAKE_OPTS += MKSTATICLIBS=yes
>>>> +endif
>>>> +
>>>> +OPENRC_MAKE_OPTS += TARGETDIR=$(TARGET_DIR)
>>
>> Arnout, TARGETDIR comes from here. Although I admit it is confusing...
> 
> This patch was introduced because OpenRC determines location for its libs,
> wheter to put them in /lib or /lib64 directory, by reading link of /lib.
> Unfortunately it has "/lib" path hardcoded and buildroot is reading that
> link from host os, so libs generated for cross x86 could land in lib64 on amd64
> hosts. I will rise this issue to OpenRC's author - maybe there is already
> solution for this and I've just invented wheel again.

 Yes but as I mentioned somewhere else in my review: if this is only used at
install time and not at build time then you should use $(DESTDIR)/$(PREFIX) and
not some new TARGETDIR variable.

 Oh and don't forget to send that patch upstream for review.

> 
>>>> +OPENRC_MAKE_OPTS += MKNET=yes
>>>> +OPENRC_MAKE_OPTS += MKPKGCONFIG=no
>>>> +OPENRC_MAKE_OPTS += MKSELINUX=no
>>>> +OPENRC_MAKE_OPTS += MKSYSVINIT=yes
>>
>> When doing a sequence, I's nicer to write:
>>
>>     OPENRC_MAKE_OPTS += \
>>         TARGETDIR=$(TARGET_DIR) \
>>         MKNET=yes \
>>         MKPKGCONFIG=no \
>>         [...]

 Actually, I disagree that it is nicer, and we already have both patterns in
Buildroot.

>>
>>>> +OPENRC_MAKE_OPTS += BRANDING=$(BR2_OPENRC_BRANDING)
>>>  We typically use an explicit qstrip and adding quotes here. That way, it works
>>> correctly even if you override it from the command line or in local.mk.
>>
>> To be homest, I don;t thionk overriding frm the command-line should be
>> suported that nicely, because it is not reproducible. It may work for a
>> quicky, but how much more complex is it to go to menuconfig, or edit
>> .config ?
>>>> +	ln -sf /etc/init.d/agetty.$(SYSTEM_GETTY_PORT) $(TARGET_DIR)/etc/runlevels/default
>>>> +	$(SED) '/#term_type=/c\term_type="$(SYSTEM_GETTY_TERM)"' $(TARGET_DIR)/etc/conf.d/agetty
>>>> +	$(SED) '/#agetty_options=/c\agetty_options="$(SYSTEM_GETTY_OPTIONS)"' $(TARGET_DIR)/etc/conf.d/agetty
>>>
>>>  We usually do this kind of thing in a single SED invocation, with -e to specify
>>> each command.
>>
>> ... except for the first, as $(SED) already ends with -e.
>>
>> (and I find it ugly indeed, because it makes the first pattern different
>> from the others... Bleark...)

 Eek, bleark indeed... Why do we have that -e in $(SED)?

>>
>>>> +config BR2_OPENRC_BRANDING
>>>> +	string "OpenRC branding"
>>>> +	default "Buildroot"
>>>> +	depends on BR2_INIT_OPENRC
>>>> +	help
>>>> +	  Custom string that will show up when OpenRC starts like:
>>>> +
>>>> +	  OpenRC 0.38.3.8ee04a5 is starting up <branding-name> (armv7l)
>>>
>>>  I really don't think it's worth to add a config option for this. Would it be
>>> possible to reuse one of the existing options, e.g. BR2_TARGET_GENERIC_HOSTNAME
>>> or BR2_TARGET_GENERIC_ISSUE? Or alternatively, just leave branding empty?
>>
>> Agreed.
>>
> 
> That field was not designed to place hostname there but os branding. For
> example, "GNU/Linux Debian" would be good example of branding, since multiple
> hardware with different hostnames can use single OS. Of course it's just for
> eye-candy but I would prefer to hardcode it to "Buildroot" (or empty) than put
> hostname or issue here.

 For gcc we have:

        --with-pkgversion="Buildroot $(BR2_VERSION_FULL)" \
        --with-bugurl="http://bugs.buildroot.net/"

so using the same for openrc seems appropriate.

 Maybe we should introduce a global config + variable to control this, i.e. the
NAME and PRETTY_NAME in /etc/os-release.


>>>  If it is a config option, I think it is more appropriate as a suboption of the
>>> openrc package. We do that for the systemd suboptions as well.
>>
>> I don't think it has to be a new option, BR2_TARGET_GENERIC_ISSUE is
>> exactly there for this kind of usage.
>>
>>>> +choice
>>>> +	prompt "PID1 to use with OpenRC"
>>>> +	default BR2_OPENRC_PID1_BUSYBOX
>>>> +	help
>>>> +	  Since OpenRC is not replacement for /sbin/init but works with
>>>> +	  it, you can select which PID1 service you want to run
>>>> +
>>>> +config BR2_OPENRC_PID1_BUSYBOX
>>>
>>>  Since this is a suboption of BR2_INIT_OPENRC, it should be called
>>> BR2_INIT_OPENRC_PID1_BUSYBOX. But I think the PID1 bit is a bit redundant.
>>
>> I think this should be a sub-option of the openrc package, not the 'init'
>> part.

 I disagree.

 I actually disagree that the various init packages depend on PID1 coming from
that particular package. There could be use cases to have a different init
system (usually _NONE) and still have the init executable there.

 For openrc, this is doubly so, because openrc is not a standalone init system.
So it can very obviously be used with a different PID1.

 So IMO this choice really belongs to the system config.


 Regards,
 Arnout


>>>> +endif # BR2_INIT_OPENRC
>>>> +
>>>>  choice
>>>>  	prompt "/dev management" if !BR2_INIT_SYSTEMD
>>>>  	default BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_DEVTMPFS
>>>> @@ -337,16 +376,16 @@ config BR2_TARGET_GENERIC_GETTY_BAUDRATE
>>>>  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
>>>> +	# currently observed only by busybox, sysvinit and openrc
>>>> +	depends on BR2_INIT_BUSYBOX || BR2_INIT_SYSV || BR2_INIT_OPENRC
>>>
>>>  Perhaps this should be changed into !BR2_INIT_SYSTEMD - that's actually what it
>>> is at the moment.
>>>
>>>  Come to think of it, BR2_TARGET_GENERIC_GETTY should depend on !BR2_INIT_NONE
>>> as well.
>>
>> Probably, indeed... I missed that one when doing my big overhaul of that
>> section.
> 
> I agree with statements where I didn't leave comment and will apply proposed
> fixes.
> 
> Thanks again for review.
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH] package/openrc: new init package OpenRC (0.38.3)
  2019-03-06  9:58       ` Arnout Vandecappelle
@ 2019-03-06 17:36         ` Yann E. MORIN
  2019-03-06 18:12           ` Yann E. MORIN
  2019-03-07  9:58           ` Arnout Vandecappelle
  2019-03-07  8:36         ` michal.lyszczek at bofc.pl
  1 sibling, 2 replies; 12+ messages in thread
From: Yann E. MORIN @ 2019-03-06 17:36 UTC (permalink / raw
  To: buildroot

Arnout, Micha?, All,

On 2019-03-06 10:58 +0100, Arnout Vandecappelle spake thusly:
> On 06/03/2019 00:20, michal.lyszczek at bofc.pl wrote:
> > On 2019-03-05 22:38:58, Yann E. MORIN wrote:
> >>     systemd is compatible with the SysV init system to a large degree:
> >>     SysV init scripts are supported and simply read as an alternative
> >>     (though limited) configuration file format. The SysV /dev/initctl
> >>     interface is provided, and compatibility implementations of the
> >>     various SysV client tools are available. In addition to that, various
> >>     established Unix functionality such as /etc/fstab or the utmp
> >>     database are supported.
> 
>  Yes, but at the moment we don't install the sysvinit scripts so systemd is not
> going to execute them.

Oh, I thought you were speaking about the init script that packages
installs themselves, not ours...

Of course, I was a bit off my shoes on that one.

OTOH, people usually contribute whjat they need. systemd service got
added as time passed, when people realised their pet package did not
install one.

This is usualy our mantra: things get done when people are interested in
it.

>  Ideally we'd also have a runtime test that checks if the sysvinit script indeed
> gets called.

If we do have the fallback, then yes, of course, but in the end does it
make sense to have that fallback? As I said, people interested in OpenRC
and seeing their package of interest lack a prroper startup script, can
contribute one.

IMHO, the fallback allows for lazyness and complacency.

> > So my proposition would be to not perform default 'make install' but install
> > only bare minimum set of init scrips, that are sure not to fail. 
>  Instead of doing a custom install, I prefer to just do the default install and
> remove things are not needed / wanted.

I usually prefer to whitelist rather than blacklist. I.e .I prefer a
selective install, rather than a selective removal.

Otherwise, when we bump the version, and a new tool is installed, we can
very easily miss it and leave it installed. While when we selectively
install, we can't leave cruft on the target, but we will notice easily
that something is missing.

Of course, the alternative is to go full-blown, which I also find
acceptable. We can then trim down in followup patches.

> > - Invoke "install" for each file? There are quite of them.

OTOH, there are probalby quite a bunch of files to remove too, no?

> > - Do normal 'make install' then call 'rm' on each uneeded files in target_dir?
>  Yep, that's the one.

I'm still not happy with tselective removal...

> >> (and I find it ugly indeed, because it makes the first pattern different
> >> from the others... Bleark...)
>  Eek, bleark indeed... Why do we have that -e in $(SED)?

The most powerfull of the reasons: it's historic? ;-)

> > That field was not designed to place hostname there but os branding. For
> > example, "GNU/Linux Debian" would be good example of branding, since multiple
> > hardware with different hostnames can use single OS. Of course it's just for
> > eye-candy but I would prefer to hardcode it to "Buildroot" (or empty) than put
> > hostname or issue here.
> 
>  For gcc we have:
>         --with-pkgversion="Buildroot $(BR2_VERSION_FULL)" \
>         --with-bugurl="http://bugs.buildroot.net/"
> so using the same for openrc seems appropriate.

+1

>  Maybe we should introduce a global config + variable to control this, i.e. the
> NAME and PRETTY_NAME in /etc/os-release.

I amundecided on that one... Would you also offer a way to change the
version, to match that of the product? IMHO, people that are concerned
with having their names in there can just generate os-release from a
post-build script, so that they can get the version out of their VCS for
example.

> >> I think this should be a sub-option of the openrc package, not the 'init'
> >> part.
>  I actually disagree that the various init packages depend on PID1 coming from
> that particular package. There could be use cases to have a different init
> system (usually _NONE) and still have the init executable there.

But then, what package is responsible for install /sbin/init ?

If you allow to enable busybox' init, sysvinit, and systemd (and later,
OpenRC) simultaneously, they will all compete to provide /sbin/init,
with the latest winning over the others...

(Actually 'None' should really read as 'Custom', because there better be
an kind of init system, or 'first'application' to start.)

>  For openrc, this is doubly so, because openrc is not a standalone init system.
> So it can very obviously be used with a different PID1.
>  So IMO this choice really belongs to the system config.

Well, OpenRC really looks like a weird 'non-init' system, in the end...

Maybe we're taking the problem in the wrong way, then? Can we revert the
logic, and have something like:

    System configuration  --->
        Init system
            ( ) Busybox
            (X) sysvinit
            ( ) systemd
            ( ) None
        [ ] Use OpenRC as scripts scheduler/watcher/reaper

With that new option having 'depends on !systemd'.

I haven't given it much thought, but maybe that's where we want to go?
Or something along those lines? Thoughts?

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH] package/openrc: new init package OpenRC (0.38.3)
  2019-03-06 17:36         ` Yann E. MORIN
@ 2019-03-06 18:12           ` Yann E. MORIN
  2019-03-06 19:47             ` Yann E. MORIN
  2019-03-07  8:53             ` michal.lyszczek at bofc.pl
  2019-03-07  9:58           ` Arnout Vandecappelle
  1 sibling, 2 replies; 12+ messages in thread
From: Yann E. MORIN @ 2019-03-06 18:12 UTC (permalink / raw
  To: buildroot

Micha?, Arnout, All,

On 2019-03-06 18:36 +0100, Yann E. MORIN spake thusly:
> Maybe we're taking the problem in the wrong way, then?

And actually, I think we are. As per [0]:

    Since 0.25 OpenRC includes openrc-init, which can replace /sbin/init

Also, the 0.25 release notes contain:

    This version contains an OpenRC-specific implementation of init for
    Linux which can be used in place of sysvinit or any other init process.

I checked, and it looks like this is still the case in OpenRC's master
branch.

So, I believe we should aim for this solution, because it drastically
simplifies things.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH] package/openrc: new init package OpenRC (0.38.3)
  2019-03-06 18:12           ` Yann E. MORIN
@ 2019-03-06 19:47             ` Yann E. MORIN
  2019-03-07  8:53             ` michal.lyszczek at bofc.pl
  1 sibling, 0 replies; 12+ messages in thread
From: Yann E. MORIN @ 2019-03-06 19:47 UTC (permalink / raw
  To: buildroot

Micha?, Arnout, All,

On 2019-03-06 19:12 +0100, Yann E. MORIN spake thusly:
> On 2019-03-06 18:36 +0100, Yann E. MORIN spake thusly:
> > Maybe we're taking the problem in the wrong way, then?
> And actually, I think we are. As per [0]:

[0] https://en.wikipedia.org/wiki/OpenRC

>     Since 0.25 OpenRC includes openrc-init, which can replace /sbin/init
> 
> Also, the 0.25 release notes contain:
> 
>     This version contains an OpenRC-specific implementation of init for
>     Linux which can be used in place of sysvinit or any other init process.
> 
> I checked, and it looks like this is still the case in OpenRC's master
> branch.
> 
> So, I believe we should aim for this solution, because it drastically
> simplifies things.
> 
> Regards,
> Yann E. MORIN.
> 
> -- 
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH] package/openrc: new init package OpenRC (0.38.3)
  2019-03-06  9:58       ` Arnout Vandecappelle
  2019-03-06 17:36         ` Yann E. MORIN
@ 2019-03-07  8:36         ` michal.lyszczek at bofc.pl
  2019-03-07  9:36           ` Arnout Vandecappelle
  1 sibling, 1 reply; 12+ messages in thread
From: michal.lyszczek at bofc.pl @ 2019-03-07  8:36 UTC (permalink / raw
  To: buildroot

On 2019-03-06 10:58:09, Arnout Vandecappelle wrote:
> 
> 
> On 06/03/2019 00:20, michal.lyszczek at bofc.pl wrote:
> > Yann, Arnout
> > 
> > First, thank you for your extensive review. Now to the point.
> > 
> > On 2019-03-05 22:38:58, Yann E. MORIN wrote:
> >> Micha?, Arnout,
> >>
> >> On 2019-03-04 19:59 +0100, Arnout Vandecappelle spake thusly:
> [snip]
> >>> On 28/02/2019 20:44, Micha? ?yszczek wrote:
> >>>> OpenRC is a dependency based init system that works with the system
> >>>> provided init program, normally located at /sbin/init. It is not a
> >>>> replacement for /sbin/init.
> >>>>
> >>>> Signed-off-by: Micha? ?yszczek <michal.lyszczek@bofc.pl>
> >>>> ---
> >>>> OpenRC uses own set of scripts for sysinit thus
> >>>> BR2_PACKAGE_INITSCRIPTS is disabled. I decided to add new init
> >>>> choice and choice which program to use as PID1 since OpenRC uses
> >>>> its own scripts for services so S** scripts cannot be used. It is
> >>>> possible to add simple script to local.d that would start all S**
> >>>> scripts, but that would diminish OpenRC's features and usefullness,
> >>>> so additional startup scripts should be written and added to
> >>>> buildroot.
> >>>
> >>>  While this is true, I think you should still add that script so that packages
> >>> which don't have an OpenRC service description will still work. (We should also
> >>> have done that for systemd IMO.)
> >>
> >> Not needed, as it is my understanding that systemd autoimatically
> >> creates units for scripts in /etc/init.d/ when there is no corresponding
> >> native unit. From http://man7.org/linux/man-pages/man1/systemd.1.html :
> >>
> >>     systemd is compatible with the SysV init system to a large degree:
> >>     SysV init scripts are supported and simply read as an alternative
> >>     (though limited) configuration file format. The SysV /dev/initctl
> >>     interface is provided, and compatibility implementations of the
> >>     various SysV client tools are available. In addition to that, various
> >>     established Unix functionality such as /etc/fstab or the utmp
> >>     database are supported.
> 
>  Yes, but at the moment we don't install the sysvinit scripts so systemd is not
> going to execute them.
> 
>  Ideally we'd also have a runtime test that checks if the sysvinit script indeed
> gets called.
> 
> >>
> >>> Then you can have something like thisin
> >>> pkg-generic.mk:
> >>>
> >>> $(2)_INSTALL_INIT_OPENRC ?= $$($(2)_INSTALL_INIT_SYSV)
> >>>
> >>> so if a package defines its OpenRC file, that one will be used instead of the
> >>> sysvinit script.
> >>
> >> Is that works so easily, then this is indeed a good idea.
> >>
> >>>  With that in mind, I think an OpenRC skeelton would make sense at well. It
> >>> would be pretty empty :-).
> >>
> >> Except maybe for that one script that does the sysv-init fallback.
> > 
> > Ok, how about this:
> 
>  Er, the following paragraph is completely unrelated to what we were discussing
> above... Did you understand what we were talking about above?

I'm not sure now. Since you proposed to create separate skeleton for OpenRC I
suggested that I would put there some custom scripts that are not in OpenRC
package (like getty service) or are not entirely compatible with BusyBox
binaries (sysctl service uses --system).

> > by default OpenRC installs and enables many services that
> > are not necessarily needed, like setting hwclock, configuring network
> > interfaces, activating swap, setting keymaps. Some of these services fails or
> > may fail due to missing network interfaces, or due to missing additional
> > programs like loadkeys for setting keymaps. Also I notices that sysctl fails
> > with BusyBox version since it is missing "--system" option. These are not
> > critical and OpenRC still boots up and spawns tty with login - but it prints
> > error and thus is ugly and may confuse users.
> 
>  For services that are provided by busybox, it will be difficult to know if a
> custom busybox config enables them or not... Anyway, I think this is something
> that is better solved in a follow-up patch (which you can propose in the same
> series, of course).
> 
> > So my proposition would be to not perform default 'make install' but install
> > only bare minimum set of init scrips, that are sure not to fail. 
> 
>  Instead of doing a custom install, I prefer to just do the default install and
> remove things are not needed / wanted.
> 
> > And scripts
> > that are not compatible with default buildroot installation could land in
> > skeleton.
> 
>  Instead you can make the removal conditional.
> 
> 
> > One notable example is "agetty". It's not like it's some hardcore dependency for
> > OpenRC, but it uses it by default to spawn shell, and agetty is not compatible
> > with busybox due to one change in order:
> > 
> > agetty [options] port [baud_rate...] [term]
> > getty [OPTIONS] BAUD_RATE TTY [TERMTYPE]
> > 
> > See, baud rate is swaped with port. So I could write custom getty service for
> > OpenRC and put it into skeleton. That would lift agetty dependency.
> 
>  Yeah, again, you can do this conditionally on whether agetty is enabled or not.
> But indeed in that case it's better to put it in a skeleton package and not in
> the openrc package itself, so you can get rid of it by using a custom skeleton.
> 
> 
> > What might be best way to install only chosed files?
> 
>  In the .mk file (of the skeleton-init-openrc package), don't use rsync but
> instead do conditional $(INSTALL) calls. Something like:
> 
> ifeq ($(BR2_PACKAGE_UTIL_LINUX_AGETTY),)
> define SKELETON_INIT_OPENRC_INSTALL_GETTY
> 	$(INSTALL) -D -m 0644 $(@D)/getty $(TARGET_DIR)/etc/conf.d/getty
> 	$(RM) $(TARGET_DIR)/etc/conf.d/agetty
> endef
> endif
> 
> define SKELETON_INIT_OPENRC_INSTALL_TARGET_CMDS
> 	...
> 	$(SKELETON_INIT_OPENRC_INSTALL_GETTY)
> endef
> 
>  You can also do it with a POST_INSTALL_HOOK if you prefer that.
> 
> > 
> > - Do 'make install' to temp directory, remove what is not needed than copy to
> > target?
> > - Invoke "install" for each file? There are quite of them.
> > - Do normal 'make install' then call 'rm' on each uneeded files in target_dir?
> 
>  Yep, that's the one.
> 
> > - or maybe there is some easy way of doing it I didn't think about?
> > 
> >>>> If OpenRC init is accepted into Buildroot, I will commit
> >>>> some time to port existing sysvinit startup scripts to OpenRC.
> >>>>
> >>>> OpenRC works with default config - a single change is needed (which
> >>>> init system to use, with optional PID1 change - default BusyBox). I
> >>>> tested it with both BusyBox and sysvinit as PID1. Also tested with
> >>>> BusyBox programs enabled and disabled (standalone apps used, none
> >>>> of which was from BusyBox package). I tested it using
> >>>> qemu-x86_64_defconfig and beaglebone_defconfig.
> >>>
> >>>  If this gets added as a full BR2_INIT_ option, you should also add the runtime
> >>> tests for it (in separate patches, obviously). Look at the system tests for
>                                                                ^^^^^^
>                                                 I meant systemd here.
> 
> >>> inspiration.
> >>
> >> ... located in support/testing/tests/init/.
> >>
> >> [--SNIP--]
> >>>> +++ b/package/openrc/Config.in
> >>>> @@ -0,0 +1,19 @@
> >>>> +config BR2_PACKAGE_OPENRC
> >>>> +	bool "OpenRC"
> >>>> +	select BR2_PACKAGE_UTIL_LINUX
> >>>> +	select BR2_PACKAGE_UTIL_LINUX_AGETTY
> >>>
> >>>  Does it really depend on agetty? Seems strange...
> > 
> > It does not, answer is above. I wanted to preserve as much upstream stuff as
> > possible - but maybe that is not the way?
> 
>  So we found a way to get rid of the agetty dependency. Still, better do that in
> a follow-up patch and keep the initial patch as simple as possible.
> 
>  For sure, it should not be the openrc package itself that selects it, because
> it is only needed if you actually want to use openrc to start a getty. So it
> belongs to a skeleton package instead.

agetty is really not a dependency for openrc, it's just opnerc have chosen
agetty as default tty opener just like busybox have chosen getty. Now that you
told me to use skeleton - I really think best way is to get rid of agetty
altogether and just use getty instead - spawned with custom getty service that
will be put in skeleton.

> > 
> >>>  That said, if additional tweaks are needed to support busybox getty, that can
> >>> be done in a follow-up patch.
> >>
> >> We have a similar workaround for systemd, but I just sent a patch to
> >> actually remove it:
> >>     https://patchwork.ozlabs.org/patch/1051821/
> > 
> > As stated above, I could create simple service that would use getty instead of
> > agetty and put it in skeleon.
> > 
> >> If OpenRC does not require util-linux, it would be acceptable to have a
> >> workaround to be able to use busybox' getty instead. But if OpenRC
> >> otherwise requires util-linux (e.g. for lib"stuff"), then there is no
> >> point for such a workaround.
> > 
> > OpenRC does not need util-linux.
> > 
> >>>> +	OPENRC_MAKE_OPTS += MKZSHCOMP=yes
> >>>> +endif
> >>>> +
> >>>> +ifeq ($(BR2_SHARED_LIBS),y)
> >>>> +	OPENRC_MAKE_OPTS += MKSTATICLIBS=no
> >>>> +else
> >>>> +	OPENRC_MAKE_OPTS += MKSTATICLIBS=yes
> >>>> +endif
> >>>> +
> >>>> +OPENRC_MAKE_OPTS += TARGETDIR=$(TARGET_DIR)
> >>
> >> Arnout, TARGETDIR comes from here. Although I admit it is confusing...
> > 
> > This patch was introduced because OpenRC determines location for its libs,
> > wheter to put them in /lib or /lib64 directory, by reading link of /lib.
> > Unfortunately it has "/lib" path hardcoded and buildroot is reading that
> > link from host os, so libs generated for cross x86 could land in lib64 on amd64
> > hosts. I will rise this issue to OpenRC's author - maybe there is already
> > solution for this and I've just invented wheel again.
> 
>  Yes but as I mentioned somewhere else in my review: if this is only used at
> install time and not at build time then you should use $(DESTDIR)/$(PREFIX) and
> not some new TARGETDIR variable.
> 
>  Oh and don't forget to send that patch upstream for review.

I've looked into it more closely and openrc defines libname as "LIBNAME?="
so it's really easy to override during compilation, and I can use already
existing solution for determing lib version that is in system.mk

ifeq ($(BR2_ARCH_IS_64)$(BR2_MIPS_NABI32),y)
    OPENRC_MAKE_OPTS += LIBNAME=lib64
else
    OPENRC_MAKE_OPTS += LIBNAME=lib
endif

That should do the trick without any patches I reckon.

> > 
> >>>> +OPENRC_MAKE_OPTS += MKNET=yes
> >>>> +OPENRC_MAKE_OPTS += MKPKGCONFIG=no
> >>>> +OPENRC_MAKE_OPTS += MKSELINUX=no
> >>>> +OPENRC_MAKE_OPTS += MKSYSVINIT=yes
> >>
> >> When doing a sequence, I's nicer to write:
> >>
> >>     OPENRC_MAKE_OPTS += \
> >>         TARGETDIR=$(TARGET_DIR) \
> >>         MKNET=yes \
> >>         MKPKGCONFIG=no \
> >>         [...]
> 
>  Actually, I disagree that it is nicer, and we already have both patterns in
> Buildroot.
> 
> >>
> >>>> +OPENRC_MAKE_OPTS += BRANDING=$(BR2_OPENRC_BRANDING)
> >>>  We typically use an explicit qstrip and adding quotes here. That way, it works
> >>> correctly even if you override it from the command line or in local.mk.
> >>
> >> To be homest, I don;t thionk overriding frm the command-line should be
> >> suported that nicely, because it is not reproducible. It may work for a
> >> quicky, but how much more complex is it to go to menuconfig, or edit
> >> .config ?
> >>>> +	ln -sf /etc/init.d/agetty.$(SYSTEM_GETTY_PORT) $(TARGET_DIR)/etc/runlevels/default
> >>>> +	$(SED) '/#term_type=/c\term_type="$(SYSTEM_GETTY_TERM)"' $(TARGET_DIR)/etc/conf.d/agetty
> >>>> +	$(SED) '/#agetty_options=/c\agetty_options="$(SYSTEM_GETTY_OPTIONS)"' $(TARGET_DIR)/etc/conf.d/agetty
> >>>
> >>>  We usually do this kind of thing in a single SED invocation, with -e to specify
> >>> each command.
> >>
> >> ... except for the first, as $(SED) already ends with -e.
> >>
> >> (and I find it ugly indeed, because it makes the first pattern different
> >> from the others... Bleark...)
> 
>  Eek, bleark indeed... Why do we have that -e in $(SED)?
> 
> >>
> >>>> +config BR2_OPENRC_BRANDING
> >>>> +	string "OpenRC branding"
> >>>> +	default "Buildroot"
> >>>> +	depends on BR2_INIT_OPENRC
> >>>> +	help
> >>>> +	  Custom string that will show up when OpenRC starts like:
> >>>> +
> >>>> +	  OpenRC 0.38.3.8ee04a5 is starting up <branding-name> (armv7l)
> >>>
> >>>  I really don't think it's worth to add a config option for this. Would it be
> >>> possible to reuse one of the existing options, e.g. BR2_TARGET_GENERIC_HOSTNAME
> >>> or BR2_TARGET_GENERIC_ISSUE? Or alternatively, just leave branding empty?
> >>
> >> Agreed.
> >>
> > 
> > That field was not designed to place hostname there but os branding. For
> > example, "GNU/Linux Debian" would be good example of branding, since multiple
> > hardware with different hostnames can use single OS. Of course it's just for
> > eye-candy but I would prefer to hardcode it to "Buildroot" (or empty) than put
> > hostname or issue here.
> 
>  For gcc we have:
> 
>         --with-pkgversion="Buildroot $(BR2_VERSION_FULL)" \
>         --with-bugurl="http://bugs.buildroot.net/"
> 
> so using the same for openrc seems appropriate.
> 
>  Maybe we should introduce a global config + variable to control this, i.e. the
> NAME and PRETTY_NAME in /etc/os-release.

Agree, "Buildroot $(BR2_VERSION_FULL)" fits that field nicely.

> >>>  If it is a config option, I think it is more appropriate as a suboption of the
> >>> openrc package. We do that for the systemd suboptions as well.
> >>
> >> I don't think it has to be a new option, BR2_TARGET_GENERIC_ISSUE is
> >> exactly there for this kind of usage.
> >>
> >>>> +choice
> >>>> +	prompt "PID1 to use with OpenRC"
> >>>> +	default BR2_OPENRC_PID1_BUSYBOX
> >>>> +	help
> >>>> +	  Since OpenRC is not replacement for /sbin/init but works with
> >>>> +	  it, you can select which PID1 service you want to run
> >>>> +
> >>>> +config BR2_OPENRC_PID1_BUSYBOX
> >>>
> >>>  Since this is a suboption of BR2_INIT_OPENRC, it should be called
> >>> BR2_INIT_OPENRC_PID1_BUSYBOX. But I think the PID1 bit is a bit redundant.
> >>
> >> I think this should be a sub-option of the openrc package, not the 'init'
> >> part.
> 
>  I disagree.
> 
>  I actually disagree that the various init packages depend on PID1 coming from
> that particular package. There could be use cases to have a different init
> system (usually _NONE) and still have the init executable there.
> 
>  For openrc, this is doubly so, because openrc is not a standalone init system.
> So it can very obviously be used with a different PID1.
> 
>  So IMO this choice really belongs to the system config.
> 
> 
>  Regards,
>  Arnout
> 
> 
> >>>> +endif # BR2_INIT_OPENRC
> >>>> +
> >>>>  choice
> >>>>  	prompt "/dev management" if !BR2_INIT_SYSTEMD
> >>>>  	default BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_DEVTMPFS
> >>>> @@ -337,16 +376,16 @@ config BR2_TARGET_GENERIC_GETTY_BAUDRATE
> >>>>  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
> >>>> +	# currently observed only by busybox, sysvinit and openrc
> >>>> +	depends on BR2_INIT_BUSYBOX || BR2_INIT_SYSV || BR2_INIT_OPENRC
> >>>
> >>>  Perhaps this should be changed into !BR2_INIT_SYSTEMD - that's actually what it
> >>> is at the moment.
> >>>
> >>>  Come to think of it, BR2_TARGET_GENERIC_GETTY should depend on !BR2_INIT_NONE
> >>> as well.
> >>
> >> Probably, indeed... I missed that one when doing my big overhaul of that
> >> section.
> > 
> > I agree with statements where I didn't leave comment and will apply proposed
> > fixes.
> > 
> > Thanks again for review.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20190307/2c95c1da/attachment-0001.asc>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH] package/openrc: new init package OpenRC (0.38.3)
  2019-03-06 18:12           ` Yann E. MORIN
  2019-03-06 19:47             ` Yann E. MORIN
@ 2019-03-07  8:53             ` michal.lyszczek at bofc.pl
  1 sibling, 0 replies; 12+ messages in thread
From: michal.lyszczek at bofc.pl @ 2019-03-07  8:53 UTC (permalink / raw
  To: buildroot

On 2019-03-06 19:12:21, Yann E. MORIN wrote:
> Micha?, Arnout, All,
> 
> On 2019-03-06 18:36 +0100, Yann E. MORIN spake thusly:
> > Maybe we're taking the problem in the wrong way, then?
> 
> And actually, I think we are. As per [0]:
> 
>     Since 0.25 OpenRC includes openrc-init, which can replace /sbin/init
> 
> Also, the 0.25 release notes contain:
> 
>     This version contains an OpenRC-specific implementation of init for
>     Linux which can be used in place of sysvinit or any other init process.
> 
> I checked, and it looks like this is still the case in OpenRC's master
> branch.
> 
> So, I believe we should aim for this solution, because it drastically
> simplifies things.

I did quick audit of openrc-init, it's *very* simple init. It starts openrc,
which starts all services, opens fifo to handle shutdown/reboot, and reaps
zombies. So it does what it really is suppose to do and I don't see a way for it
to crash once it's started. So I am ok with using openrc-init as PID1
/sbin/init
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20190307/0a8ed584/attachment.asc>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH] package/openrc: new init package OpenRC (0.38.3)
  2019-03-07  8:36         ` michal.lyszczek at bofc.pl
@ 2019-03-07  9:36           ` Arnout Vandecappelle
  0 siblings, 0 replies; 12+ messages in thread
From: Arnout Vandecappelle @ 2019-03-07  9:36 UTC (permalink / raw
  To: buildroot



On 07/03/2019 09:36, michal.lyszczek at bofc.pl wrote:
> On 2019-03-06 10:58:09, Arnout Vandecappelle wrote:
>>
>>
>> On 06/03/2019 00:20, michal.lyszczek at bofc.pl wrote:
>>> Yann, Arnout
>>>
>>> First, thank you for your extensive review. Now to the point.
>>>
>>> On 2019-03-05 22:38:58, Yann E. MORIN wrote:
>>>> Micha?, Arnout,
>>>>
>>>> On 2019-03-04 19:59 +0100, Arnout Vandecappelle spake thusly:
>> [snip]
>>>>> On 28/02/2019 20:44, Micha? ?yszczek wrote:
[snip]
>>  So we found a way to get rid of the agetty dependency. Still, better do that in
>> a follow-up patch and keep the initial patch as simple as possible.
>>
>>  For sure, it should not be the openrc package itself that selects it, because
>> it is only needed if you actually want to use openrc to start a getty. So it
>> belongs to a skeleton package instead.
> 
> agetty is really not a dependency for openrc, it's just opnerc have chosen
> agetty as default tty opener just like busybox have chosen getty. Now that you
> told me to use skeleton - I really think best way is to get rid of agetty
> altogether and just use getty instead - spawned with custom getty service that
> will be put in skeleton.

 Yes obviously.

 I don't write my replies top-to-bottom, I do it topic by topic so sometimes
things get a little inconsistent.

 Still, I think you should split any such thing off in separate patches:

1. openrc package, clean and simple, no init scripts.
2. openrc as an init system (assuming no inittab is needed with openrc-init)
3. pkg-generic support for _INSTALL_INIT_OPENRC.
4. skeleton-init-openrc, empty.
5. skeleton-init-openrc getty support
6. skeleton-init-openrc legacy init script support
7. ...

[snip]
> I've looked into it more closely and openrc defines libname as "LIBNAME?="
> so it's really easy to override during compilation, and I can use already
> existing solution for determing lib version that is in system.mk
> 
> ifeq ($(BR2_ARCH_IS_64)$(BR2_MIPS_NABI32),y)
>     OPENRC_MAKE_OPTS += LIBNAME=lib64
> else
>     OPENRC_MAKE_OPTS += LIBNAME=lib
> endif
> 
> That should do the trick without any patches I reckon.

 Good catch. Remember that we don't indent inside `make` conditions though.

 By the way, since lib64 is a symlink to lib, isn't an unconditional LIBNAME=lib
sufficient?


 Regards,
 Arnout

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH] package/openrc: new init package OpenRC (0.38.3)
  2019-03-06 17:36         ` Yann E. MORIN
  2019-03-06 18:12           ` Yann E. MORIN
@ 2019-03-07  9:58           ` Arnout Vandecappelle
  1 sibling, 0 replies; 12+ messages in thread
From: Arnout Vandecappelle @ 2019-03-07  9:58 UTC (permalink / raw
  To: buildroot



On 06/03/2019 18:36, Yann E. MORIN wrote:
> Arnout, Micha?, All,
> 
> On 2019-03-06 10:58 +0100, Arnout Vandecappelle spake thusly:
[snip]
>>  Instead of doing a custom install, I prefer to just do the default install and
>> remove things are not needed / wanted.
> 
> I usually prefer to whitelist rather than blacklist. I.e .I prefer a
> selective install, rather than a selective removal.
> 
> Otherwise, when we bump the version, and a new tool is installed, we can
> very easily miss it and leave it installed. While when we selectively
> install, we can't leave cruft on the target, but we will notice easily
> that something is missing.

 That knife cuts both ways: when you do custom install, you can easily miss
something new that is required. It's almost never a problem if too much gets
installed, while a missing shared library tends to be troublesome.

 Remember that we have seen this issue several times with the qt5 packages.


> Of course, the alternative is to go full-blown, which I also find
> acceptable. We can then trim down in followup patches.

 It's not a matter of trimming down, really, because it's just a few KB. It's a
matter of avoiding warnings on the console.


>>> - Invoke "install" for each file? There are quite of them.
> 
> OTOH, there are probalby quite a bunch of files to remove too, no?

 I don't know, of course, but I would expect it to be just one or two.

[snip]
>>  Maybe we should introduce a global config + variable to control this, i.e. the
>> NAME and PRETTY_NAME in /etc/os-release.
> 
> I amundecided on that one... Would you also offer a way to change the
> version, to match that of the product? IMHO, people that are concerned
> with having their names in there can just generate os-release from a
> post-build script, so that they can get the version out of their VCS for
> example.

 You can still do that with a BR2_BRANDING config variable, e.g. by setting it
to "ExampleCo $(shell get-version)"


>>>> I think this should be a sub-option of the openrc package, not the 'init'
>>>> part.
>>  I actually disagree that the various init packages depend on PID1 coming from
>> that particular package. There could be use cases to have a different init
>> system (usually _NONE) and still have the init executable there.
> 
> But then, what package is responsible for install /sbin/init ?

 Same as always: if several packages install the same file, we have to resolve
the conflict. Nowadays we do that by adding a dependency and either avoiding
installation if it's already there (busybox) or overwriting it (most other cases).

 /sbin/init is currently the big exception to that.


> If you allow to enable busybox' init, sysvinit, and systemd (and later,
> OpenRC) simultaneously, they will all compete to provide /sbin/init,
> with the latest winning over the others...
> 
> (Actually 'None' should really read as 'Custom', because there better be
> an kind of init system, or 'first'application' to start.)

 Not at all. I've used Buildroot to create chroot environment, to create a
toolchain, ...

 Actually, I would like to have an option like BR2_SYSTEM_NONE that just
disables everything (no init, no shell, no passwd, no skeleton except for the
minimal bin,lib,sbin dir/symlink, no busybox, ...). I just never got around to
implement that.


 Regards,
 Arnout

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-03-07  9:58 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-28 19:44 [Buildroot] [PATCH] package/openrc: new init package OpenRC (0.38.3) Michał Łyszczek
2019-03-04 18:59 ` Arnout Vandecappelle
2019-03-05 21:38   ` Yann E. MORIN
2019-03-05 23:20     ` michal.lyszczek at bofc.pl
2019-03-06  9:58       ` Arnout Vandecappelle
2019-03-06 17:36         ` Yann E. MORIN
2019-03-06 18:12           ` Yann E. MORIN
2019-03-06 19:47             ` Yann E. MORIN
2019-03-07  8:53             ` michal.lyszczek at bofc.pl
2019-03-07  9:58           ` Arnout Vandecappelle
2019-03-07  8:36         ` michal.lyszczek at bofc.pl
2019-03-07  9:36           ` Arnout Vandecappelle

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.