buildchroot: do not mk-build-deps in ${S}

Message ID 20200925091632.43333-1-Cedric_Hombourger@mentor.com
State Superseded, archived
Headers show
Series buildchroot: do not mk-build-deps in ${S} | expand

Commit Message

Cedric Hombourger Sept. 25, 2020, 1:16 a.m. UTC
mk-build-deps may leave .changes and/or .buildinfo files behind despite use of
the -r option (which seems to only remove the generated .deb file). Having new
files in the source tree causes dh_clean to complain (at least when the package
uses quilt to manage patches). This problem was observed while re-building some
upstream bullseye packages. Since there are no requirements to execute the
mk-build-deps tool from the source tree, create a temporary directory and run
it from there.

Signed-off-by: Cedric Hombourger <Cedric_Hombourger@mentor.com>
---
 meta/recipes-devtools/buildchroot/files/deps.sh | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Cedric Hombourger Sept. 25, 2020, 1:33 a.m. UTC | #1
just found that Jan already posted something rather similar

https://groups.google.com/g/isar-users/c/FUC6QGcKD8Y/m/dr95lm2pAgAJ

I should have re-checked the mailing list before submitting (I wanted to 
ci-build.sh this change before submitting)

One difference though is the use of a temp directory and no relative 
paths (I did not want to make any assumptions about the file-system view 
we get when we enter the buildchroot)

We definitely want either patches applied so we can get more people to 
start porting their projects to bullseye!

If we go with Jan's, we should definitely address the comment from 
Henning (i.e. changing "cd .." to something more adequate)

Cedric

On 9/25/2020 11:16 AM, Cedric Hombourger wrote:
> mk-build-deps may leave .changes and/or .buildinfo files behind despite use of
> the -r option (which seems to only remove the generated .deb file). Having new
> files in the source tree causes dh_clean to complain (at least when the package
> uses quilt to manage patches). This problem was observed while re-building some
> upstream bullseye packages. Since there are no requirements to execute the
> mk-build-deps tool from the source tree, create a temporary directory and run
> it from there.
>
> Signed-off-by: Cedric Hombourger <Cedric_Hombourger@mentor.com>
> ---
>   meta/recipes-devtools/buildchroot/files/deps.sh | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/meta/recipes-devtools/buildchroot/files/deps.sh b/meta/recipes-devtools/buildchroot/files/deps.sh
> index 93bc9cf..40d4a1c 100644
> --- a/meta/recipes-devtools/buildchroot/files/deps.sh
> +++ b/meta/recipes-devtools/buildchroot/files/deps.sh
> @@ -34,17 +34,22 @@ if ! grep "^Architecture:" debian/control | grep -qv "all"; then
>   fi
>   
>   # Install all build deps
> +srcdir=$(pwd)
> +tmpdir=$(mktemp -d)
> +cd ${tmpdir}
>   if [ "$3" = "--download-only" ]; then
>       # this will not return 0 even when it worked
> -    mk-build-deps $set_arch -t "${install_cmd}" -i -r debian/control &> \
> +    mk-build-deps $set_arch -t "${install_cmd}" -i -r ${srcdir}/debian/control &> \
>           mk-build-deps.output || true
>       cat mk-build-deps.output
>       # we assume success when we find this
>       grep "mk-build-deps: Unable to install all build-dep packages" mk-build-deps.output
>       rm -f mk-build-deps.output
>   else
> -    mk-build-deps $set_arch -t "${install_cmd}" -i -r debian/control
> +    mk-build-deps $set_arch -t "${install_cmd}" -i -r ${srcdir}/debian/control
>   
>       # Upgrade any already installed packages in case we are partially rebuilding
>       apt-get upgrade -y --allow-downgrades
>   fi
> +cd ${srcdir}
> +rm -rf ${tmpdir}
Henning Schild Sept. 25, 2020, 8:27 a.m. UTC | #2
On Fri, 25 Sep 2020 11:33:15 +0200
Cedric Hombourger <cedric_hombourger@mentor.com> wrote:

> just found that Jan already posted something rather similar
> 
> https://groups.google.com/g/isar-users/c/FUC6QGcKD8Y/m/dr95lm2pAgAJ
> 
> I should have re-checked the mailing list before submitting (I wanted
> to ci-build.sh this change before submitting)
> 
> One difference though is the use of a temp directory and no relative 
> paths (I did not want to make any assumptions about the file-system
> view we get when we enter the buildchroot)
> 
> We definitely want either patches applied so we can get more people
> to start porting their projects to bullseye!

You found it yourself. Cool to see you guys working on bullseye. Jan
recently posted bullseye related stuff as well.
What is worrying is that the ci test does not fully cover it anymore.

> If we go with Jan's, we should definitely address the comment from 
> Henning (i.e. changing "cd .." to something more adequate)

I think i am fine with that ".." after all. And your tempdir + rm trick
is neat as well. But if the tool decided to leave logs, we probably
want them.

Henning

> Cedric
> 
> On 9/25/2020 11:16 AM, Cedric Hombourger wrote:
> > mk-build-deps may leave .changes and/or .buildinfo files behind
> > despite use of the -r option (which seems to only remove the
> > generated .deb file). Having new files in the source tree causes
> > dh_clean to complain (at least when the package uses quilt to
> > manage patches). This problem was observed while re-building some
> > upstream bullseye packages. Since there are no requirements to
> > execute the mk-build-deps tool from the source tree, create a
> > temporary directory and run it from there.
> >
> > Signed-off-by: Cedric Hombourger <Cedric_Hombourger@mentor.com>
> > ---
> >   meta/recipes-devtools/buildchroot/files/deps.sh | 9 +++++++--
> >   1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/meta/recipes-devtools/buildchroot/files/deps.sh
> > b/meta/recipes-devtools/buildchroot/files/deps.sh index
> > 93bc9cf..40d4a1c 100644 ---
> > a/meta/recipes-devtools/buildchroot/files/deps.sh +++
> > b/meta/recipes-devtools/buildchroot/files/deps.sh @@ -34,17 +34,22
> > @@ if ! grep "^Architecture:" debian/control | grep -qv "all"; then
> > fi 
> >   # Install all build deps
> > +srcdir=$(pwd)
> > +tmpdir=$(mktemp -d)
> > +cd ${tmpdir}
> >   if [ "$3" = "--download-only" ]; then
> >       # this will not return 0 even when it worked
> > -    mk-build-deps $set_arch -t "${install_cmd}" -i -r
> > debian/control &> \
> > +    mk-build-deps $set_arch -t "${install_cmd}" -i -r
> > ${srcdir}/debian/control &> \ mk-build-deps.output || true
> >       cat mk-build-deps.output
> >       # we assume success when we find this
> >       grep "mk-build-deps: Unable to install all build-dep
> > packages" mk-build-deps.output rm -f mk-build-deps.output
> >   else
> > -    mk-build-deps $set_arch -t "${install_cmd}" -i -r
> > debian/control
> > +    mk-build-deps $set_arch -t "${install_cmd}" -i -r
> > ${srcdir}/debian/control 
> >       # Upgrade any already installed packages in case we are
> > partially rebuilding apt-get upgrade -y --allow-downgrades
> >   fi
> > +cd ${srcdir}
> > +rm -rf ${tmpdir}  
>

Patch

diff --git a/meta/recipes-devtools/buildchroot/files/deps.sh b/meta/recipes-devtools/buildchroot/files/deps.sh
index 93bc9cf..40d4a1c 100644
--- a/meta/recipes-devtools/buildchroot/files/deps.sh
+++ b/meta/recipes-devtools/buildchroot/files/deps.sh
@@ -34,17 +34,22 @@  if ! grep "^Architecture:" debian/control | grep -qv "all"; then
 fi
 
 # Install all build deps
+srcdir=$(pwd)
+tmpdir=$(mktemp -d)
+cd ${tmpdir}
 if [ "$3" = "--download-only" ]; then
     # this will not return 0 even when it worked
-    mk-build-deps $set_arch -t "${install_cmd}" -i -r debian/control &> \
+    mk-build-deps $set_arch -t "${install_cmd}" -i -r ${srcdir}/debian/control &> \
         mk-build-deps.output || true
     cat mk-build-deps.output
     # we assume success when we find this
     grep "mk-build-deps: Unable to install all build-dep packages" mk-build-deps.output
     rm -f mk-build-deps.output
 else
-    mk-build-deps $set_arch -t "${install_cmd}" -i -r debian/control
+    mk-build-deps $set_arch -t "${install_cmd}" -i -r ${srcdir}/debian/control
 
     # Upgrade any already installed packages in case we are partially rebuilding
     apt-get upgrade -y --allow-downgrades
 fi
+cd ${srcdir}
+rm -rf ${tmpdir}