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

[vijai kumar <vijaikumar.kanagarajan@gmail.com>]

On Wed, Apr 8, 2020 at 6:00 PM Henning Schild
<henning.schild@siemens.com> wrote:
>
> 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!

Hi Henning,

I am sorry. But why is it not allowed? Am I missing any side effects of this
call?

Thanks,
Vijai Kumar K

>
> > > > +    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:
> > >
>