diff mbox

[v2,6/6] udisks: new package

Message ID 1357591399-3566-7-git-send-email-marek.belisko@open-nandra.com
State Superseded
Headers show

Commit Message

Marek Belisko Jan. 7, 2013, 8:43 p.m. UTC 
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Marek Belisko <marek.belisko@open-nandra.com>
---
 package/Config.in        |    1 +
 package/udisks/Config.in |   32 ++++++++++++++++++++++++++++++++
 package/udisks/udisks.mk |   28 ++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+)
 create mode 100644 package/udisks/Config.in
 create mode 100644 package/udisks/udisks.mk

Comments

Thomas Petazzoni Jan. 7, 2013, 9:47 p.m. UTC | #1 
Dear Marek Belisko,

On Mon,  7 Jan 2013 21:43:19 +0100, Marek Belisko wrote:

> --- /dev/null
> +++ b/package/udisks/Config.in
> @@ -0,0 +1,32 @@
> +config BR2_PACKAGE_UDISKS
> +	bool "udisks"
> +	depends on BR2_PACKAGE_UDEV
> +	depends on BR2_PACKAGE_DBUS
> +	select BR2_PACKAGE_SG3_UTILS
> +	select BR2_PACKAGE_UDEV_ALL_EXTRAS
> +	select BR2_PACKAGE_DBUS_GLIB
> +	select BR2_PACKAGE_POLKIT
> +	select BR2_PACKAGE_PARTED #select BR2_PACKAGE_LVM2
> +	select BR2_PACKAGE_LVM2_DMSETUP_ONLY #devmapper library

If you select BR2_PACKAGE_LVM2_DMSETUP_ONLY, then you also have to
select BR2_PACKAGE_LVM2.

> +	select BR2_PACKAGE_LIBATASMART

Also, you're doing a lot of select here. Please verify that all the
configuration options you're selecting don't have depends on
dependencies that aren't already brought through the dependency on dbus
and udev here.

> +	help
> +	  The udisks project provides
> +
> +	   o A storage daemon that implements well-defined D-Bus
> +	     interfaces that can be used to query and manipulate
> +	     storage devices.
> +
> +	   o a command-line tool, udisks(1), that can be used to query
> +	     and use the daemon
> +
> +	  http://www.freedesktop.org/wiki/Software/udisks
> +
> +config BR2_PACKAGE_UDISKS_LVM2
> +	bool "lvm2 support"
> +	depends on BR2_PACKAGE_UDISKS
> +	depends on BR2_PACKAGE_LVM2
> +	select BR2_PACKAGE_LVM2_APP_LIBRARY
> +	default n

BR2_PACKAGE_LVM2 is necessarily defined if
BR2_PACKAGE_LVM2_DMSETUP_ONLY is selected by the main
BR2_PACKAGE_UDISKS option.

If I understand correctly, you can build:

 * udisks without the lvm2 library, but it needs the dmsetup binary
   unconditionally (which sounds strange, isn't capable of supporting
   setups without LVM stuff)

 * udisks with the lvm2 library

Not sure how to handle that properly with the current BR2_PACKAGE_LVM2
options. Needs more thought.

> +#############################################################
> +#
> +# udisks
> +#
> +#############################################################
> +UDISKS_VERSION = 1.0.4
> +UDISKS_SITE    = http://hal.freedesktop.org/releases/
> +UDISKS_LICENCE = GPLv2+
> +UDISKS_LICENCE_FILES = COPYING

LICENSE.

Thomas
Belisko Marek Jan. 7, 2013, 10:03 p.m. UTC | #2 
Hi Thomas,

On Mon, Jan 7, 2013 at 10:47 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Marek Belisko,
>
> On Mon,  7 Jan 2013 21:43:19 +0100, Marek Belisko wrote:
>
>> --- /dev/null
>> +++ b/package/udisks/Config.in
>> @@ -0,0 +1,32 @@
>> +config BR2_PACKAGE_UDISKS
>> +     bool "udisks"
>> +     depends on BR2_PACKAGE_UDEV
>> +     depends on BR2_PACKAGE_DBUS
>> +     select BR2_PACKAGE_SG3_UTILS
>> +     select BR2_PACKAGE_UDEV_ALL_EXTRAS
>> +     select BR2_PACKAGE_DBUS_GLIB
>> +     select BR2_PACKAGE_POLKIT
>> +     select BR2_PACKAGE_PARTED #select BR2_PACKAGE_LVM2
>> +     select BR2_PACKAGE_LVM2_DMSETUP_ONLY #devmapper library
>
> If you select BR2_PACKAGE_LVM2_DMSETUP_ONLY, then you also have to
> select BR2_PACKAGE_LVM2.
This is selected conditionally when LVM2 support if selected.
>
>> +     select BR2_PACKAGE_LIBATASMART
>
> Also, you're doing a lot of select here. Please verify that all the
> configuration options you're selecting don't have depends on
> dependencies that aren't already brought through the dependency on dbus
> and udev here.
>
>> +     help
>> +       The udisks project provides
>> +
>> +        o A storage daemon that implements well-defined D-Bus
>> +          interfaces that can be used to query and manipulate
>> +          storage devices.
>> +
>> +        o a command-line tool, udisks(1), that can be used to query
>> +          and use the daemon
>> +
>> +       http://www.freedesktop.org/wiki/Software/udisks
>> +
>> +config BR2_PACKAGE_UDISKS_LVM2
>> +     bool "lvm2 support"
>> +     depends on BR2_PACKAGE_UDISKS
>> +     depends on BR2_PACKAGE_LVM2
>> +     select BR2_PACKAGE_LVM2_APP_LIBRARY
>> +     default n
>
> BR2_PACKAGE_LVM2 is necessarily defined if
> BR2_PACKAGE_LVM2_DMSETUP_ONLY is selected by the main
> BR2_PACKAGE_UDISKS option.
>
> If I understand correctly, you can build:
>
>  * udisks without the lvm2 library, but it needs the dmsetup binary
>    unconditionally (which sounds strange, isn't capable of supporting
>    setups without LVM stuff)
It is weird but configure phase require devmapper library (otherwise
fails) in both case (udisks with/without lvm2 support)
>
>  * udisks with the lvm2 library
>
> Not sure how to handle that properly with the current BR2_PACKAGE_LVM2
> options. Needs more thought.
>
>> +#############################################################
>> +#
>> +# udisks
>> +#
>> +#############################################################
>> +UDISKS_VERSION = 1.0.4
>> +UDISKS_SITE    = http://hal.freedesktop.org/releases/
>> +UDISKS_LICENCE = GPLv2+
>> +UDISKS_LICENCE_FILES = COPYING
>
> LICENSE.
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com

Regards,

mbe
Thomas Petazzoni Jan. 7, 2013, 10:18 p.m. UTC | #3 
Dear Belisko Marek,

On Mon, 7 Jan 2013 23:03:10 +0100, Belisko Marek wrote:
> >> +     select BR2_PACKAGE_PARTED #select BR2_PACKAGE_LVM2
> >> +     select BR2_PACKAGE_LVM2_DMSETUP_ONLY #devmapper library  
> >
> > If you select BR2_PACKAGE_LVM2_DMSETUP_ONLY, then you also have to
> > select BR2_PACKAGE_LVM2.  
> This is selected conditionally when LVM2 support if selected.

This is not what your patch does:

+config BR2_PACKAGE_UDISKS
+	bool "udisks"
+	depends on BR2_PACKAGE_UDEV
+	depends on BR2_PACKAGE_DBUS
+	select BR2_PACKAGE_SG3_UTILS
+	select BR2_PACKAGE_UDEV_ALL_EXTRAS
+	select BR2_PACKAGE_DBUS_GLIB
+	select BR2_PACKAGE_POLKIT
+	select BR2_PACKAGE_PARTED #select BR2_PACKAGE_LVM2
+	select BR2_PACKAGE_LVM2_DMSETUP_ONLY #devmapper library
+	select BR2_PACKAGE_LIBATASMART

The select of BR2_PACKAGE_LVM2 is commented.

Thomas
Belisko Marek Jan. 7, 2013, 10:25 p.m. UTC | #4 
Hi Thomas,

On Mon, Jan 7, 2013 at 11:18 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Belisko Marek,
>
> On Mon, 7 Jan 2013 23:03:10 +0100, Belisko Marek wrote:
>> >> +     select BR2_PACKAGE_PARTED #select BR2_PACKAGE_LVM2
>> >> +     select BR2_PACKAGE_LVM2_DMSETUP_ONLY #devmapper library
>> >
>> > If you select BR2_PACKAGE_LVM2_DMSETUP_ONLY, then you also have to
>> > select BR2_PACKAGE_LVM2.
>> This is selected conditionally when LVM2 support if selected.
>
> This is not what your patch does:
>
> +config BR2_PACKAGE_UDISKS
> +       bool "udisks"
> +       depends on BR2_PACKAGE_UDEV
> +       depends on BR2_PACKAGE_DBUS
> +       select BR2_PACKAGE_SG3_UTILS
> +       select BR2_PACKAGE_UDEV_ALL_EXTRAS
> +       select BR2_PACKAGE_DBUS_GLIB
> +       select BR2_PACKAGE_POLKIT
> +       select BR2_PACKAGE_PARTED #select BR2_PACKAGE_LVM2
> +       select BR2_PACKAGE_LVM2_DMSETUP_ONLY #devmapper library
> +       select BR2_PACKAGE_LIBATASMART
>
> The select of BR2_PACKAGE_LVM2 is commented.
No this is comment that BR2_PACKAGE_PARTED select also BR2_PACKAGE_LVM2.
Maybe this is wrong approach but it works :).
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com

Regards,

mbe
Thomas Petazzoni Jan. 7, 2013, 10:49 p.m. UTC | #5 
Dear Belisko Marek,

On Mon, 7 Jan 2013 23:25:30 +0100, Belisko Marek wrote:

> > The select of BR2_PACKAGE_LVM2 is commented.
> No this is comment that BR2_PACKAGE_PARTED select also BR2_PACKAGE_LVM2.
> Maybe this is wrong approach but it works :).

Ah, ok. Then please make the "select BR2_PACKAGE_LVM2" explicit, since
udisks needs it directly.

But then, you can get rid of your "depends on BR2_PACKAGE_LVM2" in
BR2_PACKAGE_UDISKS_LVM2, because the lvm2 package is surely already
enabled, since it gets selected by udisks.

I still don't like those lvm2 configuration options that much, though...

Thomas
diff mbox

Patch

diff --git a/package/Config.in b/package/Config.in
index 53bb5be..5968282 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -272,6 +272,7 @@  source "package/sysstat/Config.in"
 source "package/ti-utils/Config.in"
 source "package/uboot-tools/Config.in"
 source "package/udev/Config.in"
+source "package/udisks/Config.in"
 source "package/usb_modeswitch/Config.in"
 source "package/usb_modeswitch_data/Config.in"
 source "package/usbmount/Config.in"
diff --git a/package/udisks/Config.in b/package/udisks/Config.in
new file mode 100644
index 0000000..bdc99b3
--- /dev/null
+++ b/package/udisks/Config.in
@@ -0,0 +1,32 @@ 
+config BR2_PACKAGE_UDISKS
+	bool "udisks"
+	depends on BR2_PACKAGE_UDEV
+	depends on BR2_PACKAGE_DBUS
+	select BR2_PACKAGE_SG3_UTILS
+	select BR2_PACKAGE_UDEV_ALL_EXTRAS
+	select BR2_PACKAGE_DBUS_GLIB
+	select BR2_PACKAGE_POLKIT
+	select BR2_PACKAGE_PARTED #select BR2_PACKAGE_LVM2
+	select BR2_PACKAGE_LVM2_DMSETUP_ONLY #devmapper library
+	select BR2_PACKAGE_LIBATASMART
+	help
+	  The udisks project provides
+
+	   o A storage daemon that implements well-defined D-Bus
+	     interfaces that can be used to query and manipulate
+	     storage devices.
+
+	   o a command-line tool, udisks(1), that can be used to query
+	     and use the daemon
+
+	  http://www.freedesktop.org/wiki/Software/udisks
+
+config BR2_PACKAGE_UDISKS_LVM2
+	bool "lvm2 support"
+	depends on BR2_PACKAGE_UDISKS
+	depends on BR2_PACKAGE_LVM2
+	select BR2_PACKAGE_LVM2_APP_LIBRARY
+	default n
+
+comment "udisks requires udev and dbus to be enabled"
+	depends on !BR2_PACKAGE_UDEV || !BR2_PACKAGE_DBUS
diff --git a/package/udisks/udisks.mk b/package/udisks/udisks.mk
new file mode 100644
index 0000000..19f9da4
--- /dev/null
+++ b/package/udisks/udisks.mk
@@ -0,0 +1,28 @@ 
+#############################################################
+#
+# udisks
+#
+#############################################################
+UDISKS_VERSION = 1.0.4
+UDISKS_SITE    = http://hal.freedesktop.org/releases/
+UDISKS_LICENCE = GPLv2+
+UDISKS_LICENCE_FILES = COPYING
+
+UDISKS_DEPENDENCIES = 	\
+	sg3_utils	\
+	host-pkgconf	\
+	udev		\
+	dbus		\
+	dbus-glib	\
+	polkit		\
+	parted		\
+	lvm2		\
+	libatasmart
+
+UDISKS_CONF_OPT = --disable-remote-access --disable-gtk-doc
+
+ifeq ($(BR2_PACKAGE_UDISKS_LVM2),y)
+UDISKS_CONF_OPT += --enable-lvm2
+endif
+
+$(eval $(autotools-package))