Re: [PATCH v5 05/13] deb-dl-dir: Download files only belonging to the current image

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

[-- Attachment #1.1: Type: text/plain, Size: 3398 bytes --]



On Wednesday, April 22, 2020 at 12:55:19 PM UTC+5:30, Henning Schild wrote:
>
> On Fri, 17 Apr 2020 15:00:32 +0530 
> Vijai Kumar K <vijaikumar...@gmail.com <javascript:>> wrote: 
>
> > Avoid downloading deb-srcs for debs cached from other image builds. 
> > One way to ensure that is to see if the package is present in the dpkg 
> > status file. 
>
> Not sure i understand what it is doing/avoiding. Maybe you can go into 
> more detail with an example. 
>

We are looping over the entries in DEBDIR/deb, It would contain packages 
from
all former builds. To make sure we take only those are used in the image, 
we check
the status file. While handling this I had a thought why not to use the 
status file as the
source directly, but P7 in this series changed that. Not all is available 
in status file, 
only those which are installed. There might be chances that something is 
removed later
on like the localepurge in meta/classes/image-locales-extension.bbclass. 
Those does not
get captured in the status file or the manifest and hence the dpkg log 
check in P7.


> > Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com <javascript:>> 
> > --- 
> >  meta/classes/deb-dl-dir.bbclass | 10 ++++++++++ 
> >  1 file changed, 10 insertions(+) 
> > 
> > diff --git a/meta/classes/deb-dl-dir.bbclass 
> > b/meta/classes/deb-dl-dir.bbclass index 9399741..b3f4842 100644 
> > --- a/meta/classes/deb-dl-dir.bbclass 
> > +++ b/meta/classes/deb-dl-dir.bbclass 
> > @@ -5,6 +5,15 @@ 
> >   
> >  inherit repository 
> > 
> > +check_in_rootfs() { 
>
> I am confused by the logic again. And maybe the name. We already have 
> repo_contains_package which suggests a "boolean" answer, should this 
> one be "rootfs_contains_package"? Or maybe "has_package_installed" 
>

I have to admit, on seeing this back, it is a bit confusing, I will fix 
these naming.
 

>
> > +    local package="$( dpkg-deb --show --showformat '${Package}' 
> > "${1}" )" 
> > +    local output="$( grep -hs "^Package: ${package}" \ 
> > +            "${IMAGE_ROOTFS}"/var/lib/dpkg/status \ 
> > +            "${BUILDCHROOT_HOST_DIR}"/var/lib/dpkg/status \ 
> > +            "${BUILDCHROOT_TARGET_DIR}"/var/lib/dpkg/status )" 
>
> I think you should chroot in and ask "dpkg -i", doing this kind of 
> guessing on the outside is error-prone. 
>
> > +    [ -z "${output}" ] && return 1 || return 0 
>
> Again confused, in shell boolean "0" means true and anything else means 
> false. 
>

Sorry. Confusing indeed. I will use the below. Thanks.
 

>
> return [ -n "${output}" ] 
>
> > +} 
> > + 
> >  debsrc_download() { 
> >      export rootfs="$1" 
> >      export rootfs_distro="$2" 
> > @@ -18,6 +27,7 @@ debsrc_download() { 
> >      mount --bind "${DEBSRCDIR}" "${rootfs}/deb-src" 
> >  EOSUDO 
> >      find "${rootfs}/var/cache/apt/archives/" -maxdepth 1 -type f 
> > -iname '*\.deb' | while read package; do 
> > +        check_in_rootfs "${package}" || continue 
>
> This reads like you skip the download when you found package to _not_ 
> be "in_rootfs". 
>

Sorry that it is confusing. I will fix these to be more clear in the next 
series.
 

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

[-- Attachment #1.2: Type: text/html, Size: 5311 bytes --]