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