Prepare support for platforms variants #36
Conversation
ndechesne
requested review from
lumag,
sbanerjee-quic and
vkraleti
as code owners
Makefile
Outdated
| TOPDIR := $(PWD) | ||
| PLATFORMS := $(foreach platform,$(wildcard platforms/*),$(platform)/gpt) | ||
| PARTITIONS_XML := $(foreach platform,$(wildcard platforms/*),$(platform)/partitions.xml) | ||
| PARTITIONS := $(wildcard platforms/*/partitions.conf) $(wildcard platforms/*/*/partitions.conf) |
There was a problem hiding this comment.
Probably good to call sort here to have a stable list as well.
There was a problem hiding this comment.
I don't think we need to sort them here. It's only used as targets to define the order of makefile targets execution. but sorting is not bringing any benefit here. unless i am missing something.
There was a problem hiding this comment.
The out of order would only happen if we had a machine with platforms/machine/partitions.conf and also additional emmc/ufs files, and while OK from the syntax perspective, I don't think we will do that on purpose, so should be fine without sorted.
There was a problem hiding this comment.
actually I don't need to run wildcard twice.. i just need to use 2 pattern. will update.
gen_contents.py
Outdated
| build.remove(DownloadFile) | ||
| # Partition entires | ||
| for Partition in PartitionsSet: | ||
| for Partition in sorted(PartitionsSet): |
There was a problem hiding this comment.
This makes the output stable, but it also reorders the partitions compared to the original definition; I don't know if order matters, if it does another approach is to make PartitionsSet an array instead of a set.
There was a problem hiding this comment.
this is a valid comment. however up until now, every time we ran gen_content we would generate a different file anyway. So I suspect the order does not matter. @vkraleti ?
There was a problem hiding this comment.
I don't think that the order in contents.xml should matter.
There was a problem hiding this comment.
Partitions order in contents.xml doesn't really matter.
There was a problem hiding this comment.
if we sort the order, then let's preserve it? In any case, I agree with Nico that we should have a deterministic output
There was a problem hiding this comment.
if we sort the order, then let's preserve it? In any case, I agree with Nico that we should have a deterministic output
I do not understand what you meant here @lool . What do you mean with let's preserve it?
There was a problem hiding this comment.
Sorry, I meant "if we care to provide some deterministic order, we can preserve the original one"
There was a problem hiding this comment.
that was a good suggestion. i did it. replaced the 'set' with a list, so that the order from partition.xml is preserved!
ndechesne
force-pushed
the
emmc
branch
2 times, most recently
from
8fdceef to
ad7e860
Compare
|
This needs to be rebased now, sorry |
Running get_contents multiple times with the same input files generate different output files. This is because the list of partitions from partitions.xml are parsed and stored in a set data structure, which is unsorted. Looping on the set will return different output. Convert the set into a list instead which leads to: 1. the output is sorted, and thus the generated file is stable 2. the order from partitions.xml is preserved While making this change, I decided to put dict elements in the list instead of a list, it's easier to refer to field by name than by index. Signed-off-by: Nicolas Dechesne <[email protected]>
The makefile targets are defined mainly as a wildcard of $(wildcard platforms/*) and $(platform)/partitions.xml. They both define the location of the 'output' file (the XML file being the output file), and they hardcode the platform folder. Together they simply assume that there can be only one partitions.conf file inside the platform folder which is ./platforms/<board>. However for each board we want to be able to support multiple board configurations, such as a board with either an eMMC or a UFS disk, or perhaps a board that comes with a 16GB eMMC or 32 GB eMMC. So ideally we want to support: ./platforms/<board>/emmc/paritions.conf ./platforms/<board>/emmc-32gb/paritions.conf ./platforms/<board>/ufs/paritions.conf With this patch, we are looking for partitions.conf file, and we set the makefile targets based on that. We essentially introduce support for the following pattern: ./platforms/<board>/partitions.conf ./platforms/<board>/<variant>/partitions.conf Similarly, make sure we are looking for contexts.xml templates in both folders: ./platforms/<board>/contents.xml.in ./platforms/<board>/<variant>/contents.xml.in Signed-off-by: Nicolas Dechesne <[email protected]>
Generated files can potentially be location in a subsfolder of the platform folder. Signed-off-by: Nicolas Dechesne <[email protected]>
5 participants
I have another patch, which I am not pushing for now since it will break meta-qcom and qcom-deb-images to move all partitions.conf files into either emmc or ufs subfolder.
While preparing these changes and making sure that I was not breaking anything, i found the sorting (or lack of) issue which is fixed by first patch.