Re: [PATCH v4 2/2] meta: cache deb srcs as part of postprocessing

[Henning Schild <henning.schild@siemens.com>]

Am Wed, 8 Apr 2020 16:07:15 +0530
schrieb vijai kumar <vijaikumar.kanagarajan@gmail.com>:

> On Wed, Apr 8, 2020 at 3:34 PM Henning Schild
> <henning.schild@siemens.com> wrote:
> >
> > Am Fri, 3 Apr 2020 18:35:51 +0530
> > schrieb Vijai Kumar K <vijaikumar.kanagarajan@gmail.com>:
> >  
> > > Collect the deb sources of the corresponding deb binaries cached
> > > in DEBDIR as part of postprocess for those to be later included
> > > into the final base-apt by do_cache.
> > >
> > > Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
> > > ---
> > >  meta/classes/image.bbclass  |  2 +-
> > >  meta/classes/rootfs.bbclass | 46
> > > +++++++++++++++++++++++++++++++++++++ 2 files changed, 47
> > > insertions(+), 1 deletion(-)
> > >
> > > diff --git a/meta/classes/image.bbclass
> > > b/meta/classes/image.bbclass index 9fa58f8..1c7a527 100644
> > > --- a/meta/classes/image.bbclass
> > > +++ b/meta/classes/image.bbclass
> > > @@ -60,7 +60,7 @@ image_do_mounts() {
> > >  }
> > >
> > >  ROOTFSDIR = "${IMAGE_ROOTFS}"
> > > -ROOTFS_FEATURES += "copy-package-cache clean-package-cache
> > > generate-manifest" +ROOTFS_FEATURES += "copy-package-cache
> > > clean-package-cache generate-manifest cache-deb-src"
> > > ROOTFS_PACKAGES += "${IMAGE_PREINSTALL} ${IMAGE_INSTALL}"
> > > ROOTFS_MANIFEST_DEPLOY_DIR ?= "${DEPLOY_DIR_IMAGE}"
> > > diff --git a/meta/classes/rootfs.bbclass
> > > b/meta/classes/rootfs.bbclass index 8bb003d..7bfdfc9 100644
> > > --- a/meta/classes/rootfs.bbclass
> > > +++ b/meta/classes/rootfs.bbclass
> > > @@ -201,6 +201,52 @@ rootfs_generate_manifest () {
> > >          ${ROOTFS_MANIFEST_DEPLOY_DIR}/"${PF}".manifest
> > >  }
> > >
> > > +ROOTFS_POSTPROCESS_COMMAND +=
> > > "${@bb.utils.contains('ROOTFS_FEATURES', 'cache-deb-src',
> > > 'cache_deb_src', '', d)}" +cache_deb_src() {
> > > +    if [ "${ISAR_USE_CACHED_BASE_REPO}" = "1" ]; then
> > > +        return 0
> > > +    fi  
> >
> > Should the source packages not all end up in the cache, so they can
> > and probably should be fetched from there.  
> 
> Sorry. But I am not able to understand this. Can you please explain
> it again?

A first build without the cache will fetch all sources and drop them
into "${DEBSRCDIR}"/"${DISTRO}", just the the apt:// fetcher does.
A second build with an enabled cache will place all those src-pkgs in
base-apt (see populate_base_apt repo_add_srcpackage loop).

So a second run of this function here should be able to fetch all those
srcs-pkgs from base-apt. And it would be a good idea to actually do
that to prove that everything is available offline.

Note that for real offline BB_NO_NETWORK would be required. And that
"guard" should still be able to download from base-apt. Thinking about
it again ... i think you do not need the guard. If all src-pkgs are
available offline this function will never access the internet, if it
still tries the invalid proxy "guard" from isar_export_proxies will
trigger.

I think it boils down to removing the
[ "${ISAR_USE_CACHED_BASE_REPO}" = "1" ] && exit 0
and passing the ci offline/cache test

> >  
> > > +    mkdir -p "${DEBSRCDIR}"/"${DISTRO}"
> > > +
> > > +    sudo -s <<'EOSUDO'
> > > +    cp -L /etc/resolv.conf '${ROOTFSDIR}/etc'
> > > +    mkdir -p '${ROOTFSDIR}/deb-src'
> > > +    mountpoint -q '${ROOTFSDIR}/deb-src' || \
> > > +    mount --bind '${DEBSRCDIR}' '${ROOTFSDIR}/deb-src'
> > > +EOSUDO
> > > +
> > > +    sudo -E chroot ${ROOTFSDIR} /usr/bin/apt-get update

Why is that in here? Doing this in the image is not allowed, only for
isar-apt!

> > > +    find "${DEBDIR}"/"${DISTRO}" -name '*\.deb' | while read
> > > package; do

You are reading this without grabbing the lock. In multiconfig other
images might be filling that directory as you read it. And you might be
calling dpkg-deb on half copied files.

Try deb_dl_dir_import and looping over /var/cache/apt/archives/ ... in
which case you will find yourself dealing with isar-apt packages that
you need to skip.
In fact you should use the manifest as input to not download packages
installed in other images with the same distro but without the feature.

Yeahh multiconfig!

Henning

> > > +        local src="$( dpkg-deb --show --showformat '${Source}'
> > > "${package}" )"
> > > +        # If the binary package version and source package
> > > version are different, then the
> > > +        # source package version will be present inside "()" of
> > > the Source field.
> > > +        local version="$( echo "$src" | cut -sd "(" -f2 | cut -sd
> > > ")" -f1 )"
> > > +        if [ -z ${version} ]; then
> > > +            version="$( dpkg-deb --show --showformat '${Version}'
> > > "${package}" )"
> > > +        fi
> > > +        # Now strip any version information that might be
> > > available.
> > > +        src="$( echo "$src" | cut -d' ' -f1 )"
> > > +        # If there is no source field, then the source package
> > > has the same name as the
> > > +        # binary package.
> > > +        if [ -z "${src}" ];then
> > > +            src="$( dpkg-deb --show --showformat '${Package}'
> > > "${package}" )"
> > > +        fi
> > > +
> > > +        sudo -E chroot --userspec=$( id -u ):$( id -g )
> > > ${ROOTFSDIR} \
> > > +            sh -c 'mkdir -p "/deb-src/${1}/${2}" && cd
> > > "/deb-src/${1}/${2}" && \
> > > +                apt-get -y --download-only --only-source source
> > > "$2"="$3"' \
> > > +                download-src "${DISTRO}" "${src}" "${version}"
> > > +    done  
> >
> > Looks like we are going online without proxy configuration here. It
> > also needs a BB_NO_NETWORK guard.  
> 
> Will take care of that.
> 
> >
> > And i would suggest to generate the list of things you want to
> > fetch, factor out the fetcher from dpkg-base and reuse is instead
> > of copying it.  
> 
> Sure. I will have a look into how I can reuse that part.
> 
> >
> > And i would personally like a new series of patches to be sent
> > without "in-reply-to". Maybe its my client but i find these deeply
> > nested threads very hard to follow.  
> 
> No Problem. Will send the next series separately.
> 
> Thanks,
> Vijai Kumar K
> 
> >
> > Henning
> >  
> > > +    sudo -s <<'EOSUDO'
> > > +    mountpoint -q '${ROOTFSDIR}/deb-src' && \
> > > +    umount -l ${ROOTFSDIR}/deb-src
> > > +    rm -rf '${ROOTFSDIR}/etc/resolv.conf'
> > > +EOSUDO
> > > +}
> > > +
> > >  do_rootfs_postprocess[vardeps] = "${ROOTFS_POSTPROCESS_COMMAND}"
> > >  python do_rootfs_postprocess() {
> > >      # Take care that its correctly mounted:  
> >