Re: [PATCH] Map git objects to buildchroot

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

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