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

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(-)

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

>          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}"
> +EOFSUDO
>  }

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

On Fri, 10 Feb 2023 at 14:27, Uladzimir Bely <ubely@ilbers.de> wrote:
>
> > > +        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.

Tested code is better. I am working on something else, in fact.
However, the version that I am using has not chown at all.
After all, the APT cache is made for being used by root.

Best regards, R-