Re: [ImageBuilder 0/2] Use lopper to generate partial dts

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Stefano,

On 13/09/2022 21:28, Stefano Stabellini wrote:
> 
> 
> On Tue, 13 Sep 2022, Michal Orzel wrote:
>> Hi Ayan,
>>
>> On 12/09/2022 18:27, Ayan Kumar Halder wrote:
>>> Hi Michal,
>>>
>>> On 12/09/2022 12:59, Michal Orzel wrote:
>>>> This patch series introduces support to generate automatically passthrough
>>>> device trees using lopper. This feature should be used with care as the
>>>> corresponding lopper changes are still in an early support state. Current
>>>> integration has been tested with several devices from ZynqMP ZCU102 board
>>>> e.g. serial, spi, ahci, mmc.
>>>>
>>>> When using this feature, make sure to use the latest lopper's master branch
>>>> status [1].
>>>
>>> I am guessing that this is the first time the imagebuilder is using
>>> script from an external repo. There might always be a possibility that
>>> future changes to lopper (master branch) might not be backward
>>> compatible or might break something in imagebuilder.
>>>
>>> As such, will it make things better if lopper is included as a
>>> gitsubmodule for imagebuilder. This way a specific revision of lopper
>>> will be in sync with a specific revision of imagebuilder.
>>>
>>> Please let me know your thoughts.
>>
>> I think it could be beneficial in the future but not in the current state.
>> The reason why is that the lopper changes are in an early support state
>> (I try to highlight it on each occasion). This means that in the near
>> future we will be improving lopper extract assists to cover some corner 
>> cases.
>> Adding lopper as a submodule now, would result in a need of additional 
>> commits
>> for the ImageBuilder fetching new lopper changes each time we improve 
>> something
>> in lopper. I think we do not need such overhead at this stage.
>>
>> Also lopper's README states that "Internal interfaces are subject to change"
>> so we can assume that the interface given to the user will not change.
> 
> Forward and backward compatibility is something we'll need to think
> about at some point.
> 
> Personally I dislike git submodules and I would try to avoid using them
> unless strictly necessary. However, we could specify a commit-id or tag
> to use (the same way Yocto specifies component versions.)
> 
> Given that it is still early stage for this feature, I think we could
> ignore the problem for now and come back to it in the future.
> 
> Or we could change this patch series now to take as LOPPER_PATH input
> something like a SRC_URI in Yocto, which could be any of the following:
> 
> git://some.host/somepath;branch=branchX,branchY;name=nameX,nameY
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsome.host%2Fsomepath%3Bbranch%3DbranchX%2CbranchY%3Bname%3DnameX%2CnameY&data=05%7C01%7Cmichal.orzel%40amd.com%7C5ae88781fff24f8e6a8c08da95be2fb6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637986941323037721%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=dHzcGYqr9mIv2t746FnEE8QTrSmxqBJ4G9esebbAnu4%3D&reserved=0
> file://local.path/to/file.txt
> 
> If we did this, it would be more future proof and we could use the
> https:// URI by default with the "master" or "master-next" branch so
> that we would automatically get the latest updates. In the future we
> would specificy a stable branch instead (e.g. v0.2022.x).

This is a good idea in general but has one big drawback IMO.
Specifying the git repository such as lopper to be cloned by ImageBuilder 
results in
transferring the responsibility of installing prerequisite packages required by 
looper from a user to ImageBuilder.
Some of the packages are available to download using apt-get install and some 
of them unfortunately using pip manager which can be tricky sometimes.
In the current solution, lopper is an external dependency and the user is 
responsible for cloning the lopper and making sure the packages are installed,
in the same way as we require e.g. mkfs.ext4 to be installed. However, cloning 
a project from ImageBuilder means that it is the one who needs to fulfill the 
requirements
of the cloned project and keep up with syncing them (different package versions 
required by different versions of lopper).

Once the lopper+imagebuilder integration is in the shippable state, we can just 
inform user in the README with regards
to which lopper's branch/commit-id should be used.

~Michal