[Buildroot] [PATCH 0/5] Various init scripts modification

[Buildroot] [PATCH 0/5] Various init scripts modification

[Maxime Hadjinlian]

Hello All !

This is a short series which does the following: (the indexes match the
patches)

   1 Extract the init.d folder into its own package
   1 Make Busybox and SysV depends on it
   2 Change busybox a little bit to not install scripts if it's not the
   init
   3 Add a generic way to install startup scripts for the relevant init
   systems. This is mainly done to avoid duplicated code everywhere.
   For this to work, you need to have a variable and tell where are the
   init scripts files. This is done that way for two main reasons:
      - Many packages have/will have their init scripts packaged in
      	their sources. We don't want to keep them in the packages, so
      	you are able to specify something along the line "$(@D)/src/S99foo"
      - If you want to install init scripts conditionnaly (if there's a
      	daemon), you are able to do it.
   If a package requires customs actions, you can use the current
   LIBFOO_INSTALL_INIT_* or even mix the two together (which would be
   usefull for avahi for example)
   4-5 Two patches only to show, what the modification leads to. Theses
   two patches should not be applied as is, see question below.

Also, should the modification of each packages be done in one huge patch
or in multilple patches ?
Since it's a custom modification and not mechanical for each packages, I
would go for a patch per package.

Looking forward for comments.

Maxime Hadjinlian (5):
  initscripts: new package
  busybox: Install scripts only when needed
  infra: Add automatic install of init scripts
  acpid: Use new FOO_INIT_SYSV_FILES variable
  am33x-cm3: Use new FOO_INIT_SYSV_FILES variable

 docs/manual/adding-packages-generic.txt            | 17 +++++++++++++
 package/acpid/acpid.mk                             |  6 +----
 package/am33x-cm3/am33x-cm3.mk                     |  6 +----
 package/busybox/busybox.mk                         | 11 +++++++++
 .../inittab => package/initscripts/busybox_inittab |  0
 .../etc => package/initscripts}/init.d/S20urandom  |  0
 .../etc => package/initscripts}/init.d/S40network  |  0
 .../etc => package/initscripts}/init.d/rcK         |  0
 .../etc => package/initscripts}/init.d/rcS         |  0
 package/initscripts/initscripts.mk                 | 28 ++++++++++++++++++++++
 .../inittab => initscripts/sysvinit_inittab}       |  0
 package/pkg-generic.mk                             | 26 ++++++++++++++++----
 package/sysvinit/sysvinit.mk                       | 11 +++++----
 13 files changed, 86 insertions(+), 19 deletions(-)
 rename system/skeleton/etc/inittab => package/initscripts/busybox_inittab (100%)
 rename {system/skeleton/etc => package/initscripts}/init.d/S20urandom (100%)
 rename {system/skeleton/etc => package/initscripts}/init.d/S40network (100%)
 rename {system/skeleton/etc => package/initscripts}/init.d/rcK (100%)
 rename {system/skeleton/etc => package/initscripts}/init.d/rcS (100%)
 create mode 100644 package/initscripts/initscripts.mk
 rename package/{sysvinit/inittab => initscripts/sysvinit_inittab} (100%)

--
2.1.1

[Buildroot] [PATCH 1/5] initscripts: new package

[Maxime Hadjinlian]

The folder init.d is currently installed by default since it's part of
our skeleton.
This patch creates a package out of it and make busybox/sysvinit depends
on it.

This way, if you chose another init, you don't end up with a useless
init.d folder.

Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
---
v3 -> v4:
    - Fix bug with busybox
v2 -> v3:
    - Fix rebase issues
v1 -> v2:
    - Redo the patch using git format-patch -M
---
 package/busybox/busybox.mk                         |  4 ++++
 .../inittab => package/initscripts/busybox_inittab |  0
 .../etc => package/initscripts}/init.d/S20urandom  |  0
 .../etc => package/initscripts}/init.d/S40network  |  0
 .../etc => package/initscripts}/init.d/rcK         |  0
 .../etc => package/initscripts}/init.d/rcS         |  0
 package/initscripts/initscripts.mk                 | 28 ++++++++++++++++++++++
 .../inittab => initscripts/sysvinit_inittab}       |  0
 package/sysvinit/sysvinit.mk                       | 11 +++++----
 9 files changed, 38 insertions(+), 5 deletions(-)
 rename system/skeleton/etc/inittab => package/initscripts/busybox_inittab (100%)
 rename {system/skeleton/etc => package/initscripts}/init.d/S20urandom (100%)
 rename {system/skeleton/etc => package/initscripts}/init.d/S40network (100%)
 rename {system/skeleton/etc => package/initscripts}/init.d/rcK (100%)
 rename {system/skeleton/etc => package/initscripts}/init.d/rcS (100%)
 create mode 100644 package/initscripts/initscripts.mk
 rename package/{sysvinit/inittab => initscripts/sysvinit_inittab} (100%)

diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
index 73a99fb..422a95d 100644
--- a/package/busybox/busybox.mk
+++ b/package/busybox/busybox.mk
@@ -16,6 +16,10 @@ BUSYBOX_CFLAGS = \
 BUSYBOX_LDFLAGS = \
 	$(TARGET_LDFLAGS)
 
+# initscripts contains/install only inittab and the init.d folder with base
+# scripts.
+BUSYBOX_DEPENDENCIES = initscripts
+
 # Link against libtirpc if available so that we can leverage its RPC
 # support for NFS mounting with BusyBox
 ifeq ($(BR2_PACKAGE_LIBTIRPC),y)
diff --git a/system/skeleton/etc/inittab b/package/initscripts/busybox_inittab
similarity index 100%
rename from system/skeleton/etc/inittab
rename to package/initscripts/busybox_inittab
diff --git a/system/skeleton/etc/init.d/S20urandom b/package/initscripts/init.d/S20urandom
similarity index 100%
rename from system/skeleton/etc/init.d/S20urandom
rename to package/initscripts/init.d/S20urandom
diff --git a/system/skeleton/etc/init.d/S40network b/package/initscripts/init.d/S40network
similarity index 100%
rename from system/skeleton/etc/init.d/S40network
rename to package/initscripts/init.d/S40network
diff --git a/system/skeleton/etc/init.d/rcK b/package/initscripts/init.d/rcK
similarity index 100%
rename from system/skeleton/etc/init.d/rcK
rename to package/initscripts/init.d/rcK
diff --git a/system/skeleton/etc/init.d/rcS b/package/initscripts/init.d/rcS
similarity index 100%
rename from system/skeleton/etc/init.d/rcS
rename to package/initscripts/init.d/rcS
diff --git a/package/initscripts/initscripts.mk b/package/initscripts/initscripts.mk
new file mode 100644
index 0000000..13affe8
--- /dev/null
+++ b/package/initscripts/initscripts.mk
@@ -0,0 +1,28 @@
+################################################################################
+#
+# initscripts
+#
+################################################################################
+
+# source included in buildroot
+INITSCRIPTS_SOURCE =
+
+# Note: We need to override Busybox's inittab with an inittab compatible with
+# sysvinit if we want SYSVINIT as our init.
+ifeq ($(BR2_PACKAGE_SYSVINIT),y)
+define INITSCRIPTS_INSTALL_INITTAB
+	$(INSTALL) -D -m 0644 package/initscripts/sysvinit_inittab $(TARGET_DIR)/etc/inittab
+endef
+else
+define INITSCRIPTS_INSTALL_INITTAB
+	$(INSTALL) -D -m 0644 package/initscripts/busybox_inittab $(TARGET_DIR)/etc/inittab
+endef
+endif
+
+define INITSCRIPTS_INSTALL_TARGET_CMDS
+	mkdir -p  $(TARGET_DIR)/etc/init.d
+	$(INSTALL) -D -m 0755 package/initscripts/init.d/* $(TARGET_DIR)/etc/init.d/
+	$(INITSCRIPTS_INSTALL_INITTAB)
+endef
+
+$(eval $(generic-package))
diff --git a/package/sysvinit/inittab b/package/initscripts/sysvinit_inittab
similarity index 100%
rename from package/sysvinit/inittab
rename to package/initscripts/sysvinit_inittab
diff --git a/package/sysvinit/sysvinit.mk b/package/sysvinit/sysvinit.mk
index d588122..6793ab8 100644
--- a/package/sysvinit/sysvinit.mk
+++ b/package/sysvinit/sysvinit.mk
@@ -11,9 +11,13 @@ SYSVINIT_SITE = $(BR2_DEBIAN_MIRROR)/debian/pool/main/s/sysvinit
 SYSVINIT_LICENSE = GPLv2+
 SYSVINIT_LICENSE_FILES = COPYING
 
-# Override BusyBox implementations if BusyBox is enabled.
+# initscripts contains/install only inittab and the init.d folder with base
+# scripts.
+SYSVINIT_DEPENDENCIES = initscripts
+
+# Override Busybox implementations if Busybox is enabled.
 ifeq ($(BR2_PACKAGE_BUSYBOX),y)
-SYSVINIT_DEPENDENCIES = busybox
+SYSVINIT_DEPENDENCIES += busybox
 endif
 
 define SYSVINIT_DEBIAN_PATCHES
@@ -34,9 +38,6 @@ define SYSVINIT_INSTALL_TARGET_CMDS
 	for x in halt init shutdown killall5; do \
 		$(INSTALL) -D -m 0755 $(@D)/src/$$x $(TARGET_DIR)/sbin/$$x || exit 1; \
 	done
-	# Override BusyBox's inittab with an inittab compatible with
-	# sysvinit
-	$(INSTALL) -D -m 0644 package/sysvinit/inittab $(TARGET_DIR)/etc/inittab
 	ln -sf /sbin/halt $(TARGET_DIR)/sbin/reboot
 	ln -sf /sbin/halt $(TARGET_DIR)/sbin/poweroff
 	ln -sf killall5 $(TARGET_DIR)/sbin/pidof
-- 
2.1.1

[Buildroot] [PATCH 2/5] busybox: Install scripts only when needed

[Maxime Hadjinlian]

Init scripts are only usefull with init system that supports them, it
means that we don't want init scripts files unless Busybox or SysV has
been chosen as the init.

Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
---
 package/busybox/busybox.mk | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
index 422a95d..a8b4b24 100644
--- a/package/busybox/busybox.mk
+++ b/package/busybox/busybox.mk
@@ -167,6 +167,7 @@ define BUSYBOX_SET_INIT
 endef
 endif
 
+ifeq ($(BR2_INIT_SYSV)$(BR2_INIT_BUSYBOX), y)
 define BUSYBOX_INSTALL_LOGGING_SCRIPT
 	if grep -q CONFIG_SYSLOGD=y $(@D)/.config; then \
 		[ -f $(TARGET_DIR)/etc/init.d/S01logging ] || \
@@ -174,11 +175,16 @@ define BUSYBOX_INSTALL_LOGGING_SCRIPT
 				$(TARGET_DIR)/etc/init.d/S01logging; \
 	else rm -f $(TARGET_DIR)/etc/init.d/S01logging; fi
 endef
+endif
 
 ifeq ($(BR2_PACKAGE_BUSYBOX_WATCHDOG),y)
 define BUSYBOX_SET_WATCHDOG
         $(call KCONFIG_ENABLE_OPT,CONFIG_WATCHDOG,$(BUSYBOX_BUILD_CONFIG))
 endef
+endif
+
+ifeq ($(BR2_PACKAGE_BUSYBOX_WATCHDOG),y)
+ifeq ($(BR2_INIT_SYSV)$(BR2_INIT_BUSYBOX), y)
 define BUSYBOX_INSTALL_WATCHDOG_SCRIPT
 	[ -f $(TARGET_DIR)/etc/init.d/S15watchdog ] || \
 		$(INSTALL) -D -m 0755 package/busybox/S15watchdog \
@@ -187,6 +193,7 @@ define BUSYBOX_INSTALL_WATCHDOG_SCRIPT
 			$(TARGET_DIR)/etc/init.d/S15watchdog
 endef
 endif
+endif
 
 # Enable "noclobber" in install.sh, to prevent BusyBox from overwriting any
 # full-blown versions of apps installed by other packages with sym/hard links.
-- 
2.1.1

[Buildroot] [PATCH 3/5] infra: Add automatic install of init scripts

[Maxime Hadjinlian]

Instead of copy pasting the install code over all our packages, you may
now define variables to specify where to take the init script files.
Each file will be installed in the default folder of the relevant init
system.

For systemd, instead of installing the files into
$(TARGET_DIR)/etc/systemd/system, the file are now installed in
$(TARGET_DIR)/lib/systemd/system which appears to be the right things to
do.

If you have specific operations to perform in order to install your init
scripts or any other file relevant to the init system (tmpfiles for
systemd), you can still define the usual FOO_INSTALL_INIT_SYSTEMD for
your specific actions.

Also add the documentation explaining the mechanism.

Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
---
 docs/manual/adding-packages-generic.txt | 17 +++++++++++++++++
 package/pkg-generic.mk                  | 26 ++++++++++++++++++++++----
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
index 67a7453..a0e51f2 100644
--- a/docs/manual/adding-packages-generic.txt
+++ b/docs/manual/adding-packages-generic.txt
@@ -368,6 +368,23 @@ information is (assuming the package name is +libfoo+) :
   FLAT binary format is only 4k bytes. If the application consumes more stack,
   append the required number here.
 
+* +LIBFOO_INIT_SYSV_FILES+ and +LIBFOO_INIT_SYSTEMD_FILES+ are
+  a space-separated list of init scripts path. Respectively, for
+  the systemV-like init systems (busybox, sysvinit, etc.) and for
+  systemd.
+  Theses init scripts files will only be installed when the relevant
+  init system is installed (i.e. if systemd is selected as the init
+  system in the configuration, only +LIBFOO_INIT_SYSTEMD_FILES+ will be
+  installed).
+  Each scripts will be installed in the default folder of the relevant
+  init system.
+  Note: This doesn't forbid you from defining +LIBFOO_INSTALL_INIT_SYSV+ and
+  +LIBFOO_INSTALL_INIT_SYSTEMD+ if you need to perform specific actions.
+  Theses variables are optionnal.
+  Examples: +
+    +LIBFOO_INSTALL_INIT_SYSV=package/libfoo/S99libfoo +
+    +LIBFOO_INSTALL_INIT_SYSV=$(@D)/S99libfoo +
+
 The recommended way to define these variables is to use the following
 syntax:
 
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 259ee02..1d90ae9 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -229,10 +229,27 @@ $(BUILD_DIR)/%/.stamp_target_installed:
 	@$(call MESSAGE,"Installing to target")
 	$(foreach hook,$($(PKG)_PRE_INSTALL_TARGET_HOOKS),$(call $(hook))$(sep))
 	+$($(PKG)_INSTALL_TARGET_CMDS)
-	$(if $(BR2_INIT_SYSTEMD),\
-		$($(PKG)_INSTALL_INIT_SYSTEMD))
-	$(if $(BR2_INIT_SYSV)$(BR2_INIT_BUSYBOX),\
-		$($(PKG)_INSTALL_INIT_SYSV))
+ifeq ($(BR2_INIT_SYSTEMD),y)
+	$(Q)if test -n "$($(PKG)_INIT_SYSTEMD_FILES)" ; then \
+		mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants ; \
+		for s in $($(PKG)_INIT_SYSTEMD_FILES); do \
+			f=$$(basename $${s}); \
+			$(INSTALL) -D -m 644 $${s} $(TARGET_DIR)/lib/systemd/system/$${f} ; \
+			ln -fs /lib/systemd/system/$${f} \
+				$(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/$${f} ; \
+		done ; \
+	fi
+	$($(PKG)_INSTALL_INIT_SYSTEMD)
+endif
+ifeq ($(BR2_INIT_SYSV)$(BR2_INIT_BUSYBOX),y)
+	$(Q)if test -n "$($(PKG)_INIT_SYSV_FILES)" ; then \
+		for s in $($(PKG)_INIT_SYSV_FILES); do \
+			   f=$$(basename $${s}) ; \
+			   $(INSTALL) -D -m 0755 $${s} $(TARGET_DIR)/etc/init.d/$${f} ; \
+		done ; \
+	fi
+	$($(PKG)_INSTALL_INIT_SYSV)
+endif
 	$(foreach hook,$($(PKG)_POST_INSTALL_TARGET_HOOKS),$(call $(hook))$(sep))
 	$(Q)if test -n "$($(PKG)_CONFIG_SCRIPTS)" ; then \
 		$(RM) -f $(addprefix $(TARGET_DIR)/usr/bin/,$($(PKG)_CONFIG_SCRIPTS)) ; \
@@ -592,6 +609,7 @@ $(1)-reconfigure:	$(1)-clean-for-reconfigure $(1)
 # define the PKG variable for all targets, containing the
 # uppercase package variable prefix
 $$($(2)_TARGET_INSTALL_TARGET):		PKG=$(2)
+$$($(2)_TARGET_INSTALL_TARGET):		PKGDIR=$(pkgdir)
 $$($(2)_TARGET_INSTALL_STAGING):	PKG=$(2)
 $$($(2)_TARGET_INSTALL_IMAGES):		PKG=$(2)
 $$($(2)_TARGET_INSTALL_HOST):           PKG=$(2)
-- 
2.1.1

[Buildroot] [PATCH 4/5] acpid: Use new FOO_INIT_SYSV_FILES variable

[Maxime Hadjinlian]

With the update of pkg-generic, we don't need to define the
FOO_INSTALL_INIT_SYSV function.
In its place, we need to define FOO_INIT_SYSV_FILES with the full path
to the init script.

Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
---
 package/acpid/acpid.mk | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/package/acpid/acpid.mk b/package/acpid/acpid.mk
index 7a6a478..488034d 100644
--- a/package/acpid/acpid.mk
+++ b/package/acpid/acpid.mk
@@ -9,11 +9,7 @@ ACPID_SOURCE = acpid-$(ACPID_VERSION).tar.xz
 ACPID_SITE = http://downloads.sourceforge.net/project/acpid2
 ACPID_LICENSE = GPLv2+
 ACPID_LICENSE_FILES = COPYING
-
-define ACPID_INSTALL_INIT_SYSV
-	$(INSTALL) -D -m 0755 package/acpid/S02acpid \
-		$(TARGET_DIR)/etc/init.d/S02acpid
-endef
+ACPID_INIT_SYSV_FILES = package/acpid/S02acpid
 
 define ACPID_SET_EVENTS
 	mkdir -p $(TARGET_DIR)/etc/acpi/events
-- 
2.1.1

[Buildroot] [PATCH 5/5] am33x-cm3: Use new FOO_INIT_SYSV_FILES variable

[Maxime Hadjinlian]

With the update of pkg-generic, we don't need to define the
FOO_INSTALL_INIT_SYSV function.
In its place, we need to define FOO_INIT_SYSV_FILES with the full path
to the init script.

Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
---
 package/am33x-cm3/am33x-cm3.mk | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/package/am33x-cm3/am33x-cm3.mk b/package/am33x-cm3/am33x-cm3.mk
index 01cb9a0..ac78959 100644
--- a/package/am33x-cm3/am33x-cm3.mk
+++ b/package/am33x-cm3/am33x-cm3.mk
@@ -10,6 +10,7 @@ AM33X_CM3_SITE = http://arago-project.org/git/projects/am33x-cm3.git
 AM33X_CM3_SITE_METHOD = git
 AM33X_CM3_LICENSE = TI Publicly Available Software License
 AM33X_CM3_LICENSE_FILES = License.txt
+AM33X_CM3_INIT_SYSV_FILES = package/am33x-cm3/S93-am335x-pm-firmware-load
 
 # The build command below will use the standard cross-compiler (normally
 # build for Cortex-A8, to build the FW for the Cortex-M3.
@@ -23,9 +24,4 @@ define AM33X_CM3_INSTALL_TARGET_CMDS
 		$(TARGET_DIR)/lib/firmware/am335x-pm-firmware.bin
 endef
 
-define AM33X_CM3_INSTALL_INIT_SYSV
-	$(INSTALL) -m 0755 -D package/am33x-cm3/S93-am335x-pm-firmware-load \
-		$(TARGET_DIR)/etc/init.d/S93-am335x-pm-firmware-load
-endef
-
 $(eval $(generic-package))
-- 
2.1.1

[Buildroot] [PATCH 2/5] busybox: Install scripts only when needed

[Thomas Petazzoni]

Dear Maxime Hadjinlian,

On Sat, 18 Oct 2014 16:47:01 +0200, Maxime Hadjinlian wrote:

> +ifeq ($(BR2_INIT_SYSV)$(BR2_INIT_BUSYBOX), y)

	, y

should be

	,y

i.e, with no space.

> +ifeq ($(BR2_PACKAGE_BUSYBOX_WATCHDOG),y)
> +ifeq ($(BR2_INIT_SYSV)$(BR2_INIT_BUSYBOX), y)

Ditto.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 2/5] busybox: Install scripts only when needed

[Maxime Hadjinlian]

Hi Thomas, all
On Sat, Oct 18, 2014 at 6:53 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Maxime Hadjinlian,
>
> On Sat, 18 Oct 2014 16:47:01 +0200, Maxime Hadjinlian wrote:
>
>> +ifeq ($(BR2_INIT_SYSV)$(BR2_INIT_BUSYBOX), y)
>
>         , y
>
> should be
>
>         ,y
>
> i.e, with no space.
>
>> +ifeq ($(BR2_PACKAGE_BUSYBOX_WATCHDOG),y)
>> +ifeq ($(BR2_INIT_SYSV)$(BR2_INIT_BUSYBOX), y)
>
> Ditto.
Ah yes, I thought I had fixed that everywhere. Thanks !
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

[Buildroot] [PATCH 3/5] infra: Add automatic install of init scripts

[Thomas Petazzoni]

Dear Maxime Hadjinlian,

On Sat, 18 Oct 2014 16:47:02 +0200, Maxime Hadjinlian wrote:

> +ifeq ($(BR2_INIT_SYSTEMD),y)
> +	$(Q)if test -n "$($(PKG)_INIT_SYSTEMD_FILES)" ; then \
> +		mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants ; \
> +		for s in $($(PKG)_INIT_SYSTEMD_FILES); do \
> +			f=$$(basename $${s}); \
> +			$(INSTALL) -D -m 644 $${s} $(TARGET_DIR)/lib/systemd/system/$${f} ; \
> +			ln -fs /lib/systemd/system/$${f} \
> +				$(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/$${f} ; \
> +		done ; \
> +	fi
> +	$($(PKG)_INSTALL_INIT_SYSTEMD)
> +endif
> +ifeq ($(BR2_INIT_SYSV)$(BR2_INIT_BUSYBOX),y)
> +	$(Q)if test -n "$($(PKG)_INIT_SYSV_FILES)" ; then \
> +		for s in $($(PKG)_INIT_SYSV_FILES); do \
> +			   f=$$(basename $${s}) ; \
> +			   $(INSTALL) -D -m 0755 $${s} $(TARGET_DIR)/etc/init.d/$${f} ; \
> +		done ; \
> +	fi
> +	$($(PKG)_INSTALL_INIT_SYSV)
> +endif

Why is an explicit variable necessary ? Why not simply install all the
S* and K* files from the package directory for sysvinit/busybox and all
the .service (or some extension) of the package directory for systemd ?
I think we discussed this at the meeting, no?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 3/5] infra: Add automatic install of init scripts

[Maxime Hadjinlian]

Hi Thomas, all

On Sat, Oct 18, 2014 at 6:56 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Maxime Hadjinlian,
>
> On Sat, 18 Oct 2014 16:47:02 +0200, Maxime Hadjinlian wrote:
>
>> +ifeq ($(BR2_INIT_SYSTEMD),y)
>> +     $(Q)if test -n "$($(PKG)_INIT_SYSTEMD_FILES)" ; then \
>> +             mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants ; \
>> +             for s in $($(PKG)_INIT_SYSTEMD_FILES); do \
>> +                     f=$$(basename $${s}); \
>> +                     $(INSTALL) -D -m 644 $${s} $(TARGET_DIR)/lib/systemd/system/$${f} ; \
>> +                     ln -fs /lib/systemd/system/$${f} \
>> +                             $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/$${f} ; \
>> +             done ; \
>> +     fi
>> +     $($(PKG)_INSTALL_INIT_SYSTEMD)
>> +endif
>> +ifeq ($(BR2_INIT_SYSV)$(BR2_INIT_BUSYBOX),y)
>> +     $(Q)if test -n "$($(PKG)_INIT_SYSV_FILES)" ; then \
>> +             for s in $($(PKG)_INIT_SYSV_FILES); do \
>> +                        f=$$(basename $${s}) ; \
>> +                        $(INSTALL) -D -m 0755 $${s} $(TARGET_DIR)/etc/init.d/$${f} ; \
>> +             done ; \
>> +     fi
>> +     $($(PKG)_INSTALL_INIT_SYSV)
>> +endif
>
> Why is an explicit variable necessary ? Why not simply install all the
> S* and K* files from the package directory for sysvinit/busybox and all
> the .service (or some extension) of the package directory for systemd ?
> I think we discussed this at the meeting, no?
Indeed, but as stated in the cover letter, we need to have a variable,
because, some packages only install their init scripts conditionally
(take ntpd for example).
So you can't blindly install files only by maching their names.
Also, the current trend is that applications have their init scripts
in the sources, and we don't want to keep a copy if in our packages,
we want to take it straight from the sources, having such a variable
enable that.
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

[Buildroot] [PATCH 3/5] infra: Add automatic install of init scripts

[Thomas Petazzoni]

Dear Maxime Hadjinlian,

On Sat, 18 Oct 2014 16:47:02 +0200, Maxime Hadjinlian wrote:

> +ifeq ($(BR2_INIT_SYSV)$(BR2_INIT_BUSYBOX),y)
> +	$(Q)if test -n "$($(PKG)_INIT_SYSV_FILES)" ; then \
> +		for s in $($(PKG)_INIT_SYSV_FILES); do \
> +			   f=$$(basename $${s}) ; \
> +			   $(INSTALL) -D -m 0755 $${s} $(TARGET_DIR)/etc/init.d/$${f} ; \
> +		done ; \
> +	fi
> +	$($(PKG)_INSTALL_INIT_SYSV)
> +endif

Another comment: you're forcefully installing the init script here,
while many of our packages (but not all!) test if the script is already
installed before installing it. The use case for this is to allow a
custom version of the script to be part of the filesystem skeleton, and
therefore to not see it being overridden by a package.

I personally don't care very much about this use case, as I believe
people should rather use rootfs overlays, and also because it is anyway
not full-proof: while we can prevent the Buildroot package logic from
overwriting files in the root filesystem, we cannot prevent the build
system of each package from overwriting files.

But it's something that needs to be discussed and decided. Peter ?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 3/5] infra: Add automatic install of init scripts

[Thomas Petazzoni]

Dear Maxime Hadjinlian,

On Sat, 18 Oct 2014 18:59:03 +0200, Maxime Hadjinlian wrote:

> > Why is an explicit variable necessary ? Why not simply install all the
> > S* and K* files from the package directory for sysvinit/busybox and all
> > the .service (or some extension) of the package directory for systemd ?
> > I think we discussed this at the meeting, no?
> Indeed, but as stated in the cover letter, we need to have a variable,
> because, some packages only install their init scripts conditionally
> (take ntpd for example).

Well, we could by default install S* and K* files, except if the
<pkg>_INIT_SYSV_FILES variable is defined, in which case we only
install the ones that are mentioned. Maybe it's making the logic too
complicated, I don't know.

> So you can't blindly install files only by maching their names.
> Also, the current trend is that applications have their init scripts
> in the sources, and we don't want to keep a copy if in our packages,
> we want to take it straight from the sources, having such a variable
> enable that.

For systemd, I would expect the package build system to install the
unit files by itself, no?

And for init scripts, we generally don't use the init scripts provided
by the package itself, because they're often too complicated / not
compatible with the simple Busybox init.

But, well, maybe you're right and making things explicit is better.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 3/5] infra: Add automatic install of init scripts

[Maxime Hadjinlian]

Hi Thomas, all

On Sat, Oct 18, 2014 at 7:06 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Maxime Hadjinlian,
>
> On Sat, 18 Oct 2014 18:59:03 +0200, Maxime Hadjinlian wrote:
>
>> > Why is an explicit variable necessary ? Why not simply install all the
>> > S* and K* files from the package directory for sysvinit/busybox and all
>> > the .service (or some extension) of the package directory for systemd ?
>> > I think we discussed this at the meeting, no?
>> Indeed, but as stated in the cover letter, we need to have a variable,
>> because, some packages only install their init scripts conditionally
>> (take ntpd for example).
>
> Well, we could by default install S* and K* files, except if the
> <pkg>_INIT_SYSV_FILES variable is defined, in which case we only
> install the ones that are mentioned. Maybe it's making the logic too
> complicated, I don't know.
We could that, but as you say, maybe it makes things a little more
complex than they should be.
>
>> So you can't blindly install files only by maching their names.
>> Also, the current trend is that applications have their init scripts
>> in the sources, and we don't want to keep a copy if in our packages,
>> we want to take it straight from the sources, having such a variable
>> enable that.
>
> For systemd, I would expect the package build system to install the
> unit files by itself, no?
I would expect that too yes. It would be quite nice actually.
>
> And for init scripts, we generally don't use the init scripts provided
> by the package itself, because they're often too complicated / not
> compatible with the simple Busybox init.
>
> But, well, maybe you're right and making things explicit is better.
It's up for discussion as always :).
The intention here, is mainly to remove duplicated code and have some
enforced rules as to where the init files should be installed.
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

[Buildroot] [PATCH 3/5] infra: Add automatic install of init scripts

[Thomas Petazzoni]

Dear Maxime Hadjinlian,

On Sat, 18 Oct 2014 19:11:37 +0200, Maxime Hadjinlian wrote:

> > Well, we could by default install S* and K* files, except if the
> > <pkg>_INIT_SYSV_FILES variable is defined, in which case we only
> > install the ones that are mentioned. Maybe it's making the logic too
> > complicated, I don't know.
> We could that, but as you say, maybe it makes things a little more
> complex than they should be.

Yeah, maybe you're right. What I'm proposing is too complicated, and
what you suggest has the advantage of being completely explicit.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 3/5] infra: Add automatic install of init scripts

[André Erdmann]

Hi,

2014-10-18 16:47 GMT+02:00 Maxime Hadjinlian <maxime.hadjinlian@gmail.com>:
> Instead of copy pasting the install code over all our packages, you may
> now define variables to specify where to take the init script files.
> Each file will be installed in the default folder of the relevant init
> system.
>
> For systemd, instead of installing the files into
> $(TARGET_DIR)/etc/systemd/system, the file are now installed in
> $(TARGET_DIR)/lib/systemd/system which appears to be the right things to
> do.
>
> If you have specific operations to perform in order to install your init
> scripts or any other file relevant to the init system (tmpfiles for
> systemd), you can still define the usual FOO_INSTALL_INIT_SYSTEMD for
> your specific actions.
>
> Also add the documentation explaining the mechanism.
>
> Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
> ---
>  docs/manual/adding-packages-generic.txt | 17 +++++++++++++++++
>  package/pkg-generic.mk                  | 26 ++++++++++++++++++++++----
>  2 files changed, 39 insertions(+), 4 deletions(-)
>
> diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
> index 67a7453..a0e51f2 100644
> --- a/docs/manual/adding-packages-generic.txt
> +++ b/docs/manual/adding-packages-generic.txt
> @@ -368,6 +368,23 @@ information is (assuming the package name is +libfoo+) :
>    FLAT binary format is only 4k bytes. If the application consumes more stack,
>    append the required number here.
>
> +* +LIBFOO_INIT_SYSV_FILES+ and +LIBFOO_INIT_SYSTEMD_FILES+ are
> +  a space-separated list of init scripts path. Respectively, for
> +  the systemV-like init systems (busybox, sysvinit, etc.) and for
> +  systemd.
> +  Theses init scripts files will only be installed when the relevant
> +  init system is installed (i.e. if systemd is selected as the init
> +  system in the configuration, only +LIBFOO_INIT_SYSTEMD_FILES+ will be
> +  installed).
> +  Each scripts will be installed in the default folder of the relevant
> +  init system.
> +  Note: This doesn't forbid you from defining +LIBFOO_INSTALL_INIT_SYSV+ and
> +  +LIBFOO_INSTALL_INIT_SYSTEMD+ if you need to perform specific actions.
> +  Theses variables are optionnal.
> +  Examples: +
> +    +LIBFOO_INSTALL_INIT_SYSV=package/libfoo/S99libfoo +
> +    +LIBFOO_INSTALL_INIT_SYSV=$(@D)/S99libfoo +
> +
>  The recommended way to define these variables is to use the following
>  syntax:
>
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 259ee02..1d90ae9 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -229,10 +229,27 @@ $(BUILD_DIR)/%/.stamp_target_installed:
>         @$(call MESSAGE,"Installing to target")
>         $(foreach hook,$($(PKG)_PRE_INSTALL_TARGET_HOOKS),$(call $(hook))$(sep))
>         +$($(PKG)_INSTALL_TARGET_CMDS)
> -       $(if $(BR2_INIT_SYSTEMD),\
> -               $($(PKG)_INSTALL_INIT_SYSTEMD))
> -       $(if $(BR2_INIT_SYSV)$(BR2_INIT_BUSYBOX),\
> -               $($(PKG)_INSTALL_INIT_SYSV))
> +ifeq ($(BR2_INIT_SYSTEMD),y)
> +       $(Q)if test -n "$($(PKG)_INIT_SYSTEMD_FILES)" ; then \
> +               mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants ; \

A "set -e" or some "|| exit"s might be missing here.

> +               for s in $($(PKG)_INIT_SYSTEMD_FILES); do \
> +                       f=$$(basename $${s}); \
> +                       $(INSTALL) -D -m 644 $${s} $(TARGET_DIR)/lib/systemd/system/$${f} ; \
> +                       ln -fs /lib/systemd/system/$${f} \
> +                               $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/$${f} ; \
> +               done ; \
> +       fi
> +       $($(PKG)_INSTALL_INIT_SYSTEMD)
> +endif
> +ifeq ($(BR2_INIT_SYSV)$(BR2_INIT_BUSYBOX),y)
> +       $(Q)if test -n "$($(PKG)_INIT_SYSV_FILES)" ; then \

And here.

> +               for s in $($(PKG)_INIT_SYSV_FILES); do \
> +                          f=$$(basename $${s}) ; \
> +                          $(INSTALL) -D -m 0755 $${s} $(TARGET_DIR)/etc/init.d/$${f} ; \
> +               done ; \
> +       fi
> +       $($(PKG)_INSTALL_INIT_SYSV)
> +endif
>         $(foreach hook,$($(PKG)_POST_INSTALL_TARGET_HOOKS),$(call $(hook))$(sep))
>         $(Q)if test -n "$($(PKG)_CONFIG_SCRIPTS)" ; then \
>                 $(RM) -f $(addprefix $(TARGET_DIR)/usr/bin/,$($(PKG)_CONFIG_SCRIPTS)) ; \
> @@ -592,6 +609,7 @@ $(1)-reconfigure:   $(1)-clean-for-reconfigure $(1)
>  # define the PKG variable for all targets, containing the
>  # uppercase package variable prefix
>  $$($(2)_TARGET_INSTALL_TARGET):                PKG=$(2)
> +$$($(2)_TARGET_INSTALL_TARGET):                PKGDIR=$(pkgdir)
>  $$($(2)_TARGET_INSTALL_STAGING):       PKG=$(2)
>  $$($(2)_TARGET_INSTALL_IMAGES):                PKG=$(2)
>  $$($(2)_TARGET_INSTALL_HOST):           PKG=$(2)
> --
> 2.1.1
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

[Buildroot] [PATCH 0/5] Various init scripts modification

[Thomas Petazzoni]

Dear Maxime Hadjinlian,

On Sat, 18 Oct 2014 16:46:59 +0200, Maxime Hadjinlian wrote:

>   infra: Add automatic install of init scripts
>   acpid: Use new FOO_INIT_SYSV_FILES variable
>   am33x-cm3: Use new FOO_INIT_SYSV_FILES variable

We discussed this at the Buildroot meeting, and decided to not go down
this route. We believe the amount of redundancy with the existing
_INSTALL_INIT_SYSV and _INSTALL_INIT_SYSTEMD variables is not that
huge, and in many _INSTALL_INIT_SYSTEMD variable, a bit of custom stuff
is anyway needed. So we believe the benefits are too small compared to
the effort.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 2/5] busybox: Install scripts only when needed

[Thomas Petazzoni]

Dear Maxime Hadjinlian,

On Sat, 18 Oct 2014 16:47:01 +0200, Maxime Hadjinlian wrote:
> Init scripts are only usefull with init system that supports them, it
> means that we don't want init scripts files unless Busybox or SysV has
> been chosen as the init.
> 
> Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
> ---
>  package/busybox/busybox.mk | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
> index 422a95d..a8b4b24 100644
> --- a/package/busybox/busybox.mk
> +++ b/package/busybox/busybox.mk
> @@ -167,6 +167,7 @@ define BUSYBOX_SET_INIT
>  endef
>  endif
>  
> +ifeq ($(BR2_INIT_SYSV)$(BR2_INIT_BUSYBOX), y)
>  define BUSYBOX_INSTALL_LOGGING_SCRIPT
>  	if grep -q CONFIG_SYSLOGD=y $(@D)/.config; then \
>  		[ -f $(TARGET_DIR)/etc/init.d/S01logging ] || \
> @@ -174,11 +175,16 @@ define BUSYBOX_INSTALL_LOGGING_SCRIPT
>  				$(TARGET_DIR)/etc/init.d/S01logging; \
>  	else rm -f $(TARGET_DIR)/etc/init.d/S01logging; fi
>  endef
> +endif
>  
>  ifeq ($(BR2_PACKAGE_BUSYBOX_WATCHDOG),y)
>  define BUSYBOX_SET_WATCHDOG
>          $(call KCONFIG_ENABLE_OPT,CONFIG_WATCHDOG,$(BUSYBOX_BUILD_CONFIG))
>  endef
> +endif
> +
> +ifeq ($(BR2_PACKAGE_BUSYBOX_WATCHDOG),y)
> +ifeq ($(BR2_INIT_SYSV)$(BR2_INIT_BUSYBOX), y)
>  define BUSYBOX_INSTALL_WATCHDOG_SCRIPT
>  	[ -f $(TARGET_DIR)/etc/init.d/S15watchdog ] || \
>  		$(INSTALL) -D -m 0755 package/busybox/S15watchdog \
> @@ -187,6 +193,7 @@ define BUSYBOX_INSTALL_WATCHDOG_SCRIPT
>  			$(TARGET_DIR)/etc/init.d/S15watchdog
>  endef
>  endif
> +endif

Are you sure this is actually needed? The
BUSYBOX_INSTALL_LOGGING_SCRIPT and BUSYBOX_INSTALL_WATCHDOG_SCRIPT are
used as follows:

define BUSYBOX_INSTALL_INIT_SYSV
        $(BUSYBOX_INSTALL_MDEV_SCRIPT)
        $(BUSYBOX_INSTALL_LOGGING_SCRIPT)
        $(BUSYBOX_INSTALL_WATCHDOG_SCRIPT)
endef

So they are already only triggered for a sysv or busybox init.
Therefore, I've marked the patch as Rejected in patchwork. If we
misunderstood something, please let us know.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/5] initscripts: new package

[Thomas Petazzoni]

Dear Maxime Hadjinlian,

On Sat, 18 Oct 2014 16:47:00 +0200, Maxime Hadjinlian wrote:
> The folder init.d is currently installed by default since it's part of
> our skeleton.
> This patch creates a package out of it and make busybox/sysvinit depends
> on it.
> 
> This way, if you chose another init, you don't end up with a useless
> init.d folder.
> 
> Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>

We reviewed this and believe this is a good idea. However, we would
like:

 1/ To keep the sysvinit inittab in the sysvinit package.

 2/ Move the Busybox inittab in the busybox package, and install it
 when the Busybox init applet is enabled in the Busybox configuration
 file.

 3/ Have a Config.in file for BR2_PACKAGE_INITSCRIPTS, which is
 selected by both BR2_INIT_BUSYBOX and BR2_INIT_SYSV. Then the busybox
 and sysvinit would no longer have initscripts in their _DEPENDENCIES,
 because initscripts is not a build-time dependency for those packages,
 but only a runtime dependency.

Could you work on those 3 items, and resend an updated version of your
series?

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com