dpkg: use find to create symlinks to downloaded .deb files vs ln -sf wildcard

Message ID 20240226095517.66032-1-cedric.hombourger@siemens.com
State Accepted, archived
Headers show
Series dpkg: use find to create symlinks to downloaded .deb files vs ln -sf wildcard | expand

Commit Message

Cedric Hombourger Feb. 26, 2024, 9:55 a.m. UTC
If no .deb files were downloaded from remote feeds, use of "ln -sf *.deb" will
create a dangling symlink named "*.deb" in the target directory and will be a
broken link and make the cp command used in the finished-build-commands fail.
Use "find <dir> -exec ln -t <target> -sf {} +" to create symlinks instead of
"ln -sf wildcard". For consistency and optimization, the find command used to
copy newly downloaded .deb files from the sbuild env back to the build env is
also changed to use the "{} +" -exec flavor (instead of "{} ;" which spawns
one cp command for each single match). The issue of a dangling symlink named
"*.deb" was found while building against file:// sources and no remote feeds.

Signed-off-by: Cedric Hombourger <cedric.hombourger@siemens.com>
---
 meta/classes/dpkg.bbclass | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Cedric Hombourger Feb. 26, 2024, 10:08 a.m. UTC | #1
On Mon, 2024-02-26 at 10:55 +0100, Cedric Hombourger wrote:
> If no .deb files were downloaded from remote feeds, use of "ln -sf
> *.deb" will
> create a dangling symlink named "*.deb" in the target directory and
> will be a
> broken link and make the cp command used in the finished-build-
> commands fail.
> Use "find <dir> -exec ln -t <target> -sf {} +" to create symlinks
> instead of
> "ln -sf wildcard". For consistency and optimization, the find command
> used to
> copy newly downloaded .deb files from the sbuild env back to the
> build env is
> also changed to use the "{} +" -exec flavor (instead of "{} ;" which
> spawns
> one cp command for each single match). The issue of a dangling
> symlink named
> "*.deb" was found while building against file:// sources and no
> remote feeds.

Please consider this patch for 0.10. Currently doing some integration
tests against Isar's HEAD. This issue with the broken symlink named
"*.deb" is causing offline builds of our product to fail. Our hope is
for our next release to be based on a pristine 0.10 Isar release and we
will therefore strive to provide timely feedback and/or patches to the
various -rcX tags until 0.10

> 
> Signed-off-by: Cedric Hombourger <cedric.hombourger@siemens.com>
> ---
>  meta/classes/dpkg.bbclass | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
> index 0578977d..3fa9f604 100644
> --- a/meta/classes/dpkg.bbclass
> +++ b/meta/classes/dpkg.bbclass
> @@ -120,10 +120,10 @@ dpkg_runbuild() {
>          --chroot-setup-commands="echo \"APT::Get::allow-downgrades
> 1;\" > /etc/apt/apt.conf.d/50isar-apt" \
>          --chroot-setup-commands="rm -f /var/log/dpkg.log" \
>          --chroot-setup-commands="mkdir -p ${deb_dir}" \
> -        --chroot-setup-commands="ln -sf ${ext_deb_dir}/*.deb -t
> ${deb_dir}/" \
> +        --chroot-setup-commands="find ${ext_deb_dir} -maxdepth 1 -
> name '*.deb' -exec ln -t ${deb_dir}/ -sf {} +" \
>          --chroot-setup-commands="apt-get update -o
> Dir::Etc::SourceList=\"sources.list.d/isar-apt.list\" -o
> Dir::Etc::SourceParts=\"-\" -o APT::Get::List-Cleanup=\"0\"" \
>          --finished-build-commands="rm -f ${deb_dir}/sbuild-build-
> depends-main-dummy_*.deb" \
> -        --finished-build-commands="find ${deb_dir} -maxdepth 1 -type
> f -name '*.deb' -print -exec cp ${CP_FLAGS} -t ${ext_deb_dir}/ {} \;"
> \
> +        --finished-build-commands="find ${deb_dir} -maxdepth 1 -type
> f -name '*.deb' -print -exec cp ${CP_FLAGS} -t ${ext_deb_dir}/ {} +"
> \
>          --finished-build-commands="cp /var/log/dpkg.log
> ${ext_root}/dpkg_partial.log" \
>          --debbuildopts="--source-option=-I" \
>          --build-dir=${WORKDIR} --dist="isar" ${DSC_FILE}
Uladzimir Bely Feb. 27, 2024, 9:10 a.m. UTC | #2
On Mon, 2024-02-26 at 10:08 +0000, 'cedric.hombourger@siemens.com' via
isar-users wrote:
> On Mon, 2024-02-26 at 10:55 +0100, Cedric Hombourger wrote:
> > If no .deb files were downloaded from remote feeds, use of "ln -sf
> > *.deb" will
> > create a dangling symlink named "*.deb" in the target directory and
> > will be a
> > broken link and make the cp command used in the finished-build-
> > commands fail.
> > Use "find <dir> -exec ln -t <target> -sf {} +" to create symlinks
> > instead of
> > "ln -sf wildcard". For consistency and optimization, the find
> > command
> > used to
> > copy newly downloaded .deb files from the sbuild env back to the
> > build env is
> > also changed to use the "{} +" -exec flavor (instead of "{} ;"
> > which
> > spawns
> > one cp command for each single match). The issue of a dangling
> > symlink named
> > "*.deb" was found while building against file:// sources and no
> > remote feeds.
> 
> Please consider this patch for 0.10. Currently doing some integration
> tests against Isar's HEAD. This issue with the broken symlink named
> "*.deb" is causing offline builds of our product to fail. Our hope is
> for our next release to be based on a pristine 0.10 Isar release and
> we
> will therefore strive to provide timely feedback and/or patches to
> the
> various -rcX tags until 0.10
> 

Hello.
The patch itself is worth applying and we'll pass it through CI right
now.

Just a question - how exactly did you get empty download dir when the
package is built? Was sbuild-chroot taken from sstate cache, while the
package was not previously built or the recipe was changed.

This information may be useful for the future testcase of this
situation.

> > 
> > Signed-off-by: Cedric Hombourger <cedric.hombourger@siemens.com>
> > ---
> >  meta/classes/dpkg.bbclass | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
> > index 0578977d..3fa9f604 100644
> > --- a/meta/classes/dpkg.bbclass
> > +++ b/meta/classes/dpkg.bbclass
> > @@ -120,10 +120,10 @@ dpkg_runbuild() {
> >          --chroot-setup-commands="echo \"APT::Get::allow-downgrades
> > 1;\" > /etc/apt/apt.conf.d/50isar-apt" \
> >          --chroot-setup-commands="rm -f /var/log/dpkg.log" \
> >          --chroot-setup-commands="mkdir -p ${deb_dir}" \
> > -        --chroot-setup-commands="ln -sf ${ext_deb_dir}/*.deb -t
> > ${deb_dir}/" \
> > +        --chroot-setup-commands="find ${ext_deb_dir} -maxdepth 1 -
> > name '*.deb' -exec ln -t ${deb_dir}/ -sf {} +" \
> >          --chroot-setup-commands="apt-get update -o
> > Dir::Etc::SourceList=\"sources.list.d/isar-apt.list\" -o
> > Dir::Etc::SourceParts=\"-\" -o APT::Get::List-Cleanup=\"0\"" \
> >          --finished-build-commands="rm -f ${deb_dir}/sbuild-build-
> > depends-main-dummy_*.deb" \
> > -        --finished-build-commands="find ${deb_dir} -maxdepth 1 -
> > type
> > f -name '*.deb' -print -exec cp ${CP_FLAGS} -t ${ext_deb_dir}/ {}
> > \;"
> > \
> > +        --finished-build-commands="find ${deb_dir} -maxdepth 1 -
> > type
> > f -name '*.deb' -print -exec cp ${CP_FLAGS} -t ${ext_deb_dir}/ {}
> > +"
> > \
> >          --finished-build-commands="cp /var/log/dpkg.log
> > ${ext_root}/dpkg_partial.log" \
> >          --debbuildopts="--source-option=-I" \
> >          --build-dir=${WORKDIR} --dist="isar" ${DSC_FILE}
>
Cedric Hombourger Feb. 27, 2024, 9:46 a.m. UTC | #3
On Tue, 2024-02-27 at 12:10 +0300, Uladzimir Bely wrote:
> On Mon, 2024-02-26 at 10:08 +0000, 'cedric.hombourger@siemens.com'
> via
> isar-users wrote:
> > On Mon, 2024-02-26 at 10:55 +0100, Cedric Hombourger wrote:
> > > If no .deb files were downloaded from remote feeds, use of "ln -
> > > sf
> > > *.deb" will
> > > create a dangling symlink named "*.deb" in the target directory
> > > and
> > > will be a
> > > broken link and make the cp command used in the finished-build-
> > > commands fail.
> > > Use "find <dir> -exec ln -t <target> -sf {} +" to create symlinks
> > > instead of
> > > "ln -sf wildcard". For consistency and optimization, the find
> > > command
> > > used to
> > > copy newly downloaded .deb files from the sbuild env back to the
> > > build env is
> > > also changed to use the "{} +" -exec flavor (instead of "{} ;"
> > > which
> > > spawns
> > > one cp command for each single match). The issue of a dangling
> > > symlink named
> > > "*.deb" was found while building against file:// sources and no
> > > remote feeds.
> > 
> > Please consider this patch for 0.10. Currently doing some
> > integration
> > tests against Isar's HEAD. This issue with the broken symlink named
> > "*.deb" is causing offline builds of our product to fail. Our hope
> > is
> > for our next release to be based on a pristine 0.10 Isar release
> > and
> > we
> > will therefore strive to provide timely feedback and/or patches to
> > the
> > various -rcX tags until 0.10
> > 
> 
> Hello.

Hello! Thanks for looking into this patch.

> The patch itself is worth applying and we'll pass it through CI right
> now.

Sure thing. FWIW, I used ci_build -T fast for a quick non-regression
tests and no failures were reported. Let me know if you see any issues
with your full CI test.

> 
> Just a question - how exactly did you get empty download dir when the
> package is built? Was sbuild-chroot taken from sstate cache, while
> the
> package was not previously built or the recipe was changed.

I came across this issue while doing a clean build against our base-apt
feeds. The generated base-apt.list file reads (there are more feeds
than just base in our case but all follow the same pattern and I am
only showing the base feed for brevity)

deb file:///base-apt/base bookworm build debug extra main

Here we only have (as expected) only file:// feed(s) => apt will not
download anything in /var/cache/apt/archives/

> 
> This information may be useful for the future testcase of this
> situation.
> 
> > > 
> > > Signed-off-by: Cedric Hombourger <cedric.hombourger@siemens.com>
> > > ---
> > >  meta/classes/dpkg.bbclass | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/meta/classes/dpkg.bbclass
> > > b/meta/classes/dpkg.bbclass
> > > index 0578977d..3fa9f604 100644
> > > --- a/meta/classes/dpkg.bbclass
> > > +++ b/meta/classes/dpkg.bbclass
> > > @@ -120,10 +120,10 @@ dpkg_runbuild() {
> > >          --chroot-setup-commands="echo \"APT::Get::allow-
> > > downgrades
> > > 1;\" > /etc/apt/apt.conf.d/50isar-apt" \
> > >          --chroot-setup-commands="rm -f /var/log/dpkg.log" \
> > >          --chroot-setup-commands="mkdir -p ${deb_dir}" \
> > > -        --chroot-setup-commands="ln -sf ${ext_deb_dir}/*.deb -t
> > > ${deb_dir}/" \
> > > +        --chroot-setup-commands="find ${ext_deb_dir} -maxdepth 1
> > > -
> > > name '*.deb' -exec ln -t ${deb_dir}/ -sf {} +" \
> > >          --chroot-setup-commands="apt-get update -o
> > > Dir::Etc::SourceList=\"sources.list.d/isar-apt.list\" -o
> > > Dir::Etc::SourceParts=\"-\" -o APT::Get::List-Cleanup=\"0\"" \
> > >          --finished-build-commands="rm -f ${deb_dir}/sbuild-
> > > build-
> > > depends-main-dummy_*.deb" \
> > > -        --finished-build-commands="find ${deb_dir} -maxdepth 1 -
> > > type
> > > f -name '*.deb' -print -exec cp ${CP_FLAGS} -t ${ext_deb_dir}/ {}
> > > \;"
> > > \
> > > +        --finished-build-commands="find ${deb_dir} -maxdepth 1 -
> > > type
> > > f -name '*.deb' -print -exec cp ${CP_FLAGS} -t ${ext_deb_dir}/ {}
> > > +"
> > > \
> > >          --finished-build-commands="cp /var/log/dpkg.log
> > > ${ext_root}/dpkg_partial.log" \
> > >          --debbuildopts="--source-option=-I" \
> > >          --build-dir=${WORKDIR} --dist="isar" ${DSC_FILE}
> > 
>
Uladzimir Bely Feb. 27, 2024, 11:40 a.m. UTC | #4
On Tue, 2024-02-27 at 09:46 +0000, cedric.hombourger@siemens.com wrote:
> On Tue, 2024-02-27 at 12:10 +0300, Uladzimir Bely wrote:
> > On Mon, 2024-02-26 at 10:08 +0000, 'cedric.hombourger@siemens.com'
> > via
> > isar-users wrote:
> > > On Mon, 2024-02-26 at 10:55 +0100, Cedric Hombourger wrote:
> > > > If no .deb files were downloaded from remote feeds, use of "ln
> > > > -
> > > > sf
> > > > *.deb" will
> > > > create a dangling symlink named "*.deb" in the target directory
> > > > and
> > > > will be a
> > > > broken link and make the cp command used in the finished-build-
> > > > commands fail.
> > > > Use "find <dir> -exec ln -t <target> -sf {} +" to create
> > > > symlinks
> > > > instead of
> > > > "ln -sf wildcard". For consistency and optimization, the find
> > > > command
> > > > used to
> > > > copy newly downloaded .deb files from the sbuild env back to
> > > > the
> > > > build env is
> > > > also changed to use the "{} +" -exec flavor (instead of "{} ;"
> > > > which
> > > > spawns
> > > > one cp command for each single match). The issue of a dangling
> > > > symlink named
> > > > "*.deb" was found while building against file:// sources and no
> > > > remote feeds.
> > > 
> > > Please consider this patch for 0.10. Currently doing some
> > > integration
> > > tests against Isar's HEAD. This issue with the broken symlink
> > > named
> > > "*.deb" is causing offline builds of our product to fail. Our
> > > hope
> > > is
> > > for our next release to be based on a pristine 0.10 Isar release
> > > and
> > > we
> > > will therefore strive to provide timely feedback and/or patches
> > > to
> > > the
> > > various -rcX tags until 0.10
> > > 
> > 
> > Hello.
> 
> Hello! Thanks for looking into this patch.
> 
> > The patch itself is worth applying and we'll pass it through CI
> > right
> > now.
> 
> Sure thing. FWIW, I used ci_build -T fast for a quick non-regression
> tests and no failures were reported. Let me know if you see any
> issues
> with your full CI test.
> 
> > 
> > Just a question - how exactly did you get empty download dir when
> > the
> > package is built? Was sbuild-chroot taken from sstate cache, while
> > the
> > package was not previously built or the recipe was changed.
> 
> I came across this issue while doing a clean build against our base-
> apt
> feeds. The generated base-apt.list file reads (there are more feeds
> than just base in our case but all follow the same pattern and I am
> only showing the base feed for brevity)
> 
> deb file:///base-apt/base bookworm build debug extra main
> 
> Here we only have (as expected) only file:// feed(s) => apt will not
> download anything in /var/cache/apt/archives/
> 

Yes, I see. When URL's are local (e.g., file://), apt doesn't download
anything from them and just uses debs directly.

> > 
> > This information may be useful for the future testcase of this
> > situation.
> > 
> > > > 
> > > > Signed-off-by: Cedric Hombourger
> > > > <cedric.hombourger@siemens.com>
> > > > ---
> > > >  meta/classes/dpkg.bbclass | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/meta/classes/dpkg.bbclass
> > > > b/meta/classes/dpkg.bbclass
> > > > index 0578977d..3fa9f604 100644
> > > > --- a/meta/classes/dpkg.bbclass
> > > > +++ b/meta/classes/dpkg.bbclass
> > > > @@ -120,10 +120,10 @@ dpkg_runbuild() {
> > > >          --chroot-setup-commands="echo \"APT::Get::allow-
> > > > downgrades
> > > > 1;\" > /etc/apt/apt.conf.d/50isar-apt" \
> > > >          --chroot-setup-commands="rm -f /var/log/dpkg.log" \
> > > >          --chroot-setup-commands="mkdir -p ${deb_dir}" \
> > > > -        --chroot-setup-commands="ln -sf ${ext_deb_dir}/*.deb -
> > > > t
> > > > ${deb_dir}/" \
> > > > +        --chroot-setup-commands="find ${ext_deb_dir} -maxdepth
> > > > 1
> > > > -
> > > > name '*.deb' -exec ln -t ${deb_dir}/ -sf {} +" \
> > > >          --chroot-setup-commands="apt-get update -o
> > > > Dir::Etc::SourceList=\"sources.list.d/isar-apt.list\" -o
> > > > Dir::Etc::SourceParts=\"-\" -o APT::Get::List-Cleanup=\"0\"" \
> > > >          --finished-build-commands="rm -f ${deb_dir}/sbuild-
> > > > build-
> > > > depends-main-dummy_*.deb" \
> > > > -        --finished-build-commands="find ${deb_dir} -maxdepth 1
> > > > -
> > > > type
> > > > f -name '*.deb' -print -exec cp ${CP_FLAGS} -t ${ext_deb_dir}/
> > > > {}
> > > > \;"
> > > > \
> > > > +        --finished-build-commands="find ${deb_dir} -maxdepth 1
> > > > -
> > > > type
> > > > f -name '*.deb' -print -exec cp ${CP_FLAGS} -t ${ext_deb_dir}/
> > > > {}
> > > > +"
> > > > \
> > > >          --finished-build-commands="cp /var/log/dpkg.log
> > > > ${ext_root}/dpkg_partial.log" \
> > > >          --debbuildopts="--source-option=-I" \
> > > >          --build-dir=${WORKDIR} --dist="isar" ${DSC_FILE}
> > > 
> > 
>
Baurzhan Ismagulov Feb. 28, 2024, 9:09 a.m. UTC | #5
On 2024-02-27 09:46, 'cedric.hombourger@siemens.com' via isar-users wrote:
> > The patch itself is worth applying and we'll pass it through CI right
> > now.
> 
> Sure thing. FWIW, I used ci_build -T fast for a quick non-regression
> tests and no failures were reported.

Thanks, this is a big help.


> Let me know if you see any issues with your full CI test.

Full CI has passed. Uladzimir has also reviewed the patch; we will merge it
before 0.10.


> I came across this issue while doing a clean build against our base-apt
> feeds. The generated base-apt.list file reads (there are more feeds
> than just base in our case but all follow the same pattern and I am
> only showing the base feed for brevity)
> 
> deb file:///base-apt/base bookworm build debug extra main
> 
> Here we only have (as expected) only file:// feed(s) => apt will not
> download anything in /var/cache/apt/archives/

Independently of merging this one, do you see a possibility to prepare a
testcase for that?


With kind regards,
Baurzhan
Uladzimir Bely Feb. 29, 2024, 1:17 p.m. UTC | #6
On Mon, 2024-02-26 at 10:55 +0100, 'Cedric Hombourger' via isar-users
wrote:
> If no .deb files were downloaded from remote feeds, use of "ln -sf
> *.deb" will
> create a dangling symlink named "*.deb" in the target directory and
> will be a
> broken link and make the cp command used in the finished-build-
> commands fail.
> Use "find <dir> -exec ln -t <target> -sf {} +" to create symlinks
> instead of
> "ln -sf wildcard". For consistency and optimization, the find command
> used to
> copy newly downloaded .deb files from the sbuild env back to the
> build env is
> also changed to use the "{} +" -exec flavor (instead of "{} ;" which
> spawns
> one cp command for each single match). The issue of a dangling
> symlink named
> "*.deb" was found while building against file:// sources and no
> remote feeds.
> 
> Signed-off-by: Cedric Hombourger <cedric.hombourger@siemens.com>
> ---
>  meta/classes/dpkg.bbclass | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Applied to next, thanks.

Patch

diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
index 0578977d..3fa9f604 100644
--- a/meta/classes/dpkg.bbclass
+++ b/meta/classes/dpkg.bbclass
@@ -120,10 +120,10 @@  dpkg_runbuild() {
         --chroot-setup-commands="echo \"APT::Get::allow-downgrades 1;\" > /etc/apt/apt.conf.d/50isar-apt" \
         --chroot-setup-commands="rm -f /var/log/dpkg.log" \
         --chroot-setup-commands="mkdir -p ${deb_dir}" \
-        --chroot-setup-commands="ln -sf ${ext_deb_dir}/*.deb -t ${deb_dir}/" \
+        --chroot-setup-commands="find ${ext_deb_dir} -maxdepth 1 -name '*.deb' -exec ln -t ${deb_dir}/ -sf {} +" \
         --chroot-setup-commands="apt-get update -o Dir::Etc::SourceList=\"sources.list.d/isar-apt.list\" -o Dir::Etc::SourceParts=\"-\" -o APT::Get::List-Cleanup=\"0\"" \
         --finished-build-commands="rm -f ${deb_dir}/sbuild-build-depends-main-dummy_*.deb" \
-        --finished-build-commands="find ${deb_dir} -maxdepth 1 -type f -name '*.deb' -print -exec cp ${CP_FLAGS} -t ${ext_deb_dir}/ {} \;" \
+        --finished-build-commands="find ${deb_dir} -maxdepth 1 -type f -name '*.deb' -print -exec cp ${CP_FLAGS} -t ${ext_deb_dir}/ {} +" \
         --finished-build-commands="cp /var/log/dpkg.log ${ext_root}/dpkg_partial.log" \
         --debbuildopts="--source-option=-I" \
         --build-dir=${WORKDIR} --dist="isar" ${DSC_FILE}