[LEDE-DEV] fixing of image file names

Fri Dec 1 05:30:48 PST 2017

Hello Mathias,

On 29.11.2017 09:33, Mathias Kresin wrote:
> 28.11.2017 19:24, Daniel Golle:
>> Hi Moritz,
>> 
>> thanks for stepping forward and adressing this issue.
>> It'd be good to include the two assertions added to your list beelow.
>> 
>> On Tue, Nov 28, 2017 at 07:05:23PM +0100, Moritz Warning wrote:
>>> Hi,
>>>
>>> I noticed that there are some image file names that do not follow the "common" name scheme.
>>> Is it ok to change it? I would like to submit a patch.
>>>
>>> Some examples:
>>> - all lower case image names
>>>    - lede-ipq806x-EA8500-squashfs-sysupgrade.tar => lede-ipq806x-ea8500-squashfs-sysupgrade.tar
>>> - revision between -
>>>    - lede-mvebu-linksys-wrt1900acv2-squashfs-sysupgrade.bin => lede-mvebu-linksys-wrt1900ac-v2-squashfs-sysupgrade.bin
>>> - region specific images with region identifiers (us, eu, il, ...)
>>>   - lede-ramips-rt305x-wnce2001-squashfs-factory-northamerica.bin => lede-ramips-rt305x-wnce2001-us-squashfs-factory.bin
>>> - separate images for each version
>>>    - lede-brcm47xx-mips74k-linksys-e1000-v1-v2-v2.1-squashfs.bin => lede-brcm47xx-mips74k-linksys-e1000-v1-squashfs.bin, ...
>> 
>>   - board_name (in target userspace) == profile (in imagebuilder) == DTS name
>> 
>>   - image_filename == ${distro}-${target}-${subtarget}-${board_name}-${fstype}-${imgtype}
>> 
>> that would make identifying sysupgrade images much more straight
>> forward (and hence automatizable).
>> 
>> See also
>> https://github.com/aparcar/attendedsysupgrade-server/issues/80
> 
> I would like to propose something different which basically aims the same.

We already discussed this outside the list, let me make a summary and 
share my comments below.

> 
> 1. fix the compatible strings in the DTS files

I think we should start at the same time upstreaming vendor prefixes to 
[1], to avoid possible incompatibility/inconsistency later.

> 2. use the compatible string from the DTS in userspace (boardname)

This is a great idea and I fully support it.

> 3. use the compatible string for the image filename (board_name in above
> example)
This one might be a bit more complicated as we thought before, see below.

> The DTS compatible string is supposed to be unique across the whole
> kernel and this way we can prevent duplicates in big targets like ramips.
> 
> The compatible string includes the vendor, which will make it way easier
> to find a particular image in directories with a lot of images. In
> theory, it should be even possible to provide all images in a single
> directory without target/subtarget prefix.

FWIW, this would also solve boardname collision problems in userspace. 
For example, we have support for boards from different vendors in ar71xx 
target (it's legacy but I believe we will convert it to dts at some 
point) with the same names: Atheros AP96 and ALFA Network AP96.

> Since the underscore isn't a valid character in compatible strings, we
> can use it in the image filename as replacement for the comma to split
> vendor from boardname.

The assumption about the underscore character was my suggestion but it 
seems that it's not (fully) valid. At least, after researching this 
more, I was able to find only one reference in kernel patchwork: [2].
I suppose it's only a general convention rather than a documented 
requirement (I'm might be wrong here, anyone? Maybe we should just ask 
upstream about this) as there are already some examples in upstream 
which contain underscores inside compatible strings: [3], [4].

Assuming above and the fact that <manufacture> and <model> parts used in 
compatible string, based on dt specification: [5], might contain any 
printable characters (which might not be filename and Makefile safe), we 
shouldn't make any assumptions about that.

> Due to the sync of runtime boardname and the boardname used in the image
> filename, it should be possible to guess the used image filename more
> reliably as requested.

I would like to propose here slightly extended version of compatible 
string -> boardname part in image filename (and our building code) 
conversion function:

1. Convert all letters to lowercase.
2. Replace characters different than [a-z], [0-9] and comma with dash.
3. Replace comma with underscore.

I think that we could even include above function in userspace and 
expose the value somewhere, maybe in /tmp/sysinfo?

This would make searching for dedicated image for a specific device much 
easier. I think, it could solve some other issues too, already reported 
in [6] and in community projects like [7].

And personally, I would like to see same naming convention (no matter 
which one we select) in use across all (sub)targets. At now, as the 
"common" scheme isn't documented, there are different approaches as 
Mortiz already pointed out.

[snip]

> Any feedback on this approach (and help in migrating exists boards of
> course) is highly welcome.

I still have here some concerns/thoughts:

1. I don't know how to deal in above approach with region variant images 
but I'm sure we should _somehow_ separate region code from boardname and 
other parts of the image filename. IMHO, dash character won't work here.

2. We have some boards versions like "v2.1" which with my above approach 
would end up as "v2-1". Maybe we should keep dot as a valid character in 
version part?

3. Some of devices come in different RAM/flash configurations and in 
case where common image can't support all of them, we include 
information about RAM/flash variant sometimes only in image name and 
sometimes in both the image name and boardname/compatible string. A 
common and defined approach would be required for this as well.

[1] 
http://elixir.free-electrons.com/linux/v4.15-rc1/source/Documentation/devicetree/bindings/vendor-prefixes.txt

[2] https://patchwork.kernel.org/patch/8362521/

[3] 
http://elixir.free-electrons.com/linux/v4.15-rc1/source/arch/arm/boot/dts/kirkwood-net2big.dts#L24

[4] 
http://elixir.free-electrons.com/linux/v4.15-rc1/source/arch/arm/boot/dts/imx23-sansa.dts#L50

[5] 
https://github.com/devicetree-org/devicetree-specification/blob/master/source/devicetree-basics.rst#compatible

[6] https://bugs.lede-project.org/index.php?do=details&task_id=966

[7] https://github.com/freifunk-bielefeld/firmware-wizard

-- 
Cheers,
Piotr