On 2018-03-01 03:48, Yousong Zhou wrote: > On 28 February 2018 at 18:58, Felix Fietkau <n...@nbd.name> wrote: >> On 2018-02-28 11:48, Yousong Zhou wrote: >>> On 28 February 2018 at 16:13, Felix Fietkau <n...@nbd.name> wrote: >>>> On 2018-02-28 06:07, Yousong Zhou wrote: >>>>> This is intended to reduce build time for situations like the following >>>>> where python and python-six and their dependencies could still be built >>>>> as long as any subpackage within the srcpackage was selected regardless >>>>> of the selection state of openvswitch-python >>>>> >>>>> define Package/openvswitch-python >>>>> ... >>>>> DEPENDS:=+python +python-six >>>>> endef >>>>> >>>>> Previously we work around this by specifying the dependency as >>>>> +PACKAGE_openvswitch-python:python, which is unintuitive >>>>> >>>>> Signed-off-by: Yousong Zhou <yszhou4t...@gmail.com> >>>> The current behavior is intentional. The idea is that many packages >>>> currently do not use PKG_CONFIG_DEPENDS properly, or specify enable or >>>> disable of extra library support via configure arguments. >>>> >>>> This means that if the dependency depends on the package selection, we >>>> will get a lot of random package build failures depending on the build >>>> order. >>>> >>> >>> Hi Felix, can you describe a concrete example where failure can >>> happen? PKG_CONFIG_DEPENDS and the change here seems orthogonal to >>> each other. >> Let's take the openvswitch package as an example. If you modify it to >> remove the PKG_CONFIG_DEPENDS and PKG_BUILD_DEPENDS, theoretically the >> following scenario could happen: >> >> You make a clean build with openvswitch-python and python itself not >> selected. Since python doesn't get built, openvswitch detects that >> python is not available and can't build its language binding. >> Now you decide that you want python support after all, so you select >> openvswitch-python and run make again. >> It won't rebuild anything in openvswitch, since no stampfiles are >> affected, it will just try to package openvswitch-python. >> This will now fail, because the openvswitch-python package bindings >> could not be built the first time around. > > Current openvswitch does not have this issue as it explicitly selects > "+python" for openvswitch-python. That put aside, even if the said > situation happened where openvswitch-python does not depends on or > select python, it's a flaw in the packaging process that will affect > later usage when e.g. installing it with openvswitch-python as users > will expect the dependency should already be in place. I think you might be missing the point here. I'm not saying that openvswitch is affected now. I'm saying it would be affected if it wasn't using PKG_CONFIG_DEPENDS/PKG_BUILD_DEPENDS. That's why this patch and the use of PKG_CONFIG_DEPENDS is not orthogonal:
If you remove PKG_CONFIG_DEPENDS and PKG_BUILD_DEPENDS from it, here's the sequence that would lead to a build error: - Make a clean build with just openvswitch selected (NOT openvswitch-python or python itself). - After the build is done, select openvswitch-python and run make again. After the first round, openvswitch will have been built without python present. On the second round, it won't be rebuilt, so the python plugins will be missing. These config changes right now only work because PKG_CONFIG_DEPENDS is set properly. >> There are several other packages that have support for plugins that >> depend on various libraries. If the maintainers of those packages are >> not careful about either handling reconfiguration, or specifying >> everything as build dependencies, you can get spurious rebuild bugs like >> this. > > If you meat situations like "+CONFIG_openvpn_use_mbedtls:libmbedtls > +CONFIG_openvpn_use_openssl:libopenssl" where packagers were expected > to add these two "CONFIG_openvpn_use_xx" symbols to > PKG_CONFIG_DEPENDS. > > The suggested change here won't worse the situation: as long as > openvpn got selected, these libmbedtls and libopenssl would still have > their chance to be built. Whether re-configure should happen depends > solely on PKG_CONFIG_DEPENDS. It should still work as before even in > the situation where users switched from use_mbedtls to use_openssl in > which case libopenssl will be rebuilt and it won't intervene with > configure and use of the new lib. That's why I think the change is > orthogonal to the reconfigure issue. Please correct me if I am wrong > or missing something. openvpn is a bad example, because it uses build variants, where you have separate builds (and build dirs) for openssl and for mbedtls. To give you a better example, I just took another look at our packages and found one that would directly be affected by your change: Take a look at libs/elektra/Makefile in packages.git It has various plugins with their own dependencies. One depends on boost, one on python, one on openssl, etc. No PKG_BUILD_DEPENDS or PKG_CONFIG_DEPENDS. Let's run the same scenario as above: - do a clean build with just libelektra-core selected - select libelektra-boost and run make again Now there's two possibilities. Either the build fails on the first round already, because the package was expecting to be built with boost, which isn't available. Or, the build fails on the second round because the first one had built elektra without the boost plugins and there's nothing that triggers a rebuild. I do think that this is something that should get fixed in this package, however let's consider the case where this kind of issue is not detected. Without your patch, the package will always build, but maybe more inefficient because potentially unused dependencies are built along the way. With your patch, the snapshot build will always build (because everything is enabled all the time), however some users might have a different selection for their own build and they will run into build failures. I prefer the inefficient build over random build breakage (undetected by our automatic builds) any time. - Felix _______________________________________________ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
