Re: [PATCH v4] deb-dl-dir class rework to use faster ln -P or fallback to cp

[Uladzimir Bely <ubely@ilbers.de>]

In the email from Friday, 10 February 2023 15:48:03 +03 user Henning Schild 
wrote:
> Am Fri, 10 Feb 2023 12:09:08 +0100
> 
> schrieb roberto.foglietta@linuxteam.org:
> > From: "Roberto A. Foglietta" <roberto.foglietta@gmail.com>
> > 
> > deb-dl-dir, feature: faster when using ln -P otherwise fallback to cp
> > 
> > The original class functions deb_dl_dir_import/export were using cp to
> > copy debian package to the target rootfs but this approach is quite
> > slow while using hard link does not work if the destination and
> > source dirs are not lying on the same filesystem. Thus, ln -P should
> > fallback to cp when it does not work (which is different from
> > complaining on stderr).
> > 
> > Moreover, these two functions have been reworked to reach a straight
> > forward and more compact form. In particular, export function was
> > using bashism to do some kind of comparison which after all is useless
> > because copying back without overwriting just fulfills that part.
> > 
> > More rework using sudo in a different way plus a corner case
> > addressingi, in case the spia file exists for some other reasons.
> > 
> > Rebased on the current next and bugfix about checking the destination
> > not the source file.
> > 
> > Signed-off-by: Roberto A. Foglietta <roberto.foglietta@gmail.com>
> > ---
> > 
> >  meta/classes/deb-dl-dir.bbclass | 53
> > 
> > +++++++++++++++++++-------------- 1 file changed, 30 insertions(+),
> > 23 deletions(-)
> > 
> > diff --git a/meta/classes/deb-dl-dir.bbclass
> > b/meta/classes/deb-dl-dir.bbclass index 7db25251..2a3c7508 100644
> > --- a/meta/classes/deb-dl-dir.bbclass
> > +++ b/meta/classes/deb-dl-dir.bbclass
> > @@ -78,39 +78,46 @@ debsrc_download() {
> > 
> >  deb_dl_dir_import() {
> >  
> >      export pc="${DEBDIR}/${2}"
> > 
> > -    export rootfs="${1}"
> > -    sudo mkdir -p "${rootfs}"/var/cache/apt/archives/
> > +    export sc="${1}/var/cache/apt/archives/"
> > +    sudo mkdir -p "${sc}"
> > 
> >      [ ! -d "${pc}" ] && return 0
> > 
> > -    flock -s "${pc}".lock -c '
> > +    export tf=$(cd "${pc}"; ls -1 *.deb | head -n1)
> > +    [ ! -e "${pc}/${tf}" ] && return 0
> > +    flock -Fs "${pc}".lock sudo -Es << 'EOFSUDO'
> 
> Just curious ... what would EOFSUDO stand for? We have EOSUDO in
> many (all?) other places and every author can bring in their own style.
> 
> But EndOfSudo is clear to me ... EndOfFileSudo seems weird.
> 
> But maybe whoever used EOSUDO first meant something totally different
> with the acronym. And it does not mean EndOfSudo ...
> 
> Henning
> 

I'm also playing with this part of code and found one more funny thing (at 
least, I didn't know about it before).

I'm saying about taking EOSUDO into quotes. The behaviour is different with 
and without quotes:

For example, this code will print "/" contents:

sudo -Es << "EOSUDO"
ls -1 / |
  while read p; do
    echo $p
  done
​EOSUDO

But this one won't print it:

sudo -Es << EOSUDO
ls -1 / |
  while read p; do
    echo $p
  done
​EOSUDO

Just few empty lines printed. We appear to have to backslash \$p... But it 
works well with exported (before) variables...

> >          set -e
> >          printenv | grep -q BB_VERBOSE_LOGS && set -x
> > 
> > -        sudo find "${pc}" -type f -iname "*\.deb" -exec \
> > -            ln -Pf -t "${rootfs}"/var/cache/apt/archives/ {} +
> > -    '
> > +        rm -f "${sc}/${tf}"
> > +        ln -Pf -t "${sc}" "${pc}/${tf}" 2>/dev/null ||:
> > +        if [ -r "${sc}/${tf}" ]; then
> > +            find "${pc}" -type f -iname "*\.deb" -exec \
> > +                ln -Pf -t "${sc}" {} +
> > +        else
> > +            find "${pc}" -type f -iname "*\.deb" -exec \
> > +                cp -np owner --reflink=auto -t "${sc}" {} +
> > +        fi
> > +EOFSUDO
> > 
> >  }
> >  
> >  deb_dl_dir_export() {
> >  
> >      export pc="${DEBDIR}/${2}"
> > 
> > -    export rootfs="${1}"
> > +    export sc="${1}/var/cache/apt/archives/"
> > 
> >      mkdir -p "${pc}"
> > 
> > -    flock "${pc}".lock -c '
> > +    export tf=$(cd "${sc}"; ls -1 *.deb | head -n1)
> > +    [ ! -e "${sc}/${tf}" ] && return 0
> > +    flock -F "${pc}".lock sudo -Es << 'EOFSUDO'
> > 
> >          set -e
> >          printenv | grep -q BB_VERBOSE_LOGS && set -x
> > 
> > -        find "${rootfs}"/var/cache/apt/archives/ \
> > -            -maxdepth 1 -type f -iname '*\.deb' |\
> > -        while read p; do
> > -            # skip files from a previous export
> > -            [ -f "${pc}/${p##*/}" ] && continue
> > -            # can not reuse bitbake function here, this is basically
> > -            # "repo_contains_package"
> > -            package=$(find "${REPO_ISAR_DIR}"/"${DISTRO}" -name
> > ${p##*/})
> > -            if [ -n "$package" ]; then
> > -                cmp --silent "$package" "$p" && continue
> > -            fi
> > -            sudo ln -Pf "${p}" "${pc}"
> > -        done
> > -        sudo chown -R $(id -u):$(id -g) "${pc}"
> > -    '
> > +        rm -f "${pc}/${tf}"
> > +        ln -Pf -t "${pc}" "${sc}/${tf}" 2>/dev/null ||:
> > +        if [ -r "${pc}/${tf}" ]; then
> > +            find "${sc}" -maxdepth 1 -type f -iname '*\.deb' \
> > +                -exec ln -P -t "${pc}" {} + 2>/dev/null ||:
> > +        else
> > +            find "${sc}" -maxdepth 1 -type f -iname '*\.deb' \
> > +                -exec cp -n --reflink=auto -t "${pc}" {} +
> > +        fi
> > +        chown -R $(id -u):$(id -g) "${pc}"

2 Roberto: This also won't work as expected. Since sudo moved to the upper 
level, owner is 0:0 now instead of actual user:group, as it was before.

I'm now testing v3 of my version of the fix. It works on per-file basis and 
seems to have passed all my local thorough tests... at last.

> > +EOFSUDO
> > 
> >  }