[Buildroot] [PATCH v6 3/5] udev: convert to virtual package.

Thomas De Schampheleire patrickdepinguin at gmail.com
Wed Feb 5 12:31:14 UTC 2014


Hi Eric,

On Tue, Feb 4, 2014 at 1:10 PM, Eric Le Bihan <eric.le.bihan.dev at free.fr> wrote:
> Thomas, All,
>
> On Mon, Feb 03, 2014 at 10:41:03PM +0100, Thomas De Schampheleire wrote:
>
>> > --- a/package/systemd/Config.in
>> > +++ b/package/systemd/Config.in
>> > @@ -1,6 +1,6 @@
>> >  config BR2_PACKAGE_SYSTEMD
>> >         bool "systemd"
>> > -       depends on BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_UDEV
>> > +       depends on BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_EUDEV
>> >         depends on BR2_INET_IPV6
>> >         depends on BR2_TOOLCHAIN_HAS_THREADS # dbus
>> >         depends on BR2_USE_MMU # dbus
>> > @@ -18,7 +18,7 @@ config BR2_PACKAGE_SYSTEMD
>> >
>> >           http://freedesktop.org/wiki/Software/systemd
>> >
>> > -comment "systemd needs udev /dev management and a toolchain w/ IPv6, threads"
>> > +comment "systemd needs eudev /dev management and a toolchain w/ IPv6, threads"
>> >         depends on BR2_USE_MMU
>> > -       depends on !BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_UDEV || !BR2_INET_IPV6 || \
>> > +       depends on !BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_EUDEV || !BR2_INET_IPV6 || \\
>> >                 !BR2_TOOLCHAIN_HAS_THREADS
>>
>> These changes are odd: you indicate here that systemd is depending on
>> eudev, while eudev is precisely an alternative provider of udev than
>> systemd. So:
>>
>>        udev
>>        /   \
>> systemd     eudev
>>
>> and there is no direct relation between systemd and eudev. In the next
>> patch, you replace this again with BR2_INIT_SYSTEMD, which makes sense
>> again, but the transition is odd.
>
> At this stage, the version of systemd in Buildroot is still v44, which does
> not provide udev, but needs it. As the previous patch introduced eudev and
> this one removes BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_UDEV, the dependency on
> BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_EUDEV ensures systemd will be installed
> with a udev implementation.

Ok, understood.

>
>> Directly related to this, we question the need to rename
>> BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_UDEV to
>> BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_EUDEV. First of all, the symbol
>> BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_UDEV is already existing in users'
>> defconfig. Removing it requires the addition of a new legacy symbol
>> (not only for BR2_PACKAGE_UDEV, which you already added, but also for
>> the DEVICE_CREATION_DYNAMIC one).
>> Users that previously selected udev, should be moved to eudev, rather
>> than systemd. Our suggestion is to keep the existing
>> BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_UDEV symbol to indicate the eudev
>> choice, rendering the massive rename in the previous patch unneeded,
>> removing the above odd systemd transition, and avoiding the need for
>> legacy handling. What do you think of that?
>
> I find it odd that some packages depend on
> BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_UDEV whereas others depend on
> BR2_PACKAGE_UDEV. To me, both symbols are interpreted as "the package needs a
> udev daemon to talk to at runtime" (whether via D-Bus or libudev). So I
> replaced both with BR2_PACKAGE_HAS_UDEV to be homogeneous. It is true that if
> a package depends on BR2_PACKAGE_HAS_UDEV, it is not explicitly said if it is
> a runtime or a build dependency. But, AFAIK, programs that want to communicate
> with the udev daemon do it via libudev/libgudev, so we end up with a build
> dependency.
>
> In system/Config.in, I removed BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_UDEV because
> I want the user to understand the change: he/she should not think of 'udev' as
> a package, but as a feature which, at this stage, is only available via eudev.
> I could have kept BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_UDEV and have it select
> BR2_PACKAGE_EUDEV, but I've found it confusing.
>
> But I should have added BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_UDEV in
> Config.in.legacy to migrate to BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_EUDEV.
>
> In the end, the user selects BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_EUDEV, which
> selects BR2_PACKAGE_EUDEV, which in turn selects BR2_PACKAGE_HAS_UDEV. All the
> packages depending on it are now available.

Based on your additional input, and rechecking the changes, I
understand your reasoning and accept it.
You still should add the legacy symbol, though.

Peter, what about you?

>
>> Maybe you could add a comment in udev/Config.in and udev/udev.mk
>> clarifying that this is now a virtual package.
> Yes, indeed!
>
>> > -comment "udisks needs udev /dev management and a toolchain w/ wchar, threads"
>> > +comment "udisks needs udev /dev mgmnt, toolchain w/ wchar, threads"
>>
>> This string is not as it is described in the manual. Why did you change it?
> It was truncated on a 80x24 terminal.

In the past I have done extensive efforts in unifying all comments in
our Config files, and described the unified syntax in the manual. We
tried using 'short' strings so they would fit on a 80 char terminal.
For packages that have many dependencies, it is true that some words
may not necessarily fit.
In this case, we could decide to split the comment in two: one for the
/dev management and one for the toolchain options. Both comments
should depend on the common options (!avr32 and mmu).

So:
comment "udisks needs udev /dev management
        depends on !BR2_avr32
        depends on BR2_USE_MMU
        depends on !BR2_PACKAGE_HAS_UDEV

comment "udisks needs a toolchain w/ wchar, threads"
        depends on !BR2_avr32
        depends on BR2_USE_MMU
        depends on !BR2_USE_WCHAR || !BR2_TOOLCHAIN_HAS_THREADS

Best regards,
Thomas



More information about the buildroot mailing list