Re: [LEDE-DEV] [PATCH] metadata: compile dependencies only when the package is selected
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
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
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 packages.git > It has various plugins with their own dependencies. > One depend
Re: [LEDE-DEV] [PATCH] metadata: compile dependencies only when the package is selected
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 scenario as above: - do a clean build with just libelekt
Re: [LEDE-DEV] [PATCH] metadata: compile dependencies only when the package is selected
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
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
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
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
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