* Re: [Buildroot] User-visible Config.in feature sub-options [was: Re: [PATCH 1/1] package/mpd: explicitly disable features to avoid collision with host packages]
2022-04-09 16:09 ` Yann E. MORIN
@ 2022-04-10 5:44 ` Andreas Ziegler
2022-04-21 14:45 ` Andreas Ziegler
` (5 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: Andreas Ziegler @ 2022-04-10 5:44 UTC (permalink / raw)
To: Yann E. MORIN; +Cc: Jörg Krause, Thomas Petazzoni, buildroot
Hi Yann, Arnout, and whoever else is interested,
two additional thoughts from my side below:
On 2022-04-09 16:09, Yann E. MORIN wrote:
> Arnout, Andreas, All,
>
> On 2022-04-09 17:41 +0200, Arnout Vandecappelle spake thusly:
>> On 08/04/2022 15:22, Andreas Ziegler wrote:
>> >Outline of the proposed next version of this change:
>> >[I did not want to make so many changes, but you are right, it makes sense.]
>> >Separate logic from implementation: Remove feature dependencies from
>> >mpd.mk and put selection logic into Config.in exclusively.
>> As you can see from the description above: this is making things
>> quite
>> complicated. And one of the tenets of Buildroot is to keep things
>> simple.
> [--SNIP--]
>> We don't have a consistent policy for this case, but I believe the
>> policy
>> should be:
>>
>> - Add Config.in options only for features that are important,
>> meaningful for
>> the user (e.g. codec support).
>>
>> - Add Config.in options only for features that have a size impact
>> (usually
>> due to the dependencies they pull in).
>>
>> - In the .mk file, don't use the Config.in options but instead use
>> automatic
>> dependencies only.
>
> That would be very confusign from a user perspective: they would not
> enable a feature of a package, yet the package would have that feature
> enabled if th3e depedency is enabled.
>
> Besides the technical surprise, this could also lead to licensing
> issues
> if the combination of the two packages require special handling (e.g.
> because the library license propagates top the package).
The security aspect increasingly becomes more important: every line of
unused code is a potential attack vector. Better disable what you do not
need.
> So, the settings from Config.in must be abode by.
>
>> That way, the .mk file is kept simple (no problematic cases like the
>> libupnp/expat interaction in this patch). The Config.in is a bit
>> complicated, but it doesn't explode.
>>
>> Bottom line: I think it's actually the reverse that needs to be done.
>
> Err. I don;'t understand what you meant here... :-(
>
>> >In the makefile, a feature is responsible only to select or deselect its
>> >configure option and, if this feature relies on a library, add a
>> >dependency on this.
>>
>> So I think the .mk file should simply contain things like:
>>
>> ifeq ($(BR2_PACKAGE_LIBVORBIS),y)
>> MPD_DEPENDENCIES += libvorbis
>> MPD_CONF_OPTS += -Dvorbis=enabled -Dvorbisenc=enabled
>> else
>> MPD_CONF_OPTS += -Dvorbis=disabled -Dvorbisenc=disabled
>> endif
>>
>> and BR2_PACKAGE_MPD_VORBIS only has an indirect effect, by selecting
>> BR2_PACKAGE_LIBVORBIS.
>
> I highly disagree; the conditional should be on the package setting,
> not
> the dependency.
>
>> >Duplicated selection logic in mpd.mk will be eliminated. A build
>> >dependency will only be added if it is present in the mpd configuration,
>> >and not be inherited accidentally from concurrent packages.
>> So this is exactly the reverse of what I'd want. I think it makes the
>> .mk
>> file harder to maintain for no practical gain.
>
> Yet, the proposal by Andreas is I believe the correct way to handle the
> situation.
>
>> >This impacts the following features: expat, id3tag, yajl. id3tag already
>> >is half implemented, but not used consistently (has an option entry in
>> >Config.in, but does not select this, but handles dependencies
>> >individually). expat and yajl would be created as hidden options without
>> >visibility (no user interaction necessary).
>> Yes, that would indeed be an alternative to keep the .mk file
>> simpler. I
>> take it you mean to add a blind option BR2_PACKAGE_MPD_EXPAT that gets
>> selected by the other MPD options that rely on expat. This indeed
>> simplifies
>> things, but it is still a bit more complicated than what I propose.
>
> But more correct.
>
>> >Separate vorbis decoder and encoder options; currently the decoder feature
>> >BR2_PACKAGE_MPD_VORBIS enables both ogg/vorbis decoding and ogg/vorbis
>> >encoding, which is probably not intended.
>> I don't see a reason why you would want only one or the other. It has
>> virtually no impact on size.
>
> Indeed here, vorbis support should just enable both the
> encoder/decoder.
>
> The only reason that we might want to be able to chose, is if enabling
> one or the other have different requirements: extra size requirements,
> extra dependencies, legal issues in your jurisdiction, etc...
This behavior is not evident from the menu. Here you just have an option
to enable the decoder; the encoder is not mentioned anywhere. If the
configuration stays, at least grouping and help message of the current
item need an update.
>> In summary, I think we have the following three options for packages
>> where
>> we decide we want user-visible sub-options.
>>
>> - 1-to-1 mapping with the package configure options. Interaction
>> between the
>> options is expressed with select/depends in Config.in. The .mk file
>> simply
>> maps the Config.in options. If it's really not relevant for the user,
>> a
>> Config.in option can be made blind. This is what Andreas proposes.
>
> I am OK with that.
>
>> - Only automatic dependencies in the .mk file, except in cases where
>> it has
>> an important size or behaviour impact. Add Config.in options only in
>> case it
>> is relevant. This is what I propose.
>
> I am OK with the principle, but this does not look like what you
> proposed above, as the build-dependencies would be on the dependency
> being enabled, not the package option (e.g. BR2_PACKAGE_MPD_VORBIS vs.
> BR2_PACKAGE_LIBVORBIS as you showed above).
>
>> - Add Config.in options for important features. Express
>> interdependencies
>> between package configure options in the .mk file. This is the current
>> situation for mpd.
>
> I don;t see a difference here: "Add Config.in options only in case it
> is
> relevant" and "Add Config.in options for important features" are
> exactly
> the same in mny eyes...
>
>> So let's see what the other maintainers think. If you (or the other
>> maintainers) don't agree, we can compromise and go back to the
>> original
>> patch (with just the npupnp/expat situation resolved).
>
> So, I'll summariser my position:
>
> - add Config.in options when it makes sense:
> - important size delta
> - legal issues
> those options select the appropriate packages
>
> - in the .mk:
> - add conditional blocks based on those options, add build
> dependencies as appropriate;
> - add conditoinal dependencies on package for automatic
> dependencies
>
>> Ideally I'd like the maintainers (and anybody else, really) to come
>> to a
>> decision here and eventually formalize it in the manual. Because it's
>> not
>> the first time this discussion crops up.
Kind regards,
Andreas
> Regards,
> Yann E. MORIN.
>
> --
> .-----------------.--------------------.------------------.--------------------.
> | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics'
> conspiracy: |
> | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___
> |
> | +33 561 099 427 `------------.-------: X AGAINST | \e/ There
> is no |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v
> conspiracy. |
> '------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Buildroot] User-visible Config.in feature sub-options [was: Re: [PATCH 1/1] package/mpd: explicitly disable features to avoid collision with host packages]
2022-04-09 16:09 ` Yann E. MORIN
2022-04-10 5:44 ` Andreas Ziegler
@ 2022-04-21 14:45 ` Andreas Ziegler
2022-10-05 9:10 ` [Buildroot] [PATCH v2 0/4] User-visible Config.in feature sub-options Andreas Ziegler
` (4 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: Andreas Ziegler @ 2022-04-21 14:45 UTC (permalink / raw)
To: Yann E. MORIN; +Cc: Jörg Krause, Thomas Petazzoni, buildroot
Hi Yann, Arnout, *,
I would like to postpone this change for the moment; the root cause for
the addressed build failures lies within the meson build system (patch
coming) and fixing this will make some of the proposed modifications
unnecessary.
I would still like to implement the selection logic changes, because I
started to notice some inconsistencies and also missing features.
Kind regards,
Andreas
On 2022-04-09 16:09, Yann E. MORIN wrote:
> Arnout, Andreas, All,
>
> On 2022-04-09 17:41 +0200, Arnout Vandecappelle spake thusly:
>> On 08/04/2022 15:22, Andreas Ziegler wrote:
>> >Outline of the proposed next version of this change:
>> >[I did not want to make so many changes, but you are right, it makes sense.]
>> >Separate logic from implementation: Remove feature dependencies from
>> >mpd.mk and put selection logic into Config.in exclusively.
>> As you can see from the description above: this is making things
>> quite
>> complicated. And one of the tenets of Buildroot is to keep things
>> simple.
> [--SNIP--]
>> We don't have a consistent policy for this case, but I believe the
>> policy
>> should be:
>>
>> - Add Config.in options only for features that are important,
>> meaningful for
>> the user (e.g. codec support).
>>
>> - Add Config.in options only for features that have a size impact
>> (usually
>> due to the dependencies they pull in).
>>
>> - In the .mk file, don't use the Config.in options but instead use
>> automatic
>> dependencies only.
>
> That would be very confusign from a user perspective: they would not
> enable a feature of a package, yet the package would have that feature
> enabled if th3e depedency is enabled.
>
> Besides the technical surprise, this could also lead to licensing
> issues
> if the combination of the two packages require special handling (e.g.
> because the library license propagates top the package).
>
> So, the settings from Config.in must be abode by.
>
>> That way, the .mk file is kept simple (no problematic cases like the
>> libupnp/expat interaction in this patch). The Config.in is a bit
>> complicated, but it doesn't explode.
>>
>> Bottom line: I think it's actually the reverse that needs to be done.
>
> Err. I don;'t understand what you meant here... :-(
>
>> >In the makefile, a feature is responsible only to select or deselect its
>> >configure option and, if this feature relies on a library, add a
>> >dependency on this.
>>
>> So I think the .mk file should simply contain things like:
>>
>> ifeq ($(BR2_PACKAGE_LIBVORBIS),y)
>> MPD_DEPENDENCIES += libvorbis
>> MPD_CONF_OPTS += -Dvorbis=enabled -Dvorbisenc=enabled
>> else
>> MPD_CONF_OPTS += -Dvorbis=disabled -Dvorbisenc=disabled
>> endif
>>
>> and BR2_PACKAGE_MPD_VORBIS only has an indirect effect, by selecting
>> BR2_PACKAGE_LIBVORBIS.
>
> I highly disagree; the conditional should be on the package setting,
> not
> the dependency.
>
>> >Duplicated selection logic in mpd.mk will be eliminated. A build
>> >dependency will only be added if it is present in the mpd configuration,
>> >and not be inherited accidentally from concurrent packages.
>> So this is exactly the reverse of what I'd want. I think it makes the
>> .mk
>> file harder to maintain for no practical gain.
>
> Yet, the proposal by Andreas is I believe the correct way to handle the
> situation.
>
>> >This impacts the following features: expat, id3tag, yajl. id3tag already
>> >is half implemented, but not used consistently (has an option entry in
>> >Config.in, but does not select this, but handles dependencies
>> >individually). expat and yajl would be created as hidden options without
>> >visibility (no user interaction necessary).
>> Yes, that would indeed be an alternative to keep the .mk file
>> simpler. I
>> take it you mean to add a blind option BR2_PACKAGE_MPD_EXPAT that gets
>> selected by the other MPD options that rely on expat. This indeed
>> simplifies
>> things, but it is still a bit more complicated than what I propose.
>
> But more correct.
>
>> >Separate vorbis decoder and encoder options; currently the decoder feature
>> >BR2_PACKAGE_MPD_VORBIS enables both ogg/vorbis decoding and ogg/vorbis
>> >encoding, which is probably not intended.
>> I don't see a reason why you would want only one or the other. It has
>> virtually no impact on size.
>
> Indeed here, vorbis support should just enable both the
> encoder/decoder.
>
> The only reason that we might want to be able to chose, is if enabling
> one or the other have different requirements: extra size requirements,
> extra dependencies, legal issues in your jurisdiction, etc...
>
>> In summary, I think we have the following three options for packages
>> where
>> we decide we want user-visible sub-options.
>>
>> - 1-to-1 mapping with the package configure options. Interaction
>> between the
>> options is expressed with select/depends in Config.in. The .mk file
>> simply
>> maps the Config.in options. If it's really not relevant for the user,
>> a
>> Config.in option can be made blind. This is what Andreas proposes.
>
> I am OK with that.
>
>> - Only automatic dependencies in the .mk file, except in cases where
>> it has
>> an important size or behaviour impact. Add Config.in options only in
>> case it
>> is relevant. This is what I propose.
>
> I am OK with the principle, but this does not look like what you
> proposed above, as the build-dependencies would be on the dependency
> being enabled, not the package option (e.g. BR2_PACKAGE_MPD_VORBIS vs.
> BR2_PACKAGE_LIBVORBIS as you showed above).
>
>> - Add Config.in options for important features. Express
>> interdependencies
>> between package configure options in the .mk file. This is the current
>> situation for mpd.
>
> I don;t see a difference here: "Add Config.in options only in case it
> is
> relevant" and "Add Config.in options for important features" are
> exactly
> the same in mny eyes...
>
>> So let's see what the other maintainers think. If you (or the other
>> maintainers) don't agree, we can compromise and go back to the
>> original
>> patch (with just the npupnp/expat situation resolved).
>
> So, I'll summariser my position:
>
> - add Config.in options when it makes sense:
> - important size delta
> - legal issues
> those options select the appropriate packages
>
> - in the .mk:
> - add conditional blocks based on those options, add build
> dependencies as appropriate;
> - add conditoinal dependencies on package for automatic
> dependencies
>
>> Ideally I'd like the maintainers (and anybody else, really) to come
>> to a
>> decision here and eventually formalize it in the manual. Because it's
>> not
>> the first time this discussion crops up.
>
> Regards,
> Yann E. MORIN.
>
> --
> .-----------------.--------------------.------------------.--------------------.
> | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics'
> conspiracy: |
> | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___
> |
> | +33 561 099 427 `------------.-------: X AGAINST | \e/ There
> is no |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v
> conspiracy. |
> '------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 20+ messages in thread* [Buildroot] [PATCH v2 0/4] User-visible Config.in feature sub-options
2022-04-09 16:09 ` Yann E. MORIN
2022-04-10 5:44 ` Andreas Ziegler
2022-04-21 14:45 ` Andreas Ziegler
@ 2022-10-05 9:10 ` Andreas Ziegler
2022-10-05 9:10 ` [Buildroot] [PATCH v2 1/4] package/mpd: fix reversed logic in tcp disable Andreas Ziegler
` (3 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: Andreas Ziegler @ 2022-10-05 9:10 UTC (permalink / raw)
To: buildroot; +Cc: Andreas Ziegler, YANN E MORIN, Thomas Petazzoni
As indicated, this is the patch based on our discussion. I gave it the
version number 2, even if the content has little to do with the original
patch. I also split it into a series, to allow for partial integration.
#1 just a bug fix
#2 add /modify comments -- mostly captures additional info, collected when
looking up features in meson_options.txt and the mpd manual
#3 id3tag is a necessary sub-feature for mp3 decoders, so make them select
the (already existing) sub-option, instead of doing their own selection
and omitting id3tag
#4 introduce the sub-option logic for expat and yajl also; this cleans up
upnp configuration also
I would appreciate integration of 1-3. #3 actually solves a possible con-
figuration error: if you configure mp3 playback, but forget id3tag, mp3
files will not be recognized by mpd.
I am open for discussion on #4. I can live with automatic dependencies based
on libraries, aka
ifeq ($(BR2_PACKAGE_FOO),y)
MPD_DEPENDENCIES += libfoo
MPD_CONF_OPTS += -Dfoo=enabled
else
MPD_CONF_OPTS += -Dfoo=disabled
endif
(works for me), but would prefer dependencies based on Config.in sub-options.
libicu handling remains unchanged. libicu adds approx. 40 MB to the compressed
image; whoever needs this in his firmware is welcome to the additional feature
in mpd. It has no discernable size impact here.
Andreas Ziegler (4):
package/mpd: fix reversed logic in tcp disable
package/mpd: add/enhance (kconfig + code) comments
package/mpd: introduce id3tag feature dependency
package/mpd: introduce sub-options for expat and yajl
package/mpd/Config.in | 30 ++++++++++++++++++++++--------
package/mpd/mpd.mk | 33 +++++++++++++++++++++++----------
2 files changed, 45 insertions(+), 18 deletions(-)
--
2.34.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 20+ messages in thread* [Buildroot] [PATCH v2 1/4] package/mpd: fix reversed logic in tcp disable
2022-04-09 16:09 ` Yann E. MORIN
` (2 preceding siblings ...)
2022-10-05 9:10 ` [Buildroot] [PATCH v2 0/4] User-visible Config.in feature sub-options Andreas Ziegler
@ 2022-10-05 9:10 ` Andreas Ziegler
2023-08-06 19:42 ` Thomas Petazzoni via buildroot
2022-10-05 9:10 ` [Buildroot] [PATCH v2 2/4] package/mpd: add/enhance (kconfig + code) comments Andreas Ziegler
` (2 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Andreas Ziegler @ 2022-10-05 9:10 UTC (permalink / raw)
To: buildroot; +Cc: Andreas Ziegler, YANN E MORIN, Thomas Petazzoni
Signed-off-by: Andreas Ziegler <br015@umbiko.net>
---
Changes v1 -> v2:
- make this a separate patch
package/mpd/mpd.mk | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/package/mpd/mpd.mk b/package/mpd/mpd.mk
index 5c15953984..4d73e6de03 100644
--- a/package/mpd/mpd.mk
+++ b/package/mpd/mpd.mk
@@ -285,8 +285,9 @@ else
MPD_CONF_OPTS += -Dsqlite=disabled
endif
+# default setting is 'true'
ifneq ($(BR2_PACKAGE_MPD_TCP),y)
-MPD_CONF_OPTS += -Dtcp=true
+MPD_CONF_OPTS += -Dtcp=false
endif
ifeq ($(BR2_PACKAGE_MPD_TREMOR),y)
--
2.34.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Buildroot] [PATCH v2 1/4] package/mpd: fix reversed logic in tcp disable
2022-10-05 9:10 ` [Buildroot] [PATCH v2 1/4] package/mpd: fix reversed logic in tcp disable Andreas Ziegler
@ 2023-08-06 19:42 ` Thomas Petazzoni via buildroot
2023-09-11 7:22 ` Peter Korsgaard
0 siblings, 1 reply; 20+ messages in thread
From: Thomas Petazzoni via buildroot @ 2023-08-06 19:42 UTC (permalink / raw)
To: Andreas Ziegler; +Cc: YANN E MORIN, buildroot
Hello Andreas,
On Wed, 5 Oct 2022 11:10:29 +0200
Andreas Ziegler <br015@umbiko.net> wrote:
> Signed-off-by: Andreas Ziegler <br015@umbiko.net>
I've extended this explanation with some details on when the issue was
introduced:
package/mpd: fix reversed logic in tcp disable
In commit 54b9008d482923131191da75f09966483f4ecac1 ("package/mpd: bump
to version 0.21.11"), mpd was migrated from using the autotools build
system to the meson build system.
As part of this, the BR2_PACKAGE_MPD_TCP was incorrectly modified,
leading BR2_PACKAGE_MPD_TCP disabled to actually enable TCP, and
BR2_PACKAGE_MPD_TCP enabled to not explicitly enable TCP support.
This commit fixes that by handling this option in the common way.
Signed-off-by: Andreas Ziegler <br015@umbiko.net>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> +# default setting is 'true'
> ifneq ($(BR2_PACKAGE_MPD_TCP),y)
> -MPD_CONF_OPTS += -Dtcp=true
> +MPD_CONF_OPTS += -Dtcp=false
> endif
Changed to:
ifeq ($(BR2_PACKAGE_MPD_TCP),y)
MPD_CONF_OPTS += -Dtcp=true
else
MPD_CONF_OPTS += -Dtcp=false
endif
Applied to master with those changes.
Best regards,
Thomas
--
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Buildroot] [PATCH v2 1/4] package/mpd: fix reversed logic in tcp disable
2023-08-06 19:42 ` Thomas Petazzoni via buildroot
@ 2023-09-11 7:22 ` Peter Korsgaard
0 siblings, 0 replies; 20+ messages in thread
From: Peter Korsgaard @ 2023-09-11 7:22 UTC (permalink / raw)
To: Thomas Petazzoni via buildroot
Cc: Andreas Ziegler, YANN E MORIN, Thomas Petazzoni
>>>>> "Thomas" == Thomas Petazzoni via buildroot <buildroot@buildroot.org> writes:
> Hello Andreas,
> On Wed, 5 Oct 2022 11:10:29 +0200
> Andreas Ziegler <br015@umbiko.net> wrote:
>> Signed-off-by: Andreas Ziegler <br015@umbiko.net>
> I've extended this explanation with some details on when the issue was
> introduced:
> package/mpd: fix reversed logic in tcp disable
> In commit 54b9008d482923131191da75f09966483f4ecac1 ("package/mpd: bump
> to version 0.21.11"), mpd was migrated from using the autotools build
> system to the meson build system.
> As part of this, the BR2_PACKAGE_MPD_TCP was incorrectly modified,
> leading BR2_PACKAGE_MPD_TCP disabled to actually enable TCP, and
> BR2_PACKAGE_MPD_TCP enabled to not explicitly enable TCP support.
> This commit fixes that by handling this option in the common way.
> Signed-off-by: Andreas Ziegler <br015@umbiko.net>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Committed to 2023.02.x and 2023.05.x, thanks.
--
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Buildroot] [PATCH v2 2/4] package/mpd: add/enhance (kconfig + code) comments
2022-04-09 16:09 ` Yann E. MORIN
` (3 preceding siblings ...)
2022-10-05 9:10 ` [Buildroot] [PATCH v2 1/4] package/mpd: fix reversed logic in tcp disable Andreas Ziegler
@ 2022-10-05 9:10 ` Andreas Ziegler
2023-08-06 19:45 ` Thomas Petazzoni via buildroot
2022-10-05 9:10 ` [Buildroot] [PATCH v2 3/4] package/mpd: introduce id3tag feature dependency Andreas Ziegler
2022-10-05 9:10 ` [Buildroot] [PATCH v2 4/4] package/mpd: introduce sub-options for expat and yajl Andreas Ziegler
6 siblings, 1 reply; 20+ messages in thread
From: Andreas Ziegler @ 2022-10-05 9:10 UTC (permalink / raw)
To: buildroot; +Cc: Andreas Ziegler, YANN E MORIN, Thomas Petazzoni
Align kconfig comments with descriptions in the mpd manual.
Add a kconfig comment to highlight the impact of ogg/vorbis selection.
Add comments to makefile to explain remaining multiple dependency creation.
Signed-off-by: Andreas Ziegler <br015@umbiko.net>
---
Changes v1 -> v2:
- make this a separate patch
package/mpd/Config.in | 9 ++++++---
package/mpd/mpd.mk | 2 ++
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/package/mpd/Config.in b/package/mpd/Config.in
index 8f0af7b2d3..2606008e90 100644
--- a/package/mpd/Config.in
+++ b/package/mpd/Config.in
@@ -33,7 +33,7 @@ config BR2_PACKAGE_MPD_SQLITE
select BR2_PACKAGE_SQLITE
help
Enable sqlite database support.
- If you don't use sqlite it will use an ASCII database.
+ This is mandatory for the sticker database.
config BR2_PACKAGE_MPD_ZZIP
bool "zzip"
@@ -81,8 +81,8 @@ comment "Decoder plugins"
config BR2_PACKAGE_MPD_DSD
bool "dsd"
help
- Enable Digital Speech Decoder (DSD) support to play audio
- files encoded in a digital speech format.
+ Direct Stream Digital (DSD) support to play audio
+ files encoded in single bit format.
config BR2_PACKAGE_MPD_FAAD2
bool "faad2"
@@ -210,6 +210,9 @@ config BR2_PACKAGE_MPD_TWOLAME
help
Enable TwoLAME mp2 encoding.
+comment "for ogg/vorbis encoding enable vorbis decoder"
+ depends on !BR2_PACKAGE_MPD_VORBIS
+
comment "Input plugins"
config BR2_PACKAGE_MPD_CDIO_PARANOIA
diff --git a/package/mpd/mpd.mk b/package/mpd/mpd.mk
index 4d73e6de03..feab894f0f 100644
--- a/package/mpd/mpd.mk
+++ b/package/mpd/mpd.mk
@@ -230,6 +230,7 @@ else
MPD_CONF_OPTS += -Dopenal=disabled
endif
+# opus needs to be encapsulated in a container format, here ogg
ifeq ($(BR2_PACKAGE_MPD_OPUS),y)
MPD_DEPENDENCIES += opus libogg
MPD_CONF_OPTS += -Dopus=enabled
@@ -317,6 +318,7 @@ else ifeq ($(BR2_PACKAGE_MPD_UPNP_DISABLED),y)
MPD_CONF_OPTS += -Dupnp=disabled
endif
+# handle decoder and encoder simultaneously
ifeq ($(BR2_PACKAGE_MPD_VORBIS),y)
MPD_DEPENDENCIES += libvorbis
MPD_CONF_OPTS += -Dvorbis=enabled -Dvorbisenc=enabled
--
2.34.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Buildroot] [PATCH v2 2/4] package/mpd: add/enhance (kconfig + code) comments
2022-10-05 9:10 ` [Buildroot] [PATCH v2 2/4] package/mpd: add/enhance (kconfig + code) comments Andreas Ziegler
@ 2023-08-06 19:45 ` Thomas Petazzoni via buildroot
2023-08-08 8:14 ` Andreas Ziegler
0 siblings, 1 reply; 20+ messages in thread
From: Thomas Petazzoni via buildroot @ 2023-08-06 19:45 UTC (permalink / raw)
To: Andreas Ziegler; +Cc: YANN E MORIN, buildroot
Hello Andreas,
On Wed, 5 Oct 2022 11:10:30 +0200
Andreas Ziegler <br015@umbiko.net> wrote:
> Align kconfig comments with descriptions in the mpd manual.
> Add a kconfig comment to highlight the impact of ogg/vorbis selection.
> Add comments to makefile to explain remaining multiple dependency creation.
>
> Signed-off-by: Andreas Ziegler <br015@umbiko.net>
> ---
> Changes v1 -> v2:
> - make this a separate patch
>
> package/mpd/Config.in | 9 ++++++---
> package/mpd/mpd.mk | 2 ++
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/package/mpd/Config.in b/package/mpd/Config.in
> index 8f0af7b2d3..2606008e90 100644
> --- a/package/mpd/Config.in
> +++ b/package/mpd/Config.in
> @@ -33,7 +33,7 @@ config BR2_PACKAGE_MPD_SQLITE
> select BR2_PACKAGE_SQLITE
> help
> Enable sqlite database support.
> - If you don't use sqlite it will use an ASCII database.
Why is this removed?
> + This is mandatory for the sticker database.
Not sure what the sticker database is :-)
>
> config BR2_PACKAGE_MPD_ZZIP
> bool "zzip"
> @@ -81,8 +81,8 @@ comment "Decoder plugins"
> config BR2_PACKAGE_MPD_DSD
> bool "dsd"
> help
> - Enable Digital Speech Decoder (DSD) support to play audio
> - files encoded in a digital speech format.
> + Direct Stream Digital (DSD) support to play audio
> + files encoded in single bit format.
Is this change really relevant ?
>
> config BR2_PACKAGE_MPD_FAAD2
> bool "faad2"
> @@ -210,6 +210,9 @@ config BR2_PACKAGE_MPD_TWOLAME
> help
> Enable TwoLAME mp2 encoding.
>
> +comment "for ogg/vorbis encoding enable vorbis decoder"
> + depends on !BR2_PACKAGE_MPD_VORBIS
I don't understand why this comment is needed. We usually don't put
comments about all dependencies on other packages. Why for this one?
If you want to play Ogg/Vorbis files, it's quite obvious that the
"vorbis" option needs to be enabled.
> +# opus needs to be encapsulated in a container format, here ogg
Yes, and?
> ifeq ($(BR2_PACKAGE_MPD_OPUS),y)
> MPD_DEPENDENCIES += opus libogg
> MPD_CONF_OPTS += -Dopus=enabled
> @@ -317,6 +318,7 @@ else ifeq ($(BR2_PACKAGE_MPD_UPNP_DISABLED),y)
> MPD_CONF_OPTS += -Dupnp=disabled
> endif
>
> +# handle decoder and encoder simultaneously
Yes, and?
Sorry, but I don't really understand the value of the changes proposed
in this commit. I'll mark it as Changes Requested for the time being.
Feel free to resubmit with more details if needed. Also, look at other
Buildroot packages, we try to do things consistently, so doing
something "special" in MPD is unlikely to be accepted.
Thanks!
Thomas
--
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Buildroot] [PATCH v2 2/4] package/mpd: add/enhance (kconfig + code) comments
2023-08-06 19:45 ` Thomas Petazzoni via buildroot
@ 2023-08-08 8:14 ` Andreas Ziegler
0 siblings, 0 replies; 20+ messages in thread
From: Andreas Ziegler @ 2023-08-08 8:14 UTC (permalink / raw)
To: Thomas Petazzoni; +Cc: YANN E MORIN, buildroot
Hi Thomas, *,
Thank you for picking this up.
On 2023-08-06 19:45, Thomas Petazzoni wrote:
> Hello Andreas,
>
> On Wed, 5 Oct 2022 11:10:30 +0200
> Andreas Ziegler <br015@umbiko.net> wrote:
>
>> Align kconfig comments with descriptions in the mpd manual.
>> Add a kconfig comment to highlight the impact of ogg/vorbis selection.
>> Add comments to makefile to explain remaining multiple dependency
>> creation.
>>
>> Signed-off-by: Andreas Ziegler <br015@umbiko.net>
>> ---
>> Changes v1 -> v2:
>> - make this a separate patch
>>
>> package/mpd/Config.in | 9 ++++++---
>> package/mpd/mpd.mk | 2 ++
>> 2 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/package/mpd/Config.in b/package/mpd/Config.in
>> index 8f0af7b2d3..2606008e90 100644
>> --- a/package/mpd/Config.in
>> +++ b/package/mpd/Config.in
>> @@ -33,7 +33,7 @@ config BR2_PACKAGE_MPD_SQLITE
>> select BR2_PACKAGE_SQLITE
>> help
>> Enable sqlite database support.
>> - If you don't use sqlite it will use an ASCII database.
>
> Why is this removed?
It raises false hopes. Mpd uses an in-memory database that is persisted
in a proprietary file format. The format may change between releases and
there is no way to use a SQL database for storage. All that the
sqlite=enabled feature does is enable the sticker database.
>
>> + This is mandatory for the sticker database.
>
> Not sure what the sticker database is :-)
I will add more information from the mpd manual:
+ Support for the sticker database. “Stickers” are pieces of
+ information attached to songs. Some clients use them to store
+ ratings and other volatile data.
>
>>
>> config BR2_PACKAGE_MPD_ZZIP
>> bool "zzip"
>> @@ -81,8 +81,8 @@ comment "Decoder plugins"
>> config BR2_PACKAGE_MPD_DSD
>> bool "dsd"
>> help
>> - Enable Digital Speech Decoder (DSD) support to play audio
>> - files encoded in a digital speech format.
>> + Direct Stream Digital (DSD) support to play audio
>> + files encoded in single bit format.
>
> Is this change really relevant ?
>
The Digital Speech [1] format exists, but this is not the format
referred to here. MPD plays SA-CD rips [2]
Maybe:
+ Direct Stream Digital (DSD) support to play delta-sigma
+ encoded files in formats DSDIFF and DSF.
+
+ This is the sample format used on Super Audio CDs.
>>
>> config BR2_PACKAGE_MPD_FAAD2
>> bool "faad2"
>> @@ -210,6 +210,9 @@ config BR2_PACKAGE_MPD_TWOLAME
>> help
>> Enable TwoLAME mp2 encoding.
>>
>> +comment "for ogg/vorbis encoding enable vorbis decoder"
>> + depends on !BR2_PACKAGE_MPD_VORBIS
>
> I don't understand why this comment is needed. We usually don't put
> comments about all dependencies on other packages. Why for this one?
Options are structured by function. Since ogg/vorbis codecs must be
selected simultaneously (according to the Buildroot standard), a feature
that is located under *decoding* must be enabled for *encoding*. Breaks
up the UI a little bit, therefore a comment.
> If you want to play Ogg/Vorbis files, it's quite obvious that the
> "vorbis" option needs to be enabled.
This is a different use case. The encoder is used for streaming
protocols, similar to internet radio stations.
>> +# opus needs to be encapsulated in a container format, here ogg
>
> Yes, and?
Most mpd options have exactly one dependency. Feature name and library
name match; easy to see that everything is as it should be. So I added
comments whenever there was a deviation, just to show that this is not
an error, but intended.
>> ifeq ($(BR2_PACKAGE_MPD_OPUS),y)
>> MPD_DEPENDENCIES += opus libogg
>> MPD_CONF_OPTS += -Dopus=enabled
>> @@ -317,6 +318,7 @@ else ifeq ($(BR2_PACKAGE_MPD_UPNP_DISABLED),y)
>> MPD_CONF_OPTS += -Dupnp=disabled
>> endif
>>
>> +# handle decoder and encoder simultaneously
>
> Yes, and?
>
Again, a deviation from the one feature /one selection pattern.
> Sorry, but I don't really understand the value of the changes proposed
> in this commit. I'll mark it as Changes Requested for the time being.
> Feel free to resubmit with more details if needed. Also, look at other
> Buildroot packages, we try to do things consistently, so doing
> something "special" in MPD is unlikely to be accepted.
Some packages - for example mpd - are rather difficult to configure
correctly. So why not add a bit more information, especially in the user
interface ;-)
I expect your comments and will follow up with a new version.
Kind regards,
Andreas
> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com
[1] https://en.wikipedia.org/wiki/Digital_Speech_Standard
[2] https://en.wikipedia.org/wiki/Direct_Stream_Digital
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Buildroot] [PATCH v2 3/4] package/mpd: introduce id3tag feature dependency
2022-04-09 16:09 ` Yann E. MORIN
` (4 preceding siblings ...)
2022-10-05 9:10 ` [Buildroot] [PATCH v2 2/4] package/mpd: add/enhance (kconfig + code) comments Andreas Ziegler
@ 2022-10-05 9:10 ` Andreas Ziegler
2023-08-06 19:46 ` Thomas Petazzoni via buildroot
2023-09-11 7:23 ` Peter Korsgaard
2022-10-05 9:10 ` [Buildroot] [PATCH v2 4/4] package/mpd: introduce sub-options for expat and yajl Andreas Ziegler
6 siblings, 2 replies; 20+ messages in thread
From: Andreas Ziegler @ 2022-10-05 9:10 UTC (permalink / raw)
To: buildroot; +Cc: Andreas Ziegler, YANN E MORIN, Thomas Petazzoni
id3tag is a sub-feature that is needed to extract information from mp3 files.
It selects the corresponding library and handles config settings. Two other
features need this sub-feature, but handle all library selections themselves
and omit enabling the id3tag feature. In consequence, users have to remember
to select both mp3 library and id3tag, otherwise the mpd executable will not
process mp3 files.
Reflect feature dependency in mpd Config.in to make id3tag selection automatic.
Signed-off-by: Andreas Ziegler <br015@umbiko.net>
---
Changes v1 -> v2:
- make this a separate patch
package/mpd/Config.in | 4 ++--
package/mpd/mpd.mk | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/package/mpd/Config.in b/package/mpd/Config.in
index 2606008e90..daf1469cae 100644
--- a/package/mpd/Config.in
+++ b/package/mpd/Config.in
@@ -127,7 +127,7 @@ config BR2_PACKAGE_MPD_LIBSNDFILE
config BR2_PACKAGE_MPD_MAD
bool "mad"
default y
- select BR2_PACKAGE_LIBID3TAG
+ select BR2_PACKAGE_MPD_ID3TAG
select BR2_PACKAGE_LIBMAD
help
Enable mad input support.
@@ -141,7 +141,7 @@ config BR2_PACKAGE_MPD_MODPLUG
config BR2_PACKAGE_MPD_MPG123
bool "mpg123"
- select BR2_PACKAGE_LIBID3TAG
+ select BR2_PACKAGE_MPD_ID3TAG
select BR2_PACKAGE_MPG123
help
Enable mpg123 input support.
diff --git a/package/mpd/mpd.mk b/package/mpd/mpd.mk
index feab894f0f..1269b90661 100644
--- a/package/mpd/mpd.mk
+++ b/package/mpd/mpd.mk
@@ -190,7 +190,7 @@ MPD_CONF_OPTS += -Dsoxr=disabled
endif
ifeq ($(BR2_PACKAGE_MPD_MAD),y)
-MPD_DEPENDENCIES += libid3tag libmad
+MPD_DEPENDENCIES += libmad
MPD_CONF_OPTS += -Dmad=enabled
else
MPD_CONF_OPTS += -Dmad=disabled
@@ -204,7 +204,7 @@ MPD_CONF_OPTS += -Dmodplug=disabled
endif
ifeq ($(BR2_PACKAGE_MPD_MPG123),y)
-MPD_DEPENDENCIES += libid3tag mpg123
+MPD_DEPENDENCIES += mpg123
MPD_CONF_OPTS += -Dmpg123=enabled
else
MPD_CONF_OPTS += -Dmpg123=disabled
--
2.34.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Buildroot] [PATCH v2 3/4] package/mpd: introduce id3tag feature dependency
2022-10-05 9:10 ` [Buildroot] [PATCH v2 3/4] package/mpd: introduce id3tag feature dependency Andreas Ziegler
@ 2023-08-06 19:46 ` Thomas Petazzoni via buildroot
2023-09-11 7:23 ` Peter Korsgaard
1 sibling, 0 replies; 20+ messages in thread
From: Thomas Petazzoni via buildroot @ 2023-08-06 19:46 UTC (permalink / raw)
To: Andreas Ziegler; +Cc: YANN E MORIN, buildroot
On Wed, 5 Oct 2022 11:10:31 +0200
Andreas Ziegler <br015@umbiko.net> wrote:
> id3tag is a sub-feature that is needed to extract information from mp3 files.
> It selects the corresponding library and handles config settings. Two other
> features need this sub-feature, but handle all library selections themselves
> and omit enabling the id3tag feature. In consequence, users have to remember
> to select both mp3 library and id3tag, otherwise the mpd executable will not
> process mp3 files.
>
> Reflect feature dependency in mpd Config.in to make id3tag selection automatic.
>
> Signed-off-by: Andreas Ziegler <br015@umbiko.net>
> ---
> Changes v1 -> v2:
> - make this a separate patch
Applied to master, thanks.
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Buildroot] [PATCH v2 3/4] package/mpd: introduce id3tag feature dependency
2022-10-05 9:10 ` [Buildroot] [PATCH v2 3/4] package/mpd: introduce id3tag feature dependency Andreas Ziegler
2023-08-06 19:46 ` Thomas Petazzoni via buildroot
@ 2023-09-11 7:23 ` Peter Korsgaard
1 sibling, 0 replies; 20+ messages in thread
From: Peter Korsgaard @ 2023-09-11 7:23 UTC (permalink / raw)
To: Andreas Ziegler; +Cc: YANN E MORIN, Thomas Petazzoni, buildroot
>>>>> "Andreas" == Andreas Ziegler <br015@umbiko.net> writes:
> id3tag is a sub-feature that is needed to extract information from mp3 files.
> It selects the corresponding library and handles config settings. Two other
> features need this sub-feature, but handle all library selections themselves
> and omit enabling the id3tag feature. In consequence, users have to remember
> to select both mp3 library and id3tag, otherwise the mpd executable will not
> process mp3 files.
> Reflect feature dependency in mpd Config.in to make id3tag selection automatic.
> Signed-off-by: Andreas Ziegler <br015@umbiko.net>
> ---
> Changes v1 -> v2:
> - make this a separate patch
Committed to 2023.02.x and 2023.05.x, thanks.
--
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Buildroot] [PATCH v2 4/4] package/mpd: introduce sub-options for expat and yajl
2022-04-09 16:09 ` Yann E. MORIN
` (5 preceding siblings ...)
2022-10-05 9:10 ` [Buildroot] [PATCH v2 3/4] package/mpd: introduce id3tag feature dependency Andreas Ziegler
@ 2022-10-05 9:10 ` Andreas Ziegler
2023-08-06 20:45 ` Thomas Petazzoni via buildroot
6 siblings, 1 reply; 20+ messages in thread
From: Andreas Ziegler @ 2022-10-05 9:10 UTC (permalink / raw)
To: buildroot; +Cc: Andreas Ziegler, YANN E MORIN, Thomas Petazzoni
Make inclusion of expat and yajl libraries in the mpd build dependent on
user selection, not presence of the libraries in the target filesystem.
Signed-off-by: Andreas Ziegler <br015@umbiko.net>
---
Changes v1 -> v2:
- based on a discussion w/ Arnout Vandecapelle, Yann Morin
package/mpd/Config.in | 17 ++++++++++++++---
package/mpd/mpd.mk | 24 +++++++++++++++++-------
2 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/package/mpd/Config.in b/package/mpd/Config.in
index daf1469cae..fe4f22201a 100644
--- a/package/mpd/Config.in
+++ b/package/mpd/Config.in
@@ -20,6 +20,16 @@ menuconfig BR2_PACKAGE_MPD
if BR2_PACKAGE_MPD
+# mpd internal feature dependencies
+
+config BR2_PACKAGE_MPD_EXPAT
+ bool
+ select BR2_PACKAGE_EXPAT
+
+config BR2_PACKAGE_MPD_YAJL
+ bool
+ select BR2_PACKAGE_YAJL
+
comment "Archive plugins"
config BR2_PACKAGE_MPD_BZIP2
@@ -48,14 +58,14 @@ config BR2_PACKAGE_MPD_QOBUZ
depends on BR2_PACKAGE_LIBGPG_ERROR_ARCH_SUPPORTS
select BR2_PACKAGE_MPD_CURL
select BR2_PACKAGE_LIBGCRYPT
- select BR2_PACKAGE_YAJL
+ select BR2_PACKAGE_MPD_YAJL
help
Play songs from the commercial streaming service Qobuz.
config BR2_PACKAGE_MPD_SOUNDCLOUD
bool "soundcloud"
select BR2_PACKAGE_MPD_CURL
- select BR2_PACKAGE_YAJL
+ select BR2_PACKAGE_MPD_YAJL
help
Enable soundcloud.com playlist support.
@@ -399,9 +409,9 @@ choice
config BR2_PACKAGE_MPD_UPNP_PUPNP
bool "pupnp"
- select BR2_PACKAGE_EXPAT
select BR2_PACKAGE_LIBUPNP
select BR2_PACKAGE_MPD_CURL
+ select BR2_PACKAGE_MPD_EXPAT
help
Provides UPnP database access through libupnp
(the legacy Portable SDK for UPnP devices).
@@ -412,6 +422,7 @@ config BR2_PACKAGE_MPD_UPNP_NPUPNP
bool "npupnp"
select BR2_PACKAGE_LIBNPUPNP
select BR2_PACKAGE_MPD_CURL
+ select BR2_PACKAGE_MPD_EXPAT
help
Provides UPnP database access through libnpupnp
(a C++ reimplementation of the Portable UPnP library).
diff --git a/package/mpd/mpd.mk b/package/mpd/mpd.mk
index 1269b90661..1c3b659220 100644
--- a/package/mpd/mpd.mk
+++ b/package/mpd/mpd.mk
@@ -78,6 +78,13 @@ else
MPD_CONF_OPTS += -Ddsd=false
endif
+ifeq ($(BR2_PACKAGE_MPD_EXPAT),y)
+MPD_DEPENDENCIES += expat
+MPD_CONF_OPTS += -Dexpat=enabled
+else
+MPD_CONF_OPTS += -Dexpat=disabled
+endif
+
ifeq ($(BR2_PACKAGE_MPD_FAAD2),y)
MPD_DEPENDENCIES += faad2
MPD_CONF_OPTS += -Dfaad=enabled
@@ -252,7 +259,7 @@ MPD_CONF_OPTS += -Dpulse=disabled
endif
ifeq ($(BR2_PACKAGE_MPD_QOBUZ),y)
-MPD_DEPENDENCIES += libgcrypt yajl
+MPD_DEPENDENCIES += libgcrypt
MPD_CONF_OPTS += -Dqobuz=enabled
else
MPD_CONF_OPTS += -Dqobuz=disabled
@@ -273,7 +280,6 @@ MPD_CONF_OPTS += -Dsidplay=disabled
endif
ifeq ($(BR2_PACKAGE_MPD_SOUNDCLOUD),y)
-MPD_DEPENDENCIES += yajl
MPD_CONF_OPTS += -Dsoundcloud=enabled
else
MPD_CONF_OPTS += -Dsoundcloud=disabled
@@ -306,13 +312,10 @@ MPD_CONF_OPTS += -Dtwolame=disabled
endif
ifeq ($(BR2_PACKAGE_MPD_UPNP_PUPNP),y)
-MPD_DEPENDENCIES += \
- expat \
- libupnp
+MPD_DEPENDENCIES += libupnp
MPD_CONF_OPTS += -Dupnp=pupnp
else ifeq ($(BR2_PACKAGE_MPD_UPNP_NPUPNP),y)
-MPD_DEPENDENCIES += \
- libnpupnp
+MPD_DEPENDENCIES += libnpupnp
MPD_CONF_OPTS += -Dupnp=npupnp
else ifeq ($(BR2_PACKAGE_MPD_UPNP_DISABLED),y)
MPD_CONF_OPTS += -Dupnp=disabled
@@ -333,6 +336,13 @@ else
MPD_CONF_OPTS += -Dwavpack=disabled
endif
+ifeq ($(BR2_PACKAGE_MPD_YAJL),y)
+MPD_DEPENDENCIES += yajl
+MPD_CONF_OPTS += -Dyajl=enabled
+else
+MPD_CONF_OPTS += -Dyajl=disabled
+endif
+
ifeq ($(BR2_PACKAGE_MPD_ZZIP),y)
MPD_DEPENDENCIES += zziplib
MPD_CONF_OPTS += -Dzzip=enabled
--
2.34.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Buildroot] [PATCH v2 4/4] package/mpd: introduce sub-options for expat and yajl
2022-10-05 9:10 ` [Buildroot] [PATCH v2 4/4] package/mpd: introduce sub-options for expat and yajl Andreas Ziegler
@ 2023-08-06 20:45 ` Thomas Petazzoni via buildroot
2023-09-11 7:23 ` Peter Korsgaard
0 siblings, 1 reply; 20+ messages in thread
From: Thomas Petazzoni via buildroot @ 2023-08-06 20:45 UTC (permalink / raw)
To: Andreas Ziegler; +Cc: YANN E MORIN, buildroot
Hello Andreas,
On Wed, 5 Oct 2022 11:10:32 +0200
Andreas Ziegler <br015@umbiko.net> wrote:
> Make inclusion of expat and yajl libraries in the mpd build dependent on
> user selection, not presence of the libraries in the target filesystem.
>
> Signed-off-by: Andreas Ziegler <br015@umbiko.net>
So I handled this differently:
- First, one patch that adds the "missing" dependency of
BR2_PACKAGE_MPD_UPNP_NPUPNP on expat:
https://gitlab.com/buildroot.org/buildroot/-/commit/6cdb48a0482a093dcdf73d0d748f5435262515ab
- Second, a patch that takes care of expat and yajl, but without the
intermediate options: as soon as expat or yajl are available in
Buildroot, their support will enabled in mpq:
https://gitlab.com/buildroot.org/buildroot/-/commit/3693462a1f8bd328c1a4f8fb68f00ce9127c31cd
Thanks!
Thomas
--
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Buildroot] [PATCH v2 4/4] package/mpd: introduce sub-options for expat and yajl
2023-08-06 20:45 ` Thomas Petazzoni via buildroot
@ 2023-09-11 7:23 ` Peter Korsgaard
0 siblings, 0 replies; 20+ messages in thread
From: Peter Korsgaard @ 2023-09-11 7:23 UTC (permalink / raw)
To: Thomas Petazzoni via buildroot
Cc: Andreas Ziegler, YANN E MORIN, Thomas Petazzoni
>>>>> "Thomas" == Thomas Petazzoni via buildroot <buildroot@buildroot.org> writes:
> Hello Andreas,
> On Wed, 5 Oct 2022 11:10:32 +0200
> Andreas Ziegler <br015@umbiko.net> wrote:
>> Make inclusion of expat and yajl libraries in the mpd build dependent on
>> user selection, not presence of the libraries in the target filesystem.
>>
>> Signed-off-by: Andreas Ziegler <br015@umbiko.net>
> So I handled this differently:
> - First, one patch that adds the "missing" dependency of
> BR2_PACKAGE_MPD_UPNP_NPUPNP on expat:
> https://gitlab.com/buildroot.org/buildroot/-/commit/6cdb48a0482a093dcdf73d0d748f5435262515ab
> - Second, a patch that takes care of expat and yajl, but without the
> intermediate options: as soon as expat or yajl are available in
> Buildroot, their support will enabled in mpq:
> https://gitlab.com/buildroot.org/buildroot/-/commit/3693462a1f8bd328c1a4f8fb68f00ce9127c31cd
Committed to 2023.02.x and 2023.05.x, thanks.
--
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 20+ messages in thread