Re: [PATCH] Map git objects to buildchroot

[Henning Schild <henning.schild@siemens.com>]

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