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

[Uladzimir Bely <ubely@ilbers.de>]

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.