| Message ID | 1357591399-3566-6-git-send-email-marek.belisko@open-nandra.com |
|---|---|
| State | Accepted |
| Headers | show |
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
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
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
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
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
>>>>> "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.
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
>>>>> "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.
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*)
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(-)