Re: [PATCH v2 06/15] update-alternatives: allow hooks before renaming files

["André Draszik" <git@andred.net>]

On Tue, 2019-01-15 at 16:45 +0000, Richard Purdie wrote:
> On Tue, 2019-01-15 at 14:45 +0000, André Draszik wrote:
> > From: André Draszik <andre.draszik@jci.com>
> > 
> > At the moment, the update alternatives handling is happening
> > right after copying into PKGD during packaging time using
> > an _append OVERRIDE to the copy function. This means that at
> > this point the PACKAGES variable must have been fully
> > populated, as that is a prerequisite for update-alternatives.
> > 
> > In other words, this makes it impossible to e.g. populate
> > PACKAGES dynamically using do_split_packages() and still do
> > update-alternatives, as do_split_packages() can never
> > execute early enough in a deterministic way.
> > 
> > By converting the existing python function from a 'def'
> > type function to a 'python' type function, we now make it
> > possible for somebody to hook themselves between copying
> > into PKGD and execution of update-alternatives, via
> > a _prepend OVERRIDE, which is not possible with 'def' type
> > functions.
> > 
> > [YOCTO #13058]
> 
> I don't think you need to do this. Can you add your function to the
> start of PACKAGEBUILDPKGD rather than using
> apply_update_alternative_renames_prepend
> ?

do_split_packages() can only execute after perform_packagecopy() which is
the first function in PACKAGEBUILDPKGD. So other than redefining
PACKAGEBUILDPKGD, or adding another variable just for u/a and modifying this
var in util-linux, I'd say impossible while keeping it all legible /
maintainable.

> or maybe use PACKAGE_PREPROCESS_FUNCS?
> 
> In fact we should probably fix the update-alternatives class to use
> PACKAGE_PREPROCESS_FUNCS rather than perform_packagecopy_append too.
> 
> Regardless, I'd like to see us try and make this more readable rather
> than less which means separate functions and less prepend/appending and
> more use of the variables...

This sounds good, thank you. While I did contemplate that originally, I was
mislead by a comment in the u/a class and didn't follow that approach. I now
think it will work fine, though. Patch upcoming.

Cheers,
Andre'

> 
> Cheers,
> 
> Richard
> 
> 
> > Signed-off-by: André Draszik <andre.draszik@jci.com>
> > ---
> >  meta/classes/update-alternatives.bbclass | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/meta/classes/update-alternatives.bbclass
> > b/meta/classes/update-alternatives.bbclass
> > index f1250f877b..ffedada2f6 100644
> > --- a/meta/classes/update-alternatives.bbclass
> > +++ b/meta/classes/update-alternatives.bbclass
> > @@ -135,10 +135,10 @@ populate_packages[vardeps] += "${UPDALTVARS} 
> > ${@gen_updatealternativesvars(d)}"
> >  # place.
> >  python perform_packagecopy_append () {
> >      if update_alternatives_enabled(d):
> > -        apply_update_alternative_renames(d)
> > +        bb.build.exec_func('apply_update_alternative_renames', d)
> >  }
> >  
> > -def apply_update_alternative_renames(d):
> > +python apply_update_alternative_renames () {
> >      # Check for deprecated usage...
> >      pn = d.getVar('BPN')
> >      if d.getVar('ALTERNATIVE_LINKS') != None:
> > @@ -204,6 +204,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
> >