Patchwork [v2,5/6] lvm2: Compile and install application library.

login
register
mail settings
Submitter Marek Belisko
Date Jan. 7, 2013, 8:43 p.m.
Message ID <1357591399-3566-6-git-send-email-marek.belisko@open-nandra.com>
Download mbox | patch
Permalink /patch/210236/
State Accepted
Headers show

Comments

Marek Belisko - Jan. 7, 2013, 8:43 p.m.
Signed-off-by: Marek Belisko <marek.belisko@open-nandra.com>
---
 package/lvm2/Config.in |    6 ++++++
 package/lvm2/lvm2.mk   |    8 +++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)
Thomas Petazzoni - Jan. 7, 2013, 9:38 p.m.
Dear Marek Belisko,

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

> +config BR2_PACKAGE_LVM2_APP_LIBRARY
> +	bool "install application library"
> +	depends on BR2_PACKAGE_LVM2
> +	help
> +	  Install application library (liblvm2app2).
> +

> +ifeq ($(BR2_PACKAGE_LVM2_APP_LIBRARY),y)
> +LVM2_MAKE_OPT += liblvm
> +LVM2_INSTALL_STAGING_OPT = DESTDIR=$(STAGING_DIR) install
> +LVM2_INSTALL_TARGET_OPT = DESTDIR=$(TARGET_DIR) install
> +endif

I am not quite sure this is correct. Currently, the package works as
follows (if I understand correctly):

 * if BR2_PACKAGE_LVM2=y but BR2_PACKAGE_LVM2_DMSETUP_ONLY is not
   enabled, then LVM2_MAKE_OPT is empty, so it just runs the default
   make target, which presumably builds everything.

 * if BR2_PACKAGE_LVM2=y and BR2_PACKAGE_LVM2_DMSETUP_ONLY=y, then
   LVM2_MAKE_OPT is set to device-mapper, in which case it only builds
   the device mapper code.

So now, you add a third BR2_PACKAGE_LVM2_APP_LIBRARY option. But if I
do BR2_PACKAGE_LVM2=y and BR2_PACKAGE_LVM2_APP_LIBRARY=y, it will only
build the library and not the rest, which is not what we expect.

I'd say that the DMSETUP_ONLY thing is a bad idea from the beginning,
so I'm not sure what's the best way moving forward.

Are you sure liblvm doesn't get build and installed by the default
BR2_PACKAGE_LVM2=y configuration?

Thomas
Belisko Marek - Jan. 7, 2013, 9:58 p.m.
Hi Thomas,

On Mon, Jan 7, 2013 at 10:38 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Marek Belisko,
>
> On Mon,  7 Jan 2013 21:43:18 +0100, Marek Belisko wrote:
>
>> +config BR2_PACKAGE_LVM2_APP_LIBRARY
>> +     bool "install application library"
>> +     depends on BR2_PACKAGE_LVM2
>> +     help
>> +       Install application library (liblvm2app2).
>> +
>
>> +ifeq ($(BR2_PACKAGE_LVM2_APP_LIBRARY),y)
>> +LVM2_MAKE_OPT += liblvm
>> +LVM2_INSTALL_STAGING_OPT = DESTDIR=$(STAGING_DIR) install
>> +LVM2_INSTALL_TARGET_OPT = DESTDIR=$(TARGET_DIR) install
>> +endif
>
> I am not quite sure this is correct. Currently, the package works as
> follows (if I understand correctly):
>
>  * if BR2_PACKAGE_LVM2=y but BR2_PACKAGE_LVM2_DMSETUP_ONLY is not
>    enabled, then LVM2_MAKE_OPT is empty, so it just runs the default
>    make target, which presumably builds everything.
>
>  * if BR2_PACKAGE_LVM2=y and BR2_PACKAGE_LVM2_DMSETUP_ONLY=y, then
>    LVM2_MAKE_OPT is set to device-mapper, in which case it only builds
>    the device mapper code.
>
> So now, you add a third BR2_PACKAGE_LVM2_APP_LIBRARY option. But if I
> do BR2_PACKAGE_LVM2=y and BR2_PACKAGE_LVM2_APP_LIBRARY=y, it will only
> build the library and not the rest, which is not what we expect.
>
> I'd say that the DMSETUP_ONLY thing is a bad idea from the beginning,
> so I'm not sure what's the best way moving forward.
>
> Are you sure liblvm doesn't get build and installed by the default
> BR2_PACKAGE_LVM2=y configuration?
Nope and this was intention of this patch to add ability to build also library.
lvm app library is necessary as dependency for udisk (if enabled lvm2
support - disabled by default).
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com

Regards,

mbe
Belisko Marek - Jan. 15, 2013, 8:53 p.m.
Hi Thomas,

On Mon, Jan 7, 2013 at 10:38 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Marek Belisko,
>
> On Mon,  7 Jan 2013 21:43:18 +0100, Marek Belisko wrote:
>
>> +config BR2_PACKAGE_LVM2_APP_LIBRARY
>> +     bool "install application library"
>> +     depends on BR2_PACKAGE_LVM2
>> +     help
>> +       Install application library (liblvm2app2).
>> +
>
>> +ifeq ($(BR2_PACKAGE_LVM2_APP_LIBRARY),y)
>> +LVM2_MAKE_OPT += liblvm
>> +LVM2_INSTALL_STAGING_OPT = DESTDIR=$(STAGING_DIR) install
>> +LVM2_INSTALL_TARGET_OPT = DESTDIR=$(TARGET_DIR) install
>> +endif
>
> I am not quite sure this is correct. Currently, the package works as
> follows (if I understand correctly):
>
>  * if BR2_PACKAGE_LVM2=y but BR2_PACKAGE_LVM2_DMSETUP_ONLY is not
>    enabled, then LVM2_MAKE_OPT is empty, so it just runs the default
>    make target, which presumably builds everything.
>
>  * if BR2_PACKAGE_LVM2=y and BR2_PACKAGE_LVM2_DMSETUP_ONLY=y, then
>    LVM2_MAKE_OPT is set to device-mapper, in which case it only builds
>    the device mapper code.
>
> So now, you add a third BR2_PACKAGE_LVM2_APP_LIBRARY option. But if I
> do BR2_PACKAGE_LVM2=y and BR2_PACKAGE_LVM2_APP_LIBRARY=y, it will only
> build the library and not the rest, which is not what we expect.
>
> I'd say that the DMSETUP_ONLY thing is a bad idea from the beginning,
> so I'm not sure what's the best way moving forward.
What about to compile everything (device mapper + app library) and
only install things which user select in
config.
>
> Are you sure liblvm doesn't get build and installed by the default
> BR2_PACKAGE_LVM2=y configuration?
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com

Thanks,

marek
Thomas Petazzoni - Jan. 16, 2013, 8:06 a.m.
Dear Belisko Marek,

On Tue, 15 Jan 2013 21:53:48 +0100, Belisko Marek wrote:

> What about to compile everything (device mapper + app library) and
> only install things which user select in
> config.

That's usually what we do in relatively small packages for which the
overall build time is not so significant to be worth the effort of
doing selective compilation. We however do selective installation if
needed to only install the required files.

Best regards,

Thomas
Belisko Marek - Jan. 16, 2013, 8:49 a.m.
Hi Thomas,

On Wed, Jan 16, 2013 at 9:06 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Belisko Marek,
>
> On Tue, 15 Jan 2013 21:53:48 +0100, Belisko Marek wrote:
>
>> What about to compile everything (device mapper + app library) and
>> only install things which user select in
>> config.
>
> That's usually what we do in relatively small packages for which the
> overall build time is not so significant to be worth the effort of
> doing selective compilation. We however do selective installation if
> needed to only install the required files.
OK. Any ideas how to proceed with this patch? Without that udisks (with lvm2)
cannot work. IMO original patch is OK despite of fact it build only
liblvm (not libdm + lvm2)

>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com

Regards,

marek
Peter Korsgaard - Feb. 3, 2013, 2:20 p.m.
>>>>> "Marek" == Marek Belisko <marek.belisko@open-nandra.com> writes:

 Marek> Signed-off-by: Marek Belisko <marek.belisko@open-nandra.com>
 Marek> ---
 Marek>  package/lvm2/Config.in |    6 ++++++
 Marek>  package/lvm2/lvm2.mk   |    8 +++++++-
 Marek>  2 files changed, 13 insertions(+), 1 deletion(-)

 Marek> diff --git a/package/lvm2/Config.in b/package/lvm2/Config.in
 Marek> index 30af14e..bb41c0a 100644
 Marek> --- a/package/lvm2/Config.in
 Marek> +++ b/package/lvm2/Config.in
 Marek> @@ -21,5 +21,11 @@ config BR2_PACKAGE_LVM2_DMSETUP_ONLY
 Marek>  	help
 Marek>  	  Install dmsetup only and skip the LVM2 suite.
 
 Marek> +config BR2_PACKAGE_LVM2_APP_LIBRARY
 Marek> +	bool "install application library"

This option doesn't make much sense in combination with DMSETUP_ONLY, so
I've made it depend on !DMSETUP_ONLY


 Marek> +	depends on BR2_PACKAGE_LVM2
 Marek> +	help
 Marek> +	  Install application library (liblvm2app2).
 Marek> +
 Marek>  comment "lvm2 requires a toolchain with LARGEFILE support"
 Marek>          depends on !BR2_LARGEFILE
 Marek> diff --git a/package/lvm2/lvm2.mk b/package/lvm2/lvm2.mk
 Marek> index f54caa4..213ef65 100644
 Marek> --- a/package/lvm2/lvm2.mk
 Marek> +++ b/package/lvm2/lvm2.mk
 Marek> @@ -21,7 +21,7 @@ LVM2_BINS = \
 
 Marek>  # Make sure that binaries and libraries are installed with write
 Marek>  # permissions for the owner.
 Marek> -LVM2_CONF_OPT += --enable-write_install --enable-pkgconfig
 Marek> +LVM2_CONF_OPT += --enable-write_install --enable-pkgconfig --enable-applib

We should only enable it if _APP_LIBRARY is enabled.


 
 Marek>  # LVM2 uses autoconf, but not automake, and the build system does not
 Marek>  # take into account the CC passed at configure time.
 Marek> @@ -41,6 +41,12 @@ LVM2_INSTALL_STAGING_OPT = DESTDIR=$(STAGING_DIR) install_device-mapper
 Marek>  LVM2_INSTALL_TARGET_OPT = DESTDIR=$(TARGET_DIR) install_device-mapper
 Marek>  endif
 
 Marek> +ifeq ($(BR2_PACKAGE_LVM2_APP_LIBRARY),y)
 Marek> +LVM2_MAKE_OPT += liblvm
 Marek> +LVM2_INSTALL_STAGING_OPT = DESTDIR=$(STAGING_DIR) install
 Marek> +LVM2_INSTALL_TARGET_OPT = DESTDIR=$(TARGET_DIR) install

And then this isn't needed.

Committed with these fixes, thanks.
Belisko Marek - Feb. 3, 2013, 8:44 p.m.
Hi Peter,

On Sun, Feb 3, 2013 at 3:20 PM, Peter Korsgaard <jacmet@uclibc.org> wrote:
>>>>>> "Marek" == Marek Belisko <marek.belisko@open-nandra.com> writes:
>
>  Marek> Signed-off-by: Marek Belisko <marek.belisko@open-nandra.com>
>  Marek> ---
>  Marek>  package/lvm2/Config.in |    6 ++++++
>  Marek>  package/lvm2/lvm2.mk   |    8 +++++++-
>  Marek>  2 files changed, 13 insertions(+), 1 deletion(-)
>
>  Marek> diff --git a/package/lvm2/Config.in b/package/lvm2/Config.in
>  Marek> index 30af14e..bb41c0a 100644
>  Marek> --- a/package/lvm2/Config.in
>  Marek> +++ b/package/lvm2/Config.in
>  Marek> @@ -21,5 +21,11 @@ config BR2_PACKAGE_LVM2_DMSETUP_ONLY
>  Marek>         help
>  Marek>           Install dmsetup only and skip the LVM2 suite.
>
>  Marek> +config BR2_PACKAGE_LVM2_APP_LIBRARY
>  Marek> +       bool "install application library"
>
> This option doesn't make much sense in combination with DMSETUP_ONLY, so
> I've made it depend on !DMSETUP_ONLY
This patch was from v2 and was dropped in v3 because it wasn't necessary.
Please see http://lists.busybox.net/pipermail/buildroot/2013-January/065740.html
I've tested it also with fresh build and it was working without this patch.
>
>
>  Marek> +       depends on BR2_PACKAGE_LVM2
>  Marek> +       help
>  Marek> +         Install application library (liblvm2app2).
>  Marek> +
>  Marek>  comment "lvm2 requires a toolchain with LARGEFILE support"
>  Marek>          depends on !BR2_LARGEFILE
>  Marek> diff --git a/package/lvm2/lvm2.mk b/package/lvm2/lvm2.mk
>  Marek> index f54caa4..213ef65 100644
>  Marek> --- a/package/lvm2/lvm2.mk
>  Marek> +++ b/package/lvm2/lvm2.mk
>  Marek> @@ -21,7 +21,7 @@ LVM2_BINS = \
>
>  Marek>  # Make sure that binaries and libraries are installed with write
>  Marek>  # permissions for the owner.
>  Marek> -LVM2_CONF_OPT += --enable-write_install --enable-pkgconfig
>  Marek> +LVM2_CONF_OPT += --enable-write_install --enable-pkgconfig --enable-applib
>
> We should only enable it if _APP_LIBRARY is enabled.
>
>
>
>  Marek>  # LVM2 uses autoconf, but not automake, and the build system does not
>  Marek>  # take into account the CC passed at configure time.
>  Marek> @@ -41,6 +41,12 @@ LVM2_INSTALL_STAGING_OPT = DESTDIR=$(STAGING_DIR) install_device-mapper
>  Marek>  LVM2_INSTALL_TARGET_OPT = DESTDIR=$(TARGET_DIR) install_device-mapper
>  Marek>  endif
>
>  Marek> +ifeq ($(BR2_PACKAGE_LVM2_APP_LIBRARY),y)
>  Marek> +LVM2_MAKE_OPT += liblvm
>  Marek> +LVM2_INSTALL_STAGING_OPT = DESTDIR=$(STAGING_DIR) install
>  Marek> +LVM2_INSTALL_TARGET_OPT = DESTDIR=$(TARGET_DIR) install
>
> And then this isn't needed.
>
> Committed with these fixes, thanks.
>
> --
> Bye, Peter Korsgaard

Thanks,

marek
Peter Korsgaard - Feb. 3, 2013, 9:47 p.m.
>>>>> "Belisko" == Belisko Marek <marek.belisko@gmail.com> writes:

Hi,

 Marek> +config BR2_PACKAGE_LVM2_APP_LIBRARY
 Marek> +       bool "install application library"
 >> 
 >> This option doesn't make much sense in combination with DMSETUP_ONLY, so
 >> I've made it depend on !DMSETUP_ONLY

 Belisko> This patch was from v2 and was dropped in v3 because it wasn't
 Belisko> necessary.  Please see
 Belisko> http://lists.busybox.net/pipermail/buildroot/2013-January/065740.html
 Belisko> I've tested it also with fresh build and it was working
 Belisko> without this patch.

Hmm, it didn't work for me here.

From lvm2 configure.in:

AC_ARG_ENABLE(applib,
              AC_HELP_STRING([--enable-applib], [build application library]),
              APPLIB=$enableval, APPLIB=no)
AC_MSG_RESULT($APPLIB)

E.G. applib defaults to disabled.

Makefile.in:

ifeq ("@APPLIB@", "yes")
  SUBDIRS += liblvm
endif

So I wonder how you could have gotten the liblvm subdir built without
either passing --enable-applib or explicitly calling make liblvm.

Patch

diff --git a/package/lvm2/Config.in b/package/lvm2/Config.in
index 30af14e..bb41c0a 100644
--- a/package/lvm2/Config.in
+++ b/package/lvm2/Config.in
@@ -21,5 +21,11 @@  config BR2_PACKAGE_LVM2_DMSETUP_ONLY
 	help
 	  Install dmsetup only and skip the LVM2 suite.
 
+config BR2_PACKAGE_LVM2_APP_LIBRARY
+	bool "install application library"
+	depends on BR2_PACKAGE_LVM2
+	help
+	  Install application library (liblvm2app2).
+
 comment "lvm2 requires a toolchain with LARGEFILE support"
         depends on !BR2_LARGEFILE
diff --git a/package/lvm2/lvm2.mk b/package/lvm2/lvm2.mk
index f54caa4..213ef65 100644
--- a/package/lvm2/lvm2.mk
+++ b/package/lvm2/lvm2.mk
@@ -21,7 +21,7 @@  LVM2_BINS = \
 
 # Make sure that binaries and libraries are installed with write
 # permissions for the owner.
-LVM2_CONF_OPT += --enable-write_install --enable-pkgconfig
+LVM2_CONF_OPT += --enable-write_install --enable-pkgconfig --enable-applib
 
 # LVM2 uses autoconf, but not automake, and the build system does not
 # take into account the CC passed at configure time.
@@ -41,6 +41,12 @@  LVM2_INSTALL_STAGING_OPT = DESTDIR=$(STAGING_DIR) install_device-mapper
 LVM2_INSTALL_TARGET_OPT = DESTDIR=$(TARGET_DIR) install_device-mapper
 endif
 
+ifeq ($(BR2_PACKAGE_LVM2_APP_LIBRARY),y)
+LVM2_MAKE_OPT += liblvm
+LVM2_INSTALL_STAGING_OPT = DESTDIR=$(STAGING_DIR) install
+LVM2_INSTALL_TARGET_OPT = DESTDIR=$(TARGET_DIR) install
+endif
+
 define LVM2_UNINSTALL_STAGING_CMDS
 	rm -f $(addprefix $(STAGING_DIR)/usr/sbin/,$(LVM2_BINS))
 	rm -f $(addprefix $(STAGING_DIR)/usr/lib/,libdevmapper.so*)