Re: [PATCH v2 5/9] Move buildchroot mounts into dpkg-base class

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2018-02-14 06:54, Alexander Smirnov wrote:
> 
> Jan Kiszka <jan.kiszka@siemens.com> 13 февраля 2018 г. 23:48:02 написал:
> 
>> On 2018-02-13 21:17, Alexander Smirnov wrote:
>>>
>>>
>>> On 02/13/2018 11:05 PM, Jan Kiszka wrote:
>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>
>>>> This avoids the unfortunate setup_mounts task which is either not
>>>> executed on rebuilds or causes excessive package rebuilds. We just need
>>>> ensure prior to switching into the buildchroot that all mount points
>>>> are
>>>> populated.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> ---
>>>>   meta/classes/dpkg-base.bbclass                   | 13 +++++++++-
>>>>   meta/recipes-devtools/buildchroot/buildchroot.bb | 30
>>>> +++---------------------
>>>>   2 files changed, 15 insertions(+), 28 deletions(-)
>>>>
>>>> diff --git a/meta/classes/dpkg-base.bbclass
>>>> b/meta/classes/dpkg-base.bbclass
>>>> index a45fbce..66132da 100644
>>>> --- a/meta/classes/dpkg-base.bbclass
>>>> +++ b/meta/classes/dpkg-base.bbclass
>>>> @@ -2,7 +2,7 @@
>>>>   # Copyright (C) 2017 Siemens AG
>>>>     # Add dependency from buildchroot creation
>>>> -do_build[depends] = "buildchroot:do_setup_mounts"
>>>> +do_build[depends] = "buildchroot:do_build"
>>>>     # Add dependency between Isar recipes
>>>>   DEPENDS ?= ""
>>>> @@ -20,11 +20,22 @@ dpkg_runbuild() {
>>>>       die "This should never be called, overwrite it in your derived
>>>> class"
>>>>   }
>>>>   +mount_if_needed() {
>>>> +
>>>> +    # The test may race with parallel builds. So probe, mount if
>>>> needed, and
>>>> +    # then only fail if both the mount and yet another probe failed
>>>> as well.
>>>> +    grep -q $2 /proc/mounts || sudo mount $1 $2 || grep -q $2
>>>> /proc/mounts
>>>
>>> This could lead to have lots of remounts on the same folder during
>>> parallel execution. This should be done in a separate task and protected
>>> by [lockfiles].
>>
>> No, it won't - please re-read the code. It's a classic retry pattern,
>> and it's even detecting errors.
>>
> 
> 'mount --bind' never fails. So if several do_build() have passed first
> grep in parallel, then each one will mount 'isar-apt'. Or I don't
> understand this.

That is a valid remark - I wasn't aware of that behaviour (and
explaining this earlier would have helped a lot).

I will come up with a safe variant using flock soon.

And before you pull out your task idea again, I'd like to cite from the
bitbake manual:

"Any task that depends (possibly indirectly) on a [nostamp] task will
always be executed as well. This can cause unnecessary rebuilding if you
are not careful."

That's why its broken by design.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux