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

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

On Wed, 15 Apr 2020 17:59:12 +0530
vijai kumar <vijaikumar.kanagarajan@gmail.com> wrote:

> 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 for asking, please keep doing that when things are unclear.

An "update" stores a copy of the "view on the repo world" in the image.
It is essentially a copy of the Packages.gz or Sources.gz of all repos.
That information changes over time on the servers, while they still
(hopefully) offer to download packages referenced in older version of
those indexes.

Isar relies on that. It fetches all indexes exactly once and later
downloads packages found in the cached versions. Once you update an
index the "view of the world" moves away from "the state of the image".

On a living debian system you would always upgrade packages after
update-ing the indexes. In an "installer" - like Isar - you probably do
not want those dynamics.

So in order to keep "the state of the image" and "the view of the
world" in sync we never "apt-get update" ... except for isar-apt which
is a repo we can/do control.

If a build takes a really long time, there is a slim chance that we can
not actually fetch packages found in our old indexes because upstream
does not provide them anymore. I have not seen real evidence of that
potential problem. It could however manifest if we have a long running
build ... arm without cross ... and do additional fetches in postinst
... like you are implementing.
But whatever you can not fetch in the end, is probably not worth
fetching because it is not what was used to construct your image.

Henning

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