Bug 13469 - uboot-extlinux-config uses wrong description
Summary: uboot-extlinux-config uses wrong description
Status: RESOLVED FIXED
Alias: None
Product: OE-Core
Classification: Build System, Metadata & Runtime
Component: oe-core other (show other bugs)
Version: unspecified
Hardware: Other Multiple
: Medium normal
Target Milestone: 3.1
Assignee: Will Page
QA Contact:
URL:
Whiteboard:
Depends on:
Blocks:
 
Reported: 2019-08-06 16:56 UTC by Will Page
Modified: 2019-11-11 19:14 UTC (History)
1 user (show)

See Also:
OS type for building Yocto: ---
Type of Regression: ---
Verified:
Documentation change: Don't know


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Will Page 2019-08-06 16:56:18 UTC
U-Boot's extlinux.conf label generation facilities (defined in meta/classes/uboot-extlinux-config.bbclass) generate incorrect results under some circumstances.

When using non-default labels (e.g. UBOOT_EXTLINUX_LABELS = "default", replacing the default value of "linux"), user-supplied label descriptions are ignored (e.g. UBOOT_EXTLINUX_MENU_DESCRIPTION_default = "Foo" will always be ignored in favor of the default value supplied in UBOOT_EXTLINUX_MENU_DESCRIPTION).

This behavior results from the class using OVERRIDES to support label-specific definitions, and the default label using an identifier that corresponds to a common ${OS} value, which by default appears in OVERRIDES.

For example:

UBOOT_EXTLINUX_LABELS = "default"
UBOOT_EXTLINUX_MENU_DESCRIPTION = "Foo"

During processing of the "default" label, the OVERRIDES will be updated from "${OS}:..." to "default:${OS}:...".  Because "linux" will be the most common value for ${OS} in this situation, it will frequently be evaluated as "default:linux:...".

Later, when writing out the entries for that label, it will request the value of "UBOOT_EXTLINUX_MENU_DESCRIPTION", at which point it will iterate through the OVERRIDES list, first processing the value for UBOOT_EXTLINUX_MENU_DESCRIPTION_default, then in a later iteration finding UBOOT_EXTLINUX_MENU_DESCRIPTION_linux, and using that value instead.

The OVERRIDE precedence rules essentially guarantee that the default value assigned to the _linux variable will supercede any user-supplied values for any and all labels.

I see three possible fixes:
1. change the provided label from "linux" to "default" or "primary" or similar, something that's not a commonly used value in OVERRIDES.  If using this solution, documentation changes should probably be made to discourage using OS-, DISTRO-, or MACHINE-related values for labels.
2. in the local copy of the dictionary used for generating label entries, clear out all pre-existing values of OVERRIDES
3. instead of prepending the label to the list of OVERRIDES, append the label to the list, giving it the highest precedence (although properly speaking it should be inserted before forcevariable, if it exists, giving it the second-highest priority).

A related issue exists where values defined for the first label shadow values defined for subsequent labels (when multiple labels are defined), because the dictionary gets re-used between iterations over the labels, and the newly-added label OVERRIDE has lower precedence than the previous labels, which are still present in the OVERRIDES list.

This related issue would be solved by options #2 and #3 above, although solution #2 is likely to result in surprising results where values not set for one label would essentially default to the value used for the previous label.

Alternatively, solution #1 combined with moving the dictionary copy into the loop, giving each label iteration a fresh copy of OVERRIDES (along with everything else) would also solve this issue.
Comment 1 Will Page 2019-08-06 17:33:29 UTC
I misspoke in my original description, where I said

"although solution #2 is likely to result in surprising results where values not set for one label would essentially default to the value used for the previous label"

I meant to say that solution #3 would give the described surprising results.
Comment 2 Ross Burton 2019-08-06 22:12:30 UTC
Reassigning to Marek, the u-boot maintainer.
Comment 3 Randy MacLeod 2019-08-08 14:44:33 UTC
Will, can you send a patch to the email list to fix this issue? Marek may be on vacation for a bit.
Comment 4 Richard Purdie 2019-08-08 14:53:12 UTC
The use of overrides in this way is perhaps unfortunate. What we try to do now is use a prefix with overrides to try and avoid this problem. Perhaps we could adopt some prefix here to avoid these problems?

Will: Would you be willing to send a patch for this?
Comment 5 Will Page 2019-08-08 15:16:17 UTC
(In reply to comment #4)
> The use of overrides in this way is perhaps unfortunate. What we try to do
> now is use a prefix with overrides to try and avoid this problem. Perhaps we
> could adopt some prefix here to avoid these problems?
> 
> Will: Would you be willing to send a patch for this?

Can you point me to an example of prefixed overrides, so that I'm clear on what you're suggesting and can do it consistently with other places in the project?
Comment 6 Will Page 2019-08-13 17:45:11 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > The use of overrides in this way is perhaps unfortunate. What we try to do
> > now is use a prefix with overrides to try and avoid this problem. Perhaps we
> > could adopt some prefix here to avoid these problems?
> > 
> > Will: Would you be willing to send a patch for this?
> 
> Can you point me to an example of prefixed overrides, so that I'm clear on
> what you're suggesting and can do it consistently with other places in the
> project?

I think I've got it.  I'm working on a solution that follows a similar route to what happens in devtools-source.bbclass.
Comment 7 Randy MacLeod 2019-08-13 19:56:47 UTC
Reset the target milestone. (2.8 is gone, it's now known as 3.0)
Comment 8 Randy MacLeod 2019-11-07 16:06:00 UTC
Will can you work on this or is it already fixed?
Comment 9 Will Page 2019-11-11 19:14:38 UTC
This was fixed in master with commit 42b44fdd9e63b8d39cf83baecec08328275558ff.

https://git.openembedded.org/openembedded-core/commit/meta/classes/uboot-extlinux-config.bbclass?id=42b44fdd9e63b8d39cf83baecec08328275558ff