[PATCH] update-alternatives: Don't process duplicated entries

[PATCH] update-alternatives: Don't process duplicated entries

[Jeremy Puhlman]

If ALTERNATIVE_<pkg> lists the same entry more then once, the
update-alternatives identical install and remove actions are added for
each repeated entry. This is exhibited when you add meta-selinux, and
examine the busybox package, which installs every link twice because it
alters the links to point to the shell replacements so selinux will work
with them. This can generate warnings on do_rootfs about similarly
prioritized alternatives. Given that at this point in the processing
the addtions should always be identical there shouldn't be any good
reason to add them twice.

Signed-off-by: Jeremy Puhlman <jpuhlman@mvista.com>
---
 meta/classes/update-alternatives.bbclass | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/meta/classes/update-alternatives.bbclass b/meta/classes/update-alternatives.bbclass
index aa01058..2c01c8b 100644
--- a/meta/classes/update-alternatives.bbclass
+++ b/meta/classes/update-alternatives.bbclass
@@ -207,7 +207,11 @@ python populate_packages_updatealternatives () {
         # Create post install/removal scripts
         alt_setup_links = ""
         alt_remove_links = ""
+        already_processed = set()
         for alt_name in (d.getVar('ALTERNATIVE_%s' % pkg) or "").split():
+            if alt_name in already_processed:
+               continue
+            already_processed.add(alt_name)
             alt_link     = d.getVarFlag('ALTERNATIVE_LINK_NAME', alt_name)
             alt_target   = d.getVarFlag('ALTERNATIVE_TARGET_%s' % pkg, alt_name) or d.getVarFlag('ALTERNATIVE_TARGET', alt_name)
             alt_target   = alt_target or d.getVar('ALTERNATIVE_TARGET_%s' % pkg) or d.getVar('ALTERNATIVE_TARGET') or alt_link
-- 
2.6.4

Re: [PATCH] update-alternatives: Don't process duplicated entries

[Richard Purdie]

On Fri, 2018-06-15 at 17:59 -0700, Jeremy Puhlman wrote:
> If ALTERNATIVE_<pkg> lists the same entry more then once, the
> update-alternatives identical install and remove actions are added
> for
> each repeated entry. This is exhibited when you add meta-selinux, and
> examine the busybox package, which installs every link twice because
> it
> alters the links to point to the shell replacements so selinux will
> work
> with them. This can generate warnings on do_rootfs about similarly
> prioritized alternatives. Given that at this point in the processing
> the addtions should always be identical there shouldn't be any good
> reason to add them twice.
> 
> Signed-off-by: Jeremy Puhlman <jpuhlman@mvista.com>
> ---
>  meta/classes/update-alternatives.bbclass | 4 ++++
>  1 file changed, 4 insertions(+)

Shouldn't we detect and make this a fatal error in one of the other
sanity tests like we do for PACKAGES for example? Would there ever be a
valid use case for duplicate entries here?

Cheers,

Richard

Re: [PATCH] update-alternatives: Don't process duplicated entries

[Jeremy A. Puhlman]

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

> On Fri, 2018-06-15 at 17:59 -0700, Jeremy Puhlman wrote:
>> If ALTERNATIVE_<pkg> lists the same entry more then once, the
>> update-alternatives identical install and remove actions are added
>> for
>> each repeated entry. This is exhibited when you add meta-selinux, and
>> examine the busybox package, which installs every link twice because
>> it
>> alters the links to point to the shell replacements so selinux will
>> work
>> with them. This can generate warnings on do_rootfs about similarly
>> prioritized alternatives. Given that at this point in the processing
>> the addtions should always be identical there shouldn't be any good
>> reason to add them twice.
>> 
>> Signed-off-by: Jeremy Puhlman <jpuhlman@mvista.com>
>> ---
>>  meta/classes/update-alternatives.bbclass | 4 ++++
>>  1 file changed, 4 insertions(+)
>
> Shouldn't we detect and make this a fatal error in one of the other
> sanity tests like we do for PACKAGES for example?

That is certainly one way to go, it would also require at least a fix to the selinux append(which is what I
did first), but I figured a more generalized fix would be more useful in the long run.

>  Would there ever be a valid use case for duplicate entries here?

Given at this point in the code all the values are pretty much locked in, you can't really create links that are different, so
at least as the way the code is currently structured, there is no value in adding identical values twice as far as I can think
of.

With out any change, there does not appear to be any degradation in the on target alternatives function, in some cases it just generates
a number of "alternatives set at the same priority" warning messages during the image construction. In the PACKAGES case, I thought there
was actual real functional issues with duplicating values in package, which is why it was added. In this case the only real effect would
be cleaner alternatives files/post scripts and no warnings during image builds.

I am fine with either way, as I already have a fix up for the selinux append.

>
> Cheers,
>
> Richard




-- 
Jeremy A. Puhlman
jpuhlman@mvista.com


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