[meta-oe][PATCH] freerdp: Use DEPENDS:append

[meta-oe][PATCH] freerdp: Use DEPENDS:append

[Marek Vasut]

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

Re: [oe] [meta-oe][PATCH] freerdp: Use DEPENDS:append

[Jacob Kroon]

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

Re: [oe] [meta-oe][PATCH] freerdp: Use DEPENDS:append

[Alexander Kanavin]

[-- Attachment #1: Type: text/plain, Size: 969 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 1840 bytes --]

Re: [oe] [meta-oe][PATCH] freerdp: Use DEPENDS:append

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 1740 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 3300 bytes --]

Re: [oe] [meta-oe][PATCH] freerdp: Use DEPENDS:append

[Marek Vasut]

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.

Re: [oe] [meta-oe][PATCH] freerdp: Use DEPENDS:append

[Alexander Kanavin]

[-- Attachment #1: Type: text/plain, Size: 1231 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 2243 bytes --]

Re: [oe] [meta-oe][PATCH] freerdp: Use DEPENDS:append

[Marek Vasut]

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 ?

Re: [oe] [meta-oe][PATCH] freerdp: Use DEPENDS:append

[Alexander Kanavin]

[-- Attachment #1: Type: text/plain, Size: 622 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 1024 bytes --]

Re: [oe] [meta-oe][PATCH] freerdp: Use DEPENDS:append

[Marek Vasut]

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.

Re: [oe] [meta-oe][PATCH] freerdp: Use DEPENDS:append

[Konrad Weihmann]

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.

Re: [oe] [meta-oe][PATCH] freerdp: Use DEPENDS:append

[Konrad Weihmann]

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.

Re: [oe] [meta-oe][PATCH] freerdp: Use DEPENDS:append

[Konrad Weihmann]

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.