diff mbox

[1/1] Add more options to eudev package.

Message ID CAOcYpt3=T60_0NWWaCPsidFOC_vcZrCcGv5s9VH+AHJnb0ckjw@mail.gmail.com
State Changes Requested
Headers show

Commit Message

David Kosir Oct. 21, 2015, 11:42 a.m. UTC
From aac359a0f280476ba9654e995dd4fd5565ffc22f Mon Sep 17 00:00:00 2001
From: David Kosir <david.kosir@bylapis.com>
Date: Wed, 21 Oct 2015 12:06:58 +0200
Subject: [PATCH 1/1] Don't force kmod with eudev package

Signed-off-by: David Kosir <david.kosir@bylapis.com>
---
 package/eudev/Config.in |  1 -
 package/eudev/eudev.mk  | 12 +++++++++---
 2 files changed, 9 insertions(+), 4 deletions(-)

Comments

David Kosir Oct. 21, 2015, 11:50 a.m. UTC | #1
Yann,

I've sent first patch, for kmod dependency.

I've tried to make second patch better, but realized that it is more
complicated than I thought.
Best solution I came with to add --libdir=/lib to EUDEV_CONF_OPTS
I see that we already specify --libexecdir=/lib
That option would allow to use separate /usr, at least to start udev
normaly at boot, before /usr is mounted.
I think that is good option as it would not break anything (at least
what I see).
Do you have better idea?

--
David
Thomas Petazzoni Oct. 21, 2015, 7:24 p.m. UTC | #2
Hello David,

On Wed, 21 Oct 2015 13:50:17 +0200, David Kosir wrote:

> I've tried to make second patch better, but realized that it is more
> complicated than I thought.
> Best solution I came with to add --libdir=/lib to EUDEV_CONF_OPTS
> I see that we already specify --libexecdir=/lib
> That option would allow to use separate /usr, at least to start udev
> normaly at boot, before /usr is mounted.
> I think that is good option as it would not break anything (at least
> what I see).
> Do you have better idea?

While I understand what you are trying to do, I am not sure we can
support this in Buildroot upstream. You want eudev to be in / and
not /usr, but maybe the next person will want this other package to be
in / and not /usr. How do we handle this ? Add one more option to all
packages ? Doesn't seem really maintainable.

I don't really have a good suggestion on how to solve this problem, but
I'm pretty sure that adding an option to the eudev package is the right
solution.

Best regards,

Thomas
Arnout Vandecappelle Oct. 21, 2015, 7:37 p.m. UTC | #3
On 21-10-15 13:42, David Kosir wrote:
>>From aac359a0f280476ba9654e995dd4fd5565ffc22f Mon Sep 17 00:00:00 2001
> From: David Kosir <david.kosir@bylapis.com>
> Date: Wed, 21 Oct 2015 12:06:58 +0200
> Subject: [PATCH 1/1] Don't force kmod with eudev package

 Hi David,

 This patch looks good, but was not sent properly and did not appear in our
patch tracking system. Can you resubmit properly, using git send-email?

 Groeten,
 Arnout

> 
> Signed-off-by: David Kosir <david.kosir@bylapis.com>
> ---
>  package/eudev/Config.in |  1 -
>  package/eudev/eudev.mk  | 12 +++++++++---
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/package/eudev/Config.in b/package/eudev/Config.in
> index 76df409..10f24c0 100644
> --- a/package/eudev/Config.in
> +++ b/package/eudev/Config.in
> @@ -7,7 +7,6 @@ config BR2_PACKAGE_EUDEV
>      select BR2_PACKAGE_HAS_UDEV
>      select BR2_PACKAGE_UTIL_LINUX
>      select BR2_PACKAGE_UTIL_LINUX_LIBBLKID
> -    select BR2_PACKAGE_KMOD
>      help
>        Userspace device daemon. This is a standalone version,
>        independent of systemd. It is a fork maintained by Gentoo.
> diff --git a/package/eudev/eudev.mk b/package/eudev/eudev.mk
> index a005f45..455ce25 100644
> --- a/package/eudev/eudev.mk
> +++ b/package/eudev/eudev.mk
> @@ -24,12 +24,18 @@ EUDEV_CONF_OPTS =        \
>      --sbindir=/sbin        \
>      --libexecdir=/lib    \
>      --with-firmware-path=/lib/firmware    \
> -    --disable-introspection            \
> -    --enable-libkmod
> +    --disable-introspection
> 
> -EUDEV_DEPENDENCIES = host-gperf host-pkgconf util-linux kmod
> +EUDEV_DEPENDENCIES = host-gperf host-pkgconf util-linux
>  EUDEV_PROVIDES = udev
> 
> +ifeq ($(BR2_PACKAGE_KMOD),y)
> +EUDEV_DEPENDENCIES += kmod
> +EUDEV_CONF_OPTS += --enable-libkmod
> +else
> +EUDEV_CONF_OPTS += --disable-libkmod
> +endif
> +
>  ifeq ($(BR2_ROOTFS_MERGED_USR),)
>  EUDEV_CONF_OPTS += --with-rootlibdir=/lib --enable-split-usr
>  endif
>
Arnout Vandecappelle Oct. 21, 2015, 8 p.m. UTC | #4
On 21-10-15 21:24, Thomas Petazzoni wrote:
> Hello David,
> 
> On Wed, 21 Oct 2015 13:50:17 +0200, David Kosir wrote:
> 
>> I've tried to make second patch better, but realized that it is more
>> complicated than I thought.
>> Best solution I came with to add --libdir=/lib to EUDEV_CONF_OPTS
>> I see that we already specify --libexecdir=/lib
>> That option would allow to use separate /usr, at least to start udev
>> normaly at boot, before /usr is mounted.
>> I think that is good option as it would not break anything (at least
>> what I see).
>> Do you have better idea?
> 
> While I understand what you are trying to do, I am not sure we can
> support this in Buildroot upstream. You want eudev to be in / and
> not /usr, but maybe the next person will want this other package to be
> in / and not /usr. How do we handle this ? Add one more option to all
> packages ? Doesn't seem really maintainable.

 Actually, it makes sense to me to install it completely in / instead of /usr -
that would also remove the need to have all the --libdir etc. options that we
currently pass. The way it's currently done, eudev's install anyway adds
symlinks back from /lib/libudev.so to /usr/lib/libudev.so.

 Of course, includes and pkgconfig still have to be put in /usr.


 Regards,
 Arnout

> 
> I don't really have a good suggestion on how to solve this problem, but
> I'm pretty sure that adding an option to the eudev package is the right
> solution.
> 
> Best regards,
> 
> Thomas
>
Thomas Petazzoni Oct. 21, 2015, 8:18 p.m. UTC | #5
Hello,

On Wed, 21 Oct 2015 22:00:35 +0200, Arnout Vandecappelle wrote:

> > While I understand what you are trying to do, I am not sure we can
> > support this in Buildroot upstream. You want eudev to be in / and
> > not /usr, but maybe the next person will want this other package to be
> > in / and not /usr. How do we handle this ? Add one more option to all
> > packages ? Doesn't seem really maintainable.
> 
>  Actually, it makes sense to me to install it completely in / instead of /usr -
> that would also remove the need to have all the --libdir etc. options that we
> currently pass. The way it's currently done, eudev's install anyway adds
> symlinks back from /lib/libudev.so to /usr/lib/libudev.so.

If we install it unconditionally in /, then it's fine with me. What I
would dislike is to start having options for such things.

However, there is still the question: why eudev in /, and not any other
package that other users may find important to have in / ? Where do we
put the boundary ?

Best regards,

Thomas
Arnout Vandecappelle Oct. 21, 2015, 8:39 p.m. UTC | #6
On 21-10-15 22:18, Thomas Petazzoni wrote:
> Hello,
> 
> On Wed, 21 Oct 2015 22:00:35 +0200, Arnout Vandecappelle wrote:
> 
>>> While I understand what you are trying to do, I am not sure we can
>>> support this in Buildroot upstream. You want eudev to be in / and
>>> not /usr, but maybe the next person will want this other package to be
>>> in / and not /usr. How do we handle this ? Add one more option to all
>>> packages ? Doesn't seem really maintainable.
>>
>>  Actually, it makes sense to me to install it completely in / instead of /usr -
>> that would also remove the need to have all the --libdir etc. options that we
>> currently pass. The way it's currently done, eudev's install anyway adds
>> symlinks back from /lib/libudev.so to /usr/lib/libudev.so.
> 
> If we install it unconditionally in /, then it's fine with me. What I
> would dislike is to start having options for such things.
> 
> However, there is still the question: why eudev in /, and not any other
> package that other users may find important to have in / ? Where do we
> put the boundary ?

 Because eudev is already installed mostly in / - as I said, even if libudev.so
is installed in /usr/lib, there's still a symlink created to point to it from
/lib. Same for udevadm: installed in /usr/bin but a symlink is created from
/bin. Basically, it's eudev itself that decides it should be in / in addition to
/usr.

 That said, I don't think David's use case is one that we should support,
because IMHO it's the wrong way to go at it. I think it's better to make a
separate full filesystem and either pivot_root or union-mount it. I think
depending on the /bin or /usr/bin location is a recipe for disaster.

 Regards,
 Arnout
David Kosir Oct. 22, 2015, 6:53 a.m. UTC | #7
>> However, there is still the question: why eudev in /, and not any other
>> package that other users may find important to have in / ? Where do we
>> put the boundary ?
>
>  Because eudev is already installed mostly in / - as I said, even if libudev.so
> is installed in /usr/lib, there's still a symlink created to point to it from
> /lib. Same for udevadm: installed in /usr/bin but a symlink is created from
> /bin. Basically, it's eudev itself that decides it should be in / in addition to
> /usr.

Exactly, you can check in src/libudevMakefile, install-exec-hook.

>  This patch looks good, but was not sent properly and did not appear in our
> patch tracking system. Can you resubmit properly, using git send-email?

I will try it. I just copy-pasted in into gmail as send-mail was
complaining about something regarding ssl. I guess that is why it
didn't work.

--
David
Arnout Vandecappelle Oct. 22, 2015, 7:51 a.m. UTC | #8
On 22-10-15 08:53, David Kosir wrote:
>>> However, there is still the question: why eudev in /, and not any other
>>> package that other users may find important to have in / ? Where do we
>>> put the boundary ?
>>
>>  Because eudev is already installed mostly in / - as I said, even if libudev.so
>> is installed in /usr/lib, there's still a symlink created to point to it from
>> /lib. Same for udevadm: installed in /usr/bin but a symlink is created from
>> /bin. Basically, it's eudev itself that decides it should be in / in addition to
>> /usr.
> 
> Exactly, you can check in src/libudevMakefile, install-exec-hook.
> 
>>  This patch looks good, but was not sent properly and did not appear in our
>> patch tracking system. Can you resubmit properly, using git send-email?
> 
> I will try it. I just copy-pasted in into gmail as send-mail was
> complaining about something regarding ssl. I guess that is why it
> didn't work.

'git help send-email' gives a very good explanation about how to configure gmail.

 Regards,
 Arnout
diff mbox

Patch

diff --git a/package/eudev/Config.in b/package/eudev/Config.in
index 76df409..10f24c0 100644
--- a/package/eudev/Config.in
+++ b/package/eudev/Config.in
@@ -7,7 +7,6 @@  config BR2_PACKAGE_EUDEV
     select BR2_PACKAGE_HAS_UDEV
     select BR2_PACKAGE_UTIL_LINUX
     select BR2_PACKAGE_UTIL_LINUX_LIBBLKID
-    select BR2_PACKAGE_KMOD
     help
       Userspace device daemon. This is a standalone version,
       independent of systemd. It is a fork maintained by Gentoo.
diff --git a/package/eudev/eudev.mk b/package/eudev/eudev.mk
index a005f45..455ce25 100644
--- a/package/eudev/eudev.mk
+++ b/package/eudev/eudev.mk
@@ -24,12 +24,18 @@  EUDEV_CONF_OPTS =        \
     --sbindir=/sbin        \
     --libexecdir=/lib    \
     --with-firmware-path=/lib/firmware    \
-    --disable-introspection            \
-    --enable-libkmod
+    --disable-introspection

-EUDEV_DEPENDENCIES = host-gperf host-pkgconf util-linux kmod
+EUDEV_DEPENDENCIES = host-gperf host-pkgconf util-linux
 EUDEV_PROVIDES = udev

+ifeq ($(BR2_PACKAGE_KMOD),y)
+EUDEV_DEPENDENCIES += kmod
+EUDEV_CONF_OPTS += --enable-libkmod
+else
+EUDEV_CONF_OPTS += --disable-libkmod
+endif
+
 ifeq ($(BR2_ROOTFS_MERGED_USR),)
 EUDEV_CONF_OPTS += --with-rootlibdir=/lib --enable-split-usr
 endif