[Buildroot] [PATCH 1/1] package/mpd: explicitly disable features to avoid collision with host packages

[Andreas Ziegler]

Hi Arnout,

Thank you for taking a look.

On 2022-04-04 17:38, Arnout Vandecappelle wrote:
> On 31/03/2022 15:22, Andreas Ziegler wrote:
>> Background
>> During configuration, the meson build system tries to determine the
>> availability of optional dependencies using (host) pkgconfig and cmake 
>> in that
>> order. If a library does not exist on the target, pkg-config will 
>> fail, but
>> cmake sometimes finds and reports libraries that exist as host 
>> packages. This
>> has been observed for host-expat (cmake dependency) and host-zlib. The 
>> link
>> step subsequently fails, because necessary files are not present in 
>> the target
>> architecture.
>> 
>> Unconditionally disable optional features often found in host binaries 
>> and
>> modify the menu selection processing in mpd.mk to re-enable them where
>> necessary. Currently this concerns expat and zlib only.
>> 
>> This fixes the following build errors:
>> 
>> [expat]
>> /home/data/buildroot.x86_64/host/lib/gcc/x86_64-buildroot-linux-uclibc/11.2.0/../../../../x86_64-buildroot-linux-uclibc/bin/ld: 
>> warning: libc.so.6, needed by 
>> /home/data/buildroot.x86_64/host/lib/libexpat.so.1.8.7, not found (try 
>> using -rpath or -rpath-link)
>> /home/data/buildroot.x86_64/host/lib/gcc/x86_64-buildroot-linux-uclibc/11.2.0/../../../../x86_64-buildroot-linux-uclibc/bin/ld: 
>> /home/data/buildroot.x86_64/host/lib/libexpat.so.1.8.7: undefined 
>> reference to `__errno_location at GLIBC_2.2.5'
>> 
>> [zlib]
>> http://autobuild.buildroot.net/results/f0a/f0a9e719114f19dc9d20622ed85dd4f8e968c20f/
>> 
>> Signed-off-by: Andreas Ziegler <br015 at umbiko.net>
>> ---
>>   package/mpd/mpd.mk | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>> 
>> diff --git a/package/mpd/mpd.mk b/package/mpd/mpd.mk
>> index 12da36098f..4e67c9428c 100644
>> --- a/package/mpd/mpd.mk
>> +++ b/package/mpd/mpd.mk
>> @@ -14,6 +14,7 @@ MPD_LICENSE_FILES = COPYING
>>   # these refer to the FreeBSD PPP daemon
>>   MPD_IGNORE_CVES = CVE-2020-7465 CVE-2020-7466
>>   MPD_SELINUX_MODULES = mpd
>> +# These features are either unwanted or not selectable via the 
>> Buildroot menu
>>   MPD_CONF_OPTS = \
>>   	-Daudiofile=disabled \
>>   	-Ddocumentation=disabled \
>> @@ -21,6 +22,12 @@ MPD_CONF_OPTS = \
>>   	-Dpipewire=disabled \
>>   	-Dsnapcast=false
>>   +# Explicitly disable features where meson's dependency detection 
>> picks up host
>> +# libraries. These settings can be overridden through menu options 
>> later
>> +MPD_CONF_OPTS += \
>> +	-Dexpat=disabled \
>> +	-Dzlib=disabled
> 
>  It would be better to make these automatic conditional dependencies,
> like it's done for ICU.

Good idea, but inclusion of ICU should be initiated by Config.in, like 
every other feature.

>> +
>>   # Zeroconf support depends on libdns_sd from avahi.
>>   ifeq ($(BR2_PACKAGE_MPD_AVAHI_SUPPORT),y)
>>   MPD_DEPENDENCIES += avahi
>> @@ -300,11 +307,11 @@ ifeq ($(BR2_PACKAGE_MPD_UPNP_PUPNP),y)
>>   MPD_DEPENDENCIES += \
>>   	expat \
> 
>  That would also allow to remove this (it will be done implicitly
> since expat is selected in Config.in and the automatic dependency will
> add it).
> 
>>   	libupnp
>> -MPD_CONF_OPTS += -Dupnp=pupnp
>> +MPD_CONF_OPTS += -Dupnp=pupnp -Dexpat=enabled
>>   else ifeq ($(BR2_PACKAGE_MPD_UPNP_NPUPNP),y)
>>   MPD_DEPENDENCIES += \
>>   	libnpupnp
>> -MPD_CONF_OPTS += -Dupnp=npupnp
>> +MPD_CONF_OPTS += -Dupnp=npupnp -Dexpat=enabled
> 
>  This is wrong: no dependency on expat is added so expat may not even
> be selected or built at this point.

Actually, the dependency is inherited from libnpupnp, but you are right, 
this handling is not intuitive.

>  Marked as Changes Requested.

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.

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.

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.

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).

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.

Create entry 'unicode' for ICU in Config.in.

Set zlib to disabled (not used currently).

Thoughts?

Kind regards,
Andreas

>  Regards,
>  Arnout
> 
>>   else ifeq ($(BR2_PACKAGE_MPD_UPNP_DISABLED),y)
>>   MPD_CONF_OPTS += -Dupnp=disabled
>>   endif