[PATCH] dpkg-base: mount git source folder into buildchroot

[PATCH] dpkg-base: mount git source folder into buildchroot

[Christian Storm]

When building a Debian source package with source/format "3.0 (git)",
the git binary is used to bundle the source. For being able to do so,
<build>/tmp/downloads/git/<git_repo> needs to be mounted into the
buildchroot as .git/objects/info/alternates refers to it.

Signed-off-by: Christian Storm <christian.storm@siemens.com>
---
 meta/classes/dpkg-base.bbclass | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index 35af6d5..a98e690 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -20,14 +20,18 @@ dpkg_runbuild() {
 # Wrap the function dpkg_runbuild with the bind mount for buildroot
 do_build() {
     mkdir -p ${BUILDROOT}
-    sudo mount --bind ${WORKDIR} ${BUILDROOT}
+    [ ! -d ${BUILDCHROOT_DIR}/${GITDIR} ] && sudo install -d -m 755 ${BUILDCHROOT_DIR}/${GITDIR}
     _do_build_cleanup() {
         ret=$?
         sudo umount ${BUILDROOT} 2>/dev/null || true
         sudo rmdir ${BUILDROOT} 2>/dev/null || true
+        sudo umount ${BUILDCHROOT_DIR}/${GITDIR} 2>/dev/null || true
+        sudo rmdir ${BUILDCHROOT_DIR}/${GITDIR} 2>/dev/null || true
         (exit $ret) || bb_exit_handler
     }
     trap '_do_build_cleanup' EXIT
+    sudo mount --bind ${WORKDIR} ${BUILDROOT}
+    sudo mount --bind ${GITDIR} ${BUILDCHROOT_DIR}/${GITDIR}
     dpkg_runbuild
     _do_build_cleanup
 }
-- 
2.15.0

Re: [PATCH] dpkg-base: mount git source folder into buildchroot

[Alexander Smirnov]

Hi,

On 11/14/2017 07:48 PM, Christian Storm wrote:
> When building a Debian source package with source/format "3.0 (git)",
> the git binary is used to bundle the source. For being able to do so,
> <build>/tmp/downloads/git/<git_repo> needs to be mounted into the
> buildchroot as .git/objects/info/alternates refers to it.

Could you please provide more details regarding where it should be mounted?

> 
> Signed-off-by: Christian Storm <christian.storm@siemens.com>
> ---
>   meta/classes/dpkg-base.bbclass | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
> index 35af6d5..a98e690 100644
> --- a/meta/classes/dpkg-base.bbclass
> +++ b/meta/classes/dpkg-base.bbclass
> @@ -20,14 +20,18 @@ dpkg_runbuild() {
>   # Wrap the function dpkg_runbuild with the bind mount for buildroot
>   do_build() {
>       mkdir -p ${BUILDROOT}
> -    sudo mount --bind ${WORKDIR} ${BUILDROOT}
> +    [ ! -d ${BUILDCHROOT_DIR}/${GITDIR} ] && sudo install -d -m 755 ${BUILDCHROOT_DIR}/${GITDIR}

${GITDIR} - will be expanded to absolute path of your build tree on the 
host. Why this private path is needed in buildchroot? This doesn't look 
correct for me if you need it for build.

Also git repository is already available in buildchroot:

1. If package is fetched via git repository - it's stored in ${GITDIR}.
2. Then task "do_unpack" clones this to ${WORKDIR}/git folder
3. During build task, the folder ${WORKDIR} is mounted to ${BUILDROOT}
4. So your git repo is in: /home/builder/${PN}/git folder inside chroot.

Probably some sample recipe would be helpful to describe what's going.

Alex

>       _do_build_cleanup() {
>           ret=$?
>           sudo umount ${BUILDROOT} 2>/dev/null || true
>           sudo rmdir ${BUILDROOT} 2>/dev/null || true
> +        sudo umount ${BUILDCHROOT_DIR}/${GITDIR} 2>/dev/null || true
> +        sudo rmdir ${BUILDCHROOT_DIR}/${GITDIR} 2>/dev/null || true
>           (exit $ret) || bb_exit_handler
>       }
>       trap '_do_build_cleanup' EXIT
> +    sudo mount --bind ${WORKDIR} ${BUILDROOT}
> +    sudo mount --bind ${GITDIR} ${BUILDCHROOT_DIR}/${GITDIR}
>       dpkg_runbuild
>       _do_build_cleanup
>   }
>

Re: [PATCH] dpkg-base: mount git source folder into buildchroot

[Christian Storm]

> > When building a Debian source package with source/format "3.0 (git)",
> > the git binary is used to bundle the source. For being able to do so,
> > <build>/tmp/downloads/git/<git_repo> needs to be mounted into the
> > buildchroot as .git/objects/info/alternates refers to it.
> 
> Could you please provide more details regarding where it should be mounted?

Please see below for an explanation.


> > Signed-off-by: Christian Storm <christian.storm@siemens.com>
> > ---
> >   meta/classes/dpkg-base.bbclass | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
> > index 35af6d5..a98e690 100644
> > --- a/meta/classes/dpkg-base.bbclass
> > +++ b/meta/classes/dpkg-base.bbclass
> > @@ -20,14 +20,18 @@ dpkg_runbuild() {
> >   # Wrap the function dpkg_runbuild with the bind mount for buildroot
> >   do_build() {
> >       mkdir -p ${BUILDROOT}
> > -    sudo mount --bind ${WORKDIR} ${BUILDROOT}
> > +    [ ! -d ${BUILDCHROOT_DIR}/${GITDIR} ] && sudo install -d -m 755 ${BUILDCHROOT_DIR}/${GITDIR}
> 
> ${GITDIR} - will be expanded to absolute path of your build tree on the 
> host. Why this private path is needed in buildchroot? This doesn't look 
> correct for me if you need it for build.
> 
> Also git repository is already available in buildchroot:
> 
> 1. If package is fetched via git repository - it's stored in ${GITDIR}.
> 2. Then task "do_unpack" clones this to ${WORKDIR}/git folder
> 3. During build task, the folder ${WORKDIR} is mounted to ${BUILDROOT}
> 4. So your git repo is in: /home/builder/${PN}/git folder inside chroot.

Well, no. It contains the checkout of the source done by the fetcher.
You'll get a castrated .git/ in the buildchroot, i.e., try to run git
operations in the buildchroot when do_build'ing a particular package and
git complains:
  error: object directory /build/tmp/downloads/git/xxxxxx/objects does not exist; check .git/objects/info/alternates.
  fatal: bad object HEAD

It tries to resolve the repository "symlink" in
.git/objects/info/alternates which points to the directory the fetcher
has put it in, i.e., <build>/tmp/downloads/git/xxxxxx. This directory,
respectively, <build>/tmp/downloads/git/, however, isn't mounted in the
buildchroot and so git can't do operations.
Hence, source/format "3.0 (git)" fails as it wants to do git operations.

Does this explanation help?



Kind regards,
   Christian

-- 
Dr. Christian Storm
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Otto-Hahn-Ring 6, 81739 M�nchen, Germany

Re: [PATCH] dpkg-base: mount git source folder into buildchroot

[Henning Schild]

Am Tue, 14 Nov 2017 17:48:44 +0100
schrieb "[ext] Christian Storm" <christian.storm@siemens.com>:

> When building a Debian source package with source/format "3.0 (git)",
> the git binary is used to bundle the source. For being able to do so,
> <build>/tmp/downloads/git/<git_repo> needs to be mounted into the
> buildchroot as .git/objects/info/alternates refers to it.
> 
> Signed-off-by: Christian Storm <christian.storm@siemens.com>
> ---
>  meta/classes/dpkg-base.bbclass | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes/dpkg-base.bbclass
> b/meta/classes/dpkg-base.bbclass index 35af6d5..a98e690 100644
> --- a/meta/classes/dpkg-base.bbclass
> +++ b/meta/classes/dpkg-base.bbclass
> @@ -20,14 +20,18 @@ dpkg_runbuild() {
>  # Wrap the function dpkg_runbuild with the bind mount for buildroot
>  do_build() {
>      mkdir -p ${BUILDROOT}
> -    sudo mount --bind ${WORKDIR} ${BUILDROOT}
> +    [ ! -d ${BUILDCHROOT_DIR}/${GITDIR} ] && sudo install -d -m 755
> ${BUILDCHROOT_DIR}/${GITDIR} 

This "mkdir" should probably be done in buildchroot:do_build, maybe
with [dirs]. The current implementation has a race and the mkdir could
also be done without condition, since install would not fail anyways.

> _do_build_cleanup() {
>          ret=$?
>          sudo umount ${BUILDROOT} 2>/dev/null || true
>          sudo rmdir ${BUILDROOT} 2>/dev/null || true
> +        sudo umount ${BUILDCHROOT_DIR}/${GITDIR} 2>/dev/null || true
> +        sudo rmdir ${BUILDCHROOT_DIR}/${GITDIR} 2>/dev/null || true

The rmdir is not really needed, maybe [dirs] will handle it.

Henning

>          (exit $ret) || bb_exit_handler
>      }
>      trap '_do_build_cleanup' EXIT
> +    sudo mount --bind ${WORKDIR} ${BUILDROOT}
> +    sudo mount --bind ${GITDIR} ${BUILDCHROOT_DIR}/${GITDIR}
>      dpkg_runbuild
>      _do_build_cleanup
>  }

[PATCH] dpkg-base: mount git source folder into buildchroot

[Christian Storm]

When building a Debian source package with source/format "3.0 (git)",
the git binary is used to bundle the source. For being able to do so,
<build>/tmp/downloads/git/<git_repo> needs to be mounted into the
buildchroot as .git/objects/info/alternates refers to it.

Signed-off-by: Christian Storm <christian.storm@siemens.com>
---
 meta/classes/dpkg-base.bbclass                   | 5 ++++-
 meta/recipes-devtools/buildchroot/buildchroot.bb | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index 35af6d5..3235ebf 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -20,14 +20,17 @@ dpkg_runbuild() {
 # Wrap the function dpkg_runbuild with the bind mount for buildroot
 do_build() {
     mkdir -p ${BUILDROOT}
-    sudo mount --bind ${WORKDIR} ${BUILDROOT}
     _do_build_cleanup() {
         ret=$?
         sudo umount ${BUILDROOT} 2>/dev/null || true
         sudo rmdir ${BUILDROOT} 2>/dev/null || true
+        sudo umount ${BUILDCHROOT_DIR}/${GITDIR} 2>/dev/null || true
+        sudo rmdir ${BUILDCHROOT_DIR}/${GITDIR} 2>/dev/null || true
         (exit $ret) || bb_exit_handler
     }
     trap '_do_build_cleanup' EXIT
+    sudo mount --bind ${WORKDIR} ${BUILDROOT}
+    sudo mount --bind ${GITDIR} ${BUILDCHROOT_DIR}/${GITDIR}
     dpkg_runbuild
     _do_build_cleanup
 }
diff --git a/meta/recipes-devtools/buildchroot/buildchroot.bb b/meta/recipes-devtools/buildchroot/buildchroot.bb
index 6a94733..8652cf1 100644
--- a/meta/recipes-devtools/buildchroot/buildchroot.bb
+++ b/meta/recipes-devtools/buildchroot/buildchroot.bb
@@ -32,6 +32,7 @@ WORKDIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}"
 do_build[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
 do_build[dirs] = "${WORKDIR}/hooks_multistrap"
 
+do_build[dirs] = "${BUILDCHROOT_DIR}/${GITDIR}"
 do_build() {
     chmod +x "${WORKDIR}/setup.sh"
     chmod +x "${WORKDIR}/configscript.sh"
-- 
2.15.0

[PATCH] Map git objects to buildchroot

[Alexander Smirnov]

Hi Christian,

The patch above has 2 major issues:

1. It mounts ${GITDIR} to buildchroot during package building. This variable
is static, so it's the same for each recipe. This means that if several
packages are building in parallel, the folder ${BUILDCHROOT_DIR}/${GITDIR}
could be mounted/unmounted while there is active I/O operation by another
package. To be honest, I don't know what could happen, but believe in worst
case.

2. Using host configuration in buildchroot could lead to some problems.
Lets consider the following example:
 - User override ${DL_DIR} to point it to local cache, let's say: /home/builder/XXX
 - Isar uses  the following path to build package YYY: /home/builder/YYY
 - If XXX equals YYY, you will probably experience some problems, because your source
   tree will be overriden by incorect mount.

So I'd see the more generic solution like patch below:

8<--

dpkg-base: Map git objects to buildchroot

This patch mounts git repository to buildchroot into the respective folder:

 /git/$REPO_NAME

Also it updates git alternative to correctly point to objects in buildchroot.

Signed-off-by: Alexander Smirnov <asmirnov@ilbers.de>
---
 meta/classes/dpkg-base.bbclass | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index 35af6d5..147cfce 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -19,12 +19,25 @@ dpkg_runbuild() {
 
 # Wrap the function dpkg_runbuild with the bind mount for buildroot
 do_build() {
+    if [ -d ${WORKDIR}/git/.git ]; then
+        OBJ_PATH=$(cat ${WORKDIR}/git/.git/objects/info/alternates)
+        REPO_PATH=$(dirname $OBJ_PATH)
+        REPO_NAME=$(basename $REPO_PATH)
+        sudo mkdir -p ${BUILDCHROOT_DIR}/git/$REPO_NAME
+        sudo mount --bind $REPO_PATH ${BUILDCHROOT_DIR}/git/$REPO_NAME
+        echo "/git/$REPO_NAME/objects" > ${WORKDIR}/git/.git/objects/info/alternates
+    fi
+
     mkdir -p ${BUILDROOT}
     sudo mount --bind ${WORKDIR} ${BUILDROOT}
     _do_build_cleanup() {
         ret=$?
         sudo umount ${BUILDROOT} 2>/dev/null || true
         sudo rmdir ${BUILDROOT} 2>/dev/null || true
+        if [ -d ${WORKDIR}/git/.git ]; then
+            sudo umount ${BUILDCHROOT_DIR}/git/$REPO_NAME
+            sudo rm -rf ${BUILDCHROOT_DIR}/git/$REPO_NAME
+        fi
         (exit $ret) || bb_exit_handler
     }
     trap '_do_build_cleanup' EXIT
-- 
2.9.5

Re: [PATCH] Map git objects to buildchroot

[Henning Schild]

Am Mon, 20 Nov 2017 18:06:41 +0300
schrieb Alexander Smirnov <asmirnov@ilbers.de>:

> Hi Christian,
> 
> The patch above has 2 major issues:
> 
> 1. It mounts ${GITDIR} to buildchroot during package building. This
> variable is static, so it's the same for each recipe. This means that
> if several packages are building in parallel, the folder
> ${BUILDCHROOT_DIR}/${GITDIR} could be mounted/unmounted while there
> is active I/O operation by another package. To be honest, I don't
> know what could happen, but believe in worst case.

True, that must not happen!

> 2. Using host configuration in buildchroot could lead to some
> problems. Lets consider the following example:
>  - User override ${DL_DIR} to point it to local cache, let's
> say: /home/builder/XXX
>  - Isar uses  the following path to build package
> YYY: /home/builder/YYY
>  - If XXX equals YYY, you will probably experience some problems,
> because your source tree will be overriden by incorect mount.
> 
> So I'd see the more generic solution like patch below:
> 
> 8<--
> 
> dpkg-base: Map git objects to buildchroot
> 
> This patch mounts git repository to buildchroot into the respective
> folder:
> 
>  /git/$REPO_NAME
> 
> Also it updates git alternative to correctly point to objects in
> buildchroot.
> 
> Signed-off-by: Alexander Smirnov <asmirnov@ilbers.de>
> ---
>  meta/classes/dpkg-base.bbclass | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/meta/classes/dpkg-base.bbclass
> b/meta/classes/dpkg-base.bbclass index 35af6d5..147cfce 100644
> --- a/meta/classes/dpkg-base.bbclass
> +++ b/meta/classes/dpkg-base.bbclass
> @@ -19,12 +19,25 @@ dpkg_runbuild() {
>  
>  # Wrap the function dpkg_runbuild with the bind mount for buildroot
>  do_build() {
> +    if [ -d ${WORKDIR}/git/.git ]; then
> +        OBJ_PATH=$(cat ${WORKDIR}/git/.git/objects/info/alternates)
> +        REPO_PATH=$(dirname $OBJ_PATH)
> +        REPO_NAME=$(basename $REPO_PATH)
> +        sudo mkdir -p ${BUILDCHROOT_DIR}/git/$REPO_NAME
> +        sudo mount --bind $REPO_PATH
> ${BUILDCHROOT_DIR}/git/$REPO_NAME
> +        echo "/git/$REPO_NAME/objects" >
> ${WORKDIR}/git/.git/objects/info/alternates
> +    fi
> +
>      mkdir -p ${BUILDROOT}
>      sudo mount --bind ${WORKDIR} ${BUILDROOT}
>      _do_build_cleanup() {
>          ret=$?
>          sudo umount ${BUILDROOT} 2>/dev/null || true
>          sudo rmdir ${BUILDROOT} 2>/dev/null || true
> +        if [ -d ${WORKDIR}/git/.git ]; then
> +            sudo umount ${BUILDCHROOT_DIR}/git/$REPO_NAME
> +            sudo rm -rf ${BUILDCHROOT_DIR}/git/$REPO_NAME
> +        fi
>          (exit $ret) || bb_exit_handler
>      }
>      trap '_do_build_cleanup' EXIT

I would not do this either. There should be one mount in buildchroot.bb
and one umount. Additional umounts could be allowed in exception
handlers, in case they can not call out to functions from buildchroot.


Henning

Re: [PATCH] Map git objects to buildchroot

[Alexander Smirnov]

On 11/20/2017 06:32 PM, Henning Schild wrote:
> Am Mon, 20 Nov 2017 18:06:41 +0300
> schrieb Alexander Smirnov <asmirnov@ilbers.de>:
> 
>> Hi Christian,
>>
>> The patch above has 2 major issues:
>>
>> 1. It mounts ${GITDIR} to buildchroot during package building. This
>> variable is static, so it's the same for each recipe. This means that
>> if several packages are building in parallel, the folder
>> ${BUILDCHROOT_DIR}/${GITDIR} could be mounted/unmounted while there
>> is active I/O operation by another package. To be honest, I don't
>> know what could happen, but believe in worst case.
> 
> True, that must not happen!
> 
>> 2. Using host configuration in buildchroot could lead to some
>> problems. Lets consider the following example:
>>   - User override ${DL_DIR} to point it to local cache, let's
>> say: /home/builder/XXX
>>   - Isar uses  the following path to build package
>> YYY: /home/builder/YYY
>>   - If XXX equals YYY, you will probably experience some problems,
>> because your source tree will be overriden by incorect mount.
>>
>> So I'd see the more generic solution like patch below:
>>
>> 8<--
>>
>> dpkg-base: Map git objects to buildchroot
>>
>> This patch mounts git repository to buildchroot into the respective
>> folder:
>>
>>   /git/$REPO_NAME
>>
>> Also it updates git alternative to correctly point to objects in
>> buildchroot.
>>
>> Signed-off-by: Alexander Smirnov <asmirnov@ilbers.de>
>> ---
>>   meta/classes/dpkg-base.bbclass | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/meta/classes/dpkg-base.bbclass
>> b/meta/classes/dpkg-base.bbclass index 35af6d5..147cfce 100644
>> --- a/meta/classes/dpkg-base.bbclass
>> +++ b/meta/classes/dpkg-base.bbclass
>> @@ -19,12 +19,25 @@ dpkg_runbuild() {
>>   
>>   # Wrap the function dpkg_runbuild with the bind mount for buildroot
>>   do_build() {
>> +    if [ -d ${WORKDIR}/git/.git ]; then
>> +        OBJ_PATH=$(cat ${WORKDIR}/git/.git/objects/info/alternates)
>> +        REPO_PATH=$(dirname $OBJ_PATH)
>> +        REPO_NAME=$(basename $REPO_PATH)
>> +        sudo mkdir -p ${BUILDCHROOT_DIR}/git/$REPO_NAME
>> +        sudo mount --bind $REPO_PATH
>> ${BUILDCHROOT_DIR}/git/$REPO_NAME
>> +        echo "/git/$REPO_NAME/objects" >
>> ${WORKDIR}/git/.git/objects/info/alternates
>> +    fi
>> +
>>       mkdir -p ${BUILDROOT}
>>       sudo mount --bind ${WORKDIR} ${BUILDROOT}
>>       _do_build_cleanup() {
>>           ret=$?
>>           sudo umount ${BUILDROOT} 2>/dev/null || true
>>           sudo rmdir ${BUILDROOT} 2>/dev/null || true
>> +        if [ -d ${WORKDIR}/git/.git ]; then
>> +            sudo umount ${BUILDCHROOT_DIR}/git/$REPO_NAME
>> +            sudo rm -rf ${BUILDCHROOT_DIR}/git/$REPO_NAME
>> +        fi
>>           (exit $ret) || bb_exit_handler
>>       }
>>       trap '_do_build_cleanup' EXIT
> 
> I would not do this either. There should be one mount in buildchroot.bb
> and one umount. Additional umounts could be allowed in exception
> handlers, in case they can not call out to functions from buildchroot.

Sorry, didn't get your idea regarding one mount buildchroot.bb, what do 
you mean?

BTW: we always have option to clone git repository to workspace by 
running in ${WORKDIR}/git:

$ git repack -a

in do_build() task. With this, no extra mount/umount logic is needed.

We could consider this behavior for source 3.0 packages only by using 
some flag in recipe.

Alex

Re: [PATCH] Map git objects to buildchroot

[Christian Storm]

> [...]
> BTW: we always have option to clone git repository to workspace by 
> running in ${WORKDIR}/git:
> 
> $ git repack -a
> 
> in do_build() task. With this, no extra mount/umount logic is needed.

Yes, right, providing a (proper) git checkout in the working directory
solves the problem, as a mount --bind does. For a proper checkout,
you'll have to have the (entire) history/tags available, i.e., not a
shallow clone. For large repositories, this may take some time which a
mount --bind does not consume. On the pro side, using a (proper)
checkout, there's no way to mess with the repository from within the
buildroot, i.e., building from the same source twice - for whatever
reason - uses the exact same revision as base: Changes made to the
repository in one don't affect the other.
That said, I'm fine with both solutions.

So, would you go ahead and post a patch, Alex?


> We could consider this behavior for source 3.0 packages only by using 
> some flag in recipe.

Well, no. This way you're entangling Debian packaging internals to the
recipe. I would opt against this. Most probably you'll want to do git
operations even if you don't use source 3.0 (git) packages, e.g.,
getting the latest git tag or whatever. So, I'd opt for doing it for
every source git package, unconditionally.



Kind regards,
   Christian

-- 
Dr. Christian Storm
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Otto-Hahn-Ring 6, 81739 M�nchen, Germany

Re: [PATCH] Map git objects to buildchroot

[Henning Schild]

Am Mon, 20 Nov 2017 18:54:23 +0300
schrieb Alexander Smirnov <asmirnov@ilbers.de>:

> On 11/20/2017 06:32 PM, Henning Schild wrote:
> > Am Mon, 20 Nov 2017 18:06:41 +0300
> > schrieb Alexander Smirnov <asmirnov@ilbers.de>:
> >   
> >> Hi Christian,
> >>
> >> The patch above has 2 major issues:
> >>
> >> 1. It mounts ${GITDIR} to buildchroot during package building. This
> >> variable is static, so it's the same for each recipe. This means
> >> that if several packages are building in parallel, the folder
> >> ${BUILDCHROOT_DIR}/${GITDIR} could be mounted/unmounted while there
> >> is active I/O operation by another package. To be honest, I don't
> >> know what could happen, but believe in worst case.  
> > 
> > True, that must not happen!
> >   
> >> 2. Using host configuration in buildchroot could lead to some
> >> problems. Lets consider the following example:
> >>   - User override ${DL_DIR} to point it to local cache, let's
> >> say: /home/builder/XXX
> >>   - Isar uses  the following path to build package
> >> YYY: /home/builder/YYY
> >>   - If XXX equals YYY, you will probably experience some problems,
> >> because your source tree will be overriden by incorect mount.
> >>
> >> So I'd see the more generic solution like patch below:
> >>
> >> 8<--
> >>
> >> dpkg-base: Map git objects to buildchroot
> >>
> >> This patch mounts git repository to buildchroot into the respective
> >> folder:
> >>
> >>   /git/$REPO_NAME
> >>
> >> Also it updates git alternative to correctly point to objects in
> >> buildchroot.
> >>
> >> Signed-off-by: Alexander Smirnov <asmirnov@ilbers.de>
> >> ---
> >>   meta/classes/dpkg-base.bbclass | 13 +++++++++++++
> >>   1 file changed, 13 insertions(+)
> >>
> >> diff --git a/meta/classes/dpkg-base.bbclass
> >> b/meta/classes/dpkg-base.bbclass index 35af6d5..147cfce 100644
> >> --- a/meta/classes/dpkg-base.bbclass
> >> +++ b/meta/classes/dpkg-base.bbclass
> >> @@ -19,12 +19,25 @@ dpkg_runbuild() {
> >>   
> >>   # Wrap the function dpkg_runbuild with the bind mount for
> >> buildroot do_build() {
> >> +    if [ -d ${WORKDIR}/git/.git ]; then
> >> +        OBJ_PATH=$(cat
> >> ${WORKDIR}/git/.git/objects/info/alternates)
> >> +        REPO_PATH=$(dirname $OBJ_PATH)
> >> +        REPO_NAME=$(basename $REPO_PATH)
> >> +        sudo mkdir -p ${BUILDCHROOT_DIR}/git/$REPO_NAME
> >> +        sudo mount --bind $REPO_PATH
> >> ${BUILDCHROOT_DIR}/git/$REPO_NAME
> >> +        echo "/git/$REPO_NAME/objects" >
> >> ${WORKDIR}/git/.git/objects/info/alternates
> >> +    fi
> >> +
> >>       mkdir -p ${BUILDROOT}
> >>       sudo mount --bind ${WORKDIR} ${BUILDROOT}
> >>       _do_build_cleanup() {
> >>           ret=$?
> >>           sudo umount ${BUILDROOT} 2>/dev/null || true
> >>           sudo rmdir ${BUILDROOT} 2>/dev/null || true
> >> +        if [ -d ${WORKDIR}/git/.git ]; then
> >> +            sudo umount ${BUILDCHROOT_DIR}/git/$REPO_NAME
> >> +            sudo rm -rf ${BUILDCHROOT_DIR}/git/$REPO_NAME
> >> +        fi
> >>           (exit $ret) || bb_exit_handler
> >>       }
> >>       trap '_do_build_cleanup' EXIT  
> > 
> > I would not do this either. There should be one mount in
> > buildchroot.bb and one umount. Additional umounts could be allowed
> > in exception handlers, in case they can not call out to functions
> > from buildchroot.  
> 
> Sorry, didn't get your idea regarding one mount buildchroot.bb, what
> do you mean?

There should be one mount done from buildchroot.bb before all the
do_builds of package recipes start. And that should get umounted after
the last do_build is done.

> BTW: we always have option to clone git repository to workspace by 
> running in ${WORKDIR}/git:
> 
> $ git repack -a
> 
> in do_build() task. With this, no extra mount/umount logic is needed.

That sounds promising. But i would only do that if git is actually the
only download-tool that works with such "symlinks". If it is not the
whole dl-dir should get mounted, not just the git-subdir.

> We could consider this behavior for source 3.0 packages only by using 
> some flag in recipe.

I think that is something we can just do in any case. A flag would just
complicate things.

Henning

> Alex

Re: [PATCH] Map git objects to buildchroot

[Alexander Smirnov]

Hi,

On 11/21/2017 11:20 AM, Henning Schild wrote:
> Am Mon, 20 Nov 2017 18:54:23 +0300
> schrieb Alexander Smirnov <asmirnov@ilbers.de>:
> 
>> On 11/20/2017 06:32 PM, Henning Schild wrote:
>>> Am Mon, 20 Nov 2017 18:06:41 +0300
>>> schrieb Alexander Smirnov <asmirnov@ilbers.de>:
>>>    
>>>> Hi Christian,
>>>>
>>>> The patch above has 2 major issues:
>>>>
>>>> 1. It mounts ${GITDIR} to buildchroot during package building. This
>>>> variable is static, so it's the same for each recipe. This means
>>>> that if several packages are building in parallel, the folder
>>>> ${BUILDCHROOT_DIR}/${GITDIR} could be mounted/unmounted while there
>>>> is active I/O operation by another package. To be honest, I don't
>>>> know what could happen, but believe in worst case.
>>>
>>> True, that must not happen!
>>>    
>>>> 2. Using host configuration in buildchroot could lead to some
>>>> problems. Lets consider the following example:
>>>>    - User override ${DL_DIR} to point it to local cache, let's
>>>> say: /home/builder/XXX
>>>>    - Isar uses  the following path to build package
>>>> YYY: /home/builder/YYY
>>>>    - If XXX equals YYY, you will probably experience some problems,
>>>> because your source tree will be overriden by incorect mount.
>>>>
>>>> So I'd see the more generic solution like patch below:
>>>>
>>>> 8<--
>>>>
>>>> dpkg-base: Map git objects to buildchroot
>>>>
>>>> This patch mounts git repository to buildchroot into the respective
>>>> folder:
>>>>
>>>>    /git/$REPO_NAME
>>>>
>>>> Also it updates git alternative to correctly point to objects in
>>>> buildchroot.
>>>>
>>>> Signed-off-by: Alexander Smirnov <asmirnov@ilbers.de>
>>>> ---
>>>>    meta/classes/dpkg-base.bbclass | 13 +++++++++++++
>>>>    1 file changed, 13 insertions(+)
>>>>
>>>> diff --git a/meta/classes/dpkg-base.bbclass
>>>> b/meta/classes/dpkg-base.bbclass index 35af6d5..147cfce 100644
>>>> --- a/meta/classes/dpkg-base.bbclass
>>>> +++ b/meta/classes/dpkg-base.bbclass
>>>> @@ -19,12 +19,25 @@ dpkg_runbuild() {
>>>>    
>>>>    # Wrap the function dpkg_runbuild with the bind mount for
>>>> buildroot do_build() {
>>>> +    if [ -d ${WORKDIR}/git/.git ]; then
>>>> +        OBJ_PATH=$(cat
>>>> ${WORKDIR}/git/.git/objects/info/alternates)
>>>> +        REPO_PATH=$(dirname $OBJ_PATH)
>>>> +        REPO_NAME=$(basename $REPO_PATH)
>>>> +        sudo mkdir -p ${BUILDCHROOT_DIR}/git/$REPO_NAME
>>>> +        sudo mount --bind $REPO_PATH
>>>> ${BUILDCHROOT_DIR}/git/$REPO_NAME
>>>> +        echo "/git/$REPO_NAME/objects" >
>>>> ${WORKDIR}/git/.git/objects/info/alternates
>>>> +    fi
>>>> +
>>>>        mkdir -p ${BUILDROOT}
>>>>        sudo mount --bind ${WORKDIR} ${BUILDROOT}
>>>>        _do_build_cleanup() {
>>>>            ret=$?
>>>>            sudo umount ${BUILDROOT} 2>/dev/null || true
>>>>            sudo rmdir ${BUILDROOT} 2>/dev/null || true
>>>> +        if [ -d ${WORKDIR}/git/.git ]; then
>>>> +            sudo umount ${BUILDCHROOT_DIR}/git/$REPO_NAME
>>>> +            sudo rm -rf ${BUILDCHROOT_DIR}/git/$REPO_NAME
>>>> +        fi
>>>>            (exit $ret) || bb_exit_handler
>>>>        }
>>>>        trap '_do_build_cleanup' EXIT
>>>
>>> I would not do this either. There should be one mount in
>>> buildchroot.bb and one umount. Additional umounts could be allowed
>>> in exception handlers, in case they can not call out to functions
>>> from buildchroot.
>>
>> Sorry, didn't get your idea regarding one mount buildchroot.bb, what
>> do you mean?
> 
> There should be one mount done from buildchroot.bb before all the
> do_builds of package recipes start. And that should get umounted after
> the last do_build is done.

Are you speaking about git folder mount only? Or you are speaking about 
all the possible mounts related to building process?

Alex

> 
>> BTW: we always have option to clone git repository to workspace by
>> running in ${WORKDIR}/git:
>>
>> $ git repack -a
>>
>> in do_build() task. With this, no extra mount/umount logic is needed.
> 
> That sounds promising. But i would only do that if git is actually the
> only download-tool that works with such "symlinks". If it is not the
> whole dl-dir should get mounted, not just the git-subdir.
> 
>> We could consider this behavior for source 3.0 packages only by using
>> some flag in recipe.
> 
> I think that is something we can just do in any case. A flag would just
> complicate things.
> 
> Henning
> 

Re: [PATCH] Map git objects to buildchroot

[Henning Schild]

Am Tue, 21 Nov 2017 11:57:52 +0300
schrieb Alexander Smirnov <asmirnov@ilbers.de>:

> Hi,
> 
> On 11/21/2017 11:20 AM, Henning Schild wrote:
> > Am Mon, 20 Nov 2017 18:54:23 +0300
> > schrieb Alexander Smirnov <asmirnov@ilbers.de>:
> >   
> >> On 11/20/2017 06:32 PM, Henning Schild wrote:  
> >>> Am Mon, 20 Nov 2017 18:06:41 +0300
> >>> schrieb Alexander Smirnov <asmirnov@ilbers.de>:
> >>>      
> >>>> Hi Christian,
> >>>>
> >>>> The patch above has 2 major issues:
> >>>>
> >>>> 1. It mounts ${GITDIR} to buildchroot during package building.
> >>>> This variable is static, so it's the same for each recipe. This
> >>>> means that if several packages are building in parallel, the
> >>>> folder ${BUILDCHROOT_DIR}/${GITDIR} could be mounted/unmounted
> >>>> while there is active I/O operation by another package. To be
> >>>> honest, I don't know what could happen, but believe in worst
> >>>> case.  
> >>>
> >>> True, that must not happen!
> >>>      
> >>>> 2. Using host configuration in buildchroot could lead to some
> >>>> problems. Lets consider the following example:
> >>>>    - User override ${DL_DIR} to point it to local cache, let's
> >>>> say: /home/builder/XXX
> >>>>    - Isar uses  the following path to build package
> >>>> YYY: /home/builder/YYY
> >>>>    - If XXX equals YYY, you will probably experience some
> >>>> problems, because your source tree will be overriden by incorect
> >>>> mount.
> >>>>
> >>>> So I'd see the more generic solution like patch below:
> >>>>
> >>>> 8<--
> >>>>
> >>>> dpkg-base: Map git objects to buildchroot
> >>>>
> >>>> This patch mounts git repository to buildchroot into the
> >>>> respective folder:
> >>>>
> >>>>    /git/$REPO_NAME
> >>>>
> >>>> Also it updates git alternative to correctly point to objects in
> >>>> buildchroot.
> >>>>
> >>>> Signed-off-by: Alexander Smirnov <asmirnov@ilbers.de>
> >>>> ---
> >>>>    meta/classes/dpkg-base.bbclass | 13 +++++++++++++
> >>>>    1 file changed, 13 insertions(+)
> >>>>
> >>>> diff --git a/meta/classes/dpkg-base.bbclass
> >>>> b/meta/classes/dpkg-base.bbclass index 35af6d5..147cfce 100644
> >>>> --- a/meta/classes/dpkg-base.bbclass
> >>>> +++ b/meta/classes/dpkg-base.bbclass
> >>>> @@ -19,12 +19,25 @@ dpkg_runbuild() {
> >>>>    
> >>>>    # Wrap the function dpkg_runbuild with the bind mount for
> >>>> buildroot do_build() {
> >>>> +    if [ -d ${WORKDIR}/git/.git ]; then
> >>>> +        OBJ_PATH=$(cat
> >>>> ${WORKDIR}/git/.git/objects/info/alternates)
> >>>> +        REPO_PATH=$(dirname $OBJ_PATH)
> >>>> +        REPO_NAME=$(basename $REPO_PATH)
> >>>> +        sudo mkdir -p ${BUILDCHROOT_DIR}/git/$REPO_NAME
> >>>> +        sudo mount --bind $REPO_PATH
> >>>> ${BUILDCHROOT_DIR}/git/$REPO_NAME
> >>>> +        echo "/git/$REPO_NAME/objects" >
> >>>> ${WORKDIR}/git/.git/objects/info/alternates
> >>>> +    fi
> >>>> +
> >>>>        mkdir -p ${BUILDROOT}
> >>>>        sudo mount --bind ${WORKDIR} ${BUILDROOT}
> >>>>        _do_build_cleanup() {
> >>>>            ret=$?
> >>>>            sudo umount ${BUILDROOT} 2>/dev/null || true
> >>>>            sudo rmdir ${BUILDROOT} 2>/dev/null || true
> >>>> +        if [ -d ${WORKDIR}/git/.git ]; then
> >>>> +            sudo umount ${BUILDCHROOT_DIR}/git/$REPO_NAME
> >>>> +            sudo rm -rf ${BUILDCHROOT_DIR}/git/$REPO_NAME
> >>>> +        fi
> >>>>            (exit $ret) || bb_exit_handler
> >>>>        }
> >>>>        trap '_do_build_cleanup' EXIT  
> >>>
> >>> I would not do this either. There should be one mount in
> >>> buildchroot.bb and one umount. Additional umounts could be allowed
> >>> in exception handlers, in case they can not call out to functions
> >>> from buildchroot.  
> >>
> >> Sorry, didn't get your idea regarding one mount buildchroot.bb,
> >> what do you mean?  
> > 
> > There should be one mount done from buildchroot.bb before all the
> > do_builds of package recipes start. And that should get umounted
> > after the last do_build is done.  
> 
> Are you speaking about git folder mount only? Or you are speaking
> about all the possible mounts related to building process?

For this case i am speaking about the git-folder or better the dldir,
because that is something that is common to all packages that use
buildchroot.
Mounts that are packet specific have to stay in the individual recipes.

Henning

> Alex
> 
> >   
> >> BTW: we always have option to clone git repository to workspace by
> >> running in ${WORKDIR}/git:
> >>
> >> $ git repack -a
> >>
> >> in do_build() task. With this, no extra mount/umount logic is
> >> needed.  
> > 
> > That sounds promising. But i would only do that if git is actually
> > the only download-tool that works with such "symlinks". If it is
> > not the whole dl-dir should get mounted, not just the git-subdir.
> >   
> >> We could consider this behavior for source 3.0 packages only by
> >> using some flag in recipe.  
> > 
> > I think that is something we can just do in any case. A flag would
> > just complicate things.
> > 
> > Henning
> >   

Re: [PATCH] Map git objects to buildchroot

[Alexander Smirnov]

Hi again,

On 11/21/2017 11:14 AM, Christian Storm wrote:
>> [...]
>> BTW: we always have option to clone git repository to workspace by
>> running in ${WORKDIR}/git:
>>
>> $ git repack -a
>>
>> in do_build() task. With this, no extra mount/umount logic is needed.
> 
> Yes, right, providing a (proper) git checkout in the working directory
> solves the problem, as a mount --bind does. For a proper checkout,
> you'll have to have the (entire) history/tags available, i.e., not a
> shallow clone. For large repositories, this may take some time which a
> mount --bind does not consume. On the pro side, using a (proper)
> checkout, there's no way to mess with the repository from within the
> buildroot, i.e., building from the same source twice - for whatever
> reason - uses the exact same revision as base: Changes made to the
> repository in one don't affect the other.
> That said, I'm fine with both solutions.
> 
> So, would you go ahead and post a patch, Alex?

Yeah, could do this now.

Alex

> 
> 
>> We could consider this behavior for source 3.0 packages only by using
>> some flag in recipe.
> 
> Well, no. This way you're entangling Debian packaging internals to the
> recipe. I would opt against this. Most probably you'll want to do git
> operations even if you don't use source 3.0 (git) packages, e.g.,
> getting the latest git tag or whatever. So, I'd opt for doing it for
> every source git package, unconditionally.
>