All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system
@ 2017-08-01 22:52 Arnout Vandecappelle
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 01/14] support/testing: add runtime testing for init systems Arnout Vandecappelle
                   ` (15 more replies)
  0 siblings, 16 replies; 22+ messages in thread
From: Arnout Vandecappelle @ 2017-08-01 22:52 UTC (permalink / raw)
  To: buildroot

This series is a proposal to fix our handling of systemd as an init
system.

Our default skeleton is not well suited for systemd:

  - we have /var/log a symlink to /tmp/log, but the way systemd starts
    hides the journals:
    - start systemd-journald, stores journals in /tmp/log/ (because of
      the redirection above)
    - mounts a tmpfs over /tmp, thus hidding the journals.

  - on a read-only rootfs, systemd expects /var to be read-write, which
    we do not provide. Read-only rootfs will be handled in a separate
    series.

All of this sounds trivial, but fixing it is definitely not.

The overall idea is that we need different skeletons, one for each type
of init systems: sysv, systemd, others (aka custom).

The skeleton package becomes a virtual package, that is provided by one
of four implementations:

  - skeleton-sysv or skeleton-systemd, as their names imply, for either
    SysV- or systemd-based systems, and which depend on skeleton-common;

  - skeleton-common, that provides base files and directories comon to
    all skeletons (except custom, below); skeleton-common is also a
    provider for the skeleton virtual package, for use with no-init (aka
    custom init) systems;

  - skeleton-custom, for user-provided custom skeleton, and of which we
    expect it is fully prepared; skeleton-custom does not depend on
    skeleton-common.

Eventually, we add support for runing systemd on a read-only rootfs.
This requires that we play tricks with /var and the systemd-tmpfiles
factory feautre.

A series of runtime tests have also been added, running on the QEMU
vexpress:

    init system         rootfs      DHCP?
    -------------------------------------
    busybox             ext4        no
    busybox             ext4        eth0
    busybox             squashfs    no
    busybox             squashfs    eth0

  * systemd-ifupdown    ext4        eth0
  * systemd-ifupdown    squashfs    eth0
 ** systemd-networkd    ext4        eth0
 ** systemd-networkd    squashfs    eth0
*** systemd-full        ext4        eth0
*** systemd-full        squashfs    eth0

  + no-init             squashfs    no

  * systemd-networkd: only what gets selected automatically when
    systemd is used as init system, with all other options unset;

 ** systemd-ifupdown: like systemd-networkd, but with networkd
    disabled, thus using ifupdown instead;

*** systemd-full: all systemd options enabled, implies networkd;

  + a shell is launched as init

Those new tests are not exhaustive, especially the systemd one; instead,
they just test basic features: what init is being used, and did we get
an IP adress for eth0? More tests can be added later on.

You can also find this series on https://gitlab.com/arnout/buildroot
branch systemd-skeleton-5-by-arnout

Changes v4 -> v5:
  - moved the test infra patch all the way to the beginning of the series
  - dropped the patch that removed support for BR2_INIT_NONE with the default
    skeleton
  - to make that possible, merged the patch that re-introduced BR2_INIT_NONE
    support with the patch that breaks it, i.e. the introduction of
    skeleton-sysv and skeleton-systemd
  - split the patch that splits off skeleton-custom into 4 patches; the first
    one does only the split, the other ones do 3 clean-ups. Except for
    comments, the end result is the same as the original skeleton-custom
  - add a further cleanup to skeleton-common that renames
    SKELETON_ROOT_PASSWORD to SKELETON_CUSTOM_ROOT_PASSWORD
  - re-generate .gitlab-ci.yml in the two patches that create new tests.

Changes v3 -> v4:
  - fix the read-only support  (Arnout)
  - add reviewed-by tags from Arnout. Thanks a lot!
  - enhance some commit logs after those reviews
  - simplify no-init skeleton-common  (Arnout)

Changes v2 -> v3:
  - too much to note, mostly rewriten from scratch

Changes v1 -> v2:
  - enhance the commit logs

Regards,
Arnout

----------------------------------------------------------------
Arnout Vandecappelle (1):
  skeleton-common: rename SKELETON_ROOT_PASSWORD to
    SKELETON_CUSTOM_ROOT_PASSWORD

Yann E. MORIN (13):
  support/testing: add runtime testing for init systems
  package/skeleton: split out into skeleton-custom
  package/skeleton-custom: rework the merged_usr and building conditions
  package/skeleton-custom: also check for missing directories
  package/skeleton-custom: simplify target/staging install
  package/skeleton: split out into skeleton-common
  package/skeleton: make it a virtual package
  package/skeleton-common: simplify staging install
  package/skeleton: introduce sysv- and systemd-specific skeletons
  system: separate sysv and systemd parts of the skeleton
  fs: add pre- and post-command hooks
  system: make systemd work on a read-only rootfs
  support/testing: add runtime testing for read-only systemd

 .gitlab-ci.yml                                     |   8 ++
 fs/common.mk                                       |   4 +
 package/Config.in                                  |   4 +
 package/pkg-generic.mk                             |   4 +
 package/skeleton-common/Config.in                  |  17 +++
 package/skeleton-common/skeleton-common.mk         |  90 ++++++++++++
 package/skeleton-custom/Config.in                  |   6 +
 package/skeleton-custom/skeleton-custom.mk         | 107 +++++++++++++++
 package/skeleton-systemd/Config.in                 |   7 +
 package/skeleton-systemd/skeleton-systemd.mk       |  71 ++++++++++
 package/skeleton-sysv/Config.in                    |   7 +
 package/skeleton-sysv/skeleton-sysv.mk             |  22 +++
 {system => package/skeleton-sysv}/skeleton/dev/log |   0
 .../skeleton-sysv}/skeleton/dev/pts/.empty         |   0
 .../skeleton-sysv}/skeleton/dev/shm/.empty         |   0
 .../skeleton-sysv}/skeleton/etc/fstab              |   0
 .../skeleton-sysv}/skeleton/var/cache              |   0
 .../skeleton-sysv}/skeleton/var/lib/misc           |   0
 .../skeleton-sysv}/skeleton/var/lock               |   0
 {system => package/skeleton-sysv}/skeleton/var/log |   0
 {system => package/skeleton-sysv}/skeleton/var/run |   0
 .../skeleton-sysv}/skeleton/var/spool              |   0
 {system => package/skeleton-sysv}/skeleton/var/tmp |   0
 package/skeleton/Config.in                         |   9 +-
 package/skeleton/skeleton.mk                       | 139 +------------------
 .../testing/tests/init/__init__.py                 |   0
 support/testing/tests/init/base.py                 |  47 +++++++
 .../testing/tests/init/systemd-factory/var/foo/bar |   1 +
 support/testing/tests/init/test_busybox.py         |  67 +++++++++
 support/testing/tests/init/test_none.py            |  32 +++++
 support/testing/tests/init/test_systemd.py         | 152 +++++++++++++++++++++
 system/Config.in                                   |  12 +-
 system/skeleton/dev/{pts => }/.empty               |   0
 33 files changed, 663 insertions(+), 143 deletions(-)
 create mode 100644 package/skeleton-common/Config.in
 create mode 100644 package/skeleton-common/skeleton-common.mk
 create mode 100644 package/skeleton-custom/Config.in
 create mode 100644 package/skeleton-custom/skeleton-custom.mk
 create mode 100644 package/skeleton-systemd/Config.in
 create mode 100644 package/skeleton-systemd/skeleton-systemd.mk
 create mode 100644 package/skeleton-sysv/Config.in
 create mode 100644 package/skeleton-sysv/skeleton-sysv.mk
 rename {system => package/skeleton-sysv}/skeleton/dev/log (100%)
 copy {system => package/skeleton-sysv}/skeleton/dev/pts/.empty (100%)
 rename {system => package/skeleton-sysv}/skeleton/dev/shm/.empty (100%)
 rename {system => package/skeleton-sysv}/skeleton/etc/fstab (100%)
 rename {system => package/skeleton-sysv}/skeleton/var/cache (100%)
 rename {system => package/skeleton-sysv}/skeleton/var/lib/misc (100%)
 rename {system => package/skeleton-sysv}/skeleton/var/lock (100%)
 rename {system => package/skeleton-sysv}/skeleton/var/log (100%)
 rename {system => package/skeleton-sysv}/skeleton/var/run (100%)
 rename {system => package/skeleton-sysv}/skeleton/var/spool (100%)
 rename {system => package/skeleton-sysv}/skeleton/var/tmp (100%)
 copy system/skeleton/dev/pts/.empty => support/testing/tests/init/__init__.py (100%)
 create mode 100644 support/testing/tests/init/base.py
 create mode 100644 support/testing/tests/init/systemd-factory/var/foo/bar
 create mode 100644 support/testing/tests/init/test_busybox.py
 create mode 100644 support/testing/tests/init/test_none.py
 create mode 100644 support/testing/tests/init/test_systemd.py
 rename system/skeleton/dev/{pts => }/.empty (100%)

-- 
2.13.3

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

* [Buildroot] [PATCH v5 01/14] support/testing: add runtime testing for init systems
  2017-08-01 22:52 [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Arnout Vandecappelle
@ 2017-08-01 22:52 ` Arnout Vandecappelle
  2017-08-02 15:48   ` Thomas Petazzoni
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 02/14] package/skeleton: split out into skeleton-custom Arnout Vandecappelle
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 22+ messages in thread
From: Arnout Vandecappelle @ 2017-08-01 22:52 UTC (permalink / raw)
  To: buildroot

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

The "builtin" kernel does not boot a systemd-based system, so
we resort to building the same one as currently used by our
qemu_arm_vexpress_defconfig.

We test the 8 following combinations:

  - busybox, read-only, without network
  - busybox, read-only, with network
  - busybox, read-write, without network
  - busybox, read-write, with network

  - basic systemd, read-write, network w/ ifupdown
  - basic systemd, read-write, network w/ networkd
  - full systemd, read-write, network w/ networkd

  - no init system, read-only, without network

The tests just verify what the /sbin/init binary is, and that we were
able to grab an IP address. More tests can be added later, for example
to check each systemd features (journal, tmpfiles...)

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
[Arnout: update .gitlab-ci.yml]
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Changes v4 -> v5: (Arnout)
 - re-ordered
 - update .gitlab-ci.yml

Changes v3 -> v4:
  - drop the systemd read-only tests for now, we do not support it yet.
---
 .gitlab-ci.yml                             |  8 +++
 support/testing/tests/init/__init__.py     |  0
 support/testing/tests/init/base.py         | 47 +++++++++++++++++
 support/testing/tests/init/test_busybox.py | 67 ++++++++++++++++++++++++
 support/testing/tests/init/test_none.py    | 32 ++++++++++++
 support/testing/tests/init/test_systemd.py | 82 ++++++++++++++++++++++++++++++
 6 files changed, 236 insertions(+)
 create mode 100644 support/testing/tests/init/__init__.py
 create mode 100644 support/testing/tests/init/base.py
 create mode 100644 support/testing/tests/init/test_busybox.py
 create mode 100644 support/testing/tests/init/test_none.py
 create mode 100644 support/testing/tests/init/test_systemd.py

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index f9e5b1fa6b..cdae953bf9 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -229,6 +229,14 @@ tests.fs.test_jffs2.TestJffs2: *runtime_test
 tests.fs.test_squashfs.TestSquashfs: *runtime_test
 tests.fs.test_ubi.TestUbi: *runtime_test
 tests.fs.test_yaffs2.TestYaffs2: *runtime_test
+tests.init.test_busybox.TestInitSystemBusyboxRo: *runtime_test
+tests.init.test_busybox.TestInitSystemBusyboxRoNet: *runtime_test
+tests.init.test_busybox.TestInitSystemBusyboxRw: *runtime_test
+tests.init.test_busybox.TestInitSystemBusyboxRwNet: *runtime_test
+tests.init.test_none.TestInitSystemNone: *runtime_test
+tests.init.test_systemd.TestInitSystemSystemdRwFull: *runtime_test
+tests.init.test_systemd.TestInitSystemSystemdRwIfupdown: *runtime_test
+tests.init.test_systemd.TestInitSystemSystemdRwNetworkd: *runtime_test
 tests.package.test_dropbear.TestDropbear: *runtime_test
 tests.package.test_ipython.TestIPythonPy2: *runtime_test
 tests.package.test_ipython.TestIPythonPy3: *runtime_test
diff --git a/support/testing/tests/init/__init__.py b/support/testing/tests/init/__init__.py
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/support/testing/tests/init/base.py b/support/testing/tests/init/base.py
new file mode 100644
index 0000000000..a261d7dd46
--- /dev/null
+++ b/support/testing/tests/init/base.py
@@ -0,0 +1,47 @@
+import os
+import subprocess
+import infra.basetest
+
+class InitSystemBase(infra.basetest.BRTest):
+
+    def startEmulator(self, fs_type, kernel=None, dtb=None, init=None):
+        img = os.path.join(self.builddir, "images", "rootfs.{}".format(fs_type))
+        subprocess.call(["truncate", "-s", "%1M", img])
+
+        options = ["-drive",
+                   "file={},if=sd,format=raw".format(img),
+                   "-M", "vexpress-a9"]
+
+        if kernel is None:
+            kernel = "builtin"
+        else:
+            kernel = os.path.join(self.builddir, "images", kernel)
+            options.extend(["-dtb", os.path.join(self.builddir, "images",
+                                             "{}.dtb".format(dtb))])
+
+        kernel_cmdline = ["root=/dev/mmcblk0",
+                          "rootfstype={}".format(fs_type),
+                          "rootwait",
+                          "ro",
+                          "console=ttyAMA0"]
+
+        if not init is None:
+            kernel_cmdline.extend(["init={}".format(init)])
+
+        self.emulator.boot(arch="armv7",
+                           kernel=kernel,
+                           kernel_cmdline=kernel_cmdline,
+                           options=options)
+
+        if init is None:
+            self.emulator.login()
+
+    def checkInit(self, path):
+        cmd = "cmp /proc/1/exe {}".format(path)
+        _, exit_code = self.emulator.run(cmd)
+        self.assertEqual(exit_code, 0)
+
+    def checkNetwork(self, interface, exitCode=0):
+        cmd = "ip addr show {} |grep inet".format(interface)
+        _, exit_code = self.emulator.run(cmd)
+        self.assertEqual(exit_code, exitCode)
diff --git a/support/testing/tests/init/test_busybox.py b/support/testing/tests/init/test_busybox.py
new file mode 100644
index 0000000000..c3e425bf5d
--- /dev/null
+++ b/support/testing/tests/init/test_busybox.py
@@ -0,0 +1,67 @@
+import infra.basetest
+from tests.init.base import InitSystemBase as InitSystemBase
+
+class InitSystemBusyboxBase(InitSystemBase):
+    config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
+        """
+        # BR2_TARGET_ROOTFS_TAR is not set
+        """
+
+    def checkInit(self):
+        super(InitSystemBusyboxBase, self).checkInit("/bin/busybox")
+
+
+#-------------------------------------------------------------------------------
+class TestInitSystemBusyboxRo(InitSystemBusyboxBase):
+    config = InitSystemBusyboxBase.config + \
+        """
+        # BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is not set
+        BR2_TARGET_ROOTFS_SQUASHFS=y
+        """
+
+    def test_run(self):
+        self.startEmulator("squashfs")
+        self.checkInit()
+        self.checkNetwork("eth0", 1)
+
+
+#-------------------------------------------------------------------------------
+class TestInitSystemBusyboxRw(InitSystemBusyboxBase):
+    config = InitSystemBusyboxBase.config + \
+        """
+        BR2_TARGET_ROOTFS_EXT2=y
+        """
+
+    def test_run(self):
+        self.startEmulator("ext2")
+        self.checkInit()
+        self.checkNetwork("eth0", 1)
+
+
+#-------------------------------------------------------------------------------
+class TestInitSystemBusyboxRoNet(InitSystemBusyboxBase):
+    config = InitSystemBusyboxBase.config + \
+        """
+        BR2_SYSTEM_DHCP="eth0"
+        # BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is not set
+        BR2_TARGET_ROOTFS_SQUASHFS=y
+        """
+
+    def test_run(self):
+        self.startEmulator("squashfs")
+        self.checkInit()
+        self.checkNetwork("eth0")
+
+
+#-------------------------------------------------------------------------------
+class TestInitSystemBusyboxRwNet(InitSystemBusyboxBase):
+    config = InitSystemBusyboxBase.config + \
+        """
+        BR2_SYSTEM_DHCP="eth0"
+        BR2_TARGET_ROOTFS_EXT2=y
+        """
+
+    def test_run(self):
+        self.startEmulator("ext2")
+        self.checkInit()
+        self.checkNetwork("eth0")
diff --git a/support/testing/tests/init/test_none.py b/support/testing/tests/init/test_none.py
new file mode 100644
index 0000000000..f55db5d3e0
--- /dev/null
+++ b/support/testing/tests/init/test_none.py
@@ -0,0 +1,32 @@
+import pexpect
+
+import infra.basetest
+from tests.init.base import InitSystemBase as InitSystemBase
+
+class TestInitSystemNone(InitSystemBase):
+    config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
+        """
+        BR2_INIT_NONE=y
+        # BR2_TARGET_ROOTFS_TAR is not set
+        BR2_TARGET_ROOTFS_SQUASHFS=y
+        """
+
+    def test_run(self):
+        self.startEmulator(fs_type="squashfs", init="/bin/sh")
+        index = self.emulator.qemu.expect(["/bin/sh: can't access tty; job control turned off", pexpect.TIMEOUT], timeout=60)
+        if index != 0:
+            self.emulator.logfile.write("==> System does not boot")
+            raise SystemError("System does not boot")
+        index = self.emulator.qemu.expect(["#", pexpect.TIMEOUT], timeout=60)
+        if index != 0:
+            self.emulator.logfile.write("==> System does not boot")
+            raise SystemError("System does not boot")
+
+        out, exit_code = self.emulator.run("sh -c 'echo $PPID'")
+        self.assertEqual(exit_code, 0)
+        self.assertEqual(out[0], "1")
+
+        _, exit_code = self.emulator.run("mount -t proc none /proc")
+        self.assertEqual(exit_code, 0)
+
+        self.checkInit("/bin/sh")
diff --git a/support/testing/tests/init/test_systemd.py b/support/testing/tests/init/test_systemd.py
new file mode 100644
index 0000000000..7a80aa1145
--- /dev/null
+++ b/support/testing/tests/init/test_systemd.py
@@ -0,0 +1,82 @@
+import infra.basetest
+from tests.init.base import InitSystemBase as InitSystemBase
+
+class InitSystemSystemdBase(InitSystemBase):
+    config = \
+        """
+        BR2_arm=y
+        BR2_TOOLCHAIN_EXTERNAL=y
+        BR2_INIT_SYSTEMD=y
+        BR2_TARGET_GENERIC_GETTY_PORT="ttyAMA0"
+        BR2_LINUX_KERNEL=y
+        BR2_LINUX_KERNEL_CUSTOM_VERSION=y
+        BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="4.11.3"
+        BR2_LINUX_KERNEL_DEFCONFIG="vexpress"
+        BR2_LINUX_KERNEL_DTS_SUPPORT=y
+        BR2_LINUX_KERNEL_INTREE_DTS_NAME="vexpress-v2p-ca9"
+        # BR2_TARGET_ROOTFS_TAR is not set
+        """
+
+    def checkInit(self):
+        super(InitSystemSystemdBase, self).checkInit("/lib/systemd/systemd")
+
+
+#-------------------------------------------------------------------------------
+class TestInitSystemSystemdRwNetworkd(InitSystemSystemdBase):
+    config = InitSystemSystemdBase.config + \
+        """
+        BR2_SYSTEM_DHCP="eth0"
+        BR2_TARGET_ROOTFS_EXT2=y
+        """
+
+    def test_run(self):
+        self.startEmulator("ext2", "zImage", "vexpress-v2p-ca9")
+        self.checkInit()
+        self.checkNetwork("eth0")
+
+
+#-------------------------------------------------------------------------------
+class TestInitSystemSystemdRwIfupdown(InitSystemSystemdBase):
+    config = InitSystemSystemdBase.config + \
+        """
+        BR2_SYSTEM_DHCP="eth0"
+        # BR2_PACKAGE_SYSTEMD_NETWORKD is not set
+        # BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is not set
+        BR2_TARGET_ROOTFS_EXT2=y
+        """
+
+    def test_run(self):
+        self.startEmulator("ext2", "zImage", "vexpress-v2p-ca9")
+        self.checkInit()
+        self.checkNetwork("eth0")
+
+
+#-------------------------------------------------------------------------------
+class TestInitSystemSystemdRwFull(InitSystemSystemdBase):
+    config = InitSystemSystemdBase.config + \
+        """
+        BR2_SYSTEM_DHCP="eth0"
+        BR2_PACKAGE_SYSTEMD_JOURNAL_GATEWAY=y
+        BR2_PACKAGE_SYSTEMD_BACKLIGHT=y
+        BR2_PACKAGE_SYSTEMD_BINFMT=y
+        BR2_PACKAGE_SYSTEMD_COREDUMP=y
+        BR2_PACKAGE_SYSTEMD_FIRSTBOOT=y
+        BR2_PACKAGE_SYSTEMD_HIBERNATE=y
+        BR2_PACKAGE_SYSTEMD_IMPORTD=y
+        BR2_PACKAGE_SYSTEMD_LOCALED=y
+        BR2_PACKAGE_SYSTEMD_LOGIND=y
+        BR2_PACKAGE_SYSTEMD_MACHINED=y
+        BR2_PACKAGE_SYSTEMD_POLKIT=y
+        BR2_PACKAGE_SYSTEMD_QUOTACHECK=y
+        BR2_PACKAGE_SYSTEMD_RANDOMSEED=y
+        BR2_PACKAGE_SYSTEMD_RFKILL=y
+        BR2_PACKAGE_SYSTEMD_SMACK_SUPPORT=y
+        BR2_PACKAGE_SYSTEMD_SYSUSERS=y
+        BR2_PACKAGE_SYSTEMD_VCONSOLE=y
+        BR2_TARGET_ROOTFS_EXT2=y
+        """
+
+    def test_run(self):
+        self.startEmulator("ext2", "zImage", "vexpress-v2p-ca9")
+        self.checkInit()
+        self.checkNetwork("eth0")
-- 
2.13.3

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

* [Buildroot] [PATCH v5 02/14] package/skeleton: split out into skeleton-custom
  2017-08-01 22:52 [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Arnout Vandecappelle
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 01/14] support/testing: add runtime testing for init systems Arnout Vandecappelle
@ 2017-08-01 22:52 ` Arnout Vandecappelle
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 03/14] package/skeleton-custom: rework the merged_usr and building conditions Arnout Vandecappelle
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Arnout Vandecappelle @ 2017-08-01 22:52 UTC (permalink / raw)
  To: buildroot

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

For the custom skeleton, we practicaly do nothing, except ensure it
contains the basic, required directories, and that those are properly
setup wrt. merged /usr.

Furthermore, our current skeleton is not fit for systemd, and we'll
have to split things out into various skeletons.

So, off-load the custom skeleton into its own package.

Thus, the existing skeleton package is now limited to:

  - when using our default skeleton, install and tweak it properly;

  - when using a custom skeleton, do nothing except for depending on
    the skeleton-custom package.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
[Arnout: split off in a separate patch doing only this]
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Note: this calls for making skeleton a virtual package. This will come
in follwup patches, after skeleton has lost all its code to the various
skeleton packages.

---
Changes v4 -> v5: (Arnout)
 - separate patch for the split into skeleton-customer; the rest of
   the modifications comes in follow-up patches
 - fixed a use of SKELETON_PATH instead of SKELETON_CUSTOM_PATH
 - don't remove the "The skeleton can't depend on the toolchain" comment

Changes v3 -> v4:
  - lib{32.64} are also tweaked in the skeleton  (Arnout)
---
 package/Config.in                          |  1 +
 package/skeleton-custom/Config.in          |  3 ++
 package/skeleton-custom/skeleton-custom.mk | 81 ++++++++++++++++++++++++++++++
 package/skeleton/skeleton.mk               | 51 +------------------
 system/Config.in                           |  2 +-
 5 files changed, 88 insertions(+), 50 deletions(-)
 create mode 100644 package/skeleton-custom/Config.in
 create mode 100644 package/skeleton-custom/skeleton-custom.mk

diff --git a/package/Config.in b/package/Config.in
index 86d714dae4..f409f5a363 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -2,6 +2,7 @@ menu "Target packages"
 
 	source "package/busybox/Config.in"
 	source "package/skeleton/Config.in"
+	source "package/skeleton-custom/Config.in"
 
 menu "Audio and video applications"
 	source "package/alsa-utils/Config.in"
diff --git a/package/skeleton-custom/Config.in b/package/skeleton-custom/Config.in
new file mode 100644
index 0000000000..b12bd8f73c
--- /dev/null
+++ b/package/skeleton-custom/Config.in
@@ -0,0 +1,3 @@
+config BR2_PACKAGE_SKELETON_CUSTOM
+	bool
+	select BR2_PACKAGE_SKELETON
diff --git a/package/skeleton-custom/skeleton-custom.mk b/package/skeleton-custom/skeleton-custom.mk
new file mode 100644
index 0000000000..4aebe3f3b1
--- /dev/null
+++ b/package/skeleton-custom/skeleton-custom.mk
@@ -0,0 +1,81 @@
+################################################################################
+#
+# skeleton-custom
+#
+################################################################################
+
+# The skeleton can't depend on the toolchain, since all packages depends on the
+# skeleton and the toolchain is a target package, as is skeleton.
+# Hence, skeleton would depends on the toolchain and the toolchain would depend
+# on skeleton.
+SKELETON_CUSTOM_ADD_TOOLCHAIN_DEPENDENCY = NO
+SKELETON_CUSTOM_ADD_SKELETON_DEPENDENCY = NO
+
+# The skeleton also handles the merged /usr case in the sysroot
+SKELETON_CUSTOM_INSTALL_STAGING = YES
+
+SKELETON_CUSTOM_PATH = $(call qstrip,$(BR2_ROOTFS_SKELETON_CUSTOM_PATH))
+
+ifeq ($(BR2_PACKAGE_SKELETON_CUSTOM)$(BR_BUILDING),yy)
+ifeq ($(SKELETON_CUSTOM_PATH),)
+$(error No path specified for the custom skeleton)
+endif
+
+ifeq ($(BR2_ROOTFS_MERGED_USR),y)
+
+# Ensure the user has prepared a merged /usr.
+#
+# Extract the inode numbers for all of those directories. In case any is
+# a symlink, we want to get the inode of the pointed-to directory, so we
+# append '/.' to be sure we get the target directory. Since the symlinks
+# can be anyway (/bin -> /usr/bin or /usr/bin -> /bin), we do that for
+# all of them.
+#
+SKELETON_CUSTOM_LIB_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/lib/.)
+SKELETON_CUSTOM_BIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/bin/.)
+SKELETON_CUSTOM_SBIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/sbin/.)
+SKELETON_CUSTOM_USR_LIB_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/usr/lib/.)
+SKELETON_CUSTOM_USR_BIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/usr/bin/.)
+SKELETON_CUSTOM_USR_SBIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/usr/sbin/.)
+
+ifneq ($(SKELETON_CUSTOM_LIB_INODE),$(SKELETON_CUSTOM_USR_LIB_INODE))
+SKELETON_CUSTOM_NOT_MERGED_USR += /lib
+endif
+ifneq ($(SKELETON_CUSTOM_BIN_INODE),$(SKELETON_CUSTOM_USR_BIN_INODE))
+SKELETON_CUSTOM_NOT_MERGED_USR += /bin
+endif
+ifneq ($(SKELETON_CUSTOM_SBIN_INODE),$(SKELETON_CUSTOM_USR_SBIN_INODE))
+SKELETON_CUSTOM_NOT_MERGED_USR += /sbin
+endif
+
+ifneq ($(SKELETON_CUSTOM_NOT_MERGED_USR),)
+$(error The custom skeleton in $(SKELETON_CUSTOM_PATH) is not \
+	using a merged /usr for the following directories: \
+	$(SKELETON_CUSTOM_NOT_MERGED_USR))
+endif
+
+endif # merged /usr
+endif # ! building
+
+define SKELETON_CUSTOM_INSTALL_TARGET_CMDS
+	$(call SYSTEM_RSYNC,$(SKELETON_CUSTOM_PATH),$(TARGET_DIR))
+	$(call SYSTEM_USR_SYMLINKS_OR_DIRS,$(TARGET_DIR))
+	$(call SYSTEM_LIB_SYMLINK,$(TARGET_DIR))
+	$(INSTALL) -m 0644 support/misc/target-dir-warning.txt \
+		$(TARGET_DIR_WARNING_FILE)
+endef
+
+# For the staging dir, we don't really care about /bin and /sbin.
+# But for consistency with the target dir, and to simplify the code,
+# we still handle them for the merged or non-merged /usr cases.
+# Since the toolchain is not yet available, the staging is not yet
+# populated, so we need to create the directories in /usr
+define SKELETON_CUSTOM_INSTALL_STAGING_CMDS
+	$(INSTALL) -d -m 0755 $(STAGING_DIR)/usr/lib
+	$(INSTALL) -d -m 0755 $(STAGING_DIR)/usr/bin
+	$(INSTALL) -d -m 0755 $(STAGING_DIR)/usr/sbin
+	$(call SYSTEM_USR_SYMLINKS_OR_DIRS,$(STAGING_DIR))
+	$(call SYSTEM_LIB_SYMLINK,$(STAGING_DIR))
+endef
+
+$(eval $(generic-package))
diff --git a/package/skeleton/skeleton.mk b/package/skeleton/skeleton.mk
index 41da1778a6..f44ecf48f6 100644
--- a/package/skeleton/skeleton.mk
+++ b/package/skeleton/skeleton.mk
@@ -14,57 +14,14 @@ SKELETON_ADD_SKELETON_DEPENDENCY = NO
 # The skeleton also handles the merged /usr case in the sysroot
 SKELETON_INSTALL_STAGING = YES
 
-ifeq ($(BR2_ROOTFS_SKELETON_CUSTOM),y)
+ifeq ($(BR2_PACKAGE_SKELETON_CUSTOM),y)
 
-SKELETON_PATH = $(call qstrip,$(BR2_ROOTFS_SKELETON_CUSTOM_PATH))
-
-ifeq ($(BR_BUILDING),y)
-ifeq ($(SKELETON_PATH),)
-$(error No path specified for the custom skeleton)
-endif
-endif
-
-ifeq ($(BR2_ROOTFS_MERGED_USR),y)
-
-# Ensure the user has prepared a merged /usr.
-#
-# Extract the inode numbers for all of those directories. In case any is
-# a symlink, we want to get the inode of the pointed-to directory, so we
-# append '/.' to be sure we get the target directory. Since the symlinks
-# can be anyway (/bin -> /usr/bin or /usr/bin -> /bin), we do that for
-# all of them.
-#
-SKELETON_LIB_INODE = $(shell stat -c '%i' $(SKELETON_PATH)/lib/.)
-SKELETON_BIN_INODE = $(shell stat -c '%i' $(SKELETON_PATH)/bin/.)
-SKELETON_SBIN_INODE = $(shell stat -c '%i' $(SKELETON_PATH)/sbin/.)
-SKELETON_USR_LIB_INODE = $(shell stat -c '%i' $(SKELETON_PATH)/usr/lib/.)
-SKELETON_USR_BIN_INODE = $(shell stat -c '%i' $(SKELETON_PATH)/usr/bin/.)
-SKELETON_USR_SBIN_INODE = $(shell stat -c '%i' $(SKELETON_PATH)/usr/sbin/.)
-
-ifneq ($(SKELETON_LIB_INODE),$(SKELETON_USR_LIB_INODE))
-SKELETON_CUSTOM_NOT_MERGED_USR += /lib
-endif
-ifneq ($(SKELETON_BIN_INODE),$(SKELETON_USR_BIN_INODE))
-SKELETON_CUSTOM_NOT_MERGED_USR += /bin
-endif
-ifneq ($(SKELETON_SBIN_INODE),$(SKELETON_USR_SBIN_INODE))
-SKELETON_CUSTOM_NOT_MERGED_USR += /sbin
-endif
-
-ifneq ($(SKELETON_CUSTOM_NOT_MERGED_USR),)
-$(error The custom skeleton in $(SKELETON_PATH) is not \
-	using a merged /usr for the following directories: \
-	$(SKELETON_CUSTOM_NOT_MERGED_USR))
-endif
-
-endif # merged /usr
+SKELETON_DEPENDENCIES = skeleton-custom
 
 else # ! custom skeleton
 
 SKELETON_PATH = system/skeleton
 
-endif # ! custom skeleton
-
 define SKELETON_INSTALL_TARGET_CMDS
 	$(call SYSTEM_RSYNC,$(SKELETON_PATH),$(TARGET_DIR))
 	$(call SYSTEM_USR_SYMLINKS_OR_DIRS,$(TARGET_DIR))
@@ -86,10 +43,6 @@ define SKELETON_INSTALL_STAGING_CMDS
 	$(call SYSTEM_LIB_SYMLINK,$(STAGING_DIR))
 endef
 
-# The TARGET_FINALIZE_HOOKS must be sourced only if the users choose to use the
-# default skeleton.
-ifeq ($(BR2_ROOTFS_SKELETON_DEFAULT),y)
-
 SKELETON_TARGET_GENERIC_HOSTNAME = $(call qstrip,$(BR2_TARGET_GENERIC_HOSTNAME))
 SKELETON_TARGET_GENERIC_ISSUE = $(call qstrip,$(BR2_TARGET_GENERIC_ISSUE))
 SKELETON_TARGET_GENERIC_ROOT_PASSWD = $(call qstrip,$(BR2_TARGET_GENERIC_ROOT_PASSWD))
diff --git a/system/Config.in b/system/Config.in
index b2f5be376e..014aedfb42 100644
--- a/system/Config.in
+++ b/system/Config.in
@@ -11,7 +11,7 @@ config BR2_ROOTFS_SKELETON_DEFAULT
 
 config BR2_ROOTFS_SKELETON_CUSTOM
 	bool "custom target skeleton"
-	select BR2_PACKAGE_SKELETON
+	select BR2_PACKAGE_SKELETON_CUSTOM
 	help
 	  Use custom target skeleton.
 
-- 
2.13.3

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

* [Buildroot] [PATCH v5 03/14] package/skeleton-custom: rework the merged_usr and building conditions
  2017-08-01 22:52 [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Arnout Vandecappelle
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 01/14] support/testing: add runtime testing for init systems Arnout Vandecappelle
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 02/14] package/skeleton: split out into skeleton-custom Arnout Vandecappelle
@ 2017-08-01 22:52 ` Arnout Vandecappelle
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 04/14] package/skeleton-custom: also check for missing directories Arnout Vandecappelle
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Arnout Vandecappelle @ 2017-08-01 22:52 UTC (permalink / raw)
  To: buildroot

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

skeleton-custom.mk contains a huge condition for BR_BUILDING and for
BR2_ROOTFS_MERGED_USR. This makes the code difficult to follow, so
rework the condition a bit so that only the parts that really need to
be conditional are indeed conditional.

Note that there is no overhead in needlessly evaluation the "stat"
commands. Indeed, the assignments use late evaluation so the "stat"
is only executed when evaluating the condition - when skeleton-custom
is not selected, stat is never called.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
[Arnout: split off in a separate patch + wrote commit message]
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Changes v4 -> v5: (Arnout)
 - New patch (split off from the previous one)
---
 package/skeleton-custom/skeleton-custom.mk | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/package/skeleton-custom/skeleton-custom.mk b/package/skeleton-custom/skeleton-custom.mk
index 4aebe3f3b1..b514f8017b 100644
--- a/package/skeleton-custom/skeleton-custom.mk
+++ b/package/skeleton-custom/skeleton-custom.mk
@@ -20,11 +20,8 @@ ifeq ($(BR2_PACKAGE_SKELETON_CUSTOM)$(BR_BUILDING),yy)
 ifeq ($(SKELETON_CUSTOM_PATH),)
 $(error No path specified for the custom skeleton)
 endif
+endif
 
-ifeq ($(BR2_ROOTFS_MERGED_USR),y)
-
-# Ensure the user has prepared a merged /usr.
-#
 # Extract the inode numbers for all of those directories. In case any is
 # a symlink, we want to get the inode of the pointed-to directory, so we
 # append '/.' to be sure we get the target directory. Since the symlinks
@@ -38,6 +35,10 @@ SKELETON_CUSTOM_USR_LIB_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/usr
 SKELETON_CUSTOM_USR_BIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/usr/bin/.)
 SKELETON_CUSTOM_USR_SBIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/usr/sbin/.)
 
+# For a merged /usr, ensure that /lib, /bin and /sbin and their /usr
+# counterparts are appropriately setup as symlinks ones to the others.
+ifeq ($(BR2_ROOTFS_MERGED_USR),y)
+
 ifneq ($(SKELETON_CUSTOM_LIB_INODE),$(SKELETON_CUSTOM_USR_LIB_INODE))
 SKELETON_CUSTOM_NOT_MERGED_USR += /lib
 endif
@@ -48,14 +49,15 @@ ifneq ($(SKELETON_CUSTOM_SBIN_INODE),$(SKELETON_CUSTOM_USR_SBIN_INODE))
 SKELETON_CUSTOM_NOT_MERGED_USR += /sbin
 endif
 
+endif # merged /usr
+
+ifeq ($(BR2_PACKAGE_SKELETON_CUSTOM)$(BR_BUILDING),yy)
 ifneq ($(SKELETON_CUSTOM_NOT_MERGED_USR),)
 $(error The custom skeleton in $(SKELETON_CUSTOM_PATH) is not \
 	using a merged /usr for the following directories: \
 	$(SKELETON_CUSTOM_NOT_MERGED_USR))
 endif
-
-endif # merged /usr
-endif # ! building
+endif
 
 define SKELETON_CUSTOM_INSTALL_TARGET_CMDS
 	$(call SYSTEM_RSYNC,$(SKELETON_CUSTOM_PATH),$(TARGET_DIR))
-- 
2.13.3

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

* [Buildroot] [PATCH v5 04/14] package/skeleton-custom: also check for missing directories
  2017-08-01 22:52 [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Arnout Vandecappelle
                   ` (2 preceding siblings ...)
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 03/14] package/skeleton-custom: rework the merged_usr and building conditions Arnout Vandecappelle
@ 2017-08-01 22:52 ` Arnout Vandecappelle
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 05/14] package/skeleton-custom: simplify target/staging install Arnout Vandecappelle
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Arnout Vandecappelle @ 2017-08-01 22:52 UTC (permalink / raw)
  To: buildroot

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

In addition to checking the symlinks in the merged usr case, also check
whether /bin, /lib, /sbin and their /usr/... counterparts exist in the
non-merged case.

Missing directories are collected in the variable
SKELETON_CUSTOM_MISSING_DIRS. For symmetry and because it's more
logical, rename SKELETON_CUSTOM_NOT_MERGED_USR to
SKELETON_CUSTOM_NOT_MERGED_USR_DIRS.

When a directory is indeed missing, "stat" will print an error.
Buildroot *also* prints an error, which is clearer. So remove the error
from stat by redirecting it to /dev/null.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
[Arnout: split off in a separate patch + wrote commit message]
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Changes v4 -> v5: (Arnout)
 - New patch (split off from the previous one)
---
 package/skeleton-custom/skeleton-custom.mk | 48 +++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 11 deletions(-)

diff --git a/package/skeleton-custom/skeleton-custom.mk b/package/skeleton-custom/skeleton-custom.mk
index b514f8017b..df46b72c4c 100644
--- a/package/skeleton-custom/skeleton-custom.mk
+++ b/package/skeleton-custom/skeleton-custom.mk
@@ -28,34 +28,60 @@ endif
 # can be anyway (/bin -> /usr/bin or /usr/bin -> /bin), we do that for
 # all of them.
 #
-SKELETON_CUSTOM_LIB_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/lib/.)
-SKELETON_CUSTOM_BIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/bin/.)
-SKELETON_CUSTOM_SBIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/sbin/.)
-SKELETON_CUSTOM_USR_LIB_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/usr/lib/.)
-SKELETON_CUSTOM_USR_BIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/usr/bin/.)
-SKELETON_CUSTOM_USR_SBIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/usr/sbin/.)
+SKELETON_CUSTOM_LIB_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/lib/. 2>/dev/null)
+SKELETON_CUSTOM_BIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/bin/. 2>/dev/null)
+SKELETON_CUSTOM_SBIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/sbin/. 2>/dev/null)
+SKELETON_CUSTOM_USR_LIB_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/usr/lib/. 2>/dev/null)
+SKELETON_CUSTOM_USR_BIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/usr/bin/. 2>/dev/null)
+SKELETON_CUSTOM_USR_SBIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/usr/sbin/. 2>/dev/null)
+
+# Ensure that the custom skeleton has /lib, /bin and /sbin, and their
+# /usr counterparts
+ifeq ($(SKELETON_CUSTOM_LIB_INODE),)
+SKELETON_CUSTOM_MISSING_DIRS += /lib
+endif
+ifeq ($(SKELETON_CUSTOM_USR_LIB_INODE),)
+SKELETON_CUSTOM_MISSING_DIRS += /usr/lib
+endif
+ifeq ($(SKELETON_CUSTOM_BIN_INODE),)
+SKELETON_CUSTOM_MISSING_DIRS += /bin
+endif
+ifeq ($(SKELETON_CUSTOM_USR_BIN_INODE),)
+SKELETON_CUSTOM_MISSING_DIRS += /usr/bin
+endif
+ifeq ($(SKELETON_CUSTOM_SBIN_INODE),)
+SKELETON_CUSTOM_MISSING_DIRS += /sbin
+endif
+ifeq ($(SKELETON_CUSTOM_USR_SBIN_INODE),)
+SKELETON_CUSTOM_MISSING_DIRS += /usr/sbin
+endif
 
 # For a merged /usr, ensure that /lib, /bin and /sbin and their /usr
 # counterparts are appropriately setup as symlinks ones to the others.
 ifeq ($(BR2_ROOTFS_MERGED_USR),y)
 
 ifneq ($(SKELETON_CUSTOM_LIB_INODE),$(SKELETON_CUSTOM_USR_LIB_INODE))
-SKELETON_CUSTOM_NOT_MERGED_USR += /lib
+SKELETON_CUSTOM_NOT_MERGED_USR_DIRS += /lib
 endif
 ifneq ($(SKELETON_CUSTOM_BIN_INODE),$(SKELETON_CUSTOM_USR_BIN_INODE))
-SKELETON_CUSTOM_NOT_MERGED_USR += /bin
+SKELETON_CUSTOM_NOT_MERGED_USR_DIRS += /bin
 endif
 ifneq ($(SKELETON_CUSTOM_SBIN_INODE),$(SKELETON_CUSTOM_USR_SBIN_INODE))
-SKELETON_CUSTOM_NOT_MERGED_USR += /sbin
+SKELETON_CUSTOM_NOT_MERGED_USR_DIRS += /sbin
 endif
 
 endif # merged /usr
 
 ifeq ($(BR2_PACKAGE_SKELETON_CUSTOM)$(BR_BUILDING),yy)
-ifneq ($(SKELETON_CUSTOM_NOT_MERGED_USR),)
+ifneq ($(SKELETON_CUSTOM_MISSING_DIRS),)
+$(error The custom skeleton in $(SKELETON_CUSTOM_PATH) is \
+	missing those directories or symlinks: \
+	$(SKELETON_CUSTOM_MISSING_DIRS))
+endif
+ifneq ($(SKELETON_CUSTOM_NOT_MERGED_USR_DIRS),)
 $(error The custom skeleton in $(SKELETON_CUSTOM_PATH) is not \
 	using a merged /usr for the following directories: \
-	$(SKELETON_CUSTOM_NOT_MERGED_USR))
+	$(SKELETON_CUSTOM_NOT_MERGED_USR_DIRS))
 endif
 endif
 
-- 
2.13.3

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

* [Buildroot] [PATCH v5 05/14] package/skeleton-custom: simplify target/staging install
  2017-08-01 22:52 [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Arnout Vandecappelle
                   ` (3 preceding siblings ...)
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 04/14] package/skeleton-custom: also check for missing directories Arnout Vandecappelle
@ 2017-08-01 22:52 ` Arnout Vandecappelle
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 06/14] package/skeleton: split out into skeleton-common Arnout Vandecappelle
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Arnout Vandecappelle @ 2017-08-01 22:52 UTC (permalink / raw)
  To: buildroot

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

For target install, we don't need to check the merged-usr symlinks
any more, it was already checked before.

For staging, instead of creating directories, just copy the same
skeleton, which was already checked to be correct.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
[Arnout: split off in a separate patch + wrote commit message]
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Changes v4 -> v5: (Arnout)
 - New patch (split off from the previous one)
---
 package/skeleton-custom/skeleton-custom.mk | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/package/skeleton-custom/skeleton-custom.mk b/package/skeleton-custom/skeleton-custom.mk
index df46b72c4c..6e9d52620b 100644
--- a/package/skeleton-custom/skeleton-custom.mk
+++ b/package/skeleton-custom/skeleton-custom.mk
@@ -85,24 +85,21 @@ $(error The custom skeleton in $(SKELETON_CUSTOM_PATH) is not \
 endif
 endif
 
+# The target-dir-warning file and the lib{32,64} symlinks are the only
+# things we customise in the custom skeleton.
 define SKELETON_CUSTOM_INSTALL_TARGET_CMDS
 	$(call SYSTEM_RSYNC,$(SKELETON_CUSTOM_PATH),$(TARGET_DIR))
-	$(call SYSTEM_USR_SYMLINKS_OR_DIRS,$(TARGET_DIR))
 	$(call SYSTEM_LIB_SYMLINK,$(TARGET_DIR))
 	$(INSTALL) -m 0644 support/misc/target-dir-warning.txt \
 		$(TARGET_DIR_WARNING_FILE)
 endef
 
-# For the staging dir, we don't really care about /bin and /sbin.
-# But for consistency with the target dir, and to simplify the code,
-# we still handle them for the merged or non-merged /usr cases.
-# Since the toolchain is not yet available, the staging is not yet
-# populated, so we need to create the directories in /usr
+# For the staging dir, we don't really care what we install, but we
+# need the /lib and /usr/lib appropriately setup. Since we ensure,
+# above, that they are correct in the skeleton, we can simply copy the
+# skeleton to staging.
 define SKELETON_CUSTOM_INSTALL_STAGING_CMDS
-	$(INSTALL) -d -m 0755 $(STAGING_DIR)/usr/lib
-	$(INSTALL) -d -m 0755 $(STAGING_DIR)/usr/bin
-	$(INSTALL) -d -m 0755 $(STAGING_DIR)/usr/sbin
-	$(call SYSTEM_USR_SYMLINKS_OR_DIRS,$(STAGING_DIR))
+	$(call SYSTEM_RSYNC,$(SKELETON_CUSTOM_PATH),$(STAGING_DIR))
 	$(call SYSTEM_LIB_SYMLINK,$(STAGING_DIR))
 endef
 
-- 
2.13.3

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

* [Buildroot] [PATCH v5 06/14] package/skeleton: split out into skeleton-common
  2017-08-01 22:52 [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Arnout Vandecappelle
                   ` (4 preceding siblings ...)
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 05/14] package/skeleton-custom: simplify target/staging install Arnout Vandecappelle
@ 2017-08-01 22:52 ` Arnout Vandecappelle
  2017-08-02 17:24   ` Thomas Petazzoni
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 07/14] skeleton-common: rename SKELETON_ROOT_PASSWORD to SKELETON_CUSTOM_ROOT_PASSWORD Arnout Vandecappelle
                   ` (9 subsequent siblings)
  15 siblings, 1 reply; 22+ messages in thread
From: Arnout Vandecappelle @ 2017-08-01 22:52 UTC (permalink / raw)
  To: buildroot

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

Move all the handling of the default skeleton into a new package,
skeleton-common.

We don't name it skeleton-default, because it will be further split
later, into a skeleton for sysv and another for systemd, with some parts
still common between the two. So just name it skeleton-common right now;
this will save us a rename later.

We keep in the skeleton package all definitions of macros and variables
that can be used by other skeleton packages and/or init systems, and
just offload the actual feeding of the skeleton to the new package.

While we're at it, also assign to SKELETON_COMMON_TARGET_FINALIZE_HOOKS
instead of directly to the global FINALIZE_HOOKS. Therefore, we don't
need to do all of that in a condition BR2_PACKAGE_SKELETON_COMMON==y.

Note: it would be technically sound to move the skeleton files together
within a sub-directory of the skeleton-common package. However, we refer
the user to those files, from various locations (manual, packages). It
will indeed be easier for the user to find those files in
system/skeleton/ rather than in package/skeleton-common/skeleton/

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
[Arnout: remove the mkdir $(STAGING_DIR)/usr/include which was removed
         in skeleton.mk in master.]
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Changes v4 -> v5: (Arnout)
 - remove mkdir $(STAGING_DIR)/usr/include

Changes v3 -> v4:
  - nothing, except add a blurb why skeleton files were not moved
---
 package/Config.in                          |  1 +
 package/skeleton-common/Config.in          |  3 +
 package/skeleton-common/skeleton-common.mk | 95 ++++++++++++++++++++++++++++++
 package/skeleton/skeleton.mk               | 86 +--------------------------
 system/Config.in                           |  2 +-
 5 files changed, 101 insertions(+), 86 deletions(-)
 create mode 100644 package/skeleton-common/Config.in
 create mode 100644 package/skeleton-common/skeleton-common.mk

diff --git a/package/Config.in b/package/Config.in
index f409f5a363..7d22b47f5c 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -2,6 +2,7 @@ menu "Target packages"
 
 	source "package/busybox/Config.in"
 	source "package/skeleton/Config.in"
+	source "package/skeleton-common/Config.in"
 	source "package/skeleton-custom/Config.in"
 
 menu "Audio and video applications"
diff --git a/package/skeleton-common/Config.in b/package/skeleton-common/Config.in
new file mode 100644
index 0000000000..6c094f6466
--- /dev/null
+++ b/package/skeleton-common/Config.in
@@ -0,0 +1,3 @@
+config BR2_PACKAGE_SKELETON_COMMON
+	bool
+	select BR2_PACKAGE_SKELETON
diff --git a/package/skeleton-common/skeleton-common.mk b/package/skeleton-common/skeleton-common.mk
new file mode 100644
index 0000000000..07c434c07a
--- /dev/null
+++ b/package/skeleton-common/skeleton-common.mk
@@ -0,0 +1,95 @@
+################################################################################
+#
+# skeleton-common
+#
+################################################################################
+
+# The skeleton can't depend on the toolchain, since all packages depends on the
+# skeleton and the toolchain is a target package, as is skeleton.
+# Hence, skeleton would depends on the toolchain and the toolchain would depend
+# on skeleton.
+SKELETON_COMMON_ADD_TOOLCHAIN_DEPENDENCY = NO
+SKELETON_COMMON_ADD_SKELETON_DEPENDENCY = NO
+
+# The skeleton also handles the merged /usr case in the sysroot
+SKELETON_COMMON_INSTALL_STAGING = YES
+
+SKELETON_COMMON_PATH = system/skeleton
+
+define SKELETON_COMMON_INSTALL_TARGET_CMDS
+	$(call SYSTEM_RSYNC,$(SKELETON_COMMON_PATH),$(TARGET_DIR))
+	$(call SYSTEM_USR_SYMLINKS_OR_DIRS,$(TARGET_DIR))
+	$(call SYSTEM_LIB_SYMLINK,$(TARGET_DIR))
+	$(INSTALL) -m 0644 support/misc/target-dir-warning.txt \
+		$(TARGET_DIR_WARNING_FILE)
+endef
+
+# For the staging dir, we don't really care about /bin and /sbin.
+# But for consistency with the target dir, and to simplify the code,
+# we still handle them for the merged or non-merged /usr cases.
+# Since the toolchain is not yet available, the staging is not yet
+# populated, so we need to create the directories in /usr
+define SKELETON_COMMON_INSTALL_STAGING_CMDS
+	$(INSTALL) -d -m 0755 $(STAGING_DIR)/usr/lib
+	$(INSTALL) -d -m 0755 $(STAGING_DIR)/usr/bin
+	$(INSTALL) -d -m 0755 $(STAGING_DIR)/usr/sbin
+	$(call SYSTEM_USR_SYMLINKS_OR_DIRS,$(STAGING_DIR))
+	$(call SYSTEM_LIB_SYMLINK,$(STAGING_DIR))
+endef
+
+SKELETON_COMMON_HOSTNAME = $(call qstrip,$(BR2_TARGET_GENERIC_HOSTNAME))
+SKELETON_COMMON_ISSUE = $(call qstrip,$(BR2_TARGET_GENERIC_ISSUE))
+SKELETON_COMMON_ROOT_PASSWD = $(call qstrip,$(BR2_TARGET_GENERIC_ROOT_PASSWD))
+SKELETON_COMMON_PASSWD_METHOD = $(call qstrip,$(BR2_TARGET_GENERIC_PASSWD_METHOD))
+SKELETON_COMMON_BIN_SH = $(call qstrip,$(BR2_SYSTEM_BIN_SH))
+
+ifneq ($(SKELETON_COMMON_HOSTNAME),)
+define SKELETON_COMMON_SET_HOSTNAME
+	mkdir -p $(TARGET_DIR)/etc
+	echo "$(SKELETON_COMMON_HOSTNAME)" > $(TARGET_DIR)/etc/hostname
+	$(SED) '$$a \127.0.1.1\t$(SKELETON_COMMON_HOSTNAME)' \
+		-e '/^127.0.1.1/d' $(TARGET_DIR)/etc/hosts
+endef
+SKELETON_COMMON_TARGET_FINALIZE_HOOKS += SKELETON_COMMON_SET_HOSTNAME
+endif
+
+ifneq ($(SKELETON_COMMON_ISSUE),)
+define SKELETON_COMMON_SET_ISSUE
+	mkdir -p $(TARGET_DIR)/etc
+	echo "$(SKELETON_COMMON_ISSUE)" > $(TARGET_DIR)/etc/issue
+endef
+SKELETON_COMMON_TARGET_FINALIZE_HOOKS += SKELETON_COMMON_SET_ISSUE
+endif
+
+ifeq ($(BR2_TARGET_ENABLE_ROOT_LOGIN),y)
+ifeq ($(SKELETON_COMMON_ROOT_PASSWD),)
+SKELETON_ROOT_PASSWORD =
+else ifneq ($(filter $$1$$% $$5$$% $$6$$%,$(SKELETON_COMMON_ROOT_PASSWD)),)
+SKELETON_ROOT_PASSWORD = '$(SKELETON_COMMON_ROOT_PASSWD)'
+else
+# This variable will only be evaluated in the finalize stage, so we can
+# be sure that host-mkpasswd will have already been built by that time.
+SKELETON_ROOT_PASSWORD = "`$(MKPASSWD) -m "$(SKELETON_COMMON_PASSWD_METHOD)" "$(SKELETON_COMMON_ROOT_PASSWD)"`"
+endif
+else # !BR2_TARGET_ENABLE_ROOT_LOGIN
+SKELETON_ROOT_PASSWORD = "*"
+endif
+define SKELETON_COMMON_SET_ROOT_PASSWD
+	$(SED) s,^root:[^:]*:,root:$(SKELETON_ROOT_PASSWORD):, $(TARGET_DIR)/etc/shadow
+endef
+SKELETON_COMMON_TARGET_FINALIZE_HOOKS += SKELETON_COMMON_SET_ROOT_PASSWD
+
+ifeq ($(BR2_SYSTEM_BIN_SH_NONE),y)
+define SKELETON_COMMON_BIN_SH
+	rm -f $(TARGET_DIR)/bin/sh
+endef
+else
+ifneq ($(SKELETON_COMMON_BIN_SH),)
+define SKELETON_COMMON_BIN_SH
+	ln -sf $(SKELETON_COMMON_BIN_SH) $(TARGET_DIR)/bin/sh
+endef
+endif
+endif
+SKELETON_COMMON_TARGET_FINALIZE_HOOKS += SKELETON_COMMON_BIN_SH
+
+$(eval $(generic-package))
diff --git a/package/skeleton/skeleton.mk b/package/skeleton/skeleton.mk
index f44ecf48f6..1d3e4fbe3c 100644
--- a/package/skeleton/skeleton.mk
+++ b/package/skeleton/skeleton.mk
@@ -11,94 +11,10 @@
 SKELETON_ADD_TOOLCHAIN_DEPENDENCY = NO
 SKELETON_ADD_SKELETON_DEPENDENCY = NO
 
-# The skeleton also handles the merged /usr case in the sysroot
-SKELETON_INSTALL_STAGING = YES
-
 ifeq ($(BR2_PACKAGE_SKELETON_CUSTOM),y)
-
 SKELETON_DEPENDENCIES = skeleton-custom
-
-else # ! custom skeleton
-
-SKELETON_PATH = system/skeleton
-
-define SKELETON_INSTALL_TARGET_CMDS
-	$(call SYSTEM_RSYNC,$(SKELETON_PATH),$(TARGET_DIR))
-	$(call SYSTEM_USR_SYMLINKS_OR_DIRS,$(TARGET_DIR))
-	$(call SYSTEM_LIB_SYMLINK,$(TARGET_DIR))
-	$(INSTALL) -m 0644 support/misc/target-dir-warning.txt \
-		$(TARGET_DIR_WARNING_FILE)
-endef
-
-# For the staging dir, we don't really care about /bin and /sbin.
-# But for consistency with the target dir, and to simplify the code,
-# we still handle them for the merged or non-merged /usr cases.
-# Since the toolchain is not yet available, the staging is not yet
-# populated, so we need to create the directories in /usr
-define SKELETON_INSTALL_STAGING_CMDS
-	$(INSTALL) -d -m 0755 $(STAGING_DIR)/usr/lib
-	$(INSTALL) -d -m 0755 $(STAGING_DIR)/usr/bin
-	$(INSTALL) -d -m 0755 $(STAGING_DIR)/usr/sbin
-	$(call SYSTEM_USR_SYMLINKS_OR_DIRS,$(STAGING_DIR))
-	$(call SYSTEM_LIB_SYMLINK,$(STAGING_DIR))
-endef
-
-SKELETON_TARGET_GENERIC_HOSTNAME = $(call qstrip,$(BR2_TARGET_GENERIC_HOSTNAME))
-SKELETON_TARGET_GENERIC_ISSUE = $(call qstrip,$(BR2_TARGET_GENERIC_ISSUE))
-SKELETON_TARGET_GENERIC_ROOT_PASSWD = $(call qstrip,$(BR2_TARGET_GENERIC_ROOT_PASSWD))
-SKELETON_TARGET_GENERIC_PASSWD_METHOD = $(call qstrip,$(BR2_TARGET_GENERIC_PASSWD_METHOD))
-SKELETON_TARGET_GENERIC_BIN_SH = $(call qstrip,$(BR2_SYSTEM_BIN_SH))
-
-ifneq ($(SKELETON_TARGET_GENERIC_HOSTNAME),)
-define SKELETON_SET_HOSTNAME
-	mkdir -p $(TARGET_DIR)/etc
-	echo "$(SKELETON_TARGET_GENERIC_HOSTNAME)" > $(TARGET_DIR)/etc/hostname
-	$(SED) '$$a \127.0.1.1\t$(SKELETON_TARGET_GENERIC_HOSTNAME)' \
-		-e '/^127.0.1.1/d' $(TARGET_DIR)/etc/hosts
-endef
-TARGET_FINALIZE_HOOKS += SKELETON_SET_HOSTNAME
-endif
-
-ifneq ($(SKELETON_TARGET_GENERIC_ISSUE),)
-define SKELETON_SET_ISSUE
-	mkdir -p $(TARGET_DIR)/etc
-	echo "$(SKELETON_TARGET_GENERIC_ISSUE)" > $(TARGET_DIR)/etc/issue
-endef
-TARGET_FINALIZE_HOOKS += SKELETON_SET_ISSUE
-endif
-
-ifeq ($(BR2_TARGET_ENABLE_ROOT_LOGIN),y)
-ifeq ($(SKELETON_TARGET_GENERIC_ROOT_PASSWD),)
-SKELETON_ROOT_PASSWORD =
-else ifneq ($(filter $$1$$% $$5$$% $$6$$%,$(SKELETON_TARGET_GENERIC_ROOT_PASSWD)),)
-SKELETON_ROOT_PASSWORD = '$(SKELETON_TARGET_GENERIC_ROOT_PASSWD)'
 else
-# This variable will only be evaluated in the finalize stage, so we can
-# be sure that host-mkpasswd will have already been built by that time.
-SKELETON_ROOT_PASSWORD = "`$(MKPASSWD) -m "$(SKELETON_TARGET_GENERIC_PASSWD_METHOD)" "$(SKELETON_TARGET_GENERIC_ROOT_PASSWD)"`"
-endif
-else # !BR2_TARGET_ENABLE_ROOT_LOGIN
-SKELETON_ROOT_PASSWORD = "*"
+SKELETON_DEPENDENCIES = skeleton-common
 endif
 
-define SKELETON_SET_ROOT_PASSWD
-	$(SED) s,^root:[^:]*:,root:$(SKELETON_ROOT_PASSWORD):, $(TARGET_DIR)/etc/shadow
-endef
-TARGET_FINALIZE_HOOKS += SKELETON_SET_ROOT_PASSWD
-
-ifeq ($(BR2_SYSTEM_BIN_SH_NONE),y)
-define SKELETON_BIN_SH
-	rm -f $(TARGET_DIR)/bin/sh
-endef
-else
-ifneq ($(SKELETON_TARGET_GENERIC_BIN_SH),)
-define SKELETON_BIN_SH
-	ln -sf $(SKELETON_TARGET_GENERIC_BIN_SH) $(TARGET_DIR)/bin/sh
-endef
-endif
-endif
-TARGET_FINALIZE_HOOKS += SKELETON_BIN_SH
-
-endif # BR2_ROOTFS_SKELETON_DEFAULT
-
 $(eval $(generic-package))
diff --git a/system/Config.in b/system/Config.in
index 014aedfb42..448984e273 100644
--- a/system/Config.in
+++ b/system/Config.in
@@ -5,7 +5,7 @@ choice
 
 config BR2_ROOTFS_SKELETON_DEFAULT
 	bool "default target skeleton"
-	select BR2_PACKAGE_SKELETON
+	select BR2_PACKAGE_SKELETON_COMMON
 	help
 	  Use default target skeleton
 
-- 
2.13.3

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

* [Buildroot] [PATCH v5 07/14] skeleton-common: rename SKELETON_ROOT_PASSWORD to SKELETON_CUSTOM_ROOT_PASSWORD
  2017-08-01 22:52 [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Arnout Vandecappelle
                   ` (5 preceding siblings ...)
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 06/14] package/skeleton: split out into skeleton-common Arnout Vandecappelle
@ 2017-08-01 22:52 ` Arnout Vandecappelle
  2017-08-02 17:29   ` Thomas Petazzoni
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 08/14] package/skeleton: make it a virtual package Arnout Vandecappelle
                   ` (8 subsequent siblings)
  15 siblings, 1 reply; 22+ messages in thread
From: Arnout Vandecappelle @ 2017-08-01 22:52 UTC (permalink / raw)
  To: buildroot

Package variables should always be prefixed with the package name. For
SKELETON_CUSTOM_ROOT_PASSWORD, this means we set the variable a second
time based on the value it previously had. Fortunately, this is fine
for make, even for recursively expanded variables.

This also allows to simplify the condition of the empty password - it's
not needed to set it again to empty if it already was empty.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Changes v4 -> v5: (Arnout)
 - New patch
---
 package/skeleton-common/skeleton-common.mk | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/package/skeleton-common/skeleton-common.mk b/package/skeleton-common/skeleton-common.mk
index 07c434c07a..06e59c03df 100644
--- a/package/skeleton-common/skeleton-common.mk
+++ b/package/skeleton-common/skeleton-common.mk
@@ -62,20 +62,18 @@ SKELETON_COMMON_TARGET_FINALIZE_HOOKS += SKELETON_COMMON_SET_ISSUE
 endif
 
 ifeq ($(BR2_TARGET_ENABLE_ROOT_LOGIN),y)
-ifeq ($(SKELETON_COMMON_ROOT_PASSWD),)
-SKELETON_ROOT_PASSWORD =
-else ifneq ($(filter $$1$$% $$5$$% $$6$$%,$(SKELETON_COMMON_ROOT_PASSWD)),)
-SKELETON_ROOT_PASSWORD = '$(SKELETON_COMMON_ROOT_PASSWD)'
-else
+ifneq ($(filter $$1$$% $$5$$% $$6$$%,$(SKELETON_COMMON_ROOT_PASSWD)),)
+SKELETON_COMMON_ROOT_PASSWORD = '$(SKELETON_COMMON_ROOT_PASSWD)'
+else ifneq ($(SKELETON_COMMON_ROOT_PASSWD),)
 # This variable will only be evaluated in the finalize stage, so we can
 # be sure that host-mkpasswd will have already been built by that time.
-SKELETON_ROOT_PASSWORD = "`$(MKPASSWD) -m "$(SKELETON_COMMON_PASSWD_METHOD)" "$(SKELETON_COMMON_ROOT_PASSWD)"`"
+SKELETON_COMMON_ROOT_PASSWORD = "`$(MKPASSWD) -m "$(SKELETON_COMMON_PASSWD_METHOD)" "$(SKELETON_COMMON_ROOT_PASSWD)"`"
 endif
 else # !BR2_TARGET_ENABLE_ROOT_LOGIN
-SKELETON_ROOT_PASSWORD = "*"
+SKELETON_COMMON_ROOT_PASSWORD = "*"
 endif
 define SKELETON_COMMON_SET_ROOT_PASSWD
-	$(SED) s,^root:[^:]*:,root:$(SKELETON_ROOT_PASSWORD):, $(TARGET_DIR)/etc/shadow
+	$(SED) s,^root:[^:]*:,root:$(SKELETON_COMMON_ROOT_PASSWORD):, $(TARGET_DIR)/etc/shadow
 endef
 SKELETON_COMMON_TARGET_FINALIZE_HOOKS += SKELETON_COMMON_SET_ROOT_PASSWD
 
-- 
2.13.3

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

* [Buildroot] [PATCH v5 08/14] package/skeleton: make it a virtual package
  2017-08-01 22:52 [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Arnout Vandecappelle
                   ` (6 preceding siblings ...)
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 07/14] skeleton-common: rename SKELETON_ROOT_PASSWORD to SKELETON_CUSTOM_ROOT_PASSWORD Arnout Vandecappelle
@ 2017-08-01 22:52 ` Arnout Vandecappelle
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 09/14] package/skeleton-common: simplify staging install Arnout Vandecappelle
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Arnout Vandecappelle @ 2017-08-01 22:52 UTC (permalink / raw)
  To: buildroot

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

We now have two packages that can act as a skeleton, skeleton-common,
also known as our default skeleton, and skeleton-custom.

This means that the skeleton package can be a standard virtual package
now.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Changes v4 -> v5: (Arnout)
 - No changes

Changes v3 -> v4:
  - drop now-legacy paragraph from the commit log  (Arnout)
---
 package/skeleton-common/Config.in          | 5 ++++-
 package/skeleton-common/skeleton-common.mk | 2 ++
 package/skeleton-custom/Config.in          | 5 ++++-
 package/skeleton-custom/skeleton-custom.mk | 3 ++-
 package/skeleton/Config.in                 | 9 +++++++--
 package/skeleton/skeleton.mk               | 8 +-------
 system/Config.in                           | 4 ++++
 7 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/package/skeleton-common/Config.in b/package/skeleton-common/Config.in
index 6c094f6466..5675e873cf 100644
--- a/package/skeleton-common/Config.in
+++ b/package/skeleton-common/Config.in
@@ -1,3 +1,6 @@
 config BR2_PACKAGE_SKELETON_COMMON
 	bool
-	select BR2_PACKAGE_SKELETON
+	select BR2_PACKAGE_HAS_SKELETON
+
+config BR2_PACKAGE_PROVIDES_SKELETON
+	default "skeleton-common" if BR2_PACKAGE_SKELETON_COMMON
diff --git a/package/skeleton-common/skeleton-common.mk b/package/skeleton-common/skeleton-common.mk
index 06e59c03df..e3a52dc040 100644
--- a/package/skeleton-common/skeleton-common.mk
+++ b/package/skeleton-common/skeleton-common.mk
@@ -11,6 +11,8 @@
 SKELETON_COMMON_ADD_TOOLCHAIN_DEPENDENCY = NO
 SKELETON_COMMON_ADD_SKELETON_DEPENDENCY = NO
 
+SKELETON_COMMON_PROVIDES = skeleton
+
 # The skeleton also handles the merged /usr case in the sysroot
 SKELETON_COMMON_INSTALL_STAGING = YES
 
diff --git a/package/skeleton-custom/Config.in b/package/skeleton-custom/Config.in
index b12bd8f73c..601c3b247e 100644
--- a/package/skeleton-custom/Config.in
+++ b/package/skeleton-custom/Config.in
@@ -1,3 +1,6 @@
 config BR2_PACKAGE_SKELETON_CUSTOM
 	bool
-	select BR2_PACKAGE_SKELETON
+	select BR2_PACKAGE_HAS_SKELETON
+
+config BR2_PACKAGE_PROVIDES_SKELETON
+	default "skeleton-custom" if BR2_PACKAGE_SKELETON_CUSTOM
diff --git a/package/skeleton-custom/skeleton-custom.mk b/package/skeleton-custom/skeleton-custom.mk
index 6e9d52620b..d3c6b48211 100644
--- a/package/skeleton-custom/skeleton-custom.mk
+++ b/package/skeleton-custom/skeleton-custom.mk
@@ -11,7 +11,8 @@
 SKELETON_CUSTOM_ADD_TOOLCHAIN_DEPENDENCY = NO
 SKELETON_CUSTOM_ADD_SKELETON_DEPENDENCY = NO
 
-# The skeleton also handles the merged /usr case in the sysroot
+SKELETON_CUSTOM_PROVIDES = skeleton
+
 SKELETON_CUSTOM_INSTALL_STAGING = YES
 
 SKELETON_CUSTOM_PATH = $(call qstrip,$(BR2_ROOTFS_SKELETON_CUSTOM_PATH))
diff --git a/package/skeleton/Config.in b/package/skeleton/Config.in
index b22ac66b35..efaa1e135f 100644
--- a/package/skeleton/Config.in
+++ b/package/skeleton/Config.in
@@ -1,4 +1,9 @@
 config BR2_PACKAGE_SKELETON
 	bool
-	help
-	  The basic skeleton for your rootfs.
+	default y
+
+config BR2_PACKAGE_HAS_SKELETON
+	bool
+
+config BR2_PACKAGE_PROVIDES_SKELETON
+	string
diff --git a/package/skeleton/skeleton.mk b/package/skeleton/skeleton.mk
index 1d3e4fbe3c..d380f41649 100644
--- a/package/skeleton/skeleton.mk
+++ b/package/skeleton/skeleton.mk
@@ -11,10 +11,4 @@
 SKELETON_ADD_TOOLCHAIN_DEPENDENCY = NO
 SKELETON_ADD_SKELETON_DEPENDENCY = NO
 
-ifeq ($(BR2_PACKAGE_SKELETON_CUSTOM),y)
-SKELETON_DEPENDENCIES = skeleton-custom
-else
-SKELETON_DEPENDENCIES = skeleton-common
-endif
-
-$(eval $(generic-package))
+$(eval $(virtual-package))
diff --git a/system/Config.in b/system/Config.in
index 448984e273..8c3f903bdc 100644
--- a/system/Config.in
+++ b/system/Config.in
@@ -1,5 +1,9 @@
 menu "System configuration"
 
+# Note: usually, it is not possible to select a provider of a virtual
+# package. But here we have an exception: there are only two providers
+# and they only get selected each by separate entries in this choice.
+# So this is a safe situation.
 choice
 	prompt "Root FS skeleton"
 
-- 
2.13.3

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

* [Buildroot] [PATCH v5 09/14] package/skeleton-common: simplify staging install
  2017-08-01 22:52 [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Arnout Vandecappelle
                   ` (7 preceding siblings ...)
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 08/14] package/skeleton: make it a virtual package Arnout Vandecappelle
@ 2017-08-01 22:52 ` Arnout Vandecappelle
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 10/14] package/skeleton: introduce sysv- and systemd-specific skeletons Arnout Vandecappelle
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Arnout Vandecappelle @ 2017-08-01 22:52 UTC (permalink / raw)
  To: buildroot

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

We don't really care what's going in staging, as long as it is properly
setup for merged/non-merged /usr, especially for the lib/ directory.

So we can just copy the skeleton as-is.

This simplifies maintenance, should we ever need to tweak the layout:
we'd just have to do it once in the skeleton directory to have it
propagated to both target and staging.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Changes v4 -> v5: (Arnout)
 - No changes
---
 package/skeleton-common/skeleton-common.mk | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/package/skeleton-common/skeleton-common.mk b/package/skeleton-common/skeleton-common.mk
index e3a52dc040..bf75077a85 100644
--- a/package/skeleton-common/skeleton-common.mk
+++ b/package/skeleton-common/skeleton-common.mk
@@ -26,17 +26,14 @@ define SKELETON_COMMON_INSTALL_TARGET_CMDS
 		$(TARGET_DIR_WARNING_FILE)
 endef
 
-# For the staging dir, we don't really care about /bin and /sbin.
-# But for consistency with the target dir, and to simplify the code,
-# we still handle them for the merged or non-merged /usr cases.
-# Since the toolchain is not yet available, the staging is not yet
-# populated, so we need to create the directories in /usr
+# We don't care much about what goes in staging, as long as it is
+# correctly setup for merged/non-merged /usr. The simplest is to
+# fill it in with the content of the skeleton.
 define SKELETON_COMMON_INSTALL_STAGING_CMDS
-	$(INSTALL) -d -m 0755 $(STAGING_DIR)/usr/lib
-	$(INSTALL) -d -m 0755 $(STAGING_DIR)/usr/bin
-	$(INSTALL) -d -m 0755 $(STAGING_DIR)/usr/sbin
+	$(call SYSTEM_RSYNC,$(SKELETON_COMMON_PATH),$(STAGING_DIR))
 	$(call SYSTEM_USR_SYMLINKS_OR_DIRS,$(STAGING_DIR))
 	$(call SYSTEM_LIB_SYMLINK,$(STAGING_DIR))
+	$(INSTALL) -d -m 0755 $(STAGING_DIR)/usr/include
 endef
 
 SKELETON_COMMON_HOSTNAME = $(call qstrip,$(BR2_TARGET_GENERIC_HOSTNAME))
-- 
2.13.3

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

* [Buildroot] [PATCH v5 10/14] package/skeleton: introduce sysv- and systemd-specific skeletons
  2017-08-01 22:52 [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Arnout Vandecappelle
                   ` (8 preceding siblings ...)
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 09/14] package/skeleton-common: simplify staging install Arnout Vandecappelle
@ 2017-08-01 22:52 ` Arnout Vandecappelle
  2017-08-02 17:59   ` Thomas Petazzoni
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 11/14] system: separate sysv and systemd parts of the skeleton Arnout Vandecappelle
                   ` (5 subsequent siblings)
  15 siblings, 1 reply; 22+ messages in thread
From: Arnout Vandecappelle @ 2017-08-01 22:52 UTC (permalink / raw)
  To: buildroot

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

Currently, we use the same skeleton for sysv-like init systems and
systemd, even though systemd has some peculiarities that makes out
default skeleton unfit.

So, we'll need to provide different skeletons (really, only part of it)
for sysv-like and systemd.

Introduce two new skeleton packages, aptly named skeleton-sysv and
skeleton-systemd, one for each, that are both providers of the skeleton
package, in lieue of the skeleton-common, which is now a simple
dependency of both the two new skeletons.

Those packages are empty for now. In followup changes:
  - sysv-specific stuff will be moved out of skeleton-common and into
    skeleton-sysv;
  - systemd-specific stuff will be added to skeleton-systemd.

skeleton-common remains the provider for the BR2_INIT_NONE case, so
add BR2_PACKAGE_SKELETON_COMMON_ONLY to cover this case.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
[Arnout:
 - merge with the patch that enables the BR2_INIT_NONE case
 - simplify the BR2_PACKAGE_SKELETON_COMMON_ONLY select logic]
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Changes v4 -> v5: (Arnout)
 - merge with the patch that enables the BR2_INIT_NONE case
 - simplify the BR2_PACKAGE_SKELETON_COMMON_ONLY select logic

Changes v3 -> v4:
  - select BR2_PACKAGE_HAS_SKELETON from BR2_PACKAGE_SKELETON_COMMON_ONLY,
    drop now-unneeded comment  (Arnout)
---
 package/Config.in                            |  2 ++
 package/skeleton-common/Config.in            | 13 ++++++++++++-
 package/skeleton-common/skeleton-common.mk   |  2 --
 package/skeleton-systemd/Config.in           |  7 +++++++
 package/skeleton-systemd/skeleton-systemd.mk | 18 ++++++++++++++++++
 package/skeleton-sysv/Config.in              |  7 +++++++
 package/skeleton-sysv/skeleton-sysv.mk       | 18 ++++++++++++++++++
 system/Config.in                             | 11 +++++++----
 8 files changed, 71 insertions(+), 7 deletions(-)
 create mode 100644 package/skeleton-systemd/Config.in
 create mode 100644 package/skeleton-systemd/skeleton-systemd.mk
 create mode 100644 package/skeleton-sysv/Config.in
 create mode 100644 package/skeleton-sysv/skeleton-sysv.mk

diff --git a/package/Config.in b/package/Config.in
index 7d22b47f5c..8e0717cd90 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -4,6 +4,8 @@ menu "Target packages"
 	source "package/skeleton/Config.in"
 	source "package/skeleton-common/Config.in"
 	source "package/skeleton-custom/Config.in"
+	source "package/skeleton-systemd/Config.in"
+	source "package/skeleton-sysv/Config.in"
 
 menu "Audio and video applications"
 	source "package/alsa-utils/Config.in"
diff --git a/package/skeleton-common/Config.in b/package/skeleton-common/Config.in
index 5675e873cf..e18d6d0ff0 100644
--- a/package/skeleton-common/Config.in
+++ b/package/skeleton-common/Config.in
@@ -1,6 +1,17 @@
 config BR2_PACKAGE_SKELETON_COMMON
 	bool
+
+# skeleton-common is normally a dependency of another skeleton.
+# BR2_PACKAGE_SKELETON_COMMON_ONLY can be selected if skeleton-common
+# is the only provider of the skeleton.
+config BR2_PACKAGE_SKELETON_COMMON_ONLY
+	bool
+	select BR2_PACKAGE_SKELETON_COMMON
 	select BR2_PACKAGE_HAS_SKELETON
 
+if BR2_PACKAGE_SKELETON_COMMON_ONLY
+
 config BR2_PACKAGE_PROVIDES_SKELETON
-	default "skeleton-common" if BR2_PACKAGE_SKELETON_COMMON
+	default "skeleton-common"
+
+endif
diff --git a/package/skeleton-common/skeleton-common.mk b/package/skeleton-common/skeleton-common.mk
index bf75077a85..e9b87c3fd3 100644
--- a/package/skeleton-common/skeleton-common.mk
+++ b/package/skeleton-common/skeleton-common.mk
@@ -11,8 +11,6 @@
 SKELETON_COMMON_ADD_TOOLCHAIN_DEPENDENCY = NO
 SKELETON_COMMON_ADD_SKELETON_DEPENDENCY = NO
 
-SKELETON_COMMON_PROVIDES = skeleton
-
 # The skeleton also handles the merged /usr case in the sysroot
 SKELETON_COMMON_INSTALL_STAGING = YES
 
diff --git a/package/skeleton-systemd/Config.in b/package/skeleton-systemd/Config.in
new file mode 100644
index 0000000000..c507264598
--- /dev/null
+++ b/package/skeleton-systemd/Config.in
@@ -0,0 +1,7 @@
+config BR2_PACKAGE_SKELETON_SYSTEMD
+	bool
+	select BR2_PACKAGE_HAS_SKELETON
+	select BR2_PACKAGE_SKELETON_COMMON
+
+config BR2_PACKAGE_PROVIDES_SKELETON
+	default "skeleton-systemd" if BR2_PACKAGE_SKELETON_SYSTEMD
diff --git a/package/skeleton-systemd/skeleton-systemd.mk b/package/skeleton-systemd/skeleton-systemd.mk
new file mode 100644
index 0000000000..cec6359007
--- /dev/null
+++ b/package/skeleton-systemd/skeleton-systemd.mk
@@ -0,0 +1,18 @@
+################################################################################
+#
+# skeleton-systemd
+#
+################################################################################
+
+# The skeleton can't depend on the toolchain, since all packages depends on the
+# skeleton and the toolchain is a target package, as is skeleton.
+# Hence, skeleton would depends on the toolchain and the toolchain would depend
+# on skeleton.
+SKELETON_SYSTEMD_ADD_TOOLCHAIN_DEPENDENCY = NO
+SKELETON_SYSTEMD_ADD_SKELETON_DEPENDENCY = NO
+
+SKELETON_SYSTEMD_DEPENDENCIES = skeleton-common
+
+SKELETON_SYSTEMD_PROVIDES = skeleton
+
+$(eval $(generic-package))
diff --git a/package/skeleton-sysv/Config.in b/package/skeleton-sysv/Config.in
new file mode 100644
index 0000000000..2f6dbd9673
--- /dev/null
+++ b/package/skeleton-sysv/Config.in
@@ -0,0 +1,7 @@
+config BR2_PACKAGE_SKELETON_SYSV
+	bool
+	select BR2_PACKAGE_HAS_SKELETON
+	select BR2_PACKAGE_SKELETON_COMMON
+
+config BR2_PACKAGE_PROVIDES_SKELETON
+	default "skeleton-sysv" if BR2_PACKAGE_SKELETON_SYSV
diff --git a/package/skeleton-sysv/skeleton-sysv.mk b/package/skeleton-sysv/skeleton-sysv.mk
new file mode 100644
index 0000000000..b0c2b6bac1
--- /dev/null
+++ b/package/skeleton-sysv/skeleton-sysv.mk
@@ -0,0 +1,18 @@
+################################################################################
+#
+# skeleton-sysv
+#
+################################################################################
+
+# The skeleton can't depend on the toolchain, since all packages depends on the
+# skeleton and the toolchain is a target package, as is skeleton.
+# Hence, skeleton would depends on the toolchain and the toolchain would depend
+# on skeleton.
+SKELETON_SYSV_ADD_TOOLCHAIN_DEPENDENCY = NO
+SKELETON_SYSV_ADD_SKELETON_DEPENDENCY = NO
+
+SKELETON_SYSV_DEPENDENCIES = skeleton-common
+
+SKELETON_SYSV_PROVIDES = skeleton
+
+$(eval $(generic-package))
diff --git a/system/Config.in b/system/Config.in
index 8c3f903bdc..6d2d10269e 100644
--- a/system/Config.in
+++ b/system/Config.in
@@ -1,15 +1,18 @@
 menu "System configuration"
 
 # Note: usually, it is not possible to select a provider of a virtual
-# package. But here we have an exception: there are only two providers
-# and they only get selected each by separate entries in this choice.
-# So this is a safe situation.
+# package. But here we have an exception: there are only three providers
+# and they only get selected by separate entries in this choice and
+# under different, exclusive conditions. So this is a safe situation.
 choice
 	prompt "Root FS skeleton"
 
 config BR2_ROOTFS_SKELETON_DEFAULT
 	bool "default target skeleton"
-	select BR2_PACKAGE_SKELETON_COMMON
+	select BR2_PACKAGE_SKELETON_SYSV if BR2_INIT_SYSV
+	select BR2_PACKAGE_SKELETON_SYSV if BR2_INIT_BUSYBOX
+	select BR2_PACKAGE_SKELETON_SYSTEMD if BR2_INIT_SYSTEMD
+	select BR2_PACKAGE_SKELETON_COMMON_ONLY if BR2_INIT_NONE
 	help
 	  Use default target skeleton
 
-- 
2.13.3

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

* [Buildroot] [PATCH v5 11/14] system: separate sysv and systemd parts of the skeleton
  2017-08-01 22:52 [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Arnout Vandecappelle
                   ` (9 preceding siblings ...)
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 10/14] package/skeleton: introduce sysv- and systemd-specific skeletons Arnout Vandecappelle
@ 2017-08-01 22:52 ` Arnout Vandecappelle
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 12/14] fs: add pre- and post-command hooks Arnout Vandecappelle
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Arnout Vandecappelle @ 2017-08-01 22:52 UTC (permalink / raw)
  To: buildroot

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

For systemd, we create a simple /etc/fstab with only an entry for /, as
systemd otherwise automatically mounts what it needs where it needs it.

systemd does not like that the content of /var be symlinks to /tmp,
especially journald that starts before /tmp is mounted, and thus the
journal files are hidden from view, which causes quite a bit of fuss...

Instead, move the current /var to a sysv-only skeleton.

systemd at install time will create the /var content it needs, so we
just create an empty /var for systemd.

systemd would create /home and /srv at runtime if they are missing, but
it is better to create them right now, to simplify supporting systemd on
a RO filesystem in the (near) future.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Changes v4 -> v5: (Arnout)
 - No changes

Changes v3 -> v4:
  - add blurb about /etc/fstab  (Arnout)
  - blurb about /home and /srv moved from here to the commit log itself
---
 package/skeleton-systemd/skeleton-systemd.mk              | 7 +++++++
 package/skeleton-sysv/skeleton-sysv.mk                    | 4 ++++
 {system => package/skeleton-sysv}/skeleton/dev/log        | 0
 {system => package/skeleton-sysv}/skeleton/dev/pts/.empty | 0
 {system => package/skeleton-sysv}/skeleton/dev/shm/.empty | 0
 {system => package/skeleton-sysv}/skeleton/etc/fstab      | 0
 {system => package/skeleton-sysv}/skeleton/var/cache      | 0
 {system => package/skeleton-sysv}/skeleton/var/lib/misc   | 0
 {system => package/skeleton-sysv}/skeleton/var/lock       | 0
 {system => package/skeleton-sysv}/skeleton/var/log        | 0
 {system => package/skeleton-sysv}/skeleton/var/run        | 0
 {system => package/skeleton-sysv}/skeleton/var/spool      | 0
 {system => package/skeleton-sysv}/skeleton/var/tmp        | 0
 system/skeleton/dev/{pts => }/.empty                      | 0
 14 files changed, 11 insertions(+)
 rename {system => package/skeleton-sysv}/skeleton/dev/log (100%)
 copy {system => package/skeleton-sysv}/skeleton/dev/pts/.empty (100%)
 rename {system => package/skeleton-sysv}/skeleton/dev/shm/.empty (100%)
 rename {system => package/skeleton-sysv}/skeleton/etc/fstab (100%)
 rename {system => package/skeleton-sysv}/skeleton/var/cache (100%)
 rename {system => package/skeleton-sysv}/skeleton/var/lib/misc (100%)
 rename {system => package/skeleton-sysv}/skeleton/var/lock (100%)
 rename {system => package/skeleton-sysv}/skeleton/var/log (100%)
 rename {system => package/skeleton-sysv}/skeleton/var/run (100%)
 rename {system => package/skeleton-sysv}/skeleton/var/spool (100%)
 rename {system => package/skeleton-sysv}/skeleton/var/tmp (100%)
 rename system/skeleton/dev/{pts => }/.empty (100%)

diff --git a/package/skeleton-systemd/skeleton-systemd.mk b/package/skeleton-systemd/skeleton-systemd.mk
index cec6359007..384715e1c9 100644
--- a/package/skeleton-systemd/skeleton-systemd.mk
+++ b/package/skeleton-systemd/skeleton-systemd.mk
@@ -15,4 +15,11 @@ SKELETON_SYSTEMD_DEPENDENCIES = skeleton-common
 
 SKELETON_SYSTEMD_PROVIDES = skeleton
 
+define SKELETON_SYSTEMD_INSTALL_TARGET_CMDS
+	mkdir -p $(TARGET_DIR)/home
+	mkdir -p $(TARGET_DIR)/srv
+	mkdir -p $(TARGET_DIR)/var
+	echo "/dev/root / auto rw 0 1" >$(TARGET_DIR)/etc/fstab
+endef
+
 $(eval $(generic-package))
diff --git a/package/skeleton-sysv/skeleton-sysv.mk b/package/skeleton-sysv/skeleton-sysv.mk
index b0c2b6bac1..b5fa396ac2 100644
--- a/package/skeleton-sysv/skeleton-sysv.mk
+++ b/package/skeleton-sysv/skeleton-sysv.mk
@@ -15,4 +15,8 @@ SKELETON_SYSV_DEPENDENCIES = skeleton-common
 
 SKELETON_SYSV_PROVIDES = skeleton
 
+define SKELETON_SYSV_INSTALL_TARGET_CMDS
+	$(call SYSTEM_RSYNC,$(SKELETON_SYSV_PKGDIR)/skeleton,$(TARGET_DIR))
+endef
+
 $(eval $(generic-package))
diff --git a/system/skeleton/dev/log b/package/skeleton-sysv/skeleton/dev/log
similarity index 100%
rename from system/skeleton/dev/log
rename to package/skeleton-sysv/skeleton/dev/log
diff --git a/system/skeleton/dev/pts/.empty b/package/skeleton-sysv/skeleton/dev/pts/.empty
similarity index 100%
copy from system/skeleton/dev/pts/.empty
copy to package/skeleton-sysv/skeleton/dev/pts/.empty
diff --git a/system/skeleton/dev/shm/.empty b/package/skeleton-sysv/skeleton/dev/shm/.empty
similarity index 100%
rename from system/skeleton/dev/shm/.empty
rename to package/skeleton-sysv/skeleton/dev/shm/.empty
diff --git a/system/skeleton/etc/fstab b/package/skeleton-sysv/skeleton/etc/fstab
similarity index 100%
rename from system/skeleton/etc/fstab
rename to package/skeleton-sysv/skeleton/etc/fstab
diff --git a/system/skeleton/var/cache b/package/skeleton-sysv/skeleton/var/cache
similarity index 100%
rename from system/skeleton/var/cache
rename to package/skeleton-sysv/skeleton/var/cache
diff --git a/system/skeleton/var/lib/misc b/package/skeleton-sysv/skeleton/var/lib/misc
similarity index 100%
rename from system/skeleton/var/lib/misc
rename to package/skeleton-sysv/skeleton/var/lib/misc
diff --git a/system/skeleton/var/lock b/package/skeleton-sysv/skeleton/var/lock
similarity index 100%
rename from system/skeleton/var/lock
rename to package/skeleton-sysv/skeleton/var/lock
diff --git a/system/skeleton/var/log b/package/skeleton-sysv/skeleton/var/log
similarity index 100%
rename from system/skeleton/var/log
rename to package/skeleton-sysv/skeleton/var/log
diff --git a/system/skeleton/var/run b/package/skeleton-sysv/skeleton/var/run
similarity index 100%
rename from system/skeleton/var/run
rename to package/skeleton-sysv/skeleton/var/run
diff --git a/system/skeleton/var/spool b/package/skeleton-sysv/skeleton/var/spool
similarity index 100%
rename from system/skeleton/var/spool
rename to package/skeleton-sysv/skeleton/var/spool
diff --git a/system/skeleton/var/tmp b/package/skeleton-sysv/skeleton/var/tmp
similarity index 100%
rename from system/skeleton/var/tmp
rename to package/skeleton-sysv/skeleton/var/tmp
diff --git a/system/skeleton/dev/pts/.empty b/system/skeleton/dev/.empty
similarity index 100%
rename from system/skeleton/dev/pts/.empty
rename to system/skeleton/dev/.empty
-- 
2.13.3

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

* [Buildroot] [PATCH v5 12/14] fs: add pre- and post-command hooks
  2017-08-01 22:52 [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Arnout Vandecappelle
                   ` (10 preceding siblings ...)
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 11/14] system: separate sysv and systemd parts of the skeleton Arnout Vandecappelle
@ 2017-08-01 22:52 ` Arnout Vandecappelle
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 13/14] system: make systemd work on a read-only rootfs Arnout Vandecappelle
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Arnout Vandecappelle @ 2017-08-01 22:52 UTC (permalink / raw)
  To: buildroot

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

In some cases, the directory structure we want in the filesystem is not
exactly what we have in target/

For example, when systemd is used on a read-only rootfs, /var must be a
tmpfs. However, we may have packages that install stuff in there, and
set important rights (via the permission-table). So, at build time, we
need /var to be a symlink to the remanent location (/usr/share/factory)
while at runtime we need /var to be a directory.

One option would have seen to have /var as a real directory even during
build time, and in a target-finalize hook, move everything out of there
and into the "factory" location. However, that's not possible because
it's too early: some packages may want to set ownership and/or acces
rights on directories or files in /var, and this is only done in the
fakeroot script, which is called only later during the assembling of the
filesystem images.

Also, there would have been no way to undo the tweak (i.e. we need to
restore the /var symlink so that subsequent builds continue to work) if
it were done as a target-finalize hook.

The only solution is to allow packages to register pre- and post-hooks
that are called right before and right after the rootfs commands are
executed, and inside in the fakeroot script.

We can however not re-use the BR2_ROOTFS_POST_FAKEROOT_SCRIPT feature
either because it is done before the filesystem command, but there is
nothing that is done after. Also, we don't want to add to, and modify a
user-supplied variable.

So, we introduce two new variables that packages can set to add the
commands they need to run to tweak the filesystem right at the last
moment.

Those hooks are not documented on-purpose; they are probably going to
only ever be used by systemd.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Reviewed-by: Romain Naour <romain.naour@gmail.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Changes v4 -> v5: (Arnout)
 - No changes

Changes v2 -> v3:
  - add space between >> and $$   (Romain)
---
 fs/common.mk           | 4 ++++
 package/pkg-generic.mk | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/fs/common.mk b/fs/common.mk
index 14e0a6b868..9a7758ff49 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -95,10 +95,14 @@ endif
 	$$(foreach s,$$(call qstrip,$$(BR2_ROOTFS_POST_FAKEROOT_SCRIPT)),\
 		echo "echo '$$(TERM_BOLD)>>>   Executing fakeroot script $$(s)$$(TERM_RESET)'" >> $$(FAKEROOT_SCRIPT); \
 		echo $$(s) $$(TARGET_DIR) $$(BR2_ROOTFS_POST_SCRIPT_ARGS) >> $$(FAKEROOT_SCRIPT)$$(sep))
+	$$(foreach hook,$$(ROOTFS_PRE_CMD_HOOKS),\
+		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT))
 ifeq ($$(BR2_REPRODUCIBLE),y)
 	echo "find $$(TARGET_DIR) -print0 | xargs -0 -r touch -hd @$$(SOURCE_DATE_EPOCH)" >> $$(FAKEROOT_SCRIPT)
 endif
 	$$(call PRINTF,$$(ROOTFS_$(2)_CMD)) >> $$(FAKEROOT_SCRIPT)
+	$$(foreach hook,$$(ROOTFS_POST_CMD_HOOKS),\
+		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT))
 	chmod a+x $$(FAKEROOT_SCRIPT)
 	PATH=$$(BR_PATH) $$(HOST_DIR)/bin/fakeroot -- $$(FAKEROOT_SCRIPT)
 	$$(INSTALL) -m 0644 support/misc/target-dir-warning.txt $$(TARGET_DIR_WARNING_FILE)
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index f17f110115..ae03051987 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -612,6 +612,8 @@ $(2)_POST_INSTALL_IMAGES_HOOKS  ?=
 $(2)_PRE_LEGAL_INFO_HOOKS       ?=
 $(2)_POST_LEGAL_INFO_HOOKS      ?=
 $(2)_TARGET_FINALIZE_HOOKS      ?=
+$(2)_ROOTFS_PRE_CMD_HOOKS       ?=
+$(2)_ROOTFS_POST_CMD_HOOKS      ?=
 
 # human-friendly targets and target sequencing
 $(1):			$(1)-install
@@ -931,6 +933,8 @@ ifneq ($$($(2)_USERS),)
 PACKAGES_USERS += $$($(2)_USERS)$$(sep)
 endif
 TARGET_FINALIZE_HOOKS += $$($(2)_TARGET_FINALIZE_HOOKS)
+ROOTFS_PRE_CMD_HOOKS += $$($(2)_ROOTFS_PRE_CMD_HOOKS)
+ROOTFS_POST_CMD_HOOKS += $$($(2)_ROOTFS_POST_CMD_HOOKS)
 
 ifeq ($$($(2)_SITE_METHOD),svn)
 DL_TOOLS_DEPENDENCIES += svn
-- 
2.13.3

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

* [Buildroot] [PATCH v5 13/14] system: make systemd work on a read-only rootfs
  2017-08-01 22:52 [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Arnout Vandecappelle
                   ` (11 preceding siblings ...)
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 12/14] fs: add pre- and post-command hooks Arnout Vandecappelle
@ 2017-08-01 22:52 ` Arnout Vandecappelle
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 14/14] support/testing: add runtime testing for read-only systemd Arnout Vandecappelle
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 22+ messages in thread
From: Arnout Vandecappelle @ 2017-08-01 22:52 UTC (permalink / raw)
  To: buildroot

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

When the rootfs is readonly, systemd will expect /var to be writable.
Because we do not really have a R/W filesystem to mount on /var, we make
it a tmpfs [*], and use the systemd-tmpfiles feature to populate it with
"factory" defaults.

We obtain those factory defaults by redirecting /var to that location at
build time, using a symlink /var -> /usr/share/factory which is the
location in which systemd-tmpfiles will look for when instructed to
"recursively copy" a directory.

With a line like:

    C /var/something - - - -

it will look for /usr/share/factory/something and copy it (recursively
if it is a directory) to /var/something, but only if it does not already
exist there.

We also mark this copy with the exclamation mark, as it is only safe to
copy on boot, not when changing targets.

To be noted: the real format for such lines are:

    C /var/something - - - - /from/where/to/copy/something

But if the source is not given, then it is implicitly taken from
/usr/share/factory (which in our case is as-good a location as whatever
else, so we use it, and thus we need not specify the source of the
copy).

Note that we treat symlinks a little bit specially, by creating symlinks
to the factory defaults rather than copying them.

Finally, /var at build time is a symlink, but at runtime, it must be a
directory (so we can mount the tmpfs over there). We can't change that
as a target-finalize hook, because:

  - some packages may want to set ownership and/or access rights on
    files or directories in /var, and that only happens while assembling
    the filesystem images; changing /var from a symlink to a (then
    empty) directory would break this;

  - /var would be a directory on sub-sequent builds (until the next
    "make clean").

Instead, we use the newly-introduce pre- and post-rootfs command hooks,
to turn /var into a directory before assembling the image, and back to a
symlink after assembling the image.

[*] People who want the factory-defaults only on first boot will have
    to tweak the fstab to mount something else than a tmpfs on /var.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Reviewed-by: Romain Naour <romain.naour@gmail.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Changes v4 -> v5: (Arnout)
 - No changes

Changes v2 -> v3;
  - fix typoes in commit log  (Romain)
---
 package/skeleton-systemd/skeleton-systemd.mk | 50 ++++++++++++++++++++++++++--
 system/Config.in                             |  1 -
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/package/skeleton-systemd/skeleton-systemd.mk b/package/skeleton-systemd/skeleton-systemd.mk
index 384715e1c9..860ebea2cd 100644
--- a/package/skeleton-systemd/skeleton-systemd.mk
+++ b/package/skeleton-systemd/skeleton-systemd.mk
@@ -15,11 +15,57 @@ SKELETON_SYSTEMD_DEPENDENCIES = skeleton-common
 
 SKELETON_SYSTEMD_PROVIDES = skeleton
 
+ifeq ($(BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW),y)
+
+define SKELETON_SYSTEMD_ROOT_RO_OR_RW
+	echo "/dev/root / auto rw 0 1" >$(TARGET_DIR)/etc/fstab
+	mkdir -p $(TARGET_DIR)/var
+endef
+
+else
+
+# On a R/O rootfs, /var is a tmpfs filesystem. So, at build time, we
+# redirect /var to the "factory settings" location. Just before the
+# filesystem gets created, the /var symlink will be replaced with
+# a real (but empty) directory, and the "factory files" will be copied
+# back there by the tmpfiles.d mechanism.
+define SKELETON_SYSTEMD_ROOT_RO_OR_RW
+	mkdir -p $(TARGET_DIR)/etc/systemd/tmpfiles.d
+	mkdir -p $(TARGET_DIR)/usr/share/factory/var
+	ln -s usr/share/factory/var $(TARGET_DIR)/var
+	echo "/dev/root / auto ro 0 1" >$(TARGET_DIR)/etc/fstab
+	echo "tmpfs /var tmpfs mode=1777 0 0" >>$(TARGET_DIR)/etc/fstab
+endef
+
+define SKELETON_SYSTEMD_PRE_ROOTFS_VAR
+	rm -f $(TARGET_DIR)/var
+	mkdir $(TARGET_DIR)/var
+	for i in $(TARGET_DIR)/usr/share/factory/var/*; do \
+		j="$${i#$(TARGET_DIR)/usr/share/factory}"; \
+		if [ -L "$${i}" ]; then \
+			printf "L+! %s - - - - %s\n" \
+				"$${j}" "../usr/share/factory/$${j}" \
+			|| exit 1; \
+		else \
+			printf "C! %s - - - -\n" "$${j}" \
+			|| exit 1; \
+		fi; \
+	done >$(TARGET_DIR)/etc/tmpfiles.d/var-factory.conf
+endef
+SKELETON_SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SKELETON_SYSTEMD_PRE_ROOTFS_VAR
+
+define SKELETON_SYSTEMD_POST_ROOTFS_VAR
+	rm -rf $(TARGET_DIR)/var
+	ln -s usr/share/factory/var $(TARGET_DIR)/var
+endef
+SKELETON_SYSTEMD_ROOTFS_POST_CMD_HOOKS += SKELETON_SYSTEMD_POST_ROOTFS_VAR
+
+endif
+
 define SKELETON_SYSTEMD_INSTALL_TARGET_CMDS
 	mkdir -p $(TARGET_DIR)/home
 	mkdir -p $(TARGET_DIR)/srv
-	mkdir -p $(TARGET_DIR)/var
-	echo "/dev/root / auto rw 0 1" >$(TARGET_DIR)/etc/fstab
+	$(SKELETON_SYSTEMD_ROOT_RO_OR_RW)
 endef
 
 $(eval $(generic-package))
diff --git a/system/Config.in b/system/Config.in
index 6d2d10269e..124d7f59d3 100644
--- a/system/Config.in
+++ b/system/Config.in
@@ -136,7 +136,6 @@ config BR2_INIT_SYSTEMD
 	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_10
 	select BR2_ROOTFS_MERGED_USR
 	select BR2_PACKAGE_SYSTEMD
-	select BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW if BR2_ROOTFS_SKELETON_DEFAULT
 
 comment "systemd needs a glibc toolchain, headers >= 3.10"
 	depends on !(BR2_TOOLCHAIN_USES_GLIBC \
-- 
2.13.3

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

* [Buildroot] [PATCH v5 14/14] support/testing: add runtime testing for read-only systemd
  2017-08-01 22:52 [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Arnout Vandecappelle
                   ` (12 preceding siblings ...)
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 13/14] system: make systemd work on a read-only rootfs Arnout Vandecappelle
@ 2017-08-01 22:52 ` Arnout Vandecappelle
  2017-08-02 17:51 ` [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Yann E. MORIN
  2017-08-02 19:19 ` Thomas Petazzoni
  15 siblings, 0 replies; 22+ messages in thread
From: Arnout Vandecappelle @ 2017-08-01 22:52 UTC (permalink / raw)
  To: buildroot

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

We add the 3 following combinations:

  - basic systemd, read-only, network w/ ifupdown
  - basic systemd, read-only, network w/ networkd
  - full systemd, read-only, network w/ networkd

The tests just verify what the /sbin/init binary is, and that we were
able to grab an IP address. More tests can be added later, for example
to check each systemd features (journal, tmpfiles...)

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
[Arnout: regenerate .gitlab-ci.yml]
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Changes v4 -> v5: (Arnout)
 - regenerate .gitlab-ci.yml
---
 .gitlab-ci.yml                                     |  3 +
 .../testing/tests/init/systemd-factory/var/foo/bar |  1 +
 support/testing/tests/init/test_systemd.py         | 70 ++++++++++++++++++++++
 3 files changed, 74 insertions(+)
 create mode 100644 support/testing/tests/init/systemd-factory/var/foo/bar

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index cdae953bf9..9cd52c6798 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -234,6 +234,9 @@ tests.init.test_busybox.TestInitSystemBusyboxRoNet: *runtime_test
 tests.init.test_busybox.TestInitSystemBusyboxRw: *runtime_test
 tests.init.test_busybox.TestInitSystemBusyboxRwNet: *runtime_test
 tests.init.test_none.TestInitSystemNone: *runtime_test
+tests.init.test_systemd.TestInitSystemSystemdRoFull: *runtime_test
+tests.init.test_systemd.TestInitSystemSystemdRoIfupdown: *runtime_test
+tests.init.test_systemd.TestInitSystemSystemdRoNetworkd: *runtime_test
 tests.init.test_systemd.TestInitSystemSystemdRwFull: *runtime_test
 tests.init.test_systemd.TestInitSystemSystemdRwIfupdown: *runtime_test
 tests.init.test_systemd.TestInitSystemSystemdRwNetworkd: *runtime_test
diff --git a/support/testing/tests/init/systemd-factory/var/foo/bar b/support/testing/tests/init/systemd-factory/var/foo/bar
new file mode 100644
index 0000000000..323fae03f4
--- /dev/null
+++ b/support/testing/tests/init/systemd-factory/var/foo/bar
@@ -0,0 +1 @@
+foobar
diff --git a/support/testing/tests/init/test_systemd.py b/support/testing/tests/init/test_systemd.py
index 7a80aa1145..b1b6517373 100644
--- a/support/testing/tests/init/test_systemd.py
+++ b/support/testing/tests/init/test_systemd.py
@@ -22,6 +22,28 @@ class InitSystemSystemdBase(InitSystemBase):
 
 
 #-------------------------------------------------------------------------------
+class TestInitSystemSystemdRoNetworkd(InitSystemSystemdBase):
+    config = InitSystemSystemdBase.config + \
+        """
+        BR2_SYSTEM_DHCP="eth0"
+        # BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is not set
+        BR2_ROOTFS_OVERLAY="{}"
+        BR2_TARGET_ROOTFS_SQUASHFS=y
+        """.format(infra.filepath("tests/init/systemd-factory"))
+
+    def test_run(self):
+        self.startEmulator("squashfs", "zImage", "vexpress-v2p-ca9")
+        self.checkInit()
+        self.checkNetwork("eth0")
+
+        # This one must be executed on the target, to check that
+        # the factory feature works as expected
+        out, exit_code = self.emulator.run("cat /var/foo/bar")
+        self.assertEqual(exit_code, 0)
+        self.assertEqual(out[0], "foobar")
+
+
+#-------------------------------------------------------------------------------
 class TestInitSystemSystemdRwNetworkd(InitSystemSystemdBase):
     config = InitSystemSystemdBase.config + \
         """
@@ -36,6 +58,22 @@ class TestInitSystemSystemdRwNetworkd(InitSystemSystemdBase):
 
 
 #-------------------------------------------------------------------------------
+class TestInitSystemSystemdRoIfupdown(InitSystemSystemdBase):
+    config = InitSystemSystemdBase.config + \
+        """
+        BR2_SYSTEM_DHCP="eth0"
+        # BR2_PACKAGE_SYSTEMD_NETWORKD is not set
+        # BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is not set
+        BR2_TARGET_ROOTFS_SQUASHFS=y
+        """
+
+    def test_run(self):
+        self.startEmulator("squashfs", "zImage", "vexpress-v2p-ca9")
+        self.checkInit()
+        self.checkNetwork("eth0")
+
+
+#-------------------------------------------------------------------------------
 class TestInitSystemSystemdRwIfupdown(InitSystemSystemdBase):
     config = InitSystemSystemdBase.config + \
         """
@@ -52,6 +90,38 @@ class TestInitSystemSystemdRwIfupdown(InitSystemSystemdBase):
 
 
 #-------------------------------------------------------------------------------
+class TestInitSystemSystemdRoFull(InitSystemSystemdBase):
+    config = InitSystemSystemdBase.config + \
+        """
+        BR2_SYSTEM_DHCP="eth0"
+        # BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is not set
+        BR2_PACKAGE_SYSTEMD_JOURNAL_GATEWAY=y
+        BR2_PACKAGE_SYSTEMD_BACKLIGHT=y
+        BR2_PACKAGE_SYSTEMD_BINFMT=y
+        BR2_PACKAGE_SYSTEMD_COREDUMP=y
+        BR2_PACKAGE_SYSTEMD_FIRSTBOOT=y
+        BR2_PACKAGE_SYSTEMD_HIBERNATE=y
+        BR2_PACKAGE_SYSTEMD_IMPORTD=y
+        BR2_PACKAGE_SYSTEMD_LOCALED=y
+        BR2_PACKAGE_SYSTEMD_LOGIND=y
+        BR2_PACKAGE_SYSTEMD_MACHINED=y
+        BR2_PACKAGE_SYSTEMD_POLKIT=y
+        BR2_PACKAGE_SYSTEMD_QUOTACHECK=y
+        BR2_PACKAGE_SYSTEMD_RANDOMSEED=y
+        BR2_PACKAGE_SYSTEMD_RFKILL=y
+        BR2_PACKAGE_SYSTEMD_SMACK_SUPPORT=y
+        BR2_PACKAGE_SYSTEMD_SYSUSERS=y
+        BR2_PACKAGE_SYSTEMD_VCONSOLE=y
+        BR2_TARGET_ROOTFS_SQUASHFS=y
+        """
+
+    def test_run(self):
+        self.startEmulator("squashfs", "zImage", "vexpress-v2p-ca9")
+        self.checkInit()
+        self.checkNetwork("eth0")
+
+
+#-------------------------------------------------------------------------------
 class TestInitSystemSystemdRwFull(InitSystemSystemdBase):
     config = InitSystemSystemdBase.config + \
         """
-- 
2.13.3

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

* [Buildroot] [PATCH v5 01/14] support/testing: add runtime testing for init systems
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 01/14] support/testing: add runtime testing for init systems Arnout Vandecappelle
@ 2017-08-02 15:48   ` Thomas Petazzoni
  0 siblings, 0 replies; 22+ messages in thread
From: Thomas Petazzoni @ 2017-08-02 15:48 UTC (permalink / raw)
  To: buildroot

Hello,

On Wed, 2 Aug 2017 00:52:11 +0200, Arnout Vandecappelle
(Essensium/Mind) wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> The "builtin" kernel does not boot a systemd-based system, so
> we resort to building the same one as currently used by our
> qemu_arm_vexpress_defconfig.

[...]

Thanks, I've applied. I like the test case definitions themselves, they
are small and very readable. However, there are a few other things that
I think could be better, see below.


> +class InitSystemBase(infra.basetest.BRTest):
> +
> +    def startEmulator(self, fs_type, kernel=None, dtb=None, init=None):
> +        img = os.path.join(self.builddir, "images", "rootfs.{}".format(fs_type))
> +        subprocess.call(["truncate", "-s", "%1M", img])
> +
> +        options = ["-drive",
> +                   "file={},if=sd,format=raw".format(img),
> +                   "-M", "vexpress-a9"]
> +
> +        if kernel is None:
> +            kernel = "builtin"
> +        else:
> +            kernel = os.path.join(self.builddir, "images", kernel)
> +            options.extend(["-dtb", os.path.join(self.builddir, "images",
> +                                             "{}.dtb".format(dtb))])
> +
> +        kernel_cmdline = ["root=/dev/mmcblk0",
> +                          "rootfstype={}".format(fs_type),
> +                          "rootwait",
> +                          "ro",
> +                          "console=ttyAMA0"]

The beginning of this function partly duplicates some of the logic in
the Emulator() class, like passing -M vexpress-a9, etc. The fact that
you needed this additional helper function on top indicates IMO that
the self.emulator.boot() method should be improved.

> +        if not init is None:

if init is not None ?

> +            kernel_cmdline.extend(["init={}".format(init)])
> +
> +        self.emulator.boot(arch="armv7",
> +                           kernel=kernel,
> +                           kernel_cmdline=kernel_cmdline,
> +                           options=options)
> +
> +        if init is None:
> +            self.emulator.login()

Reading this condition is really weird. Indeed when you have an init
such as systemd or busybox, the init variable is None, and that's why
you login. And when you have *no* init, then the init variable has a
value, and you can't login.

But the naming of the variable can cause confusion here: "if init is
None" can be understood as "if there's no init program". But it's
actually exactly the opposite.

It would be nice to find a way to clarify that.

> +class TestInitSystemNone(InitSystemBase):
> +    config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
> +        """
> +        BR2_INIT_NONE=y
> +        # BR2_TARGET_ROOTFS_TAR is not set
> +        BR2_TARGET_ROOTFS_SQUASHFS=y
> +        """
> +
> +    def test_run(self):
> +        self.startEmulator(fs_type="squashfs", init="/bin/sh")
> +        index = self.emulator.qemu.expect(["/bin/sh: can't access tty; job control turned off", pexpect.TIMEOUT], timeout=60)
> +        if index != 0:
> +            self.emulator.logfile.write("==> System does not boot")
> +            raise SystemError("System does not boot")
> +        index = self.emulator.qemu.expect(["#", pexpect.TIMEOUT], timeout=60)
> +        if index != 0:
> +            self.emulator.logfile.write("==> System does not boot")
> +            raise SystemError("System does not boot")

The self.emulator class should provide a method to wait for a string so
that you don't have to use the internals of self.emulator.qemu.

Thanks!

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

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

* [Buildroot] [PATCH v5 06/14] package/skeleton: split out into skeleton-common
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 06/14] package/skeleton: split out into skeleton-common Arnout Vandecappelle
@ 2017-08-02 17:24   ` Thomas Petazzoni
  0 siblings, 0 replies; 22+ messages in thread
From: Thomas Petazzoni @ 2017-08-02 17:24 UTC (permalink / raw)
  To: buildroot

Hello,

On Wed, 2 Aug 2017 00:52:16 +0200, Arnout Vandecappelle
(Essensium/Mind) wrote:

> We don't name it skeleton-default, because it will be further split
> later, into a skeleton for sysv and another for systemd, with some parts
> still common between the two. So just name it skeleton-common right now;
> this will save us a rename later.
> 
> We keep in the skeleton package all definitions of macros and variables
> that can be used by other skeleton packages and/or init systems, and
> just offload the actual feeding of the skeleton to the new package.

This paragraph was actually no longer true: after this patch,
package/skeleton/skeleton.mk is basically empty, with just a dependency
on skeleton-custom or skeleton-common:

==
SKELETON_ADD_TOOLCHAIN_DEPENDENCY = NO
SKELETON_ADD_SKELETON_DEPENDENCY = NO

ifeq ($(BR2_PACKAGE_SKELETON_CUSTOM),y)
SKELETON_DEPENDENCIES = skeleton-custom
else
SKELETON_DEPENDENCIES = skeleton-common
endif

$(eval $(generic-package))
==

So I've dropped this paragraph from the commit log.

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

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

* [Buildroot] [PATCH v5 07/14] skeleton-common: rename SKELETON_ROOT_PASSWORD to SKELETON_CUSTOM_ROOT_PASSWORD
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 07/14] skeleton-common: rename SKELETON_ROOT_PASSWORD to SKELETON_CUSTOM_ROOT_PASSWORD Arnout Vandecappelle
@ 2017-08-02 17:29   ` Thomas Petazzoni
  2017-08-02 19:22     ` Arnout Vandecappelle
  0 siblings, 1 reply; 22+ messages in thread
From: Thomas Petazzoni @ 2017-08-02 17:29 UTC (permalink / raw)
  To: buildroot

Hello,

On Wed, 2 Aug 2017 00:52:17 +0200, Arnout Vandecappelle
(Essensium/Mind) wrote:
> Package variables should always be prefixed with the package name. For
> SKELETON_CUSTOM_ROOT_PASSWORD, this means we set the variable a second

The variable is now named SKELETON_COMMON_ROOT_PASSWORD, not
SKELETON_CUSTOM_ROOT_PASSWORD. I've fixed that in both the commit title
and in the above sentence.

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

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

* [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system
  2017-08-01 22:52 [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Arnout Vandecappelle
                   ` (13 preceding siblings ...)
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 14/14] support/testing: add runtime testing for read-only systemd Arnout Vandecappelle
@ 2017-08-02 17:51 ` Yann E. MORIN
  2017-08-02 19:19 ` Thomas Petazzoni
  15 siblings, 0 replies; 22+ messages in thread
From: Yann E. MORIN @ 2017-08-02 17:51 UTC (permalink / raw)
  To: buildroot

Arnout, All,

On 2017-08-02 00:52 +0200, Arnout Vandecappelle (Essensium/Mind) spake thusly:
> This series is a proposal to fix our handling of systemd as an init
> system.
[--SNIP--]
> Changes v4 -> v5:
>   - moved the test infra patch all the way to the beginning of the series
>   - dropped the patch that removed support for BR2_INIT_NONE with the default
>     skeleton
>   - to make that possible, merged the patch that re-introduced BR2_INIT_NONE
>     support with the patch that breaks it, i.e. the introduction of
>     skeleton-sysv and skeleton-systemd
>   - split the patch that splits off skeleton-custom into 4 patches; the first
>     one does only the split, the other ones do 3 clean-ups. Except for
>     comments, the end result is the same as the original skeleton-custom
>   - add a further cleanup to skeleton-common that renames
>     SKELETON_ROOT_PASSWORD to SKELETON_CUSTOM_ROOT_PASSWORD
>   - re-generate .gitlab-ci.yml in the two patches that create new tests.

Thanks for cleaning it up faster than I could! ;-)

I'll try to dedicate a bit of time tonight to look at the patches that
are not yet applied.

Regards,
Yann E. MORIN.

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

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

* [Buildroot] [PATCH v5 10/14] package/skeleton: introduce sysv- and systemd-specific skeletons
  2017-08-01 22:52 ` [Buildroot] [PATCH v5 10/14] package/skeleton: introduce sysv- and systemd-specific skeletons Arnout Vandecappelle
@ 2017-08-02 17:59   ` Thomas Petazzoni
  0 siblings, 0 replies; 22+ messages in thread
From: Thomas Petazzoni @ 2017-08-02 17:59 UTC (permalink / raw)
  To: buildroot

Hello,

On Wed, 2 Aug 2017 00:52:20 +0200, Arnout Vandecappelle
(Essensium/Mind) wrote:

> diff --git a/package/skeleton-common/Config.in b/package/skeleton-common/Config.in
> index 5675e873cf..e18d6d0ff0 100644
> --- a/package/skeleton-common/Config.in
> +++ b/package/skeleton-common/Config.in
> @@ -1,6 +1,17 @@
>  config BR2_PACKAGE_SKELETON_COMMON
>  	bool
> +
> +# skeleton-common is normally a dependency of another skeleton.
> +# BR2_PACKAGE_SKELETON_COMMON_ONLY can be selected if skeleton-common
> +# is the only provider of the skeleton.
> +config BR2_PACKAGE_SKELETON_COMMON_ONLY
> +	bool
> +	select BR2_PACKAGE_SKELETON_COMMON
>  	select BR2_PACKAGE_HAS_SKELETON

I was not really happy with this BR2_PACKAGE_SKELETON_COMMON_ONLY
solution, where skeleton-common is sometimes a provider of the
skeleton virtual package, sometimes not. It feels a bit hackish.

So I've replaced that with what I believe is a simpler and cleaner
solution, even if it requires a little bit more code: I've added a
skeleton-none package. At the point of this commit, it is completely
identical to skeleton-sysv and skeleton-systemd: it does nothing but
depend on skeleton-common. But this way, we have a clean situation
where the skeleton virtual package has four providers: skeleton-none,
skeleton-sysv, skeleton-systemd and skeleton-custom. All of
skeleton-none, skeleton-sysv and skeleton-systemd depend on
skeleton-common.

> +endif
> diff --git a/package/skeleton-common/skeleton-common.mk b/package/skeleton-common/skeleton-common.mk
> index bf75077a85..e9b87c3fd3 100644
> --- a/package/skeleton-common/skeleton-common.mk
> +++ b/package/skeleton-common/skeleton-common.mk
> @@ -11,8 +11,6 @@
>  SKELETON_COMMON_ADD_TOOLCHAIN_DEPENDENCY = NO
>  SKELETON_COMMON_ADD_SKELETON_DEPENDENCY = NO
>  
> -SKELETON_COMMON_PROVIDES = skeleton

Removing this was correct when skeleton-common was *not* a provider of
skeleton (i.e when skeleton-common is a dependency of skeleton-sysv or
skeleton-systemd). But it should have been kept in the
BR2_PACKAGE_SKELETON_COMMON_ONLY, because in this case, skeleton-common
was directly a provider of the skeleton virtual package.

But thanks to the addition of a skeleton-none package, skeleton-common
is really never a provider of skeleton, and therefore removing this
line unconditionally becomes correct.

Hopefully I haven't messed up my change introducing skeleton-none :-)

>  config BR2_ROOTFS_SKELETON_DEFAULT
>  	bool "default target skeleton"
> -	select BR2_PACKAGE_SKELETON_COMMON
> +	select BR2_PACKAGE_SKELETON_SYSV if BR2_INIT_SYSV
> +	select BR2_PACKAGE_SKELETON_SYSV if BR2_INIT_BUSYBOX
> +	select BR2_PACKAGE_SKELETON_SYSTEMD if BR2_INIT_SYSTEMD
> +	select BR2_PACKAGE_SKELETON_COMMON_ONLY if BR2_INIT_NONE

So this last line is now:

	select BR2_PACKAGE_SKELETON_NONE if BR2_INIT_NONE

Thanks!

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

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

* [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system
  2017-08-01 22:52 [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Arnout Vandecappelle
                   ` (14 preceding siblings ...)
  2017-08-02 17:51 ` [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Yann E. MORIN
@ 2017-08-02 19:19 ` Thomas Petazzoni
  15 siblings, 0 replies; 22+ messages in thread
From: Thomas Petazzoni @ 2017-08-02 19:19 UTC (permalink / raw)
  To: buildroot

Hello,

On Wed, 2 Aug 2017 00:52:10 +0200, Arnout Vandecappelle
(Essensium/Mind) wrote:

> Arnout Vandecappelle (1):
>   skeleton-common: rename SKELETON_ROOT_PASSWORD to
>     SKELETON_CUSTOM_ROOT_PASSWORD
> 
> Yann E. MORIN (13):
>   support/testing: add runtime testing for init systems
>   package/skeleton: split out into skeleton-custom
>   package/skeleton-custom: rework the merged_usr and building conditions
>   package/skeleton-custom: also check for missing directories
>   package/skeleton-custom: simplify target/staging install
>   package/skeleton: split out into skeleton-common
>   package/skeleton: make it a virtual package
>   package/skeleton-common: simplify staging install
>   package/skeleton: introduce sysv- and systemd-specific skeletons
>   system: separate sysv and systemd parts of the skeleton
>   fs: add pre- and post-command hooks
>   system: make systemd work on a read-only rootfs
>   support/testing: add runtime testing for read-only systemd

I've now applied the entire series. Thanks to Yann for all the work,
and thanks Arnout for the final polishing.

I made a few changes here and there, which I commented as replies to
the respective patches.

Thanks!

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

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

* [Buildroot] [PATCH v5 07/14] skeleton-common: rename SKELETON_ROOT_PASSWORD to SKELETON_CUSTOM_ROOT_PASSWORD
  2017-08-02 17:29   ` Thomas Petazzoni
@ 2017-08-02 19:22     ` Arnout Vandecappelle
  0 siblings, 0 replies; 22+ messages in thread
From: Arnout Vandecappelle @ 2017-08-02 19:22 UTC (permalink / raw)
  To: buildroot



On 02-08-17 19:29, Thomas Petazzoni wrote:
> Hello,
> 
> On Wed, 2 Aug 2017 00:52:17 +0200, Arnout Vandecappelle
> (Essensium/Mind) wrote:
>> Package variables should always be prefixed with the package name. For
>> SKELETON_CUSTOM_ROOT_PASSWORD, this means we set the variable a second
> 
> The variable is now named SKELETON_COMMON_ROOT_PASSWORD, not
> SKELETON_CUSTOM_ROOT_PASSWORD. I've fixed that in both the commit title
> and in the above sentence.

 Yeah, I sent it at 00:52, so you can imagine...

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

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

end of thread, other threads:[~2017-08-02 19:22 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-01 22:52 [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Arnout Vandecappelle
2017-08-01 22:52 ` [Buildroot] [PATCH v5 01/14] support/testing: add runtime testing for init systems Arnout Vandecappelle
2017-08-02 15:48   ` Thomas Petazzoni
2017-08-01 22:52 ` [Buildroot] [PATCH v5 02/14] package/skeleton: split out into skeleton-custom Arnout Vandecappelle
2017-08-01 22:52 ` [Buildroot] [PATCH v5 03/14] package/skeleton-custom: rework the merged_usr and building conditions Arnout Vandecappelle
2017-08-01 22:52 ` [Buildroot] [PATCH v5 04/14] package/skeleton-custom: also check for missing directories Arnout Vandecappelle
2017-08-01 22:52 ` [Buildroot] [PATCH v5 05/14] package/skeleton-custom: simplify target/staging install Arnout Vandecappelle
2017-08-01 22:52 ` [Buildroot] [PATCH v5 06/14] package/skeleton: split out into skeleton-common Arnout Vandecappelle
2017-08-02 17:24   ` Thomas Petazzoni
2017-08-01 22:52 ` [Buildroot] [PATCH v5 07/14] skeleton-common: rename SKELETON_ROOT_PASSWORD to SKELETON_CUSTOM_ROOT_PASSWORD Arnout Vandecappelle
2017-08-02 17:29   ` Thomas Petazzoni
2017-08-02 19:22     ` Arnout Vandecappelle
2017-08-01 22:52 ` [Buildroot] [PATCH v5 08/14] package/skeleton: make it a virtual package Arnout Vandecappelle
2017-08-01 22:52 ` [Buildroot] [PATCH v5 09/14] package/skeleton-common: simplify staging install Arnout Vandecappelle
2017-08-01 22:52 ` [Buildroot] [PATCH v5 10/14] package/skeleton: introduce sysv- and systemd-specific skeletons Arnout Vandecappelle
2017-08-02 17:59   ` Thomas Petazzoni
2017-08-01 22:52 ` [Buildroot] [PATCH v5 11/14] system: separate sysv and systemd parts of the skeleton Arnout Vandecappelle
2017-08-01 22:52 ` [Buildroot] [PATCH v5 12/14] fs: add pre- and post-command hooks Arnout Vandecappelle
2017-08-01 22:52 ` [Buildroot] [PATCH v5 13/14] system: make systemd work on a read-only rootfs Arnout Vandecappelle
2017-08-01 22:52 ` [Buildroot] [PATCH v5 14/14] support/testing: add runtime testing for read-only systemd Arnout Vandecappelle
2017-08-02 17:51 ` [Buildroot] [PATCH v5 00/14] system: properly handle systemd as init system Yann E. MORIN
2017-08-02 19:19 ` Thomas Petazzoni

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.