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

[Yousong Zhou]

On 2 March 2018 at 17:19, Felix Fietkau  wrote:
> On 2018-03-02 10:07, Yousong Zhou wrote:
>> On 1 March 2018 at 17:36, Felix Fietkau  wrote:
>>> 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.
> That will not work in all cases. For some packages, using
> STAMP_CONFIGURED might not be enough (it might cache previous values).
> Different packages need different ways of dealing with this, which is a
> strong reason to not use this patch and keep the behavior opt-in (either
> via the existing conditional depend syntax, or by adding some new
> syntactic sugar).
>

Well, I thought for autotools we the autoreconf fixup can workaround
this, but it seems maybe the situation is just not that simple ;)

>> 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 don't know of any case where we need to add PKG_BUILD_DEPENDS, it
> could only serve as a workaround for breakage caused by your patch. But
> I don't think we should go that route.
>
> - Felix

Got it.  At least the current behaviour provides a consistent
configure environment for each package.  That's a good feature to
have.

Thanks,
yousong

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

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

[Felix Fietkau]

On 2018-03-02 10:07, Yousong Zhou wrote:
> On 1 March 2018 at 17:36, Felix Fietkau  wrote:
>> 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.
That will not work in all cases. For some packages, using
STAMP_CONFIGURED might not be enough (it might cache previous values).
Different packages need different ways of dealing with this, which is a
strong reason to not use this patch and keep the behavior opt-in (either
via the existing conditional depend syntax, or by adding some new
syntactic sugar).

> 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 don't know of any case where we need to add PKG_BUILD_DEPENDS, it
could only serve as a workaround for breakage caused by your patch. But
I don't think we should go that route.

- Felix

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

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

[Yousong Zhou]

On 1 March 2018 at 17:36, Felix Fietkau  wrote:
> On 2018-03-01 03:48, Yousong Zhou wrote:
>> On 28 February 2018 at 18:58, Felix Fietkau  wrote:
>>> On 2018-02-28 11:48, Yousong Zhou wrote:
 On 28 February 2018 at 16:13, Felix Fietkau  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 
> 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 

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

[Felix Fietkau]

On 2018-03-01 03:48, Yousong Zhou wrote:
> On 28 February 2018 at 18:58, Felix Fietkau  wrote:
>> On 2018-02-28 11:48, Yousong Zhou wrote:
>>> On 28 February 2018 at 16:13, Felix Fietkau  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 
 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 

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

[Yousong Zhou]

On 28 February 2018 at 18:58, Felix Fietkau  wrote:
> On 2018-02-28 11:48, Yousong Zhou wrote:
>> On 28 February 2018 at 16:13, Felix Fietkau  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 
>>> 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.

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

Regards,
yousong

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

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

[Felix Fietkau]

On 2018-02-28 11:48, Yousong Zhou wrote:
> On 28 February 2018 at 16:13, Felix Fietkau  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 
>> 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.

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.

The conditional dependency is a way to tell the build system which
dependencies are properly vetted for making the build dependency
conditional as well.

- Felix

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

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

[Yousong Zhou]

On 28 February 2018 at 16:13, Felix Fietkau  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 
> 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.

Regards,
yousong

> The idea behind this is if a package is known to handle this well, it
> can use the conditional dep syntax to indicate it.
>
> I agree that the syntax is not very intuitive, but I think it would be
> much better to add some syntactic sugar instead of adding more
> potentially hard to find build bugs.

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

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

[Felix Fietkau]

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

The idea behind this is if a package is known to handle this well, it
can use the conditional dep syntax to indicate it.

I agree that the syntax is not very intuitive, but I think it would be
much better to add some syntactic sugar instead of adding more
potentially hard to find build bugs.

- Felix

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

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

[Alexandru Ardelean]

On Wed, Feb 28, 2018 at 7:07 AM, 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

This is pretty cool actually.
I never thought of changing this logic ; I just took this behavior as
a given behavior for packages.

Thanks for this
Alex

>
> Signed-off-by: Yousong Zhou 
> ---
>  scripts/package-metadata.pl | 13 ++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/package-metadata.pl b/scripts/package-metadata.pl
> index 53bb45a..b827154 100755
> --- a/scripts/package-metadata.pl
> +++ b/scripts/package-metadata.pl
> @@ -382,6 +382,7 @@ sub gen_package_mk() {
> my %deplines = ('' => {});
>
> foreach my $pkg (@{$src->{packages}}) {
> +   my @pkgdeplines;
> foreach my $dep (@{$pkg->{depends}}) {
> next if ($dep =~ /@/);
>
> @@ -410,10 +411,17 @@ sub gen_package_mk() {
> }
> my $depline = 
> get_conditional_dep($condition, $depstr);
> if ($depline) {
> -   $deplines{''}{$depline}++;
> +   push @pkgdeplines, $depline;
> }
> }
> }
> +   if (@pkgdeplines) {
> +   my $depstr = join(' ', @pkgdeplines);
> +   if ($depstr) {
> +   my $depline = 
> get_conditional_dep('PACKAGE_'.$pkg->{name}, $depstr);
> +   $deplines{''}{$depline}++;
> +   }
> +   }
>
> my $config = '';
> $config = sprintf '$(CONFIG_PACKAGE_%s)', 
> $pkg->{name} unless $pkg->{buildonly};
> @@ -486,8 +494,7 @@ sub gen_package_mk() {
> }
>
> foreach my $suffix (sort keys %deplines) {
> -   my $depline = join(" ", sort keys 
> %{$deplines{$suffix}});
> -   if ($depline) {
> +   for my $depline (sort keys %{$deplines{$suffix}}) {
> $line .= sprintf "\$(curdir)/%s/compile += 
> %s\n", $src->{path}.$suffix, $depline;
> }
> }
> --
> 1.8.3.1
>

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev