Re: [PATCH v6] dpkg: Restore support for replacing pre-installed packages in sbuild-chroot

[Uladzimir Bely <ubely@ilbers.de>]

On Mon, 2024-01-22 at 10:59 +0300, Uladzimir Bely wrote:
> On Fri, 2024-01-19 at 10:49 +0100, Jan Kiszka wrote:
> > On 19.01.24 10:42, Jan Kiszka wrote:
> > > On 19.01.24 10:15, Uladzimir Bely wrote:
> > > > On Fri, 2024-01-19 at 08:49 +0100, Jan Kiszka wrote:
> > > > > On 19.01.24 08:44, Jan Kiszka wrote:
> > > > > > On 19.01.24 08:40, Jan Kiszka wrote:
> > > > > > > On 19.01.24 08:37, Uladzimir Bely wrote:
> > > > > > > > On Fri, 2024-01-19 at 10:20 +0300, Uladzimir Bely
> > > > > > > > wrote:
> > > > > > > > > On Thu, 2024-01-18 at 14:50 +0100, Jan Kiszka wrote:
> > > > > > > > > > From: Jan Kiszka <jan.kiszka@siemens.com>
> > > > > > > > > > 
> > > > > > > > > > During the migration to sbuild, support for using
> > > > > > > > > > self-
> > > > > > > > > > built
> > > > > > > > > > packages
> > > > > > > > > > in
> > > > > > > > > > the build environment was lost if those were
> > > > > > > > > > already
> > > > > > > > > > part
> > > > > > > > > > of the
> > > > > > > > > > sbuild-chroot. This restores it by adding --apt-
> > > > > > > > > > distupgrade
> > > > > > > > > > to the
> > > > > > > > > > sbuild call. But that is not enough because sbuild
> > > > > > > > > > will
> > > > > > > > > > only
> > > > > > > > > > upgrade
> > > > > > > > > > packages from already configured sources, not those
> > > > > > > > > > specified via
> > > > > > > > > > --extra-repository. We therefore have to switch
> > > > > > > > > > back
> > > > > > > > > > to
> > > > > > > > > > configuring
> > > > > > > > > > isar-apt during sbuild-chroot creation.
> > > > > > > > > > 
> > > > > > > > > > As rootfs_configure_isar_apt configures the isar
> > > > > > > > > > repo
> > > > > > > > > > under
> > > > > > > > > > /isar-apt, we bind-mount the one in
> > > > > > > > > > /home/builder/${PN} to
> > > > > > > > > > that
> > > > > > > > > > folder.
> > > > > > > > > > Another difference is that we now need to run apt-
> > > > > > > > > > get
> > > > > > > > > > update
> > > > > > > > > > explicitly,
> > > > > > > > > > but only for isar-apt.
> > > > > > > > > > 
> > > > > > > > > > For apt fetching, we neither need nor want isar-apt
> > > > > > > > > > to be
> > > > > > > > > > available.
> > > > > > > > > > Rebuilding self-generated apt packages is generally
> > > > > > > > > > pointless as
> > > > > > > > > > the
> > > > > > > > > > needs are better addressed in the generating recipe
> > > > > > > > > > itself.
> > > > > > > > > > Exposing
> > > > > > > > > > isar-apt to the fetch may furthermore lead to
> > > > > > > > > > fetching a
> > > > > > > > > > previously
> > > > > > > > > > built source package of the same recipe, rather
> > > > > > > > > > than
> > > > > > > > > > pulling the
> > > > > > > > > > external version.
> > > > > > > > > > 
> > > > > > > > > > And because the sbuild-chroots are now left behind
> > > > > > > > > > with
> > > > > > > > > > isar-apt
> > > > > > > > > > configured, the configuration in imager_run can be
> > > > > > > > > > removed
> > > > > > > > > > (credits
> > > > > > > > > > to
> > > > > > > > > > Srinuvasan Arjunan).
> > > > > > > > > > 
> > > > > > > > > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > > > > > > > > > ---
> > > > > > > > > > 
> > > > > > > > > > Changes in v6:
> > > > > > > > > >  - basically, going back to v4 but disabling isar-
> > > > > > > > > > apt
> > > > > > > > > > in
> > > > > > > > > > do_apt_fetch
> > > > > > > > > > 
> > > > > > > > > > I was able to resolve the scenario that Uladzimir
> > > > > > > > > > was
> > > > > > > > > > sharing this
> > > > > > > > > > way. 
> > > > > > > > > > And I also realized that apt fetching as well as
> > > > > > > > > > unpacking
> > > > > > > > > > should
> > > > > > > > > > have 
> > > > > > > > > > no deal with isar-apt. So, v5 was taking a wrong
> > > > > > > > > > turn.
> > > > > > > > > > 
> > > > > > > > > >  meta/classes/dpkg-base.bbclass                | 29
> > > > > > > > > > +++++++++++----
> > > > > > > > > > --
> > > > > > > > > > --
> > > > > > > > > >  meta/classes/dpkg.bbclass                     |  5
> > > > > > > > > > ++--
> > > > > > > > > >  meta/classes/image-tools-extension.bbclass    | 13
> > > > > > > > > > -
> > > > > > > > > > ------
> > > > > > > > > > --
> > > > > > > > > >  .../sbuild-chroot/sbuild-chroot.inc           |  2
> > > > > > > > > > -
> > > > > > > > > > -
> > > > > > > > > >  4 files changed, 20 insertions(+), 29 deletions(-)
> > > > > > > > > > 
> > > > > > > > > > diff --git a/meta/classes/dpkg-base.bbclass
> > > > > > > > > > b/meta/classes/dpkg-
> > > > > > > > > > base.bbclass
> > > > > > > > > > index 7b054d3f..80686677 100644
> > > > > > > > > > --- a/meta/classes/dpkg-base.bbclass
> > > > > > > > > > +++ b/meta/classes/dpkg-base.bbclass
> > > > > > > > > > @@ -19,8 +19,6 @@ DEPENDS:append:riscv64 = "${@'
> > > > > > > > > > crossbuild-
> > > > > > > > > > essential-riscv64' if d.getVar('ISAR_C
> > > > > > > > > >  DEB_BUILD_PROFILES ?= ""
> > > > > > > > > >  DEB_BUILD_OPTIONS ?= ""
> > > > > > > > > >  
> > > > > > > > > > -ISAR_APT_REPO ?= "deb [trusted=yes]
> > > > > > > > > > file:///home/builder/${PN}/isar-
> > > > > > > > > > apt/${DISTRO}-
> > > > > > > > > > ${DISTRO_ARCH}/apt/${DISTRO} ${DEBDISTRONAME}
> > > > > > > > > > main"
> > > > > > > > > > -
> > > > > > > > > >  python do_adjust_git() {
> > > > > > > > > >      import subprocess
> > > > > > > > > >  
> > > > > > > > > > @@ -115,6 +113,8 @@ do_apt_fetch() {
> > > > > > > > > >      trap 'exit 1' INT HUP QUIT TERM ALRM USR1
> > > > > > > > > >      trap 'schroot_cleanup' EXIT
> > > > > > > > > >  
> > > > > > > > > > +    schroot -d / -u root -c ${SBUILD_CHROOT} -- \
> > > > > > > > > > +        rm /etc/apt/sources.list.d/isar-apt.list
> > > > > > > > > > /etc/apt/preferences.d/isar-apt
> > > > > > > > > 
> > > > > > > > > The issue is still reproducible with the steps I
> > > > > > > > > posted
> > > > > > > > > below, as
> > > > > > > > > well
> > > > > > > > > as CI still fails with v6.
> > > > > > > > > I think, the steps above are not sufficient to
> > > > > > > > > completely
> > > > > > > > > stop
> > > > > > > > > dealing
> > > > > > > > > with isar-apt in do_apt_fetch() and we need some kind
> > > > > > > > > of 'apt
> > > > > > > > > update'
> > > > > > > > > here to consider removed list and preferences. The
> > > > > > > > > question
> > > > > > > > > is how to
> > > > > > > > > say apt to forget about isar-apt and do not run apt
> > > > > > > > > update
> > > > > > > > > for other
> > > > > > > > > source lists... maybe, simply remove isar-apt files
> > > > > > > > > from
> > > > > > > > > /var/lib/apt/lists/.
> > > > > > > > 
> > > > > > > > An addition:
> > > > > > > > 
> > > > > > > > It seems that files are not really removed from the
> > > > > > > > session for
> > > > > > > > some
> > > > > > > > reason. I added some kind of "ls" near remove code:
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > +    schroot -d / -u root -c ${SBUILD_CHROOT} -- \
> > > > > > > > +        sh -c 'ls -la /etc/apt/sources.list.d/'
> > > > > > > >      schroot -d / -u root -c ${SBUILD_CHROOT} -- \
> > > > > > > >          rm /etc/apt/sources.list.d/isar-apt.list
> > > > > > > > /etc/apt/preferences.d/isar-apt
> > > > > > > > +    schroot -d / -u root -c ${SBUILD_CHROOT} -- \
> > > > > > > > +        sh -c 'ls -la /etc/apt/sources.list.d/'
> > > > > > > > ```
> > > > > > > > , but still see the following in the log:
> > > > > > > > ```
> > > > > > > > > DEBUG: Executing shell function do_apt_fetch
> > > > > > > > > total 16
> > > > > > > > > drwxr-xr-x 2 root root 4096 Jan 19 07:09 .
> > > > > > > > > drwxr-xr-x 8 root root 4096 Jan 19 07:09 ..
> > > > > > > > > -rw-r--r-- 1 root root  569 Jan 19 07:09
> > > > > > > > > bootstrap.list
> > > > > > > > > -rw-r--r-- 1 root root   94 Jan 19 07:09 isar-
> > > > > > > > > apt.list
> > > > > > > > > total 16
> > > > > > > > > drwxr-xr-x 2 root root 4096 Jan 19 07:09 .
> > > > > > > > > drwxr-xr-x 8 root root 4096 Jan 19 07:09 ..
> > > > > > > > > -rw-r--r-- 1 root root  569 Jan 19 07:09
> > > > > > > > > bootstrap.list
> > > > > > > > > -rw-r--r-- 1 root root   94 Jan 19 07:09 isar-
> > > > > > > > > apt.list
> > > > > > > > > Reading package lists...
> > > > > > > > ```
> > > > > > > > Fith the apt_fetch failure followed, because of
> > > > > > > > existing
> > > > > > > > of
> > > > > > > > previously
> > > > > > > > built hello source package in the isar-apt repo.
> > > > > > > > 
> > > > > > > 
> > > > > > > OK... last-minute change of mine, splitting up the
> > > > > > > purging
> > > > > > > and
> > > > > > > the
> > > > > > > actual fetching into two schroot runs to avoid having to
> > > > > > > run the
> > > > > > > latter
> > > > > > > with "-u root". I didn't find a way to de-configure isar-
> > > > > > > apt
> > > > > > > unprivileged. Back to the drawing board. :(
> > > > > > > 
> > > > > > 
> > > > > > Comparing the split run of schroot with image-tool-
> > > > > > extension,
> > > > > > I
> > > > > > think
> > > > > > I'm just missing a proper session ID here.
> > > > > > 
> > > > > 
> > > > > Before sending v7: can you try this on top?
> > > > > 
> > > > > diff --git a/meta/classes/dpkg-base.bbclass
> > > > > b/meta/classes/dpkg-
> > > > > base.bbclass
> > > > > index 80686677..1a3333ac 100644
> > > > > --- a/meta/classes/dpkg-base.bbclass
> > > > > +++ b/meta/classes/dpkg-base.bbclass
> > > > > @@ -113,10 +113,13 @@ do_apt_fetch() {
> > > > >      trap 'exit 1' INT HUP QUIT TERM ALRM USR1
> > > > >      trap 'schroot_cleanup' EXIT
> > > > >  
> > > > > -    schroot -d / -u root -c ${SBUILD_CHROOT} -- \
> > > > > +    session_id=$(schroot -q -b -c ${SBUILD_CHROOT})
> > > > > +    echo "Started session: ${session_id}"
> > > > > +
> > > > > +    schroot -r -c ${session_id} -d / -u root -- \
> > > > >          rm /etc/apt/sources.list.d/isar-apt.list
> > > > > /etc/apt/preferences.d/isar-apt
> > > > >      for uri in "${SRC_APT}"; do
> > > > > -        schroot -d / -c ${SBUILD_CHROOT} -- \
> > > > > +        schroot -r -c ${session_id} -d / -- \
> > > > >              sh -c 'mkdir -p /downloads/deb-src/"$1"/"$2" &&
> > > > > cd
> > > > > /downloads/deb-src/"$1"/"$2" && apt-get -y --download-only --
> > > > > only-
> > > > > source source "$2"' my_script "${BASE_DISTRO}-
> > > > > ${BASE_DISTRO_CODENAME}" "${uri}"
> > > > >      done
> > > > >      schroot_delete_configs
> > > > > 
> > > > > Jan
> > > > > 
> > > > 
> > > > Yes, with persistent session it works, at least with reproduce
> > > > steps I
> > > > used. I just added session cleanup (e.g, "schroot -e -c
> > > > ${session_id}")
> > > > at the end.
> > > > 
> > > > Before sending v7 I could also check if it is not reproduced
> > > > anymore in
> > > > CI.
> > > > 
> > > > Also, due to using schroot session we probably also need to
> > > > care
> > > > about
> > > > proper session removing in case commands under schroot fail,
> > > > similar to
> > > > how it's done in image-tools-extention by imager_cleanup().
> > > 
> > > Yeah, still need to check that - schroot is also still new to me,
> > > learned a lot about it these days.
> > > 
> > > Jan
> > > 
> > 
> > diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-
> > base.bbclass
> > index 80686677..85d0a495 100644
> > --- a/meta/classes/dpkg-base.bbclass
> > +++ b/meta/classes/dpkg-base.bbclass
> > @@ -107,18 +107,24 @@ do_apt_fetch() {
> >      E="${@ isar_export_proxies(d)}"
> >      schroot_create_configs
> >  
> > +    session_id=$(schroot -q -b -c ${SBUILD_CHROOT})
> > +    echo "Started session: ${session_id}"
> > +
> >      schroot_cleanup() {
> > +        schroot -q -f -e -c ${session_id} > /dev/null 2>&1
> >          schroot_delete_configs
> >      }
> >      trap 'exit 1' INT HUP QUIT TERM ALRM USR1
> >      trap 'schroot_cleanup' EXIT
> >  
> > -    schroot -d / -u root -c ${SBUILD_CHROOT} -- \
> > +    schroot -r -c ${session_id} -d / -u root -- \
> >          rm /etc/apt/sources.list.d/isar-apt.list
> > /etc/apt/preferences.d/isar-apt
> >      for uri in "${SRC_APT}"; do
> > -        schroot -d / -c ${SBUILD_CHROOT} -- \
> > +        schroot -r -c ${session_id} -d / -- \
> >              sh -c 'mkdir -p /downloads/deb-src/"$1"/"$2" && cd
> > /downloads/deb-src/"$1"/"$2" && apt-get -y --download-only --only-
> > source source "$2"' my_script "${BASE_DISTRO}-
> > ${BASE_DISTRO_CODENAME}" "${uri}"
> >      done
> > +
> > +    schroot -e -c ${session_id}
> >      schroot_delete_configs
> >  }
> >  
> > 
> > Jan
> > 
> 
> Hello.
> 
> Checked in CI during the weekend - the issue seems not to be
> reproduced
> anymore. So, v7 should be OK.
> 

Since there is still no v7 on list, I'll send it on my own.

The patch is checked in CI as well as in the downstream that requires
downgrade for linux-libc-dev package. Everything looks OK.