| Message ID | 20230104194646.2545099-1-roberto.foglietta@gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v2] rootfs class lock undefined in corner cases fix | expand |
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
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
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-
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