Re: [PATCH 1/2] add support for derived sbuild chroots

[Uladzimir Bely <ubely@ilbers.de>]

On Tue, 2023-06-20 at 09:25 +0300, Uladzimir Bely wrote:
> On Mon, 2023-06-19 at 21:10 +0800, Moessbauer Felix wrote:
> > On Mon, 2023-06-19 at 07:58 +0200, Jan Kiszka wrote:
> > > On 13.06.23 08:24, Uladzimir Bely wrote:
> > > > On Thu, 2023-04-13 at 07:00 +0000, 'Felix Moessbauer' via isar-
> > > > users
> > > > wrote:
> > > > > This patch adds support to create derived sbuild chroots to
> > > > > speedup
> > > > > the
> > > > > build process. For packages that share a large set of common
> > > > > build
> > > > > dependencies, a derived sbuild chroot can be created to avoid
> > > > > the
> > > > > overhead of installing all base build-deps on each sbuild
> > > > > invocation.
> > > > > 
> > > > > Signed-off-by: Felix Moessbauer
> > > > > <felix.moessbauer@siemens.com>
> > > > > ---
> > > > >  doc/user_manual.md                            | 27
> > > > > +++++++++++++++++++
> > > > >  meta/classes/crossvars.bbclass                | 11 +++++---
> > > > >  .../sbuild-chroot/sbuild-chroot.inc           |  7 ++++-
> > > > >  3 files changed, 41 insertions(+), 4 deletions(-)
> > > > 
> > > > We are going to merge this in near few days in spite of an open
> > > > question left in discussion. The reasons:
> > > > 
> > > > 1. It passes internal CI (dev/fast/full).
> > > > 
> > > > 2. It was useful in case of 'meta-iot2050' downstream for
> > > > rewriting
> > > > "npm.bbclass" in order not to use buildchroot.bbclass (that is
> > > > going to
> > > > be deleted with "Imager schroot migration" patchset that is to
> > > > be
> > > > merged soon too.
> > > 
> > > To track what I discussed with Baurzhan offlist: I would be good
> > > to
> > > have
> > > some QA check running when using a derived sbuild chroot that the
> > > package built has all extra packages as part of its Build-Depends
> > > so
> > > that we are not create again silently broken debianizations. Any
> > > ideas
> > > how to achieve that best, considering also implicit inclusions of
> > > the
> > > Build-Depends?
> > 
> > This is hard to achieve. The only way I could currently imagine is
> > to
> > run the same package build in the standard chroot and check if it
> > still
> > properly builds (by definition, the derived chroot has a superset
> > of
> > the packages). For the created artifact, too-many packages in a
> > build
> > chroot are not problematic. Given that, at least the created
> > packages
> > are fine.
> > 
> 
> Could not it be some variable (disabled by default) that globally
> prohibits changes of SBUILD_FLAVOR and
> SBUILD_CHROOT_PREINSTALL_COMMON
> by packages/bbclasses?
> 
> It could be enabled in CI in order to build all packages with "clean"
> schroots, but disabled by default to speedup development process.
> 

I've experimented with such kind of "global disabling" derived schroots
and in general it works, but some moments are not still clear for me.

In the patch 2, we have SBUILD_FLAVOR="clang" set for `samefile`
package and that makes it use schroot with clang preinstalled. Also,
debian rules file is modified with `export CC=clang` in this case. As a
result, we have a package built with clang.

When I globally disable derived schroots, building `samefile` still
works since it is now built with gcc (debian rules are not modified).

I would expect a bit different behavior: debian rules files should not
be modified. Instead, `clang` is preinstalled as a dependency in
"clean" schroot and used as a compiler. So, in both cases (with
specific schroot and without it) the same compiler should be used - the
difference would be only in built time.

Probably, for the demo purposes it's OK, but I would expect that any
package is built with the same tools whether a derived schroot used or
not.

> > > 
> > > Furthermore, I had a small comment on patch 2.
> > 
> > I would rather postpone the "derive chroot name" aspect to later.
> > It
> > is
> > a minor thing and currently I lack the time to implement it. Feel
> > free
> > to add a patch on top.
> > 
> > Felix
> > 
> > > 
> > > Jan
> > > 
> > 
>