[PATCH v2] rootfs class lock undefined in corner cases fix

[PATCH v2] rootfs class lock undefined in corner cases fix

[roberto.foglietta]

From: "Roberto A. Foglietta" <roberto.foglietta@gmail.com>

rootfs class lock undefined in corner cases fix

Signed-off-by: Roberto A. Foglietta <roberto.foglietta@gmail.com>
---
 meta/classes/rootfs.bbclass | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
index b3faade..63735d4 100644
--- a/meta/classes/rootfs.bbclass
+++ b/meta/classes/rootfs.bbclass
@@ -198,11 +198,14 @@ python do_rootfs_install() {
         if (d.getVarFlag(cmd, 'isar-apt-lock') or "") == "acquire-before":
             lock = bb.utils.lockfile(d.getVar("REPO_ISAR_DIR") + "/isar.lock",
                                      shared=True)
+        else:
+            lock = ""
 
         bb.build.exec_func(cmd, d)
 
         if (d.getVarFlag(cmd, 'isar-apt-lock') or "") == "release-after":
-            bb.utils.unlockfile(lock)
+            if (lock != ""):
+                bb.utils.unlockfile(lock)
     progress_reporter.finish()
 }
 addtask rootfs_install before do_rootfs_postprocess after do_unpack
-- 
2.34.1

Re: [PATCH v2] rootfs class lock undefined in corner cases fix

[Henning Schild]

Am Wed,  4 Jan 2023 20:46:46 +0100
schrieb roberto.foglietta@gmail.com:

> From: "Roberto A. Foglietta" <roberto.foglietta@gmail.com>
> 
> rootfs class lock undefined in corner cases fix

Could you go into details of what those corner cases are?

We would obviously all assume that your base is Isar next, in case we
are talking about a fork already on bb2 that might all be a reply to
the bb2 series, or a patch based on top.

> Signed-off-by: Roberto A. Foglietta <roberto.foglietta@gmail.com>
> ---
>  meta/classes/rootfs.bbclass | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
> index b3faade..63735d4 100644
> --- a/meta/classes/rootfs.bbclass
> +++ b/meta/classes/rootfs.bbclass
> @@ -198,11 +198,14 @@ python do_rootfs_install() {
>          if (d.getVarFlag(cmd, 'isar-apt-lock') or "") ==
> "acquire-before": lock = bb.utils.lockfile(d.getVar("REPO_ISAR_DIR")
> + "/isar.lock", shared=True)
> +        else:
> +            lock = ""

if we fail to grab that lock for any reason ... are you sure we can
keep going and release the lock only when we got it?

There must be some reason for the lock and not getting it likely means
we should not continue ignoring the problem.

Maybe we rather have to keep looping and trying until we get that lock.

Henning

>  
>          bb.build.exec_func(cmd, d)
>  
>          if (d.getVarFlag(cmd, 'isar-apt-lock') or "") ==
> "release-after":
> -            bb.utils.unlockfile(lock)
> +            if (lock != ""):
> +                bb.utils.unlockfile(lock)
>      progress_reporter.finish()
>  }
>  addtask rootfs_install before do_rootfs_postprocess after do_unpack

Re: [PATCH v2] rootfs class lock undefined in corner cases fix

[Jan Kiszka]

On 04.01.23 20:46, roberto.foglietta@gmail.com wrote:
> From: "Roberto A. Foglietta" <roberto.foglietta@gmail.com>
> 
> rootfs class lock undefined in corner cases fix
> 
> Signed-off-by: Roberto A. Foglietta <roberto.foglietta@gmail.com>
> ---
>  meta/classes/rootfs.bbclass | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
> index b3faade..63735d4 100644
> --- a/meta/classes/rootfs.bbclass
> +++ b/meta/classes/rootfs.bbclass
> @@ -198,11 +198,14 @@ python do_rootfs_install() {
>          if (d.getVarFlag(cmd, 'isar-apt-lock') or "") == "acquire-before":
>              lock = bb.utils.lockfile(d.getVar("REPO_ISAR_DIR") + "/isar.lock",
>                                       shared=True)
> +        else:
> +            lock = ""
>  
>          bb.build.exec_func(cmd, d)
>  
>          if (d.getVarFlag(cmd, 'isar-apt-lock') or "") == "release-after":
> -            bb.utils.unlockfile(lock)
> +            if (lock != ""):
> +                bb.utils.unlockfile(lock)
>      progress_reporter.finish()
>  }
>  addtask rootfs_install before do_rootfs_postprocess after do_unpack

I thinks you didn't understand the logic yet when you are proposing such
a change and you are rather papering over a downstream issue: missing
"acquire-before" in a commend chain that later on has a "release-after".

We can improve detecting such bugs by asserting on lock being defined
when running into a "release-after".

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

Re: [PATCH v2] rootfs class lock undefined in corner cases fix

[Roberto A. Foglietta]

On Thu, 5 Jan 2023 at 08:18, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> On 04.01.23 20:46, roberto.foglietta@gmail.com wrote:

>
> I thinks you didn't understand the logic yet when you are proposing such
> a change and you are rather papering over a downstream issue: missing
> "acquire-before" in a commend chain that later on has a "release-after".
>

I did not care about the logic / timing of the actions. I simply
notice that there is a corner case in which that variable is undefined
but tried to be used. This is a fact despite the reason because it
happens and IMHO should be fixed in the simplest way possible at
coding level before being fixed the cause at logic level. A more
elegant coding would be:

if 'lock' in locals():
    bb.utils.unlockfile(lock)

without any other changes.

Thanks for the feedback, R-