All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] uboot-sign: Refactor do_deploy prefunc to do_deploy_prepend
@ 2020-06-30 11:38 Daniel Klauer
  2020-06-30 11:38 ` [PATCH 2/2] deploy.bbclass: Clean DEPLOYDIR before do_deploy Daniel Klauer
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Klauer @ 2020-06-30 11:38 UTC (permalink / raw)
  To: openembedded-core; +Cc: Otavio Salvador

When inherited by the u-boot recipe (UBOOT_PN), uboot-sign.bbclass adds
a concat_dtb step, which places additional files into ${DEPLOYDIR}
before do_deploy. By turning this from a prefunc into a part of the normal
do_deploy function, it becomes possible to use
  do_deploy[cleandirs] = "${DEPLOYDIR}"
in the future, without deleting the files produced by concat_dtb.

As before, care is taken to not interfere with the kernel's do_deploy
definition, since concat_dtb was only needed for u-boot.

Signed-off-by: Daniel Klauer <daniel.klauer@gin.de>
---
 meta/classes/uboot-sign.bbclass | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/meta/classes/uboot-sign.bbclass b/meta/classes/uboot-sign.bbclass
index 982ed46d01..713196df41 100644
--- a/meta/classes/uboot-sign.bbclass
+++ b/meta/classes/uboot-sign.bbclass
@@ -117,15 +117,16 @@ do_install_append() {
 	fi
 }
 
+do_deploy_prepend_pn-${UBOOT_PN}() {
+	if [ "${UBOOT_SIGN_ENABLE}" = "1" -a -n "${UBOOT_DTB_BINARY}" ]; then
+		concat_dtb
+	fi
+}
+
 python () {
     if d.getVar('UBOOT_SIGN_ENABLE') == '1' and d.getVar('PN') == d.getVar('UBOOT_PN') and d.getVar('UBOOT_DTB_BINARY'):
         kernel_pn = d.getVar('PREFERRED_PROVIDER_virtual/kernel')
 
         # Make "bitbake u-boot -cdeploy" deploys the signed u-boot.dtb
         d.appendVarFlag('do_deploy', 'depends', ' %s:do_deploy' % kernel_pn)
-
-        # kernerl's do_deploy is a litle special, so we can't use
-        # do_deploy_append, otherwise it would override
-        # kernel_do_deploy.
-        d.appendVarFlag('do_deploy', 'prefuncs', ' concat_dtb')
 }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] deploy.bbclass: Clean DEPLOYDIR before do_deploy
  2020-06-30 11:38 [PATCH 1/2] uboot-sign: Refactor do_deploy prefunc to do_deploy_prepend Daniel Klauer
@ 2020-06-30 11:38 ` Daniel Klauer
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Klauer @ 2020-06-30 11:38 UTC (permalink / raw)
  To: openembedded-core; +Cc: Otavio Salvador

do_deploy should clean up ${DEPLOYDIR} before running, just like do_install
cleans up ${D} before running. This reduces the risk of DEPLOYDIR being
accidentally contaminated by files from previous runs, possibly even with
different config, in case of incremental builds.

It is convenient to have this in deploy.bbclass, so it doesn't have to be
duplicated in every recipe, considering for example meta-freescale, which
has 23 affected recipes.

All recipes using deploy.bbclass (grep -r 'inherit .*deploy') in poky,
meta-openembedded and meta-freescale look like they either benefit from
this or are at least not affected negatively by it. The only exception
I've noticed was uboot-sign.bbclass, which was however fixed by the
previous patch.

Signed-off-by: Daniel Klauer <daniel.klauer@gin.de>
---
 meta/classes/deploy.bbclass            | 1 +
 meta/classes/kernel.bbclass            | 2 --
 meta/recipes-core/meta/signing-keys.bb | 2 --
 meta/recipes-core/ovmf/ovmf_git.bb     | 1 -
 4 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/meta/classes/deploy.bbclass b/meta/classes/deploy.bbclass
index 6d52908783..737c26122b 100644
--- a/meta/classes/deploy.bbclass
+++ b/meta/classes/deploy.bbclass
@@ -8,4 +8,5 @@ python do_deploy_setscene () {
 }
 addtask do_deploy_setscene
 do_deploy[dirs] = "${DEPLOYDIR} ${B}"
+do_deploy[cleandirs] = "${DEPLOYDIR}"
 do_deploy[stamp-extra-info] = "${MACHINE_ARCH}"
diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index a1219f3156..301e3b86b2 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -729,8 +729,6 @@ kernel_do_deploy() {
 		done
 	fi
 }
-do_deploy[cleandirs] = "${DEPLOYDIR}"
-do_deploy[dirs] = "${DEPLOYDIR} ${B}"
 do_deploy[prefuncs] += "package_get_auto_pr"
 
 addtask deploy after do_populate_sysroot do_packagedata
diff --git a/meta/recipes-core/meta/signing-keys.bb b/meta/recipes-core/meta/signing-keys.bb
index 1e1c7e3459..5bab94aa36 100644
--- a/meta/recipes-core/meta/signing-keys.bb
+++ b/meta/recipes-core/meta/signing-keys.bb
@@ -67,8 +67,6 @@ do_deploy () {
     fi
 }
 do_deploy[sstate-outputdirs] = "${DEPLOY_DIR_RPM}"
-# cleandirs should possibly be in deploy.bbclass but we need it
-do_deploy[cleandirs] = "${DEPLOYDIR}"
 # clear stamp-extra-info since MACHINE_ARCH is normally put there by
 # deploy.bbclass
 do_deploy[stamp-extra-info] = ""
diff --git a/meta/recipes-core/ovmf/ovmf_git.bb b/meta/recipes-core/ovmf/ovmf_git.bb
index 7292ebbb77..84547630e8 100644
--- a/meta/recipes-core/ovmf/ovmf_git.bb
+++ b/meta/recipes-core/ovmf/ovmf_git.bb
@@ -220,7 +220,6 @@ do_deploy[depends] += "${DEPLOYDEP}"
 
 do_deploy() {
 }
-do_deploy[cleandirs] = "${DEPLOYDIR}"
 do_deploy_class-target() {
     # For use with "runqemu ovmf".
     for i in \
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] uboot-sign: Refactor do_deploy prefunc to do_deploy_prepend
  2019-08-29  9:05   ` Daniel Klauer
@ 2019-08-29 13:00     ` richard.purdie
  0 siblings, 0 replies; 6+ messages in thread
From: richard.purdie @ 2019-08-29 13:00 UTC (permalink / raw)
  To: Daniel Klauer; +Cc: Patches and discussions about the oe-core layer

On Thu, 2019-08-29 at 11:05 +0200, Daniel Klauer wrote:
> > That commit message isn't correct.
> > 
> > It removes cleandirs for the prefunc at the start of the prefunc and
> > cleandirs for the main function at the start of that main function.
> 
> Oh ok, so the prefuncs/postfuncs can have their own [cleandirs], that's
> interesting.
> 
> How about the following (trying to find a good way to explain it):

Yes, that sounds fine. The bit in brackets probably isn't needed.

Cheers,

Richard

> 
> -----
> When inherited by the u-boot recipe (UBOOT_PN), uboot-sign.bbclass adds
> a concat_dtb step, which places additional files into ${DEPLOYDIR}
> before do_deploy.
> 
> The use of prefuncs for this prevents us from adding
>   do_deploy[cleandirs] = "${DEPLOYDIR}"
> because that would remove the files produced by the prefunc.
> (Each of prefuncs/task/postfuncs functions has their own cleandirs, and
> each function's cleandirs are removed at the start of that function.)
> 
> Thus, it seems good to make concat_dtb a part of do_deploy, such that it
> runs after removal of do_deploy's cleandirs. As before, care is taken to
> not interfere with the kernel's do_deploy definition, since concat_dtb
> was only needed for u-boot.
> -----
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] uboot-sign: Refactor do_deploy prefunc to do_deploy_prepend
  2019-08-28 14:01 ` Richard Purdie
@ 2019-08-29  9:05   ` Daniel Klauer
  2019-08-29 13:00     ` richard.purdie
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Klauer @ 2019-08-29  9:05 UTC (permalink / raw)
  To: Richard Purdie; +Cc: Patches and discussions about the oe-core layer

> That commit message isn't correct.
>
> It removes cleandirs for the prefunc at the start of the prefunc and
> cleandirs for the main function at the start of that main function.

Oh ok, so the prefuncs/postfuncs can have their own [cleandirs], that's
interesting.

How about the following (trying to find a good way to explain it):

-----
When inherited by the u-boot recipe (UBOOT_PN), uboot-sign.bbclass adds
a concat_dtb step, which places additional files into ${DEPLOYDIR}
before do_deploy.

The use of prefuncs for this prevents us from adding
  do_deploy[cleandirs] = "${DEPLOYDIR}"
because that would remove the files produced by the prefunc.
(Each of prefuncs/task/postfuncs functions has their own cleandirs, and
each function's cleandirs are removed at the start of that function.)

Thus, it seems good to make concat_dtb a part of do_deploy, such that it
runs after removal of do_deploy's cleandirs. As before, care is taken to
not interfere with the kernel's do_deploy definition, since concat_dtb
was only needed for u-boot.
-----


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] uboot-sign: Refactor do_deploy prefunc to do_deploy_prepend
  2019-08-28 11:41 [PATCH 1/2] uboot-sign: Refactor do_deploy prefunc to do_deploy_prepend Daniel Klauer
@ 2019-08-28 14:01 ` Richard Purdie
  2019-08-29  9:05   ` Daniel Klauer
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Purdie @ 2019-08-28 14:01 UTC (permalink / raw)
  To: Daniel Klauer, openembedded-core

On Wed, 2019-08-28 at 13:41 +0200, Daniel Klauer wrote:
> bitbake removes cleandirs once per prefunc and then again for the actual
> task.

That commit message isn't correct.

It removes cleandirs for the prefunc at the start of the prefunc and
cleandirs for the main function at the start of that main function.

The code is in lib/bb/build.py:

            for func in (prefuncs or '').split():
                exec_func(func, localdata)
            exec_func(task, localdata)
            for func in (postfuncs or '').split():
                exec_func(func, localdata)

with the cleandirs happening in exec_func() for each function.

This does mean the cleandirs happens after the prefunc is called but
not as described above.

>  By moving the concat_dtb step here from prefunc to main task we can
> add
>     do_deploy[cleandirs] = "${DEPLOYDIR}"
> to deploy.bbclass without losing the files produced by concat_dtb.

I think you also need to state here that this is only expected to be
run for the uboot recipe and not the kernel.

The patch itself is probably ok but I'd ask the commit message be
corrected please.

Cheers,

Richard

> It looks like using do_deploy(_append) was the original goal anyways.
> 
> I tested this with poky, putting the following in local.conf:
>   UBOOT_SIGN_ENABLE = "1"
>   KERNEL_CLASSES = " kernel-fitimage "
>   KERNEL_IMAGETYPE = "fitImage"
> and then doing
>   bitbake core-image-minimal
>   bitbake u-boot
> It builds successfully, the kernel and uboot recipes' temp/run.do_deploy
> scripts look good (kernel's do_deploy is not broken, and uboot's do_deploy
> still calls concat_dtb).
> 
> Signed-off-by: Daniel Klauer <daniel.klauer@gin.de>
> ---
>  meta/classes/uboot-sign.bbclass | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/meta/classes/uboot-sign.bbclass b/meta/classes/uboot-sign.bbclass
> index 982ed46d01..713196df41 100644
> --- a/meta/classes/uboot-sign.bbclass
> +++ b/meta/classes/uboot-sign.bbclass
> @@ -117,15 +117,16 @@ do_install_append() {
>  	fi
>  }
>  
> +do_deploy_prepend_pn-${UBOOT_PN}() {
> +	if [ "${UBOOT_SIGN_ENABLE}" = "1" -a -n "${UBOOT_DTB_BINARY}" ]; then
> +		concat_dtb
> +	fi
> +}
> +
>  python () {
>      if d.getVar('UBOOT_SIGN_ENABLE') == '1' and d.getVar('PN') == d.getVar('UBOOT_PN') and d.getVar('UBOOT_DTB_BINARY'):
>          kernel_pn = d.getVar('PREFERRED_PROVIDER_virtual/kernel')
>  
>          # Make "bitbake u-boot -cdeploy" deploys the signed u-boot.dtb
>          d.appendVarFlag('do_deploy', 'depends', ' %s:do_deploy' % kernel_pn)
> -
> -        # kernerl's do_deploy is a litle special, so we can't use
> -        # do_deploy_append, otherwise it would override
> -        # kernel_do_deploy.
> -        d.appendVarFlag('do_deploy', 'prefuncs', ' concat_dtb')
>  





^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] uboot-sign: Refactor do_deploy prefunc to do_deploy_prepend
@ 2019-08-28 11:41 Daniel Klauer
  2019-08-28 14:01 ` Richard Purdie
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Klauer @ 2019-08-28 11:41 UTC (permalink / raw)
  To: openembedded-core

bitbake removes cleandirs once per prefunc and then again for the actual
task. By moving the concat_dtb step here from prefunc to main task we can
add
    do_deploy[cleandirs] = "${DEPLOYDIR}"
to deploy.bbclass without losing the files produced by concat_dtb.

It looks like using do_deploy(_append) was the original goal anyways.

I tested this with poky, putting the following in local.conf:
  UBOOT_SIGN_ENABLE = "1"
  KERNEL_CLASSES = " kernel-fitimage "
  KERNEL_IMAGETYPE = "fitImage"
and then doing
  bitbake core-image-minimal
  bitbake u-boot
It builds successfully, the kernel and uboot recipes' temp/run.do_deploy
scripts look good (kernel's do_deploy is not broken, and uboot's do_deploy
still calls concat_dtb).

Signed-off-by: Daniel Klauer <daniel.klauer@gin.de>
---
 meta/classes/uboot-sign.bbclass | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/meta/classes/uboot-sign.bbclass b/meta/classes/uboot-sign.bbclass
index 982ed46d01..713196df41 100644
--- a/meta/classes/uboot-sign.bbclass
+++ b/meta/classes/uboot-sign.bbclass
@@ -117,15 +117,16 @@ do_install_append() {
 	fi
 }
 
+do_deploy_prepend_pn-${UBOOT_PN}() {
+	if [ "${UBOOT_SIGN_ENABLE}" = "1" -a -n "${UBOOT_DTB_BINARY}" ]; then
+		concat_dtb
+	fi
+}
+
 python () {
     if d.getVar('UBOOT_SIGN_ENABLE') == '1' and d.getVar('PN') == d.getVar('UBOOT_PN') and d.getVar('UBOOT_DTB_BINARY'):
         kernel_pn = d.getVar('PREFERRED_PROVIDER_virtual/kernel')
 
         # Make "bitbake u-boot -cdeploy" deploys the signed u-boot.dtb
         d.appendVarFlag('do_deploy', 'depends', ' %s:do_deploy' % kernel_pn)
-
-        # kernerl's do_deploy is a litle special, so we can't use
-        # do_deploy_append, otherwise it would override
-        # kernel_do_deploy.
-        d.appendVarFlag('do_deploy', 'prefuncs', ' concat_dtb')
 }
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-06-30 11:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-30 11:38 [PATCH 1/2] uboot-sign: Refactor do_deploy prefunc to do_deploy_prepend Daniel Klauer
2020-06-30 11:38 ` [PATCH 2/2] deploy.bbclass: Clean DEPLOYDIR before do_deploy Daniel Klauer
  -- strict thread matches above, loose matches on Subject: below --
2019-08-28 11:41 [PATCH 1/2] uboot-sign: Refactor do_deploy prefunc to do_deploy_prepend Daniel Klauer
2019-08-28 14:01 ` Richard Purdie
2019-08-29  9:05   ` Daniel Klauer
2019-08-29 13:00     ` richard.purdie

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.