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

[claudius....@siemens.com]

From: Claudius Heine <c...@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 <c...@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 = ""
inherit isar-bootstrap-helper

do_generate_keyring[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
--
2.20.1

[Maxim Yu. Osipov]

On 4/25/19 3:44 PM, claudius....@siemens.com wrote:
> From: Claudius Heine <c...@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 <c...@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.

Some time ago I thought about introduction of HOST_DISTRO_APT_KEYS to
avoid confusion between target and host apt keys.


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
mos...@ilbers.de
http://ilbers.de/
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov

[Claudius Heine]

Hi Maxim,

Quoting Maxim Yu. Osipov (2019-04-25 20:20:59)

> On 4/25/19 3:44 PM, claudius....@siemens.com wrote:
> > From: Claudius Heine <c...@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 <c...@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.

> 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.

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
> mos...@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+...@googlegroups.com.
> To post to this group, send email to isar-...@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: c...@denx.de

PGP key: 6FF2 E59F 00C6 BC28 31D8 64C1 1173 CB19 9808 B153
Keyserver: hkp://pool.sks-keyservers.net

[Maxim Yu. Osipov]

Hi Claudius, Andreas,

@Andreas
Your input is very welcome at this topic as you were busy with all this
APT keys stuff.

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.

[Claudius Heine]

Quoting Maxim Yu. Osipov (2019-04-26 10:41:16)

The HOST_DISTRO is set to 'debian-stretch' in
isar-bootstrap-helper.bbclass (included by isar-bootstrap-host.bb)
results in isar-bootstap-host only bootstrapping debian-stretch
repositories, since the 'HOST_DISTRO_APT_SOURCES' (set in
isar-bootstrap.inc) variable only points to 'debian-stretch.list'.

Yes that is quite a mess of spaghetti code, which this patch set is
trying to sort out somewhat and also probably why this patch set now
reveals some hidden bugs or limitations like this.

A patch later in this patch series sets 'HOST_DISTRO' to 'DISTRO' per
default. But that only opens the possibility to use other debian
versions (buster) for buildchroot-host, not allowing non-debian distros.
So the implicit limitation is somewhat loosened with this patch series.

> And bootstrapping itself (function isar_bootsrap in isar-bootstrap.inc)
> differs only by passing extra '--arch' to target DISTRO_ARCH. Nothing
> regarding

Well 'isar-bootstrap-host' does contain some issues that needs fixing but
this is not in scope of this patch series, which focuses mainly on
the rootfs creation process and only secondary on the isar-bootstrap*
stuff.

> >> 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?

With that reasoning you could also say that my patch series should fix
every issue in existence because that might be an 'improvement'.

Fixing the world is not in scope of this patch set.

> I need more arguments for introduction of this limitation.

Again, that limitation is already present. I don't get why you are
often so sure of yourself, especially about code you have not written
or worked with intensively.

Claudius

[Claudius Heine]

Quoting Claudius Heine (2019-04-26 12:39:27)

> Quoting Maxim Yu. Osipov (2019-04-26 10:41:16)

> > I need more arguments for introduction of this limitation.
>
> Again, that limitation is already present. I don't get why you are
> often so sure of yourself, especially about code you have not written
> or worked with intensively.

Ok sorry, I take that back. You have written and worked with this code.
I thought that was written mostly by Alex.

[Maxim Yu. Osipov]

On 4/26/19 12:45 PM, Claudius Heine wrote:
> Quoting Claudius Heine (2019-04-26 12:39:27)
>> Quoting Maxim Yu. Osipov (2019-04-26 10:41:16)
>>> I need more arguments for introduction of this limitation.
>>
>> Again, that limitation is already present. I don't get why you are
>> often so sure of yourself, especially about code you have not written
>> or worked with intensively.

I suggest you to choose the correct words before writing such kind of
sentences. I'm not a boy and you are not my mentor.

> Ok sorry, I take that back. You have written and worked with this code.
> I thought that was written mostly by Alex.

It doesn't matter if this my code or not.

I'm quite tired from this offensive style of communication - I consider
such style as misbehavior. I hope that was my last warning to you.

Maxim.

> Claudius
>
> --
> 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: c...@denx.de
>
> PGP key: 6FF2 E59F 00C6 BC28 31D8 64C1 1173 CB19 9808 B153
> Keyserver: hkp://pool.sks-keyservers.net
>

[Jan Kiszka]

If that is just about adding and documenting another variable, let's not discuss
about when and who because just doing that will already be faster, even if it's
a "drive-by" improvement /wrt this patchset.

Jan

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

[Jan Kiszka]

On 26.04.19 13:22, Maxim Yu. Osipov wrote:
> On 4/26/19 12:45 PM, Claudius Heine wrote:
>> Quoting Claudius Heine (2019-04-26 12:39:27)
>>> Quoting Maxim Yu. Osipov (2019-04-26 10:41:16)
>>>> I need more arguments for introduction of this limitation.
>>>
>>> Again, that limitation is already present. I don't get why you are
>>> often so sure of yourself, especially about code you have not written
>>> or worked with intensively.
>
> I suggest you to choose the correct words before writing such kind of sentences.
> I'm not a boy and you are not my mentor.
>
>> Ok sorry, I take that back. You have written and worked with this code.
>> I thought that was written mostly by Alex.
>
> It doesn't matter if this my code or not.
>
> I'm quite tired from this offensive style of communication - I consider such
> style as misbehavior. I hope that was my last warning to you.
>

In general, I would appreciate when everyone focuses more again on understanding
the issues we still have in code base and tooling, rather then discussing about
where they came from, who to blame and who should therefore fix them. And the
result needs to be understandable for the core contributors - and ideally not
only for them.

Of course, if there is evidence that a particular way of working is inefficient,
a way of designing is causing problems too often, please make that transparent,
with facts.

Thanks,

[Jan Kiszka]

OTOH, I don't get the problem yet from just reading the commit message: Wasn't
DISTRO_APT_KEYS designed to be a superset of all needed keys? We are appending
raspbian to it when using that distro. So, we are at least missing the reasoning
here why that model didn't work and cannot be made working for the host/target
case. And then we can refer to that when introducing split key sets.

Thanks,

[Andreas Reichel]

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

--
Andreas Reichel
Dipl.-Phys. (Univ.)
Software Consultant

Andreas...@tngtech.com, +49-174-3180074
TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterfoehring
Geschaeftsfuehrer: Henrik Klagges, Dr. Robert Dahlke, Gerhard Mueller
Sitz: Unterfoehring * Amtsgericht Muenchen * HRB 135082

[Maxim Yu. Osipov]

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.

[Andreas Reichel]

On Tue, Apr 30, 2019 at 04:30:46PM +0300, Maxim Yu. Osipov wrote:
> 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?
>

I am OK with these.

However I want MY patch series (version 9) merged before this one,
because it is now very well reviewed and weeks old.
And please don't argue with failed CI now. Investigating, understanding
and fixing CI is a different topic and out of my scope.

Regards,
Andreas

[Jan Kiszka]

So, to state this differently: You observation is that current next or master
does not reliably build in Isar upstream CI, right? Then this issue would be of
topmost priority to understand and resolve.

Andreas, Henning, Claudius, do we have current references to upstream CI build
with that behaviour, even if that was sporadic?

[Maxim Yu. Osipov]

I'll not merge your v9 series - more than one week ago I pointed you to
the problematic test case which is easily reproducible (see forwarded
email below):

So I expect your feedback/next version of series fixing mentioned problem.


-------- Forwarded Message --------
Subject: Re: [PATCH v9 0/5] Fix usage of additional apt keys and repos
Date: Mon, 22 Apr 2019 14:22:24 +0200
From: Maxim Yu. Osipov <mos...@ilbers.de>
Organization: ilbers GmbH
To: Andreas J. Reichel <andreas.r...@siemens.com>,
isar-...@googlegroups.com

Hi Andreas,

I've tested your series (with the docker use-case example you described
in last patch in series). It works as described in the default case -
without local apt caching enabled.

I've tested it with with signed local apt caching feature enabled.

The first stage - creation of local repo passed OK -

bitbake -c cache_base_repo multiconfig:qemuarm64-stretch:isar-image-base

But on the second stage the build failed (see log below).

I've double checked 'signed local apt caching feature' works fine in the
current 'next'.

My local.conf is attached for convenience.

Regards,
Maxim.

[Andreas Reichel]

Sorry I overlooked that problem. I just sent the fix with v10.

[Maxim Yu. Osipov]

Hi Claudius,

Any feedback on Jan's question (see below)?

I've applied v10 series "Fix usage of additional apt keys and repos"
from Andreas, this results that your v4 series "Cleanup rootfs creation"
has to be rebased. I can rebase it - no problem - but all pending
questions should be answered first.

Regards,
Maxim.

[Claudius Heine]

Hi Jan,

DISTRO_APT_KEYS is only used for distros that aren't Debian, because
debootstrap uses the keys of the host distro (Debian) per default.
THIRD_PARTY_APT_KEYS is for the keys of other third party repositories
that are not used to bootstrap from.

We currently only support raspbian as a non-debian distribution and
before this patchset only 'debian-stretch' as buildchroot-host.
After this patchset we support all Debian versions we currently support
for the target for the buildchroot-host as well. Since raspbian does not
supply packages that can be used for a buildchroot-host environment, it
makes sense to just use the Debian host keys for bootstrapping the
buildchroot-host rootfs in general.

If at one point we want to support other non-debian apt/dpkg based
distributions like ubuntu, that can be used for the target as well as
the host root file system, then it makes sense to allow specifying
additional keys for the buildchroot-host as well. Until that time
however this fix is enough to go forward.

regards,
Claudius

> We
> are appending raspbian to it when using that distro. So, we are at least
> missing the reasoning here why that model didn't work and cannot be made
> working for the host/target case. And then we can refer to that when
> introducing split key sets.
>
> Thanks,
> Jan
>
>

--

[Claudius Heine]

small errata:

On 14/05/2019 13.32, Claudius Heine wrote:

>>
>> OTOH, I don't get the problem yet from just reading the commit
>> message: Wasn't DISTRO_APT_KEYS designed to be a superset of all
>> needed keys?
>
> DISTRO_APT_KEYS is only used for distros that aren't Debian, because
> debootstrap uses the keys of the host distro (Debian) per default.

DISTRO_APT_KEYS was renamed by Andreas new patchset and is now called
DISTRO_BOOTSTRAP_KEYS.