[PATCH] Fix corrupted magic number in ipk on populate_sdk task

[PATCH] Fix corrupted magic number in ipk on populate_sdk task

[Tomasz Dziendzielski]

If any do_package_write_ipk runs parallel with populate_sdk task it can
happen that opkg script arfile.py tries to get magic number of all
packages in DEPLOY_DIR_IPK while some package is being copied there at
the same time. This results with:
| AssertionError: Old ipk format (non-deb) is unsupported, file:
| build/tmp/deploy/ipk/foo/bar_1.0-r0_foo.ipk, magic: b'', expected
| !<arch>
which isn't really the case here.

To fix this I added lockfile in package_write_ipk task and in
OpkgIndexer class. Even though package_write_ipk is writing to a
common deploy directory we can use shared lock, because we are sure we
don't write to same files. On the other hand in OpkgIndex to read magic
number I added not-shared lock, because unfortunately we don't have a
mechanism to share lockfiles only with specific group of tasks. That's
not a problem though, since indexer runs fast and usually there are not
many recipes using populate_sdk class.

Signed-off-by: Tomasz Dziendzielski <tomasz.dziendzielski@gmail.com>
Signed-off-by: Kamil Kwiek <kamil.kwiek@gmail.com>
Signed-off-by: Jan Brzezanski <jan.brzezanski@gmail.com>
Signed-off-by: Paulo Neves <ptsneves@gmail.com>
---
 meta/classes/package_ipk.bbclass            | 10 ++++++++++
 meta/lib/oe/package_manager/ipk/__init__.py |  3 +++
 2 files changed, 13 insertions(+)

diff --git a/meta/classes/package_ipk.bbclass b/meta/classes/package_ipk.bbclass
index 600b3ac90c..16d4ae2032 100644
--- a/meta/classes/package_ipk.bbclass
+++ b/meta/classes/package_ipk.bbclass
@@ -256,7 +256,12 @@ python do_package_write_ipk_setscene () {
     if os.access(os.path.join(tmpdir, "stamps", "IPK_PACKAGE_INDEX_CLEAN"), os.R_OK):
         os.unlink(os.path.join(tmpdir, "stamps", "IPK_PACKAGE_INDEX_CLEAN"))
 
+    ipk_index_lock = os.path.join(d.getVar("DEPLOY_DIR_IPK"), 'ipk.lock')
+    lf = bb.utils.lockfile(ipk_index_lock, shared=True)
+
     sstate_setscene(d)
+
+    bb.utils.unlockfile(lf)
 }
 addtask do_package_write_ipk_setscene
 
@@ -268,8 +273,13 @@ python () {
 }
 
 python do_package_write_ipk () {
+    ipk_index_lock = os.path.join(d.getVar("DEPLOY_DIR_IPK"), 'ipk.lock')
+    lf = bb.utils.lockfile(ipk_index_lock, shared=True)
+
     bb.build.exec_func("read_subpackage_metadata", d)
     bb.build.exec_func("do_package_ipk", d)
+
+    bb.utils.unlockfile(lf)
 }
 do_package_write_ipk[dirs] = "${PKGWRITEDIRIPK}"
 do_package_write_ipk[cleandirs] = "${PKGWRITEDIRIPK}"
diff --git a/meta/lib/oe/package_manager/ipk/__init__.py b/meta/lib/oe/package_manager/ipk/__init__.py
index 4cd3963111..c40dd3890d 100644
--- a/meta/lib/oe/package_manager/ipk/__init__.py
+++ b/meta/lib/oe/package_manager/ipk/__init__.py
@@ -48,7 +48,10 @@ class OpkgIndexer(Indexer):
             bb.note("There are no packages in %s!" % self.deploy_dir)
             return
 
+        ipk_index_lock = os.path.join(self.d.getVar("DEPLOY_DIR_IPK"), 'ipk.lock')
+        lf = bb.utils.lockfile(ipk_index_lock, shared=False)
         oe.utils.multiprocess_launch(create_index, index_cmds, self.d)
+        bb.utils.unlockfile(lf)
 
         if signer:
             feed_sig_type = self.d.getVar('PACKAGE_FEED_GPG_SIGNATURE_TYPE')
-- 
2.31.1

Re: [OE-core] [PATCH] Fix corrupted magic number in ipk on populate_sdk task

[Richard Purdie]

On Tue, 2021-06-01 at 17:21 +0200, Tomasz Dziendzielski wrote:
> If any do_package_write_ipk runs parallel with populate_sdk task it can
> happen that opkg script arfile.py tries to get magic number of all
> packages in DEPLOY_DIR_IPK while some package is being copied there at
> the same time. This results with:
> > AssertionError: Old ipk format (non-deb) is unsupported, file:
> > build/tmp/deploy/ipk/foo/bar_1.0-r0_foo.ipk, magic: b'', expected
> > !<arch>
> which isn't really the case here.
> 
> To fix this I added lockfile in package_write_ipk task and in
> OpkgIndexer class. Even though package_write_ipk is writing to a
> common deploy directory we can use shared lock, because we are sure we
> don't write to same files. On the other hand in OpkgIndex to read magic
> number I added not-shared lock, because unfortunately we don't have a
> mechanism to share lockfiles only with specific group of tasks. That's
> not a problem though, since indexer runs fast and usually there are not
> many recipes using populate_sdk class.
> 
> Signed-off-by: Tomasz Dziendzielski <tomasz.dziendzielski@gmail.com>
> Signed-off-by: Kamil Kwiek <kamil.kwiek@gmail.com>
> Signed-off-by: Jan Brzezanski <jan.brzezanski@gmail.com>
> Signed-off-by: Paulo Neves <ptsneves@gmail.com>
> ---
>  meta/classes/package_ipk.bbclass            | 10 ++++++++++
>  meta/lib/oe/package_manager/ipk/__init__.py |  3 +++
>  2 files changed, 13 insertions(+)

I would much prefer we didn't do this, we've had such lockfiles before and they
are a bottleneck when for example expanding a new build directory from sstate.

The rpm code creates a symlink tree copy of the packages it has the right to access
and then only index those. I did a quick grep and I thought this was being used
for ipk and deb too:

deb/__init__.py:        create_packages_dir(self.d, self.deploy_dir, d.getVar("DEPLOY_DIR_DEB"), "package_write_deb", filterbydependencies)
__init__.py:def create_packages_dir(d, subrepo_dir, deploydir, taskname, filterbydependencies):
ipk/__init__.py:            create_packages_dir(self.d, self.deploy_dir, d.getVar("DEPLOY_DIR_IPK"), "package_write_ipk", filterbydependencies)
rpm/__init__.py:            create_packages_dir(self.d, oe.path.join(self.rpm_repo_dir, "rpm"), d.getVar("DEPLOY_DIR_RPM"), "package_write_rpm", filterbydependencies)

Which release did you see the issue on and is there some codepath which
isn't using these tree copies?

One reason for the above is also to stop an image seeing packages it doesn't
depend upon which is important when package globbing is used to install
packages.

Cheers,

Richard

Re: [OE-core] [PATCH] Fix corrupted magic number in ipk on populate_sdk task

[Tomasz Dziendzielski]

[-- Attachment #1: Type: text/plain, Size: 3301 bytes --]

Where I found the issue it was Krogoth. You're right, this
"create_packages_dir" is also fixing the issue. It was really hard to
reproduce the error so when creating this patch I just compared the code -
it was almost the same, doing the same with the same variables, but in fact
in the latest poky it's not a common deploy dir anymore. Please disregard
this patch.

Best regards,
Tomasz Dziendzielski

wt., 1 cze 2021 o 17:48 Richard Purdie <richard.purdie@linuxfoundation.org>
napisał(a):

> On Tue, 2021-06-01 at 17:21 +0200, Tomasz Dziendzielski wrote:
> > If any do_package_write_ipk runs parallel with populate_sdk task it can
> > happen that opkg script arfile.py tries to get magic number of all
> > packages in DEPLOY_DIR_IPK while some package is being copied there at
> > the same time. This results with:
> > > AssertionError: Old ipk format (non-deb) is unsupported, file:
> > > build/tmp/deploy/ipk/foo/bar_1.0-r0_foo.ipk, magic: b'', expected
> > > !<arch>
> > which isn't really the case here.
> >
> > To fix this I added lockfile in package_write_ipk task and in
> > OpkgIndexer class. Even though package_write_ipk is writing to a
> > common deploy directory we can use shared lock, because we are sure we
> > don't write to same files. On the other hand in OpkgIndex to read magic
> > number I added not-shared lock, because unfortunately we don't have a
> > mechanism to share lockfiles only with specific group of tasks. That's
> > not a problem though, since indexer runs fast and usually there are not
> > many recipes using populate_sdk class.
> >
> > Signed-off-by: Tomasz Dziendzielski <tomasz.dziendzielski@gmail.com>
> > Signed-off-by: Kamil Kwiek <kamil.kwiek@gmail.com>
> > Signed-off-by: Jan Brzezanski <jan.brzezanski@gmail.com>
> > Signed-off-by: Paulo Neves <ptsneves@gmail.com>
> > ---
> >  meta/classes/package_ipk.bbclass            | 10 ++++++++++
> >  meta/lib/oe/package_manager/ipk/__init__.py |  3 +++
> >  2 files changed, 13 insertions(+)
>
> I would much prefer we didn't do this, we've had such lockfiles before and
> they
> are a bottleneck when for example expanding a new build directory from
> sstate.
>
> The rpm code creates a symlink tree copy of the packages it has the right
> to access
> and then only index those. I did a quick grep and I thought this was being
> used
> for ipk and deb too:
>
> deb/__init__.py:        create_packages_dir(self.d, self.deploy_dir,
> d.getVar("DEPLOY_DIR_DEB"), "package_write_deb", filterbydependencies)
> __init__.py:def create_packages_dir(d, subrepo_dir, deploydir, taskname,
> filterbydependencies):
> ipk/__init__.py:            create_packages_dir(self.d, self.deploy_dir,
> d.getVar("DEPLOY_DIR_IPK"), "package_write_ipk", filterbydependencies)
> rpm/__init__.py:            create_packages_dir(self.d,
> oe.path.join(self.rpm_repo_dir, "rpm"), d.getVar("DEPLOY_DIR_RPM"),
> "package_write_rpm", filterbydependencies)
>
> Which release did you see the issue on and is there some codepath which
> isn't using these tree copies?
>
> One reason for the above is also to stop an image seeing packages it
> doesn't
> depend upon which is important when package globbing is used to install
> packages.
>
> Cheers,
>
> Richard
>
>
>

[-- Attachment #2: Type: text/html, Size: 4230 bytes --]