Re: [OE-core] [PATCH v3 06/15] update-alternatives: convert file renames to PACKAGE_PREPROCESS_FUNCS

2019-01-16 Thread Richard Purdie
On Wed, 2019-01-16 at 12:00 +, André Draszik wrote:
> From: André Draszik 
> 
> At the moment, the update-alternatives file renaming is
> happening right after copying into PKGD during packaging
> time using an _append OVERRIDE to the copy function
> perform_packagecopy().
> 
> This is not really readable and hard to maintain.
> 
> Additionally, this makes it impossible to e.g. populate
> PACKAGES dynamically using do_split_packages() and still
> do update-alternatives - update-alternatives file renaming
> requires the PACKAGES variable to have been fully populated
> to work correctly. On the other hand, do_split_packages()
> can only execute after perform_packagecopy(), as it needs
> PKGD populated; so it's impossible to insert
> do_split_packages() early enough in a deterministic way in
> this use-case.
> 
> As there doesn't seem to be a reason not to, convert
> this to a proper function and use PACKAGE_PREPROCESS_FUNCS
> instead - after all, that's what this is meant for.
> 
> No other classes or recipes in oe-core or meta-openembededd
> seem to have a hard requirement on update-alterantives
> executing before any other PACKAGE_PREPROCESS_FUNCS, so
> this should be perfectly fine.
> 
> The only implication is that if compress_doc is inherited,
> compressed man-page file names will end up being, e.g.
> eject.1.util-linux.gz
>   or
> eject.1.gz.util-linux
> based on the include of compress_doc.bbclass vs.
> update-alternatives.bbclass order, but the symlink created
> (alternative name) will always be correct.
> 
> This solves both problems:
> * the code is easier to read / follow
> * the above described use-case can be accomodated easily
> 
> [YOCTO #13058]
> 
> Signed-off-by: André Draszik 
> ---
>  meta/classes/update-alternatives.bbclass | 13 +
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/meta/classes/update-alternatives.bbclass
> b/meta/classes/update-alternatives.bbclass
> index f1250f877b..78291e7e45 100644
> --- a/meta/classes/update-alternatives.bbclass
> +++ b/meta/classes/update-alternatives.bbclass
> @@ -131,14 +131,10 @@ def gen_updatealternativesvars(d):
>  populate_packages[vardeps] += "${UPDALTVARS} ${@gen_updatealternativ
> esvars(d)}"
>  
>  # We need to do the rename after the image creation step, but before
> -# the split and strip steps..  packagecopy seems to be the earliest
> reasonable
> -# place.
> -python perform_packagecopy_append () {
> -if update_alternatives_enabled(d):
> -apply_update_alternative_renames(d)
> -}
> -
> -def apply_update_alternative_renames(d):
> +# the split and strip steps..  PACKAGE_PREPROCESS_FUNCS is the right
> +# place for that.
> +PACKAGE_PREPROCESS_FUNCS += "${@'apply_update_alternative_renames'
> if update_alternatives_enabled(d) else ''}"
> +python apply_update_alternative_renames () {

Looks good, how about adding:

if not update_alternatives_enabled(d):
   return

here to make it slightly more readable?

>  # Check for deprecated usage...
>  pn = d.getVar('BPN')
>  if d.getVar('ALTERNATIVE_LINKS') != None:
> @@ -204,6 +200,7 @@ def apply_update_alternative_renames(d):
>  os.unlink(src)
>  else:
>  bb.warn('%s: Unable to resolve dangling symlink:
> %s' % (pn, alt_target))
> +}


Cheers,

Richard

-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core


[OE-core] [PATCH v3 06/15] update-alternatives: convert file renames to PACKAGE_PREPROCESS_FUNCS

2019-01-16 Thread André Draszik
From: André Draszik 

At the moment, the update-alternatives file renaming is
happening right after copying into PKGD during packaging
time using an _append OVERRIDE to the copy function
perform_packagecopy().

This is not really readable and hard to maintain.

Additionally, this makes it impossible to e.g. populate
PACKAGES dynamically using do_split_packages() and still
do update-alternatives - update-alternatives file renaming
requires the PACKAGES variable to have been fully populated
to work correctly. On the other hand, do_split_packages()
can only execute after perform_packagecopy(), as it needs
PKGD populated; so it's impossible to insert
do_split_packages() early enough in a deterministic way in
this use-case.

As there doesn't seem to be a reason not to, convert
this to a proper function and use PACKAGE_PREPROCESS_FUNCS
instead - after all, that's what this is meant for.

No other classes or recipes in oe-core or meta-openembededd
seem to have a hard requirement on update-alterantives
executing before any other PACKAGE_PREPROCESS_FUNCS, so
this should be perfectly fine.

The only implication is that if compress_doc is inherited,
compressed man-page file names will end up being, e.g.
eject.1.util-linux.gz
  or
eject.1.gz.util-linux
based on the include of compress_doc.bbclass vs.
update-alternatives.bbclass order, but the symlink created
(alternative name) will always be correct.

This solves both problems:
* the code is easier to read / follow
* the above described use-case can be accomodated easily

[YOCTO #13058]

Signed-off-by: André Draszik 
---
 meta/classes/update-alternatives.bbclass | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/meta/classes/update-alternatives.bbclass 
b/meta/classes/update-alternatives.bbclass
index f1250f877b..78291e7e45 100644
--- a/meta/classes/update-alternatives.bbclass
+++ b/meta/classes/update-alternatives.bbclass
@@ -131,14 +131,10 @@ def gen_updatealternativesvars(d):
 populate_packages[vardeps] += "${UPDALTVARS} ${@gen_updatealternativesvars(d)}"
 
 # We need to do the rename after the image creation step, but before
-# the split and strip steps..  packagecopy seems to be the earliest reasonable
-# place.
-python perform_packagecopy_append () {
-if update_alternatives_enabled(d):
-apply_update_alternative_renames(d)
-}
-
-def apply_update_alternative_renames(d):
+# the split and strip steps..  PACKAGE_PREPROCESS_FUNCS is the right
+# place for that.
+PACKAGE_PREPROCESS_FUNCS += "${@'apply_update_alternative_renames' if 
update_alternatives_enabled(d) else ''}"
+python apply_update_alternative_renames () {
 # Check for deprecated usage...
 pn = d.getVar('BPN')
 if d.getVar('ALTERNATIVE_LINKS') != None:
@@ -204,6 +200,7 @@ def apply_update_alternative_renames(d):
 os.unlink(src)
 else:
 bb.warn('%s: Unable to resolve dangling symlink: %s' % 
(pn, alt_target))
+}
 
 PACKAGESPLITFUNCS_prepend = "populate_packages_updatealternatives "
 
-- 
2.20.1

-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core