[PATCH v2] u-boot: add /boot and ${sysconfdir} to SYSROOT_DIRS

[PATCH v2] u-boot: add /boot and ${sysconfdir} to SYSROOT_DIRS

[Diego Sueiro]

Since do_install is shipping files to /boot and ${sysconfdir}, we can include
these dirs to SYSROOT_DIRS and allow recipes which depend on u-boot to use its
installed artifacts from RECIPE_SYSROOT instead of DEPLOY_DIR_IMAGE.

Signed-off-by: Diego Sueiro <diego.sueiro@arm.com>
---
 meta/recipes-bsp/u-boot/u-boot.inc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meta/recipes-bsp/u-boot/u-boot.inc b/meta/recipes-bsp/u-boot/u-boot.inc
index 251178db33..c53bcd05cc 100644
--- a/meta/recipes-bsp/u-boot/u-boot.inc
+++ b/meta/recipes-bsp/u-boot/u-boot.inc
@@ -266,6 +266,8 @@ FILES_${PN}-env = " \
 "
 
 FILES_${PN} = "/boot ${datadir}"
+SYSROOT_DIRS += "/boot ${sysconfdir}"
+
 RDEPENDS_${PN} += "${PN}-env"
 
 do_deploy () {
-- 
2.17.1

Re: [OE-core] [PATCH v2] u-boot: add /boot and ${sysconfdir} to SYSROOT_DIRS

[Richard Purdie]

On Mon, 2021-01-04 at 08:30 +0000, Diego Sueiro wrote:
> Since do_install is shipping files to /boot and ${sysconfdir}, we can
> include
> these dirs to SYSROOT_DIRS and allow recipes which depend on u-boot
> to use its
> installed artifacts from RECIPE_SYSROOT instead of DEPLOY_DIR_IMAGE.
> 
> Signed-off-by: Diego Sueiro <diego.sueiro@arm.com>
> ---
>  meta/recipes-bsp/u-boot/u-boot.inc | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/meta/recipes-bsp/u-boot/u-boot.inc b/meta/recipes-bsp/u-
> boot/u-boot.inc
> index 251178db33..c53bcd05cc 100644
> --- a/meta/recipes-bsp/u-boot/u-boot.inc
> +++ b/meta/recipes-bsp/u-boot/u-boot.inc
> @@ -266,6 +266,8 @@ FILES_${PN}-env = " \
>  "
>  
>  FILES_${PN} = "/boot ${datadir}"
> +SYSROOT_DIRS += "/boot ${sysconfdir}"
> +
>  RDEPENDS_${PN} += "${PN}-env"
>  
>  do_deploy () {

I think I did ask about this on irc but we never managed to discuss.

I'm a bit worried about this creating confusion about where files are
placed (deploy or sysroot). I particularly want to avoid placing files
in both.

Are these files in /boot normally available in the rootfs at this
location? If so, the patch probably makes sense but if not, I think its
creating disparity between the target filesystem and the build one and
I'm a lot more reluctant to do that.

Cheers,

Richard

Re: [OE-core] [PATCH v2] u-boot: add /boot and ${sysconfdir} to SYSROOT_DIRS

[Diego Sueiro]

>-----Original Message-----
>From: Richard Purdie <richard.purdie@linuxfoundation.org>
>Sent: 04 January 2021 10:28
>To: Diego Sueiro <Diego.Sueiro@arm.com>; openembedded-
>core@lists.openembedded.org
>Cc: nd <nd@arm.com>
>Subject: Re: [OE-core] [PATCH v2] u-boot: add /boot and ${sysconfdir} to
>SYSROOT_DIRS
>
>On Mon, 2021-01-04 at 08:30 +0000, Diego Sueiro wrote:
>> Since do_install is shipping files to /boot and ${sysconfdir}, we can
>> include these dirs to SYSROOT_DIRS and allow recipes which depend on
>> u-boot to use its installed artifacts from RECIPE_SYSROOT instead of
>> DEPLOY_DIR_IMAGE.
>>
>> Signed-off-by: Diego Sueiro <diego.sueiro@arm.com>
>> ---
>>  meta/recipes-bsp/u-boot/u-boot.inc | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/meta/recipes-bsp/u-boot/u-boot.inc b/meta/recipes-bsp/u-
>> boot/u-boot.inc index 251178db33..c53bcd05cc 100644
>> --- a/meta/recipes-bsp/u-boot/u-boot.inc
>> +++ b/meta/recipes-bsp/u-boot/u-boot.inc
>> @@ -266,6 +266,8 @@ FILES_${PN}-env = " \
>>  "
>>
>>  FILES_${PN} = "/boot ${datadir}"
>> +SYSROOT_DIRS += "/boot ${sysconfdir}"
>> +
>>  RDEPENDS_${PN} += "${PN}-env"
>>
>>  do_deploy () {
>
>I think I did ask about this on irc but we never managed to discuss.
>
>I'm a bit worried about this creating confusion about where files are placed
>(deploy or sysroot). I particularly want to avoid placing files in both.
>
>Are these files in /boot normally available in the rootfs at this location? If so,
>the patch probably makes sense but if not, I think its creating disparity
>between the target filesystem and the build one and I'm a lot more reluctant
>to do that.
>
>Cheers,
>
>Richard

Hi Richard,

Sorry, I missed your message in the IRC and was on holidays just returning today.

For example, in trusted-firmware-a recipe in meta-arm (http://git.yoctoproject.org/cgit/cgit.cgi/meta-arm/tree/meta-arm/recipes-bsp/trusted-firmware-a/trusted-firmware-a.inc),
we have an option to include the u-boot binary (as BL33) in the FIP image. And this
is achieved by passing the DEPLOY_DIR_IMAGE path.

The idea for using the SYSROOT_DIRS is to not depend on u-boot:do_deploy in order
to be able to use its generated artifacts. I'm just trying to follow the recommendation
listed in the manual: https://www.yoctoproject.org/docs/latest/mega-manual/mega-manual.html#new-sharing-files-between-recipes.

Note that some of these artifacts are not expected to be included in the filesystem.

In meta-arm, we are using the SYSROOT_DIRS approach for other firmware related
recipes to share files between them.

We had some situations (which I don't remember exactly) where dependency cycles
occurred between do_deploy tasks when trying to signing or creating wic images, and
by using the combination of SYSROOT_DIRS and DEPENDS we managed to overcome
these issues.

Re: [OE-core] [PATCH v2] u-boot: add /boot and ${sysconfdir} to SYSROOT_DIRS

[Richard Purdie]

On Mon, 2021-01-04 at 10:56 +0000, Diego Sueiro wrote:
> 
> Sorry, I missed your message in the IRC and was on holidays just
> returning today.

No problem, I think we're all catching up after the holidays!

> For example, in trusted-firmware-a recipe in meta-arm ( 
> http://git.yoctoproject.org/cgit/cgit.cgi/meta-arm/tree/meta-arm/recipes-bsp/trusted-firmware-a/trusted-firmware-a.inc
> ),
> we have an option to include the u-boot binary (as BL33) in the FIP
> image. And this
> is achieved by passing the DEPLOY_DIR_IMAGE path.
> 
> The idea for using the SYSROOT_DIRS is to not depend on u-
> boot:do_deploy in order
> to be able to use its generated artifacts. I'm just trying to follow
> the recommendation
> listed in the manual:  
> https://www.yoctoproject.org/docs/latest/mega-manual/mega-manual.html#new-sharing-files-between-recipes
> .
> 
> Note that some of these artifacts are not expected to be included in
> the filesystem.
> 
> In meta-arm, we are using the SYSROOT_DIRS approach for other
> firmware related
> recipes to share files between them.
> 
> We had some situations (which I don't remember exactly) where
> dependency cycles
> occurred between do_deploy tasks when trying to signing or creating
> wic images, and
> by using the combination of SYSROOT_DIRS and DEPENDS we managed to
> overcome
> these issues.

I think you are reading more into that manual section than is intended.
For files on the general filesystem, the sysroot is definitely the way
to share between recipes. For files that are not within the filesystem,
I think deploy is more appropriate. We could improve the docs to
mention that.

As such I don't really want to add a mechanism where we confuse those
lines if we can help it and I am reluctant to take the patch. do_deploy
was designed for these cases.

If you can find out more details of the issue you were running into we
might be able to help figure out how to break the circular
dependencies.

Cheers,

Richard

Re: [OE-core] [PATCH v2] u-boot: add /boot and ${sysconfdir} to SYSROOT_DIRS

[Diego Sueiro]

>-----Original Message-----
>From: Richard Purdie <richard.purdie@linuxfoundation.org>
>Sent: 04 January 2021 11:32
>To: Diego Sueiro <Diego.Sueiro@arm.com>; openembedded-
>core@lists.openembedded.org
>Cc: nd <nd@arm.com>
>Subject: Re: [OE-core] [PATCH v2] u-boot: add /boot and ${sysconfdir} to
>SYSROOT_DIRS
>
>On Mon, 2021-01-04 at 10:56 +0000, Diego Sueiro wrote:
>>
>> Sorry, I missed your message in the IRC and was on holidays just
>> returning today.
>
>No problem, I think we're all catching up after the holidays!
>
>> For example, in trusted-firmware-a recipe in meta-arm (
>> http://git.yoctoproject.org/cgit/cgit.cgi/meta-arm/tree/meta-arm/recip
>> es-bsp/trusted-firmware-a/trusted-firmware-a.inc
>> ),
>> we have an option to include the u-boot binary (as BL33) in the FIP
>> image. And this is achieved by passing the DEPLOY_DIR_IMAGE path.
>>
>> The idea for using the SYSROOT_DIRS is to not depend on u-
>> boot:do_deploy in order to be able to use its generated artifacts. I'm
>> just trying to follow the recommendation listed in the manual:
>> https://www.yoctoproject.org/docs/latest/mega-manual/mega-
>manual.html#
>> new-sharing-files-between-recipes
>> .
>>
>> Note that some of these artifacts are not expected to be included in
>> the filesystem.
>>
>> In meta-arm, we are using the SYSROOT_DIRS approach for other firmware
>> related recipes to share files between them.
>>
>> We had some situations (which I don't remember exactly) where
>> dependency cycles occurred between do_deploy tasks when trying to
>> signing or creating wic images, and by using the combination of
>> SYSROOT_DIRS and DEPENDS we managed to overcome these issues.
>
>I think you are reading more into that manual section than is intended.
>For files on the general filesystem, the sysroot is definitely the way to share
>between recipes. For files that are not within the filesystem, I think deploy is
>more appropriate. We could improve the docs to mention that.
>
>As such I don't really want to add a mechanism where we confuse those lines
>if we can help it and I am reluctant to take the patch. do_deploy was designed
>for these cases.
>
For some arm based machines we end up generating loads of intermediate binaries
which will be further processed and combined as standalone images, for example.
I think doesn't make sense to have them in the deploy dir since it will make it
polluted and I do much prefer to have a tidy deploy dir with only artifacts for
flashing/update the target, rather than a list of useless artifacts that can't be used
directly.

And I do agree with you that using the SYSROOT_DIRS for this purpose might lead
to  confusion. Is it plausible to come up with a new variable and do_populate related
tasks to address use cases like the one I described?

>If you can find out more details of the issue you were running into we might
>be able to help figure out how to break the circular dependencies.
>
As soon as I come across this kind of problem I'll let you know.

>Cheers,
>
>Richard
>

Cheers,

Diego Sueiro

Re: [OE-core] [PATCH v2] u-boot: add /boot and ${sysconfdir} to SYSROOT_DIRS

[Richard Purdie]

On Mon, 2021-01-04 at 13:07 +0000, Diego Sueiro wrote:
> > From: Richard Purdie <richard.purdie@linuxfoundation.org>
> > I think you are reading more into that manual section than is
> > intended.
> > For files on the general filesystem, the sysroot is definitely the
> > way to share
> > between recipes. For files that are not within the filesystem, I
> > think deploy is
> > more appropriate. We could improve the docs to mention that.
> > 
> > As such I don't really want to add a mechanism where we confuse
> > those lines
> > if we can help it and I am reluctant to take the patch. do_deploy
> > was designed
> > for these cases.
> > 
> For some arm based machines we end up generating loads of
> intermediate binaries
> which will be further processed and combined as standalone images,
> for example.
> I think doesn't make sense to have them in the deploy dir since it
> will make it
> polluted and I do much prefer to have a tidy deploy dir with only
> artifacts for
> flashing/update the target, rather than a list of useless artifacts
> that can't be used
> directly.
> 
> And I do agree with you that using the SYSROOT_DIRS for this purpose
> might lead
> to  confusion. Is it plausible to come up with a new variable and
> do_populate related
> tasks to address use cases like the one I described?

Not wanting to pollute DEPLOY_DIR_IMAGE with unneeded files is a
sensible thing to want to do. I'm wondering if we could use the deploy
code but point it at some different output directory?

The code for "do_deploy" is much simpler than populate_sysroot and
prepare_recipe_sysroot so I'd look at deploy.bbclass, its actually
really simple. In theory you could change do_deploy[sstate-outputdirs]
and it would place files in a different location. We'd just need to
figure out a good name for that alternative location...

Cheers,

Richard

Re: [OE-core] [PATCH v2] u-boot: add /boot and ${sysconfdir} to SYSROOT_DIRS

[Richard Purdie]

On Mon, 2021-01-04 at 13:07 +0000, Diego Sueiro wrote:
> For some arm based machines we end up generating loads of
> intermediate binaries
> which will be further processed and combined as standalone images,
> for example.
> I think doesn't make sense to have them in the deploy dir since it
> will make it
> polluted and I do much prefer to have a tidy deploy dir with only
> artifacts for
> flashing/update the target, rather than a list of useless artifacts
> that can't be used
> directly.
> 
> And I do agree with you that using the SYSROOT_DIRS for this purpose
> might lead
> to  confusion. Is it plausible to come up with a new variable and
> do_populate related
> tasks to address use cases like the one I described

Just thinking out loud, maybe we can use SYSROOT_DIRS if we use
something which is named clearly such that its obvious it will never be
on the target rootfs? 

"/boot" is confusing as some systems do have/use that but there is
nothing stopping us creating something specific to this purpose. We'd
have to clearly document it should never be used for anything also
shared using do_deploy and was for intermediate files only?

Cheers,

Richard

Re: [OE-core] [PATCH v2] u-boot: add /boot and ${sysconfdir} to SYSROOT_DIRS

[Diego Sueiro]

>-----Original Message-----
>From: Richard Purdie <richard.purdie@linuxfoundation.org>
>Sent: 04 January 2021 13:23
>To: Diego Sueiro <Diego.Sueiro@arm.com>; openembedded-
>core@lists.openembedded.org
>Cc: nd <nd@arm.com>
>Subject: Re: [OE-core] [PATCH v2] u-boot: add /boot and ${sysconfdir} to
>SYSROOT_DIRS
>
>On Mon, 2021-01-04 at 13:07 +0000, Diego Sueiro wrote:
>> For some arm based machines we end up generating loads of intermediate
>> binaries which will be further processed and combined as standalone
>> images, for example.
>> I think doesn't make sense to have them in the deploy dir since it
>> will make it polluted and I do much prefer to have a tidy deploy dir
>> with only artifacts for flashing/update the target, rather than a list
>> of useless artifacts that can't be used directly.
>>
>> And I do agree with you that using the SYSROOT_DIRS for this purpose
>> might lead to  confusion. Is it plausible to come up with a new
>> variable and do_populate related tasks to address use cases like the
>> one I described
>
>Just thinking out loud, maybe we can use SYSROOT_DIRS if we use something
>which is named clearly such that its obvious it will never be on the target
>rootfs?
>
>"/boot" is confusing as some systems do have/use that but there is nothing
>stopping us creating something specific to this purpose. We'd have to clearly
>document it should never be used for anything also shared using do_deploy
>and was for intermediate files only?
>
This might be the simplest solution with low code and documentation impacts.
In meta-arm we make usage of the "/firmware" directory to include these
intermediate files.
I suspect that in case of u-boot we could also consider it as a "firmware" artifact?

Cheers,

Diego Sueiro

Re: [OE-core] [PATCH v2] u-boot: add /boot and ${sysconfdir} to SYSROOT_DIRS

[Richard Purdie]

On Mon, 2021-01-04 at 13:37 +0000, Diego Sueiro wrote:
> > Just thinking out loud, maybe we can use SYSROOT_DIRS if we use
> > something
> > which is named clearly such that its obvious it will never be on
> > the target
> > rootfs?
> > 
> > "/boot" is confusing as some systems do have/use that but there is
> > nothing
> > stopping us creating something specific to this purpose. We'd have
> > to clearly
> > document it should never be used for anything also shared using
> > do_deploy
> > and was for intermediate files only?
> > 
> This might be the simplest solution with low code and documentation
> impacts.
> In meta-arm we make usage of the "/firmware" directory to include
> these
> intermediate files.
> I suspect that in case of u-boot we could also consider it as a
> "firmware" artifact?

I'm wondering about "intermediate-deploy" as its then very clear what
its for and that its separate from any normal rootfs structure?

Cheers,

Richard

Re: [OE-core] [PATCH v2] u-boot: add /boot and ${sysconfdir} to SYSROOT_DIRS

[Diego Sueiro]

>-----Original Message-----
>From: openembedded-core@lists.openembedded.org <openembedded-
>core@lists.openembedded.org> On Behalf Of Richard Purdie via
>lists.openembedded.org
>Sent: 04 January 2021 13:41
>To: Diego Sueiro <Diego.Sueiro@arm.com>; openembedded-
>core@lists.openembedded.org
>Cc: nd <nd@arm.com>
>Subject: Re: [OE-core] [PATCH v2] u-boot: add /boot and ${sysconfdir} to
>SYSROOT_DIRS
>
>On Mon, 2021-01-04 at 13:37 +0000, Diego Sueiro wrote:
>> > Just thinking out loud, maybe we can use SYSROOT_DIRS if we use
>> > something which is named clearly such that its obvious it will never
>> > be on the target rootfs?
>> >
>> > "/boot" is confusing as some systems do have/use that but there is
>> > nothing stopping us creating something specific to this purpose.
>> > We'd have to clearly document it should never be used for anything
>> > also shared using do_deploy and was for intermediate files only?
>> >
>> This might be the simplest solution with low code and documentation
>> impacts.
>> In meta-arm we make usage of the "/firmware" directory to include
>> these intermediate files.
>> I suspect that in case of u-boot we could also consider it as a
>> "firmware" artifact?
>
>I'm wondering about "intermediate-deploy" as its then very clear what its for
>and that its separate from any normal rootfs structure?
>

Sorry for the delay. I'll be sending another patch series shortly.

Cheers,

Diego Sueiro