Re: [PATCH v4 1/9] isar-bootstrap-host: disable DISTRO_APT_KEYS usage

["Maxim Yu. Osipov" <mosipov@ilbers.de>]

On 4/30/19 12:34 PM, Andreas Reichel wrote:
> On Fri, Apr 26, 2019 at 11:41:16AM +0300, Maxim Yu. Osipov wrote:
>> Hi Claudius, Andreas,
>>
>> @Andreas
>> Your input is very welcome at this topic as you were busy with all this APT
>> keys stuff.
>>
> 
> Thank you. Well in my eyes, Claudius delivers important changes to Isar
> which improve code quality and the build steps as a whole. Also as I
> know him, he is never ever interested in any personal "affairs" whatsoever,
> which I also learnt from his reviews on my code. He has a sharp mind and
> always tries to get out the best of the code up to his knowledge and
> techniques. Furthermore, if he may sometimes sound direct or even
> annoying to some - this is merely a personal question of how one focuses
> on mails. I also did not understand everything he criticized on my code
> in the beginning - but after I understood him, it was always great
> improvement. So, Maxim, I beg that you do not take anything personal on
> any mail, regarding any words or writing style but just focus on the
> code as I always try - which always helps to go on further with the
> project and to improve it. That's my input.
> 
> Andreas

I would prefer to focus on technical aspects of the modification under 
review. Are you OK with this modification?

Regards,
Maxim.

>> On 4/26/19 9:36 AM, Claudius Heine wrote:
>>> Hi Maxim,
>>>
>>> Quoting Maxim Yu. Osipov (2019-04-25 20:20:59)
>>>> On 4/25/19 3:44 PM, claudius.heine.ext@siemens.com wrote:
>>>>> From: Claudius Heine <ch@denx.de>
>>>>>
>>>>> isar-bootstrap-host only supports bootstrapping Debian root file
>>>>> systems. Therefore deactivate any DISTRO_APT_KEYS from other
>>>>> distributions.
>>>>>
>>>>> Signed-off-by: Claudius Heine <ch@denx.de>
>>>>> ---
>>>>>     meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb | 2 ++
>>>>>     1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb b/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb
>>>>> index 08b068f..3e96281 100644
>>>>> --- a/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb
>>>>> +++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb
>>>>> @@ -12,6 +12,8 @@ DEPLOY_ISAR_BOOTSTRAP = "${DEPLOY_DIR_BOOTSTRAP}/${HOST_DISTRO}-${HOST_ARCH}"
>>>>>     ISAR_BOOTSTRAP_LOCK = "${DEPLOY_DIR_BOOTSTRAP}/${HOST_DISTRO}-${HOST_ARCH}.lock"
>>>>>     require isar-bootstrap.inc
>>>>> +# We only build debian host buildchroot environments
>>>>> +DISTRO_APT_KEYS = ""
>>>>
>>>>    From the first glance this modification limits functionality.
>>>> It looks like a hack and I would suggest to avoid this modification.
>>>
>>> Well it is a fix and that limited functionality was already present but
>>> just implicit, hidden behind some bug and the cleanup just made it
>>> appear.
>>
>>
>> Could you please point to this hidden, implicit place where mentioned bug
>> persists?
>>
>> I've looked under meta/recipes-core/isar-bootstrap/
>>
>> It seems that keyring stuff is quite symmetrical (in terms of host/target):
>>
>> isar/meta/recipes-core/isar-bootstrap$ grep -ri keyring *
>> isar-bootstrap-host.bb:do_generate_keyring[stamp-extra-info] =
>> "${DISTRO}-${DISTRO_ARCH}"
>> isar-bootstrap-host.bb:addtask bootstrap before do_build after
>> do_generate_keyring
>> isar-bootstrap.inc:APTKEYRING = "${WORKDIR}/apt-keyring.gpg"
>> isar-bootstrap.inc:DEBOOTSTRAP_KEYRING = ""
>> isar-bootstrap.inc:        d.setVar("DEBOOTSTRAP_KEYRING", "--keyring
>> ${APTKEYRING}")
>> isar-bootstrap.inc:            d.setVar("DEBOOTSTRAP_KEYRING", "--keyring
>> ${APTKEYRING}")
>> isar-bootstrap.inc:do_generate_keyring[dirs] = "${DL_DIR}"
>> isar-bootstrap.inc:do_generate_keyring[vardeps] += "DISTRO_APT_KEYS"
>> isar-bootstrap.inc:do_generate_keyring() {
>> isar-bootstrap.inc:           gpg --no-default-keyring --keyring
>> "${APTKEYRING}" \
>> isar-bootstrap.inc:addtask generate_keyring before do_build after do_unpack
>> isar-bootstrap.inc:                               ${DEBOOTSTRAP_KEYRING} \
>> isar-bootstrap.inc: ${DEBOOTSTRAP_KEYRING} \
>> isar-bootstrap-target.bb:do_generate_keyring[stamp-extra-info] =
>> "${DISTRO}-${DISTRO_ARCH}"
>> isar-bootstrap-target.bb:addtask bootstrap before do_build after
>> do_generate_keyring
>> isar/meta/recipes-core/isar-bootstrap$
>>
>>
>> And bootstrapping itself (function isar_bootsrap in isar-bootstrap.inc)
>> differs only by passing extra '--arch' to target DISTRO_ARCH. Nothing
>> regarding
>>
>>              if [ ${IS_HOST} ]; then
>>                  ${DEBOOTSTRAP} $debootstrap_args \
>>                                 ${@get_distro_components_argument(d, True)} \
>>                                 ${DEBOOTSTRAP_KEYRING} \
>>                                 "${@get_distro_suite(d, True)}" \
>>                                 "${ROOTFSDIR}" \
>>                                 "${@get_distro_source(d, True)}"
>>
>>              else
>>                   "${DEBOOTSTRAP}" $debootstrap_args \
>>                                    --arch="${DISTRO_ARCH}" \
>>                                    ${@get_distro_components_argument(d,
>> False)} \
>>                                    ${DEBOOTSTRAP_KEYRING} \
>>                                    "${@get_distro_suite(d, False)}" \
>>                                    "${ROOTFSDIR}" \
>>                                    "${@get_distro_source(d, False)}"
>>              fi
>>
>>
>>
>>
>>>
>>>> Some time ago I thought about introduction of HOST_DISTRO_APT_KEYS to
>>>> avoid confusion between target and host apt keys.
>>>
>>> Good idea. But that would be a new feature/improvement.
>>
>> Yes. But your series is also improvement, isn't?
>>
>> I need more arguments for introduction of this limitation.
>>
>> Maxim.
>>
>>> Also thanks for looking at the code!
>>>
>>> Claudius
>>>
>>>>
>>>>
>>>> Maxim.
>>>>
>>>>
>>>>
>>>>>     inherit isar-bootstrap-helper
>>>>>     do_generate_keyring[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>>>>>
>>>>
>>>>
>>>> -- 
>>>> Maxim Osipov
>>>> ilbers GmbH
>>>> Maria-Merian-Str. 8
>>>> 85521 Ottobrunn
>>>> Germany
>>>> +49 (151) 6517 6917
>>>> mosipov@ilbers.de
>>>> http://ilbers.de/
>>>> Commercial register Munich, HRB 214197
>>>> General Manager: Baurzhan Ismagulov
>>>>
>>>> -- 
>>>> You received this message because you are subscribed to the Google Groups "isar-users" group.
>>>> To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
>>>> To post to this group, send email to isar-users@googlegroups.com.
>>>> To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/ccc13295-982c-7b25-cfc2-e079033689c0%40ilbers.de.
>>>> For more options, visit https://groups.google.com/d/optout.
>>>
>>> --
>>> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>>> Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch@denx.de
>>>
>>>              PGP key: 6FF2 E59F 00C6 BC28 31D8 64C1 1173 CB19 9808 B153
>>>                                Keyserver: hkp://pool.sks-keyservers.net
>>>
>>
>>
>> -- 
>> Maxim Osipov
>> ilbers GmbH
>> Maria-Merian-Str. 8
>> 85521 Ottobrunn
>> Germany
>> +49 (151) 6517 6917
>> mosipov@ilbers.de
>> http://ilbers.de/
>> Commercial register Munich, HRB 214197
>> General Manager: Baurzhan Ismagulov
> 


-- 
Maxim Osipov
ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn
Germany
+49 (151) 6517 6917
mosipov@ilbers.de
http://ilbers.de/
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov