Re: [PATCH] deb-dl-dir: Use dpkg-deb --field in debsrc_download

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

Am Thu, 10 Feb 2022 19:55:04 +0300
schrieb Uladzimir Bely <ubely@ilbers.de>:

> In the email from Thursday, 10 February 2022 15:25:15 +03 user
> Henning Schild wrote:
> > Am Thu, 27 Jan 2022 11:45:44 +0100
> > 
> > schrieb Uladzimir Bely <ubely@ilbers.de>:  
> > > In Debian >= bullseye 'dpkg-deb --show --showformat' is currently
> > > broken in case of requesting virtual fields like
> > > 'source:<FieldName>'. This makes function 'debsrc_download'
> > > broken, so build fails when ISAR_USE_CACHED_BASE_REPO is enabled
> > > in local.conf
> > > 
> > > The regression came with the fix for bug #972580 in Debian.
> > > The issue is reported to Debian bugtracker as bug #1004372.
> > > 
> > > Signed-off-by: Uladzimir Bely <ubely@ilbers.de>
> > > ---
> > > 
> > >  meta/classes/deb-dl-dir.bbclass | 10 ++++++++--
> > >  1 file changed, 8 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/meta/classes/deb-dl-dir.bbclass
> > > b/meta/classes/deb-dl-dir.bbclass index ffbff077..ded9b3be 100644
> > > --- a/meta/classes/deb-dl-dir.bbclass
> > > +++ b/meta/classes/deb-dl-dir.bbclass
> > > @@ -51,8 +51,14 @@ debsrc_download() {
> > > 
> > >      printenv | grep -q BB_VERBOSE_LOGS && set -x
> > >      find "${rootfs}/var/cache/apt/archives/" -maxdepth 1 -type f
> > > 
> > > -iname '*\.deb' | while read package; do
> > > is_not_part_of_current_build "${package}" && continue
> > > -        local src="$( dpkg-deb --show --showformat
> > > '${source:Package}' "${package}" )"
> > > -        local version="$( dpkg-deb --show --showformat
> > > '${source:Version}' "${package}" )"
> > > +        local src="$( dpkg-deb --field "${package}" Source | awk
> > > '{printf $1}' )"
> > > +        if [ -z "$src" ]; then
> > > +            src="$( dpkg-deb --field "${package}" Package )"
> > > +        fi
> > > +        local version="$( dpkg-deb --field "${package}" Source |
> > > awk '{gsub(/[()]/,""); printf $2}')"
> > > +        if [ -z "$version" ]; then
> > > +            version="$( dpkg-deb --field "${package}" Version )"
> > > +        fi  
> > 
> > I personally find that hard to read and would appreciate some
> > comments on it.
> >   
> Initially, we assume that the "source package" name and version are
> the same as "package". It's usually true.
> 
> But sometimes "source package" name may differ. In this case,
> "Source" field should be specified and we use the value from it.
> Also, "source package" version may be also specified using
> parentheses (if it differ from "package" version), in this case we
> also take it.
> 
> Some example, when names and version of source package are different:
> 
> | dpkg-deb --field librtmp1_2.4+20151223.gitfa8646d.1-2+b2_amd64.deb
> | grep "Source:\|Version\|Package"
> | Package: librtmp1
> | Source: rtmpdump (2.4+20151223.gitfa8646d.1-2)
> | Version: 2.4+20151223.gitfa8646d.1-2+b2

I read and understood that code yesterday. So i know. I was talking
about comments in the code, making it readable and saying these things.
Maybe parts in the commit message in case you want to go really deep.
But not here.

> > In addition i would keep the original code and only take the
> > alternative approach when src == "". Adding a comment and a TODO
> > revert pointing to the bug.
> >   
> I agree that this should be a temorary workaround. Unfortunately,
> there is no activity at
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1004372 for two
> weeks...

Maybe we take it, but keeping the old code and marking the new with
"TODO revert". In that case we maybe ... hopefully do not need good
comments for the future ... because the code will for sure not be there
for long. Or anyone reading it in 4 months will see the TODO and
propose the revert.

Thanks for taking care of that upstream!

Henning

> > Looking at that bug it seems to be a matter of not much time, so the
> > workaround in isar might not be needed if one had some patience or
> > used kas-docker 2.6.
> > 
> > In fact i would prefer to keep that patch out alltogether and wait
> > for upstream fixes.
> > Maybe skip the tests doing ISAR_USE_CACHED_BASE_REPO when on
> > bullseye, or use kas.  
> 
> Oh, I seem to copypasted wrong variable to the commit message.
> 
> Actually, it is not ISAR_USE_CACHED_BASE_REPO, but BASE_REPO_FEATURES
> ?= "cache-deb-src". I will fix commit message in the v2 patchset.
>
> > 
> > I will send a patch to pin down kas in isar CI.
> > Jan please note that kas 3.0 will have to wait for #1004372
> > 
> > regards,
> > Henning
> >   
> > >          local dscfile=$(find "${DEBSRCDIR}"/"${rootfs_distro}"
> > > -name
> > > 
> > > "${src}_${version}.dsc") [ -n "$dscfile" ] && continue  
> 
>