--- Begin Message ---
TLDR: this avoids recursive dependencies that have haunted python
packages, and has been tested to ensure it does not break anything.
Sorry for the long post, but I intend to document this in the wiki, so
my explanation here (skip the testing) should be reviewed as well.
The motivation behind the patch:
python packages have always been on the verge of creating circular
dependencies. For example:
https://github.com/openwrt/packages/issues/8535
https://github.com/openwrt/packages/pull/8407
They are caused by a conditional dependency for bluetooth support, which
depends on USB_SUPPORT, a symbol that cannot be selected, since it is
defined based on the chosen target.
The python-light package's current DEPENDS line is:
DEPENDS:=+python-base +libffi +libbz2 +PYTHON_BLUETOOTH_SUPPORT:bluez-libs
While adding 'depends on USB_SUPPORT' to the PYTHON_BLUETOOTH_SUPPORT
definition would prevent the compilation of bluetooth support for
systems that lack USB support, it will not satisfy the config.in
generator.
So the real fix is to select bluez-libs only if USB_SUPPORT is defined:
DEPENDS:=...libbz2 +(PYTHON_BLUETOOTH_SUPPORT&&USB_SUPPORT):bluez-libs
How the build system works with DEPENDS:
There are three parts of the build systems that deal with the DEPENDS
line:
The config.in generator (I'm making up names as I go), which generates
tmp/.config-package.in (among others), borrowed from the linux kernel,
does the menuconfig logic. It has extensive support for operators, and
correctly implements nesting with parentheses () and the ! operator in
all positions. This is not being touched.
Then, there are scripts/package-metadata.pl, that generates the
tmp/.packageinfo file, gathering info for all installed packages, and
tmp/.packagedeps, which contains the Makefile dependencies for every
package.
Finally, the include/package-ipkg.mk file has a recipe to build the
package 'control' file, used by opkg to figure out dependencies at
install time.
How the syntax works:
Currently the build system understands a few conditional operators, but
their support is not consistent. menuconfig support is comp
lete, the other components are not:
@: these conditions apply only to the menuconfig (config.in), so they
are ignored by the other components. Its syntax is more flexible
and powerful than the other two components.
(): parentheses are completely ignored by package-metadata.pl, and by
package-ipk.mk, but work as expected for @ conditions. So use it
for readability only elsewhere, they don't change the order of
evaluation. Be careful to not end up with one set of conditions
working in menuconfig, and a different set with the opkg package
deps!
!: reverse the condition; with menuconfig, it works everywhere; it only
works in package-metadata.pl and package-ipk.mk if it is the first
operator, so it reverses the whole condition. You can write
+!TARGET_x86:package, but +(!TARGET_x86||USB_SUPPORT):somepackage
won't work as expected. One would expect it to select
somepackage if either TARGET_x86 is not selected or if USB_SUPPORT
is. It will, however, select somepackage unless TARGET_x86 or
USB_SUPPORT is selected, it would be the equivalent of
+!(TARGET_x86||USB_SUPPORT):somepackage).
Don't write an "unexpected" condition on purpose, as the menuconfig
component does understand proper ()/! syntax.
&&: logical and: this has been added with this patch to
package-metadata.pl and to package-ipk.mk, where it is always
evaluated before ||.
||: logical or: this has been supported in package-ipk.mk, but not in
package-metadata.pl before this patch. Now it works with all
components.
Testing:
It took me so long to test this, and I was trying to be so cautious not
to break things, that I might as well inflict it on people willing to
read this through. If you don't care, you may stop now.
To make sure this does not break anything, I started with a snapshot
from 2019-03-15, in which I had all packages previously built. I'm
building this for a Linksys WRT3200ACM. I've applied the current patch
and rebuilt the whole tree, then I used the following script to compare
the 'Depends:' lines in the control file of every package:
#!/bin/bash
get_deps()
{
tar xzOf $1 ./control.tar.gz | tar xzOf - ./control | egrep '^Depends:'
}
cd /home/equeiroz/src/openwrt/bin
n=0
failed=0
for f in $(find -name *.ipk); do
let n++
pkg=$(basename $f)
bin_dep=$(get_deps $f)
www_dep=$(get_deps ../www/cote-2019-03-15/$f)
echo Testing ${pkg}...
if [ "${bin_dep}" != "${www_dep}" ]; then
let failed++
echo Difference in: $pkg
echo www: ${www_dep}
echo bin: ${bin_dep}
echo
fi
done
echo Complete: ${n} packages tested, ${failed} changes detected.
The result:
Complete: 7390 packages tested, 0 changes detected.
git diff --no-index --word-diff {old,new}/.packagedeps yielded 5
differences, all as intended. You can notice here where the system
fails to produce a valid dependency (it would just write CONFIG_ before
the whole condition:
$(curdir)/feeds/packages/flashrom/compile +=
$(curdir)/feeds/packages/libftdi1/compile
$(curdir)/feeds/packages/pciutils/compile $(curdir)/libs/libusb-compat/compile
$(curdir)/libs/libusb/compile $(curdir)/libs/toolchain/compile
$(if[-$(CONFIG_(TARGET_x86||TARGET_x86_64)),$(curdir)/feeds/packages/dmidecode/compile)
$(if-] $(CONFIG_GCC_LIBSSP),$(curdir)/libs/toolchain/compile) $(if
{+$(CONFIG_TARGET_x86)$(CONFIG_TARGET_x86_64),$(curdir)/feeds/packages/dmidecode/compile)
$(if+} $(CONFIG_USE_GLIBC),$(curdir)/libs/toolchain/compile)
$(curdir)/network/utils/iproute2/compile += $(curdir)/kernel/linux/compile
$(curdir)/libs/elfutils/compile $(curdir)/libs/libmnl/compile
$(curdir)/libs/libnl-tiny/compile $(curdir)/libs/toolchain/compile
$(curdir)/network/utils/iptables/compile
$(if[-$(CONFIG_(PACKAGE_devlink||PACKAGE_rdma)),$(curdir)/libs/libmnl/compile)
$(if $(CONFIG_(PACKAGE_tc||PACKAGE_ip-full)),$(curdir)/libs/elfutils/compile)
$(if-] $(CONFIG_BUILD_NLS),,$(curdir)/libs/gettext/compile) $(if
$(CONFIG_BUILD_NLS),,$(curdir)/libs/libiconv/compile) $(if
$(CONFIG_GCC_LIBSSP),$(curdir)/libs/toolchain/compile) $(if
{+$(CONFIG_PACKAGE_devlink)$(CONFIG_PACKAGE_rdma),$(curdir)/libs/libmnl/compile)
$(if
$(CONFIG_PACKAGE_tc)$(CONFIG_PACKAGE_ip-full),$(curdir)/libs/elfutils/compile)
$(if+} $(CONFIG_USE_GLIBC),$(curdir)/libs/toolchain/compile)
$(curdir)/kernel/mac80211/compile +=
$(curdir)/firmware/b43legacy-firmware/compile
$(curdir)/firmware/linux-firmware/compile
$(curdir)/firmware/prism54-firmware/compile
$(curdir)/firmware/wireless-regdb/compile $(curdir)/kernel/linux/compile
$(curdir)/network/services/hostapd/compile
$(if[-$(CONFIG_(TARGET_brcm47xx||TARGET_brcm63xx)),,$(curdir)/kernel/linux/compile)
$(if-] $(CONFIG_ATH10K_THERMAL),$(curdir)/kernel/linux/compile) $(if
$(CONFIG_BRCMFMAC_SDIO),$(curdir)/kernel/linux/compile) $(if
$(CONFIG_BRCMFMAC_USB),$(curdir)/firmware/linux-firmware/compile) $(if
$(CONFIG_BRCMFMAC_USB),$(curdir)/kernel/linux/compile) $(if
$(CONFIG_BRCMSMAC_USE_FW_FROM_WL),,$(curdir)/firmware/linux-firmware/compile)
$(if $(CONFIG_PACKAGE_iw),$(curdir)/network/utils/iw/compile) $(if
$(CONFIG_PACKAGE_iw-full),$(curdir)/network/utils/iw/compile) $(if
{+$(CONFIG_TARGET_brcm47xx)$(CONFIG_TARGET_brcm63xx),,$(curdir)/kernel/linux/compile)
$(if+} $(CONFIG_TARGET_brcm47xx),,$(curdir)/kernel/linux/compile)
$(curdir)/feeds/packages/nginx/compile +=
$(curdir)/feeds/packages/expat/compile $(curdir)/feeds/packages/pcre/compile
$(curdir)/feeds/packages/uwsgi-cgi/compile $(curdir)/libs/openssl/compile
$(curdir)/libs/toolchain/compile $(curdir)/libs/zlib/compile
$(curdir)/utils/lua/compile
$(if[-$(CONFIG_(NGINX_SSL||NGINX_HTTP_CACHE||NGINX_HTTP_AUTH_BASIC)),$(curdir)/libs/openssl/compile)
$(if-] $(CONFIG_GCC_LIBSSP),$(curdir)/libs/toolchain/compile) $(if
$(CONFIG_NGINX_DAV),$(curdir)/feeds/packages/expat/compile) $(if
$(CONFIG_NGINX_HTTP_GZIP),$(curdir)/libs/zlib/compile) $(if
$(CONFIG_NGINX_LUA),$(curdir)/utils/lua/compile) $(if
$(CONFIG_NGINX_PCRE),$(curdir)/feeds/packages/pcre/compile) $(if
{+$(CONFIG_NGINX_SSL)$(CONFIG_NGINX_HTTP_CACHE)$(CONFIG_NGINX_HTTP_AUTH_BASIC),$(curdir)/libs/openssl/compile)
$(if+} $(CONFIG_USE_GLIBC),$(curdir)/libs/toolchain/compile)
$(curdir)/devel/perf/compile += $(curdir)/devel/binutils/compile
$(curdir)/libs/elfutils/compile $(curdir)/libs/toolchain/compile
$(if[-$(CONFIG_(mips||mipsel||powerpc||i386||x86_64||arm||aarch64)),$(curdir)/libs/libunwind/compile)
$(if-] $(CONFIG_GCC_LIBSSP),$(curdir)/libs/toolchain/compile) $(if
$(CONFIG_USE_GLIBC),$(curdir)/libs/toolchain/compile) {+$(if
$(CONFIG_mips)$(CONFIG_mipsel)$(CONFIG_powerpc)$(CONFIG_i386)$(CONFIG_x86_64)$(CONFIG_arm)$(CONFIG_aarch64),$(curdir)/libs/libunwind/compile)+}
It's tricky to do a straight diff with the tmp/.config-package.in files
as the order of the various select lines is different with every
invocation, so I examined parts of the file by hand, and ran diff after
sort: no differences found, as expected.
To test this in action, I changed various packages' DEPENDS lines and
made sure they worked as expected.
I could not consistently measure a difference in package building time.
--- End Message ---
