[LEDE-DEV] [PATCH] metadata: compile dependencies only when the package is selected

[Yousong Zhou]

On 1 March 2018 at 17:36, Felix Fietkau <nbd at nbd.name> wrote:
> On 2018-03-01 03:48, Yousong Zhou wrote:
>> On 28 February 2018 at 18:58, Felix Fietkau <nbd at nbd.name> wrote:
>>> On 2018-02-28 11:48, Yousong Zhou wrote:
>>>> On 28 February 2018 at 16:13, Felix Fietkau <nbd at 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 <yszhou4tech at 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 see.  I can fix the 2nd case by letting STAMP_CONFIGURED depend on
selection state of subpackages.  It's indeed pain if packagers need to
guess whether they should put each CONFIG_PACKAGE_xx symbol into
PKG_CONFIG_DEPENDS.

As for PKG_BUILD_DEPENDS, if any error happens it's not "random"
right?  Can we be more strict here by requiring packagers to put
correct values for this symbol?

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

Thank you for the detailed explanation.

Regards,
                yousong