| Message ID | 20211207113954.8959-1-marex@denx.de |
|---|---|
| State | New |
| Headers | show |
| Series | [meta-oe] freerdp: Use DEPENDS:append | expand |
On 12/7/21 12:39, Marek Vasut wrote: > The build system might have put something into DEPENDS already, > adhere to the recommended best practice and use DEPENDS:append > to avoid overriding the DEPENDS set by the build system. > Which document states that this is considered best practice ? Jacob
I don't think this makes sense either FWIW. The standard practice is that the recipe sets DEPENDS and then the classes add to it. Alex On Tue, 7 Dec 2021 at 13:04, Jacob Kroon <jacob.kroon@gmail.com> wrote: > On 12/7/21 12:39, Marek Vasut wrote: > > The build system might have put something into DEPENDS already, > > adhere to the recommended best practice and use DEPENDS:append > > to avoid overriding the DEPENDS set by the build system. > > > > Which document states that this is considered best practice ? > > Jacob > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#94228): > https://lists.openembedded.org/g/openembedded-devel/message/94228 > Mute This Topic: https://lists.openembedded.org/mt/87562882/1686489 > Group Owner: openembedded-devel+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-devel/unsub [ > alex.kanavin@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- > >
> The build system might have put something into DEPENDS already There is only copyright notice before this chunk, so this doesn't make much sense to me (and no other recipe does this). On Tue, Dec 7, 2021 at 12:40 PM Marek Vasut <marex@denx.de> wrote: > The build system might have put something into DEPENDS already, > adhere to the recommended best practice and use DEPENDS:append > to avoid overriding the DEPENDS set by the build system. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Alexander Kanavin <alex@linutronix.de> > Cc: Khem Raj <raj.khem@gmail.com> > --- > meta-oe/recipes-support/freerdp/freerdp_2.4.1.bb | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/meta-oe/recipes-support/freerdp/freerdp_2.4.1.bb > b/meta-oe/recipes-support/freerdp/freerdp_2.4.1.bb > index 571ba5fcb..1604838f1 100644 > --- a/meta-oe/recipes-support/freerdp/freerdp_2.4.1.bb > +++ b/meta-oe/recipes-support/freerdp/freerdp_2.4.1.bb > @@ -3,7 +3,7 @@ > > DESCRIPTION = "FreeRDP RDP client & server library" > HOMEPAGE = "http://www.freerdp.com" > -DEPENDS = "openssl alsa-lib libusb1" > +DEPENDS:append = " openssl alsa-lib libusb1" > SECTION = "net" > LICENSE = "Apache-2.0" > LIC_FILES_CHKSUM = "file://LICENSE;md5=3b83ef96387f14655fc854ddc3c6bd57" > -- > 2.33.0 > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#94227): > https://lists.openembedded.org/g/openembedded-devel/message/94227 > Mute This Topic: https://lists.openembedded.org/mt/87562882/3617156 > Group Owner: openembedded-devel+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-devel/unsub [ > Martin.Jansa@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- > >
On 12/7/21 13:35, Martin Jansa wrote: >> The build system might have put something into DEPENDS already > > There is only copyright notice before this chunk, so this doesn't make much > sense to me (and no other recipe does this). At least oelint-adv warns about it here: https://github.com/priv-kweihmann/oelint-adv/blob/master/oelint_adv/rule_base/rule_var_depends_append.py And as far as I can tell, bitbake can stick something into DEPENDS before it parses this recipe, so the :append is valid. Furthermore, git grep through oe-core already indicates a few recipes which use DEPENDS:append.
Perhaps this issue should be addressed to Konrad first before rushing to write a patch? Alex On Tue, 7 Dec 2021 at 14:22, Marek Vasut <marex@denx.de> wrote: > On 12/7/21 13:35, Martin Jansa wrote: > >> The build system might have put something into DEPENDS already > > > > There is only copyright notice before this chunk, so this doesn't make > much > > sense to me (and no other recipe does this). > > At least oelint-adv warns about it here: > > https://github.com/priv-kweihmann/oelint-adv/blob/master/oelint_adv/rule_base/rule_var_depends_append.py > > And as far as I can tell, bitbake can stick something into DEPENDS > before it parses this recipe, so the :append is valid. > > Furthermore, git grep through oe-core already indicates a few recipes > which use DEPENDS:append. > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#94231): > https://lists.openembedded.org/g/openembedded-devel/message/94231 > Mute This Topic: https://lists.openembedded.org/mt/87562882/1686489 > Group Owner: openembedded-devel+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-devel/unsub [ > alex.kanavin@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- > >
On 12/7/21 14:23, Alexander Kanavin wrote: > Perhaps this issue should be addressed to Konrad first before rushing to > write a patch? > > Alex > > On Tue, 7 Dec 2021 at 14:22, Marek Vasut <marex@denx.de> wrote: > >> On 12/7/21 13:35, Martin Jansa wrote: >>>> The build system might have put something into DEPENDS already >>> >>> There is only copyright notice before this chunk, so this doesn't make >> much >>> sense to me (and no other recipe does this). >> >> At least oelint-adv warns about it here: >> >> https://github.com/priv-kweihmann/oelint-adv/blob/master/oelint_adv/rule_base/rule_var_depends_append.py >> >> And as far as I can tell, bitbake can stick something into DEPENDS >> before it parses this recipe, so the :append is valid. >> >> Furthermore, git grep through oe-core already indicates a few recipes >> which use DEPENDS:append. I believe the warning is valid, see above ?
On Tue, 7 Dec 2021 at 14:27, Marek Vasut <marex@denx.de> wrote: > >> And as far as I can tell, bitbake can stick something into DEPENDS > >> before it parses this recipe, so the :append is valid. > >> > >> Furthermore, git grep through oe-core already indicates a few recipes > >> which use DEPENDS:append. > > I believe the warning is valid, see above ? > The linter is an obsessive pedant, and is not actually always right, that's why I want its author to comment :) 'DEPENDS = ... ' is used throughout all layers, and doesn't seem to cause much grief, so why fix it here specifically, is there an actual issue? Alex
On 12/7/21 14:33, Alexander Kanavin wrote: > On Tue, 7 Dec 2021 at 14:27, Marek Vasut <marex@denx.de> wrote: > >>>> And as far as I can tell, bitbake can stick something into DEPENDS >>>> before it parses this recipe, so the :append is valid. >>>> >>>> Furthermore, git grep through oe-core already indicates a few recipes >>>> which use DEPENDS:append. >> >> I believe the warning is valid, see above ? >> > > The linter is an obsessive pedant, and is not actually always right, that's > why I want its author to comment :) > 'DEPENDS = ... ' is used throughout all layers, and doesn't seem to cause > much grief, so why fix it here specifically, is there an actual issue? I believe it would be a good idea to start fixing it all over the place. So yes, let's wait for Konrad.
In this case I have to support Alex's viewpoint partially... On way how a DEPENDS operation before that can be injected is via INHERIT += "some-class" in local/distro/etc .conf... and if this class adds their own DEPENDS, it would get overwritten here, that's why I would say (esp as the described use case is very rare) it should be fine for now to use it as it is - *but* I would actually take that discussion to the oe-arch list - guess it's worth discussing that in a broader scope (as we already did in the past for those corner cases of variable modifications) BTW thx for the praise of the linter for being overly pedantic sometimes :-) On 07.12.21 14:35, Marek Vasut wrote: > On 12/7/21 14:33, Alexander Kanavin wrote: >> On Tue, 7 Dec 2021 at 14:27, Marek Vasut <marex@denx.de> wrote: >> >>>>> And as far as I can tell, bitbake can stick something into DEPENDS >>>>> before it parses this recipe, so the :append is valid. >>>>> >>>>> Furthermore, git grep through oe-core already indicates a few recipes >>>>> which use DEPENDS:append. >>> >>> I believe the warning is valid, see above ? >>> >> >> The linter is an obsessive pedant, and is not actually always right, >> that's >> why I want its author to comment :) >> 'DEPENDS = ... ' is used throughout all layers, and doesn't seem to cause >> much grief, so why fix it here specifically, is there an actual issue? > > I believe it would be a good idea to start fixing it all over the place. > So yes, let's wait for Konrad.
I added something to the oe-arch ML [1] - please let's discuss the issue and all of the implications there [1] https://lists.openembedded.org/g/openembedded-architecture/message/1372 On 07.12.21 14:35, Marek Vasut wrote: > On 12/7/21 14:33, Alexander Kanavin wrote: >> On Tue, 7 Dec 2021 at 14:27, Marek Vasut <marex@denx.de> wrote: >> >>>>> And as far as I can tell, bitbake can stick something into DEPENDS >>>>> before it parses this recipe, so the :append is valid. >>>>> >>>>> Furthermore, git grep through oe-core already indicates a few recipes >>>>> which use DEPENDS:append. >>> >>> I believe the warning is valid, see above ? >>> >> >> The linter is an obsessive pedant, and is not actually always right, >> that's >> why I want its author to comment :) >> 'DEPENDS = ... ' is used throughout all layers, and doesn't seem to cause >> much grief, so why fix it here specifically, is there an actual issue? > > I believe it would be a good idea to start fixing it all over the place. > So yes, let's wait for Konrad.
After it seems like we reached an agreement on the oe-arch ML, I just released a new version of the linter that doesn't mark this issue anymore. On 07.12.21 14:35, Marek Vasut wrote: > On 12/7/21 14:33, Alexander Kanavin wrote: >> On Tue, 7 Dec 2021 at 14:27, Marek Vasut <marex@denx.de> wrote: >> >>>>> And as far as I can tell, bitbake can stick something into DEPENDS >>>>> before it parses this recipe, so the :append is valid. >>>>> >>>>> Furthermore, git grep through oe-core already indicates a few recipes >>>>> which use DEPENDS:append. >>> >>> I believe the warning is valid, see above ? >>> >> >> The linter is an obsessive pedant, and is not actually always right, >> that's >> why I want its author to comment :) >> 'DEPENDS = ... ' is used throughout all layers, and doesn't seem to cause >> much grief, so why fix it here specifically, is there an actual issue? > > I believe it would be a good idea to start fixing it all over the place. > So yes, let's wait for Konrad.
diff --git a/meta-oe/recipes-support/freerdp/freerdp_2.4.1.bb b/meta-oe/recipes-support/freerdp/freerdp_2.4.1.bb index 571ba5fcb..1604838f1 100644 --- a/meta-oe/recipes-support/freerdp/freerdp_2.4.1.bb +++ b/meta-oe/recipes-support/freerdp/freerdp_2.4.1.bb @@ -3,7 +3,7 @@ DESCRIPTION = "FreeRDP RDP client & server library" HOMEPAGE = "http://www.freerdp.com" -DEPENDS = "openssl alsa-lib libusb1" +DEPENDS:append = " openssl alsa-lib libusb1" SECTION = "net" LICENSE = "Apache-2.0" LIC_FILES_CHKSUM = "file://LICENSE;md5=3b83ef96387f14655fc854ddc3c6bd57"
The build system might have put something into DEPENDS already, adhere to the recommended best practice and use DEPENDS:append to avoid overriding the DEPENDS set by the build system. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Alexander Kanavin <alex@linutronix.de> Cc: Khem Raj <raj.khem@gmail.com> --- meta-oe/recipes-support/freerdp/freerdp_2.4.1.bb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)