Re: [PATCH v2 1/6] sdk: Add support for adding self-defined sdk packages

[Jan Kiszka <jan.kiszka@siemens.com>]

On 12.06.20 12:05, [ext] Jan Kiszka wrote:
> On 12.06.20 11:54, Jan Kiszka wrote:
>> On 12.06.20 11:34, Baurzhan Ismagulov wrote:
>>> Hello Jan,
>>>
>>> On Mon, Jun 08, 2020 at 12:22:26PM +0200, Jan Kiszka wrote:
>>>> I just realized another design issue with current sdk building:
>>>> sdkchroot is shared with all image recipes that request
>>>> do_populated_sdk. You may expect that defining SDP_[PRE]INSTALL there
>>>> would have any impact, but that's not true therefore. Only adding those
>>>> to an appended sdkchroot recipe or the global configuration works right
>>>> now. As these additions may very well be specific to individual target
>>>> images, neither of those options are perfect. We rather need to make the
>>>> sdkchroot specific to the target image requesting it.
>>>>
>>>> That said, such a rework could happen later because the current form is
>>>> already fine for a lot of use cases (and is being used for a longer
>>>> while now).
>>>
>>> Thanks for the heads up, this was not on our radar. Let me check it and get
>>> back to you.
>>>
>>> Our comments so far:
>>>
>>> 1. I think the series is a good improvement, thanks.
>>>
>>> 2. isar-apt is added to the SDK so that the developer can install anything
>>>    after the initial preparation of the SDK. E.g., the user has SDK_INSTALL +=
>>>    libhello-dev but during the work he decides to install also libbye-dev using
>>>    apt. I suggest to keep isar-apt configured unless there is a better solution
>>>    for this use case. This also implies that we should keep both approaches --
>>>    relocate and chroot -- usable.
>>
>> If you want something in the SDK, add it upfront, via patch 1. Shipping
>> isar-apt is pointless bloat. Therefore the removal.
>>
>>>
>>> 3. relocate-sdk -r puts two slashes into the binaries. Also, strictly speaking,
>>>    it doesn't restore the binary into the state it was before, since rpath was
>>>    not set by default. Should we just remove rpath instead?
>>
>> Of course, we could also remore rpath. Does it practically make a
>> difference?
>>
>>>
>>> 4. It is also possible to set rpath to a relative path (in combination with
>>>    $ORIGIN). In that way, the same SDK would be usable with or without chroot
>>>    -- the use case is e.g. #2.
>>
>> How does that work then? Would we have to set rpath relative to the
>> respective binary location? If that worked, it would completely obsolete
>> a runtime script, right?
> 
> --set-rpath
> '$ORIGIN/../../usr/lib:$ORIGIN/../../usr/lib/${arch}-linux-gnu' ? That
> would be cool...
> 
> 
>>
>>>
>>> 5. I'd suggest to drop "recommended" and "fallback" from README.sdk.
>>>    relocate-sdk is already listed as the first option. The rest is a matter of
>>>    developers' preference. Or is there a specific reason to prefer one?
>>
>> It's not just a preference because the chroot is more involved, requires
>> privileged and prevents many SDK use cases. Therefore this clear
>> recommendation.
>>
>>>
>>>    E.g., I personally use chroot to avoid host environment leakage into the
>>>    built stuff. I'd like to work on the host, but in a Debian way (think
>>>    multiarch -- which IIUC currently doesn't support amd64-stretch on
>>>    amd64-buster).
>>
>> The whole point of the SDK is to enable easy application development.
>> Production builds would come an Isar environment, or a package builder.
>> You surely do not want to tell an ordinary developer to use sudo chroot
>> for daily application building.
>>
>>>
>>> 6. rpath and wrapping doesn't address all use cases.
>>>
>>>    1. Shared libraries may use other shared libraries.
>>>
>>>       E.g., ffmpeg fails to find libopenal.so.1 if
>>>       usr/lib/x86_64-linux-gnu/libavdevice.so.58 is not patched.
>>
>> LD_LIBRARY_PATH
>>
>> We may need some env tuning script for that.
>>
> 
> Or can we simply include the libs into patching?
> 

Any idea on that? And what exactly did you test?

> Jan
> 
>>>
>>>    2. Binaries outside of bin are not patched, e.g. clang-format.
>>
>> Then let's fix that.

Not sure if that need to be in scope. If you look into a cross
toolchain, you find <arch>-linux-gnu-* binaries, and this is generally
sufficient.

>>
>>>
>>>    3. Wrapping doesn't cover e.g. arm-linux-gnueabihf-gcc on qemuarm-buster.
>>>
>>
>> Then let's fix that.

Trivially fixed now.

>>
>>>    What is your vision on this? I think we will never cover all possible cases
>>>    and should rather strive for a working subset -- which your patch provides.
>>>    I also think this should be documented, because I assume that Yocto SDK is
>>>    not buggy in this regard (manual usability due to -isystem and stuff left
>>>    aside) and encountering such problems with the Isar SDK would be a bad
>>>    surprise.
>>
>> The vision is clearly to resolve the remaining issues. It will be a
>> product feature and the way we promote the SDK. chroot is for legacy
>> only, may even be phased out eventually. Then we can remove also all
>> those unneeded debian tools from the image, only providing the compiler
>> with its libs.
>>
> 

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux