Re: [PATCH] Map git objects to buildchroot

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

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