Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Arnout Vandecappelle <arnout@mind.be>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH] lvm2: replace !BR2_PACKAGE_LVM2_DMSETUP_ONLY by BR2_PACKAGE_LVM2_FULL_SUITE
Date: Thu, 28 Nov 2013 08:13:14 +0100	[thread overview]
Message-ID: <5296ED0A.8020200@mind.be> (raw)
In-Reply-To: <CAAXf6LVhLB1yqkhz0Fupd646ZQu9-4k0uw1LxkTbz-EWO_JOdw@mail.gmail.com>

On 25/11/13 22:21, Thomas De Schampheleire wrote:
> On Mon, Nov 25, 2013 at 12:02 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>> On 25/11/13 10:46, Thomas De Schampheleire wrote:
> [..]
>>>> diff --git a/package/lvm2/Config.in b/package/lvm2/Config.in
>>>> index b3988ea..849aab3 100644
>>>> --- a/package/lvm2/Config.in
>>>> +++ b/package/lvm2/Config.in
>>>> @@ -16,16 +16,18 @@ config BR2_PACKAGE_LVM2
>>>>             user-space. This makes it useful for not only LVM, but EVMS,
>>>>             software raid, and other drivers that create "virtual" block
>>>> devices.
>>>>
>>>> -config BR2_PACKAGE_LVM2_DMSETUP_ONLY
>>>> -       bool "install dmsetup only"
>>>> +config BR2_PACKAGE_LVM2_FULL_SUITE
>>>> +       bool "install the full LVM2 suite"
>>>> +       default y if !BR2_PACKAGE_LVM2_DMSETUP_ONLY # legacy 2013.11
>>>>           depends on BR2_PACKAGE_LVM2
>>>>           help
>>>> -         Install dmsetup only and skip the LVM2 suite.
>>>> +         Install the full LVM2 suite. When this option is not set, only
>>>> +         dmsetup is installed.
>>>>
>>>>    config BR2_PACKAGE_LVM2_APP_LIBRARY
>>>>           bool "install application library"
>>>>           depends on BR2_PACKAGE_LVM2
>>>> -       depends on !BR2_PACKAGE_LVM2_DMSETUP_ONLY
>>>> +       select BR2_PACKAGE_LVM2_FULL_SUITE
>>>>           help
>>>>             Install application library (liblvm2app).
>>>
>>>
>>> It seems odd to me that the 'full suite' does not include the app
>>> library: it is not 'full'.
>>
>>
>>   Granted, but I can't think of a better name. Anyone?
>
> I was trying an alternative approach with 'choice', but failed.
> Here is what I tried:
>
> $ hg diff package/
> diff --git a/package/lvm2/Config.in b/package/lvm2/Config.in
> --- a/package/lvm2/Config.in
> +++ b/package/lvm2/Config.in
> @@ -16,19 +16,29 @@ config BR2_PACKAGE_LVM2
>            user-space. This makes it useful for not only LVM, but EVMS,
>            software raid, and other drivers that create "virtual" block devices.
>
> +if BR2_PACKAGE_LVM2
> +
> +choice
> +       prompt "Installation type"
> +
> +config BR2_PACKAGE_LVM2_STANDARD_INSTALL
> +       bool "standard installation"
> +
>   config BR2_PACKAGE_LVM2_DMSETUP_ONLY
>          bool "install dmsetup only"
> -       depends on BR2_PACKAGE_LVM2
>          help
>            Install dmsetup only and skip the LVM2 suite.
>
> +endchoice
> +
>   config BR2_PACKAGE_LVM2_APP_LIBRARY
>          bool "install application library"
> -       depends on BR2_PACKAGE_LVM2
> -       depends on !BR2_PACKAGE_LVM2_DMSETUP_ONLY
> +       select BR2_PACKAGE_LVM2_STANDARD_INSTALL
>          help
>            Install application library (liblvm2app).
>
> +endif
> +

  So, what didn't work with this?


>   comment "lvm2 needs a toolchain w/ largefile"
>          depends on BR2_USE_MMU
>          depends on !BR2_LARGEFILE
> diff --git a/package/udisks/Config.in b/package/udisks/Config.in
> --- a/package/udisks/Config.in
> +++ b/package/udisks/Config.in
> @@ -30,6 +30,7 @@ if BR2_PACKAGE_UDISKS
>   config BR2_PACKAGE_UDISKS_LVM2
>          bool "lvm2 support"
>          select BR2_PACKAGE_LVM2_APP_LIBRARY
> +       select BR2_PACKAGE_LVM2_STANDARD_INSTALL
>          help
>            Enable LVM2 support
>
>
> If anyone can make this work, I'd prefer it over the 'magic'
> if-not-full-suite-you-get-dmonly approach.

  I don't.

* Choice is not covered by randconfig, so not tried by the autobuilders.

* Choice is more cumbersome for the user

* I don't see what is wrong with a boolean option that installs more stuff.

>
> If not, what about the naming 'standard installation' to refer to what
> is called 'full suite' in your patch. If it is not standard install,
> it is dmonly.

  I called it:

config BR2_PACKAGE_LVM2_STANDARD_INSTALL
	bool "standard install instead of only dmsetup"
	default y if !BR2_PACKAGE_LVM2_DMSETUP_ONLY # legacy 2013.11
	help
	  Install the standard suite of lvm2 programs. When this option
	  is not set, only dmsetup is installed.



  Regards,
  Arnout
-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

  reply	other threads:[~2013-11-28  7:13 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-23  7:30 [Buildroot] [autobuild.buildroot.net] Build results for 2013-11-22 Thomas Petazzoni
2013-11-23 13:11 ` Thomas De Schampheleire
2013-11-23 15:09   ` Thomas De Schampheleire
2013-11-23 17:37     ` Thomas Petazzoni
2013-11-23 23:50       ` [Buildroot] [PATCH] lvm2: replace !BR2_PACKAGE_LVM2_DMSETUP_ONLY by BR2_PACKAGE_LVM2_FULL_SUITE Arnout Vandecappelle
2013-11-25  9:46         ` Thomas De Schampheleire
2013-11-25 11:02           ` Arnout Vandecappelle
2013-11-25 15:10             ` Thomas De Schampheleire
2013-11-25 15:35               ` Arnout Vandecappelle
2013-11-25 15:39                 ` Arnout Vandecappelle
2013-11-25 15:51                 ` Thomas De Schampheleire
2013-11-25 16:06                   ` Arnout Vandecappelle
2013-11-25 21:21             ` Thomas De Schampheleire
2013-11-28  7:13               ` Arnout Vandecappelle [this message]
2013-11-28 10:54                 ` Thomas De Schampheleire
2013-11-23 17:26   ` [Buildroot] [autobuild.buildroot.net] Build results for 2013-11-22 Thomas Petazzoni
2013-11-25  9:26     ` Thomas De Schampheleire
2013-11-25  9:35       ` Thomas Petazzoni
2013-11-23 20:17   ` Baruch Siach
2013-11-25  9:25     ` Thomas De Schampheleire
2013-11-23 20:38 ` Romain Naour
2013-11-23 21:22   ` Yann E. MORIN
2013-11-23 21:34     ` Yann E. MORIN
2013-11-23 23:15   ` Yann E. MORIN

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5296ED0A.8020200@mind.be \
    --to=arnout@mind.be \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox