[Buildroot] [PATCH 0/2] systemd/udev/eudev polishing

[Buildroot] [PATCH 0/2] systemd/udev/eudev polishing

[Eric Le Bihan]

This patch series contains enhancements for systemd, udev and eudev, intended
for next branch:

- use of proper virtual package infrastructure (suggested by Arnout).
- enable required kernel features for systemd (suggested by SamuelM).

Best regards,
ELB

Eric Le Bihan (2):
  udev: update udev to new virtual package infra.
  systemd: enable required kernel features.

 linux/linux.mk            |    6 +++++-
 package/eudev/Config.in   |    3 +++
 package/systemd/Config.in |   15 +++++++++++++--
 package/udev/Config.in    |    8 ++++----
 package/udev/udev.mk      |   11 ++---------
 5 files changed, 27 insertions(+), 16 deletions(-)

-- 
1.7.9.5

[Buildroot] [PATCH 1/2] udev: update udev to new virtual package infra.

[Eric Le Bihan]

The udev virtual package and its current providers (eudev and systemd)
have been updated to use the latest version of virtual package
infrastructure.

A provider should now select BR2_PACKAGE_HAS_UDEV and well as set a
value for BR2_PACKAGE_PROVIDES_UDEV.

Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>
---
 package/eudev/Config.in   |    3 +++
 package/systemd/Config.in |    3 +++
 package/udev/Config.in    |    8 ++++----
 package/udev/udev.mk      |   11 ++---------
 4 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/package/eudev/Config.in b/package/eudev/Config.in
index bfad99a..40ff721 100644
--- a/package/eudev/Config.in
+++ b/package/eudev/Config.in
@@ -21,6 +21,9 @@ config BR2_PACKAGE_EUDEV
 
 if BR2_PACKAGE_EUDEV
 
+config BR2_PACKAGE_PROVIDES_UDEV
+	default "eudev"
+
 config BR2_PACKAGE_EUDEV_RULES_GEN
 	bool "enable rules generator"
 	help
diff --git a/package/systemd/Config.in b/package/systemd/Config.in
index e3ea433..eacc23c 100644
--- a/package/systemd/Config.in
+++ b/package/systemd/Config.in
@@ -40,6 +40,9 @@ config BR2_PACKAGE_SYSTEMD
 
 if BR2_PACKAGE_SYSTEMD
 
+config BR2_PACKAGE_PROVIDES_UDEV
+	default "systemd"
+
 config BR2_PACKAGE_SYSTEMD_ALL_EXTRAS
 	bool "enable all extras"
 	select BR2_PACKAGE_XZ
diff --git a/package/udev/Config.in b/package/udev/Config.in
index 5cd4051..941a515 100644
--- a/package/udev/Config.in
+++ b/package/udev/Config.in
@@ -1,6 +1,6 @@
 config BR2_PACKAGE_HAS_UDEV
 	bool
-	help
-	  udev is now a virtual package.
-	  Packages requiring udevd at run time or libudev/gudev at build time
-	  should select it.
+
+config BR2_PACKAGE_PROVIDES_UDEV
+	depends on BR2_PACKAGE_HAS_UDEV
+	string
diff --git a/package/udev/udev.mk b/package/udev/udev.mk
index a5d0f80..cb544d6 100644
--- a/package/udev/udev.mk
+++ b/package/udev/udev.mk
@@ -5,18 +5,11 @@
 ################################################################################
 
 UDEV_SOURCE =
-
-ifeq ($(BR2_PACKAGE_EUDEV),y)
-UDEV_DEPENDENCIES += eudev
-endif
-
-ifeq ($(BR2_PACKAGE_SYSTEMD),y)
-UDEV_DEPENDENCIES += systemd
-endif
+UDEV_DEPENDENCIES = $(call qstrip,$(BR2_PACKAGE_PROVIDES_UDEV))
 
 ifeq ($(UDEV_DEPENDENCIES),)
 define UDEV_CONFIGURE_CMDS
-	echo "No Udev implementation selected. Configuration error."
+	$(error No Udev implementation selected. Configuration error)
 	exit 1
 endef
 endif
-- 
1.7.9.5

[Buildroot] [PATCH 1/2] udev: update udev to new virtual package infra.

[Thomas Petazzoni]

Dear Eric Le Bihan,

On Mon, 17 Feb 2014 11:22:15 +0100, Eric Le Bihan wrote:

> -ifeq ($(BR2_PACKAGE_EUDEV),y)
> -UDEV_DEPENDENCIES += eudev
> -endif
> -
> -ifeq ($(BR2_PACKAGE_SYSTEMD),y)
> -UDEV_DEPENDENCIES += systemd
> -endif
> +UDEV_DEPENDENCIES = $(call qstrip,$(BR2_PACKAGE_PROVIDES_UDEV))
>  
>  ifeq ($(UDEV_DEPENDENCIES),)
>  define UDEV_CONFIGURE_CMDS
> -	echo "No Udev implementation selected. Configuration error."
> +	$(error No Udev implementation selected. Configuration error)
>  	exit 1
>  endef
>  endif

Why are we doing this check in UDEV_CONFIGURE_CMDS ? I believe we can
simply do:

ifeq ($(<foo>_DEPENDENCIES),)
$(error No <foo> implementation selected)
endif

Best regards,

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

[Buildroot] [PATCH 1/2] udev: update udev to new virtual package infra.

[Eric Le Bihan]

Hi!

On Mon, Feb 17, 2014 at 11:49:06AM +0100, Thomas Petazzoni wrote:
> Dear Eric Le Bihan,
>
> On Mon, 17 Feb 2014 11:22:15 +0100, Eric Le Bihan wrote:
>
> > -ifeq ($(BR2_PACKAGE_EUDEV),y)
> > -UDEV_DEPENDENCIES += eudev
> > -endif
> > -
> > -ifeq ($(BR2_PACKAGE_SYSTEMD),y)
> > -UDEV_DEPENDENCIES += systemd
> > -endif
> > +UDEV_DEPENDENCIES = $(call qstrip,$(BR2_PACKAGE_PROVIDES_UDEV))
> >
> >  ifeq ($(UDEV_DEPENDENCIES),)
> >  define UDEV_CONFIGURE_CMDS
> > -	echo "No Udev implementation selected. Configuration error."
> > +	$(error No Udev implementation selected. Configuration error)
> >  	exit 1
> >  endef
> >  endif
>
> Why are we doing this check in UDEV_CONFIGURE_CMDS ? I believe we can
> simply do:
>
> ifeq ($(<foo>_DEPENDENCIES),)
> $(error No <foo> implementation selected)
> endif
I followed the example in package/opengl/libegl/libegl.mk, but I mixed the
Makefile control function $(error text...) and some Shell script without
thinking...

I will revert it to the pure shell version to be coherent.

With the <foo>_CONFIGURE_CMDS method, we go through all the dummy steps, and
this is very verbose:

  >>> udev undefined Extracting
  >>> udev undefined Patching
  >>> udev undefined Configuring
  echo "No Udev implementation selected. Configuration error."
  No Udev implementation selected. Configuration error.
  exit 1
  make: *** [/home/eric/build/elebihan/demo-systemd/arm/build/udev-undefined/.stamp_configured] Erreur 1

With the pure Makefile version, the error message is clearer:

  package/udev/udev.mk:11: *** No Udev implementation selected. Configuration error. Stop.

I find it better. If it suits everyone, I can propose a patch to convert all
the virtual packages to it and re-send my tutorial patch about virtual
package.

Best regards,
ELB

[Buildroot] [PATCH 1/2] udev: update udev to new virtual package infra.

[Thomas Petazzoni]

Dear Eric Le Bihan,

On Mon, 17 Feb 2014 15:33:13 +0100, Eric Le Bihan wrote:

> > Why are we doing this check in UDEV_CONFIGURE_CMDS ? I believe we can
> > simply do:
> >
> > ifeq ($(<foo>_DEPENDENCIES),)
> > $(error No <foo> implementation selected)
> > endif
> I followed the example in package/opengl/libegl/libegl.mk, but I mixed the
> Makefile control function $(error text...) and some Shell script without
> thinking...
> 
> I will revert it to the pure shell version to be coherent.

I guess you mean "to the pure make version", according to what you say
below, right?

> With the <foo>_CONFIGURE_CMDS method, we go through all the dummy steps, and
> this is very verbose:
> 
>   >>> udev undefined Extracting
>   >>> udev undefined Patching
>   >>> udev undefined Configuring
>   echo "No Udev implementation selected. Configuration error."
>   No Udev implementation selected. Configuration error.
>   exit 1
>   make: *** [/home/eric/build/elebihan/demo-systemd/arm/build/udev-undefined/.stamp_configured] Erreur 1
> 
> With the pure Makefile version, the error message is clearer:
> 
>   package/udev/udev.mk:11: *** No Udev implementation selected. Configuration error. Stop.

Yes, I believe this is better. And the message is given at the very
beginning of the build, not when udev starts to be built, which is also
nicer.

> I find it better. If it suits everyone, I can propose a patch to convert all
> the virtual packages to it and re-send my tutorial patch about virtual
> package.

That would be really great!

Thanks!

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

[Buildroot] [PATCH 1/2] udev: update udev to new virtual package infra.

[Eric Le Bihan]

On Mon, Feb 17, 2014 at 03:37:45PM +0100, Thomas Petazzoni wrote:
> Dear Eric Le Bihan,
>
> On Mon, 17 Feb 2014 15:33:13 +0100, Eric Le Bihan wrote:
>
> > > Why are we doing this check in UDEV_CONFIGURE_CMDS ? I believe we can
> > > simply do:
> > >
> > > ifeq ($(<foo>_DEPENDENCIES),)
> > > $(error No <foo> implementation selected)
> > > endif
> > I followed the example in package/opengl/libegl/libegl.mk, but I mixed the
> > Makefile control function $(error text...) and some Shell script without
> > thinking...
> >
> > I will revert it to the pure shell version to be coherent.
>
> I guess you mean "to the pure make version", according to what you say
> below, right?
>
No. I was thinking about keeping the shell version for this series and making
the change in the new series that would update all the virtual packages, but
thinking twice about it, it is better to make it now (patches could be applied
in any order).

Best regards,
ELB

[Buildroot] [PATCH 1/2] udev: update udev to new virtual package infra.

[Eric Le Bihan]

Hi!

On Mon, Feb 17, 2014 at 03:37:45PM +0100, Thomas Petazzoni wrote:

> > With the <foo>_CONFIGURE_CMDS method, we go through all the dummy steps, and
> > this is very verbose:
> >
> >   >>> udev undefined Extracting
> >   >>> udev undefined Patching
> >   >>> udev undefined Configuring
> >   echo "No Udev implementation selected. Configuration error."
> >   No Udev implementation selected. Configuration error.
> >   exit 1
> >   make: *** [/home/eric/build/elebihan/demo-systemd/arm/build/udev-undefined/.stamp_configured] Erreur 1
> >
> > With the pure Makefile version, the error message is clearer:
> >
> >   package/udev/udev.mk:11: *** No Udev implementation selected. Configuration error. Stop.
>
> Yes, I believe this is better. And the message is given at the very
> beginning of the build, not when udev starts to be built, which is also
> nicer.
I've made some further testing and unfortunately, the pure "make" solution
does not work. The test on <foo>_DEPENDENCIES is performed when the Makefile
is evaluated, whether <foo> virtual package is selected or not.

If virtual package <foo> is not selected, <foo>_DEPENDENCIES will be empty, so
the test is true and the error message is displayed. That's not what we want.

Using <foo>_CONFIGURE_CMDS, the test will only be performed if the package is
selected.

So, I'll stick with the current method (I'll had some '@' to silence the shell
commands though).

Best regards,
ELB

[Buildroot] [PATCH 1/2] udev: update udev to new virtual package infra.

[Thomas Petazzoni]

Dear Eric Le Bihan,

On Tue, 18 Feb 2014 11:03:33 +0100, Eric Le Bihan wrote:

> I've made some further testing and unfortunately, the pure "make" solution
> does not work. The test on <foo>_DEPENDENCIES is performed when the Makefile
> is evaluated, whether <foo> virtual package is selected or not.
> 
> If virtual package <foo> is not selected, <foo>_DEPENDENCIES will be empty, so
> the test is true and the error message is displayed. That's not what we want.
> 
> Using <foo>_CONFIGURE_CMDS, the test will only be performed if the package is
> selected.

Then, just improve the test so that the test against the dependencies
is only made when the virtual package is selected:

ifeq ($(BR2_PACKAGE_<virtual package>),y)
ifeq ($(<virtual package>_DEPENDENCIES),)
$(error "Something wrong going on!")
endif
endif

Best regards,

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

[Buildroot] [PATCH 2/2] systemd: enable required kernel features.

[Eric Le Bihan]

When systemd is chosen as init system, the required kernel features are
enabled.

Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>
---
 linux/linux.mk            |    6 +++++-
 package/systemd/Config.in |   12 ++++++++++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/linux/linux.mk b/linux/linux.mk
index d7f51b3..f948e6c 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -195,7 +195,11 @@ define LINUX_CONFIGURE_CMDS
 		$(call KCONFIG_ENABLE_OPT,CONFIG_FUNCTION_TRACER,$(@D)/.config))
 	$(if $(BR2_PACKAGE_SYSTEMD),
 		$(call KCONFIG_ENABLE_OPT,CONFIG_CGROUPS,$(@D)/.config)
-		$(call KCONFIG_ENABLE_OPT,CONFIG_INOTIFY_USER,$(@D)/.config))
+		$(call KCONFIG_ENABLE_OPT,CONFIG_INOTIFY_USER,$(@D)/.config)
+		$(call KCONFIG_ENABLE_OPT,CONFIG_FHANDLE,$(@D)/.config)
+		$(call KCONFIG_ENABLE_OPT,CONFIG_AUTOFS4_FS,$(@D)/.config)
+		$(call KCONFIG_ENABLE_OPT,CONFIG_TMPFS_POSIX_ACL,$(@D)/.config)
+		$(call KCONFIG_ENABLE_OPT,CONFIG_TMPFS_POSIX_XATTR,$(@D)/.config))
 	$(if $(BR2_LINUX_KERNEL_APPENDED_DTB),
 		$(call KCONFIG_ENABLE_OPT,CONFIG_ARM_APPENDED_DTB,$(@D)/.config))
 	yes '' | $(TARGET_MAKE_ENV) $(MAKE1) $(LINUX_MAKE_FLAGS) -C $(@D) oldconfig
diff --git a/package/systemd/Config.in b/package/systemd/Config.in
index eacc23c..716da2c 100644
--- a/package/systemd/Config.in
+++ b/package/systemd/Config.in
@@ -26,8 +26,16 @@ config BR2_PACKAGE_SYSTEMD
 	  elaborate transactional dependency-based service control logic.
 	  It can work as a drop-in replacement for sysvinit.
 
-	  Systemd requires a Linux kernel >= 3.0, with inotify, devtmpfs,
-	  tmpfs vfs and tmpfs POSIX ACL enabled.
+	  Systemd requires a Linux kernel >= 3.0.
+
+	  The following features will be enabled in the kernel:
+
+	  - CONFIG_CGROUPS
+	  - CONFIG_INOTIFY_USER
+	  - CONFIG_FHANDLE
+	  - CONFIG_AUTOFS4_FS
+	  - CONFIG_TMPFS_POSIX_ACL
+	  - CONFIG_TMPFS_XATTR
 
 	  Systemd also provides udev, the userspace device daemon.
 
-- 
1.7.9.5

[Buildroot] [PATCH 2/2] systemd: enable required kernel features.

[Thomas Petazzoni]

Dear Eric Le Bihan,

On Mon, 17 Feb 2014 11:22:16 +0100, Eric Le Bihan wrote:

> diff --git a/package/systemd/Config.in b/package/systemd/Config.in
> index eacc23c..716da2c 100644
> --- a/package/systemd/Config.in
> +++ b/package/systemd/Config.in
> @@ -26,8 +26,16 @@ config BR2_PACKAGE_SYSTEMD
>  	  elaborate transactional dependency-based service control logic.
>  	  It can work as a drop-in replacement for sysvinit.
>  
> -	  Systemd requires a Linux kernel >= 3.0, with inotify, devtmpfs,
> -	  tmpfs vfs and tmpfs POSIX ACL enabled.
> +	  Systemd requires a Linux kernel >= 3.0.
> +
> +	  The following features will be enabled in the kernel:
> +
> +	  - CONFIG_CGROUPS
> +	  - CONFIG_INOTIFY_USER
> +	  - CONFIG_FHANDLE
> +	  - CONFIG_AUTOFS4_FS
> +	  - CONFIG_TMPFS_POSIX_ACL
> +	  - CONFIG_TMPFS_XATTR

This formulation is not correct for people who build their kernel
outside of Buildroot (while the earlier formulation worked for both
situations).

What about something like:

	  systemd requires the following options to be enabled in the
	  kernel:

	   - CONFIG_<foo>
	   - CONFIG_<bar>

	  These options will be automatically enabled by Buildroot if
	  it is responsible for building the kernel. Otherwise, if you
	  are building your kernel outside of Buildroot, make sure
	  these options are enabled.

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

[Buildroot] [PATCH 2/2] systemd: enable required kernel features.

[Eric Le Bihan]

Hi!

On Mon, Feb 17, 2014 at 11:57:21AM +0100, Thomas Petazzoni wrote:
> Dear Eric Le Bihan,
>
> On Mon, 17 Feb 2014 11:22:16 +0100, Eric Le Bihan wrote:
>
> > diff --git a/package/systemd/Config.in b/package/systemd/Config.in
> > index eacc23c..716da2c 100644
> > --- a/package/systemd/Config.in
> > +++ b/package/systemd/Config.in
> > @@ -26,8 +26,16 @@ config BR2_PACKAGE_SYSTEMD
> >  	  elaborate transactional dependency-based service control logic.
> >  	  It can work as a drop-in replacement for sysvinit.
> >
> > -	  Systemd requires a Linux kernel >= 3.0, with inotify, devtmpfs,
> > -	  tmpfs vfs and tmpfs POSIX ACL enabled.
> > +	  Systemd requires a Linux kernel >= 3.0.
> > +
> > +	  The following features will be enabled in the kernel:
> > +
> > +	  - CONFIG_CGROUPS
> > +	  - CONFIG_INOTIFY_USER
> > +	  - CONFIG_FHANDLE
> > +	  - CONFIG_AUTOFS4_FS
> > +	  - CONFIG_TMPFS_POSIX_ACL
> > +	  - CONFIG_TMPFS_XATTR
>
> This formulation is not correct for people who build their kernel
> outside of Buildroot (while the earlier formulation worked for both
> situations).
>
> What about something like:
>
> 	  systemd requires the following options to be enabled in the
> 	  kernel:
>
> 	   - CONFIG_<foo>
> 	   - CONFIG_<bar>
>
> 	  These options will be automatically enabled by Buildroot if
> 	  it is responsible for building the kernel. Otherwise, if you
> 	  are building your kernel outside of Buildroot, make sure
> 	  these options are enabled.
>
Thanks for the suggestion. I'll add it (the more information, the better).

Best regards,
ELB