From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH] package/modem-manager: test for the used packages, not for the MM option
Date: Mon, 27 Jul 2020 21:12:13 +0200 [thread overview]
Message-ID: <20200727211213.070edbbb@windsurf.home> (raw)
In-Reply-To: <20200505120720.258993-1-aleksander@aleksander.es>
On Tue, 5 May 2020 14:07:20 +0200
Aleksander Morgado <aleksander@aleksander.es> wrote:
> This allows user to either explicitly select the libraries it
> needs (e.g. libqmi and/or libmbim) and otherwise explicitly configure
> to request the QMI/MBIM features in ModemManager.
>
> This simplifies the build logic, and we avoid possible issues where a
> user that did select the library forgot to select the MM feature.
>
> Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
> ---
> package/modem-manager/modem-manager.mk | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/package/modem-manager/modem-manager.mk b/package/modem-manager/modem-manager.mk
> index 75fc5811db..6b4e9b6250 100644
> --- a/package/modem-manager/modem-manager.mk
> +++ b/package/modem-manager/modem-manager.mk
> @@ -13,7 +13,7 @@ MODEM_MANAGER_DEPENDENCIES = host-pkgconf libglib2 $(TARGET_NLS_DEPENDENCIES)
> MODEM_MANAGER_INSTALL_STAGING = YES
> MODEM_MANAGER_CONF_OPTS = --disable-more-warnings
>
> -ifeq ($(BR2_PACKAGE_MODEM_MANAGER_LIBQMI),y)
> +ifeq ($(BR2_PACKAGE_LIBQMI),y)
I don't remember if we had discuss this, and where this patch comes
from, but I disagree with it.
When a package has sub-options to enable optional features, the
optional features should really be disabled when the option is
disabled, regardless of whether the needed dependencies are present or
not.
Overall, we have two ways of handling optional features:
- "Automatic handling": when the dependencies are available, enable
optional features. Doesn't require any explicit Config sub-options.
- "Explicit handling": explicit Config.in sub-options are added, which
select the appropriate dependencies, and make sure the optional
feature is enabled.
Here you're kind of mixing both, which isn't good IMO.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
prev parent reply other threads:[~2020-07-27 19:12 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-05 12:07 [Buildroot] [PATCH] package/modem-manager: test for the used packages, not for the MM option Aleksander Morgado
2020-07-27 19:12 ` Thomas Petazzoni [this message]
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=20200727211213.070edbbb@windsurf.home \
--to=thomas.petazzoni@bootlin.com \
--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