[PATCH] package_ipk.bbclass DON'T PUSH THIS, but PLEASE read it

[PATCH] package_ipk.bbclass DON'T PUSH THIS, but PLEASE read it

[Martin Jansa]

There is quite common error about non-existent lockfile path

I've seen that everytime when recipe changes ARCH (ie when removed machine specific config)
and package is no longer in tmp/work/om-gta01.. but tmp/work/armv4t.., then whole build works
ok, but in the end it's looking for lockfile in right directory but in wrong arch dir.

Today I've seen similar error from mwester:
18:10:59 < mwester> ERROR: Error, lockfile path does not exist!: /u/slug/slugos/tmp/work/armv5teb-linux-gnueabi/gpsd-2.39-r0/packages-split

So with a bit of debugging (in patch) it's clear that it's broken after executing read_subpackage_metadata function
NOTE: WORKDIR-before /OE/tmpdir-dev-shr/work/armv5te-oe-linux-gnueabi/gpsd-2.90-r4.0
NOTE: WORKDIR-after  /OE/tmpdir-dev-shr/work/armv5te-oe-linux-gnueabi/gpsd-2.90-r0

If I comment-out that last setVar as used in patch it works OK.

I'm not sure why we wan't all those subpkg data with same keys set directly in data. But just grep for values it wants to set I can see

where broken WORKDIR goes from:
bitbake@jama ~/build.dev.shr.spitz $ grep "PR\|PKGR" variables.set.log
NOTE: Setting pkg 'libgps' subdata key 'PR', value 'r0'
NOTE: Setting pkg 'libgps' subdata key 'PKGR', value 'r0.4'
NOTE: Setting pkg 'python-pygps' subdata key 'PR', value 'r0'
NOTE: Setting pkg 'python-pygps' subdata key 'PKGR', value 'r0.4'
NOTE: Setting pkg 'gpsd-udev' subdata key 'PR', value 'r4.0'
NOTE: Setting pkg 'gpsd-udev' subdata key 'PKGR', value 'r4.0.4'
NOTE: Setting pkg 'gpsd-conf' subdata key 'PR', value 'r0'
NOTE: Setting pkg 'gpsd-conf' subdata key 'PKGR', value 'r0.4'
NOTE: Setting pkg 'gpsd-gpsctl' subdata key 'PR', value 'r0'
NOTE: Setting pkg 'gpsd-gpsctl' subdata key 'PKGR', value 'r0.4'
NOTE: Setting pkg 'gps-utils' subdata key 'PR', value 'r0'
NOTE: Setting pkg 'gps-utils' subdata key 'PKGR', value 'r0.4'
NOTE: Setting pkg 'gpsd-dbg' subdata key 'PR', value 'r0'
NOTE: Setting pkg 'gpsd-dbg' subdata key 'PKGR', value 'r0.4'
NOTE: Setting pkg 'gpsd' subdata key 'PR', value 'r0'
NOTE: Setting pkg 'gpsd' subdata key 'PKGR', value 'r0.4'
NOTE: Setting pkg 'gpsd-doc' subdata key 'PR', value 'r0'
NOTE: Setting pkg 'gpsd-doc' subdata key 'PKGR', value 'r0.4'
NOTE: Setting pkg 'gpsd-dev' subdata key 'PR', value 'r0'
NOTE: Setting pkg 'gpsd-dev' subdata key 'PKGR', value 'r0.4'
NOTE: Setting pkg 'gpsd-static' subdata key 'PR', value 'r0'
NOTE: Setting pkg 'gpsd-static' subdata key 'PKGR', value 'r0.4'
NOTE: Setting pkg 'gpsd-locale' subdata key 'PR', value 'r0'
NOTE: Setting pkg 'gpsd-locale' subdata key 'PKGR', value 'r0.4'

so the right value is only in gpsd-udev, but that's only because gpsd-udev is new subpackage and wasn't been built before
NOTE: pkg: gpsd-udev
NOTE: pkg: gpsd-udev - /OE/tmpdir-dev-shr/pkgdata/armv5te-oe-linux-gnueabi/runtime/gpsd-udev
while other recipes have both arch
NOTE: pkg: python-pygps
NOTE: pkg: python-pygps - /OE/tmpdir-dev-shr/pkgdata/spitz-oe-linux-gnueabi/runtime/python-pygps
NOTE: pkg: python-pygps - /OE/tmpdir-dev-shr/pkgdata/armv5te-oe-linux-gnueabi/runtime/python-pygps

In this case when I return last arch found, then it works again, but that's only because we're changing app from spitz to armv5te,
when someone adds some machine specific config then, right version will be in spitz and wrong in armv5te

Please someone fix this :).
---
 classes/package_ipk.bbclass |    4 ++++
 classes/packagedata.bbclass |    9 ++++++++-
 2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/classes/package_ipk.bbclass b/classes/package_ipk.bbclass
index 420c892..44241d6 100644
--- a/classes/package_ipk.bbclass
+++ b/classes/package_ipk.bbclass
@@ -320,7 +320,11 @@ python do_package_write_ipk () {
 		bb.debug(1, "No PACKAGES defined, nothing to package")
 		return
 
+	workdir = bb.data.getVar('WORKDIR', d, 1)
+	bb.note("WORKDIR-before %s" % workdir)
 	bb.build.exec_func("read_subpackage_metadata", d)
+	workdir = bb.data.getVar('WORKDIR', d, 1)
+	bb.note("WORKDIR-after %s" % workdir)
 	bb.build.exec_func("do_package_ipk", d)
 }
 do_package_write_ipk[dirs] = "${D}"
diff --git a/classes/packagedata.bbclass b/classes/packagedata.bbclass
index 86f18a9..581c020 100644
--- a/classes/packagedata.bbclass
+++ b/classes/packagedata.bbclass
@@ -27,10 +27,15 @@ def get_subpkgedata_fn(pkg, d):
 	archs.reverse()
 	pkgdata = bb.data.expand('${TMPDIR}/pkgdata/', d)
 	targetdir = bb.data.expand('${TARGET_VENDOR}-${TARGET_OS}/runtime/', d)
+	bb.note("pkg: %s" % pkg)
+	rval = None
 	for arch in archs:
 		fn = pkgdata + arch + targetdir + pkg
 		if os.path.exists(fn):
-			return fn
+			bb.note("pkg: %s - %s" % (pkg, fn))
+			rval = fn
+	if rval != None:
+		return rval
 	return bb.data.expand('${PKGDATA_DIR}/runtime/%s' % pkg, d)
 
 def has_subpkgdata(pkg, d):
@@ -51,11 +56,13 @@ python read_subpackage_metadata () {
 	data = read_pkgdata(bb.data.getVar('PN', d, 1), d)
 
 	for key in data.keys():
+		bb.note("Setting key '%s', value '%s'" % (key, data[key]));
 		bb.data.setVar(key, data[key], d)
 
 	for pkg in bb.data.getVar('PACKAGES', d, 1).split():
 		sdata = read_subpkgdata(pkg, d)
 		for key in sdata.keys():
+			bb.note("Setting pkg '%s' subdata key '%s', value '%s'" % (pkg, key, sdata[key]));
 			bb.data.setVar(key, sdata[key], d)
 }
 
-- 
1.7.0

Re: [PATCH] package_ipk.bbclass DON'T PUSH THIS, but PLEASE read it

[Martin Jansa]

On Fri, Feb 26, 2010 at 10:15 PM, Martin Jansa <martin.jansa@gmail.com> wrote:
> There is quite common error about non-existent lockfile path
>
> I've seen that everytime when recipe changes ARCH (ie when removed machine specific config)
> and package is no longer in tmp/work/om-gta01.. but tmp/work/armv4t.., then whole build works
> ok, but in the end it's looking for lockfile in right directory but in wrong arch dir.
>
> Today I've seen similar error from mwester:
> 18:10:59 < mwester> ERROR: Error, lockfile path does not exist!: /u/slug/slugos/tmp/work/armv5teb-linux-gnueabi/gpsd-2.39-r0/packages-split

ping?

I think this is quite big bug as wrong vars can be used in more places
and create bigger problems than missing lockfile path.

At least please point me to example why we need collapsed subpackage
data in global scope.

RP: sorry for adding you to CC directly, but you would probably know,
if the variable name collapsing fuction from Poky:
http://git.openembedded.org/cgit.cgi/openembedded/commit/?id=634c76d02e5796f5e908b6d6a79dcfef6f8af3d1
should be somewhere in this code path.

Regards,

>
> So with a bit of debugging (in patch) it's clear that it's broken after executing read_subpackage_metadata function
> NOTE: WORKDIR-before /OE/tmpdir-dev-shr/work/armv5te-oe-linux-gnueabi/gpsd-2.90-r4.0
> NOTE: WORKDIR-after  /OE/tmpdir-dev-shr/work/armv5te-oe-linux-gnueabi/gpsd-2.90-r0
>
> If I comment-out that last setVar as used in patch it works OK.
>
> I'm not sure why we wan't all those subpkg data with same keys set directly in data. But just grep for values it wants to set I can see
>
> where broken WORKDIR goes from:
> bitbake@jama ~/build.dev.shr.spitz $ grep "PR\|PKGR" variables.set.log
> NOTE: Setting pkg 'libgps' subdata key 'PR', value 'r0'
> NOTE: Setting pkg 'libgps' subdata key 'PKGR', value 'r0.4'
> NOTE: Setting pkg 'python-pygps' subdata key 'PR', value 'r0'
> NOTE: Setting pkg 'python-pygps' subdata key 'PKGR', value 'r0.4'
> NOTE: Setting pkg 'gpsd-udev' subdata key 'PR', value 'r4.0'
> NOTE: Setting pkg 'gpsd-udev' subdata key 'PKGR', value 'r4.0.4'
> NOTE: Setting pkg 'gpsd-conf' subdata key 'PR', value 'r0'
> NOTE: Setting pkg 'gpsd-conf' subdata key 'PKGR', value 'r0.4'
> NOTE: Setting pkg 'gpsd-gpsctl' subdata key 'PR', value 'r0'
> NOTE: Setting pkg 'gpsd-gpsctl' subdata key 'PKGR', value 'r0.4'
> NOTE: Setting pkg 'gps-utils' subdata key 'PR', value 'r0'
> NOTE: Setting pkg 'gps-utils' subdata key 'PKGR', value 'r0.4'
> NOTE: Setting pkg 'gpsd-dbg' subdata key 'PR', value 'r0'
> NOTE: Setting pkg 'gpsd-dbg' subdata key 'PKGR', value 'r0.4'
> NOTE: Setting pkg 'gpsd' subdata key 'PR', value 'r0'
> NOTE: Setting pkg 'gpsd' subdata key 'PKGR', value 'r0.4'
> NOTE: Setting pkg 'gpsd-doc' subdata key 'PR', value 'r0'
> NOTE: Setting pkg 'gpsd-doc' subdata key 'PKGR', value 'r0.4'
> NOTE: Setting pkg 'gpsd-dev' subdata key 'PR', value 'r0'
> NOTE: Setting pkg 'gpsd-dev' subdata key 'PKGR', value 'r0.4'
> NOTE: Setting pkg 'gpsd-static' subdata key 'PR', value 'r0'
> NOTE: Setting pkg 'gpsd-static' subdata key 'PKGR', value 'r0.4'
> NOTE: Setting pkg 'gpsd-locale' subdata key 'PR', value 'r0'
> NOTE: Setting pkg 'gpsd-locale' subdata key 'PKGR', value 'r0.4'
>
> so the right value is only in gpsd-udev, but that's only because gpsd-udev is new subpackage and wasn't been built before
> NOTE: pkg: gpsd-udev
> NOTE: pkg: gpsd-udev - /OE/tmpdir-dev-shr/pkgdata/armv5te-oe-linux-gnueabi/runtime/gpsd-udev
> while other recipes have both arch
> NOTE: pkg: python-pygps
> NOTE: pkg: python-pygps - /OE/tmpdir-dev-shr/pkgdata/spitz-oe-linux-gnueabi/runtime/python-pygps
> NOTE: pkg: python-pygps - /OE/tmpdir-dev-shr/pkgdata/armv5te-oe-linux-gnueabi/runtime/python-pygps
>
> In this case when I return last arch found, then it works again, but that's only because we're changing app from spitz to armv5te,
> when someone adds some machine specific config then, right version will be in spitz and wrong in armv5te
>
> Please someone fix this :).
> ---
>  classes/package_ipk.bbclass |    4 ++++
>  classes/packagedata.bbclass |    9 ++++++++-
>  2 files changed, 12 insertions(+), 1 deletions(-)
>
> diff --git a/classes/package_ipk.bbclass b/classes/package_ipk.bbclass
> index 420c892..44241d6 100644
> --- a/classes/package_ipk.bbclass
> +++ b/classes/package_ipk.bbclass
> @@ -320,7 +320,11 @@ python do_package_write_ipk () {
>                bb.debug(1, "No PACKAGES defined, nothing to package")
>                return
>
> +       workdir = bb.data.getVar('WORKDIR', d, 1)
> +       bb.note("WORKDIR-before %s" % workdir)
>        bb.build.exec_func("read_subpackage_metadata", d)
> +       workdir = bb.data.getVar('WORKDIR', d, 1)
> +       bb.note("WORKDIR-after %s" % workdir)
>        bb.build.exec_func("do_package_ipk", d)
>  }
>  do_package_write_ipk[dirs] = "${D}"
> diff --git a/classes/packagedata.bbclass b/classes/packagedata.bbclass
> index 86f18a9..581c020 100644
> --- a/classes/packagedata.bbclass
> +++ b/classes/packagedata.bbclass
> @@ -27,10 +27,15 @@ def get_subpkgedata_fn(pkg, d):
>        archs.reverse()
>        pkgdata = bb.data.expand('${TMPDIR}/pkgdata/', d)
>        targetdir = bb.data.expand('${TARGET_VENDOR}-${TARGET_OS}/runtime/', d)
> +       bb.note("pkg: %s" % pkg)
> +       rval = None
>        for arch in archs:
>                fn = pkgdata + arch + targetdir + pkg
>                if os.path.exists(fn):
> -                       return fn
> +                       bb.note("pkg: %s - %s" % (pkg, fn))
> +                       rval = fn
> +       if rval != None:
> +               return rval
>        return bb.data.expand('${PKGDATA_DIR}/runtime/%s' % pkg, d)
>
>  def has_subpkgdata(pkg, d):
> @@ -51,11 +56,13 @@ python read_subpackage_metadata () {
>        data = read_pkgdata(bb.data.getVar('PN', d, 1), d)
>
>        for key in data.keys():
> +               bb.note("Setting key '%s', value '%s'" % (key, data[key]));
>                bb.data.setVar(key, data[key], d)
>
>        for pkg in bb.data.getVar('PACKAGES', d, 1).split():
>                sdata = read_subpkgdata(pkg, d)
>                for key in sdata.keys():
> +                       bb.note("Setting pkg '%s' subdata key '%s', value '%s'" % (pkg, key, sdata[key]));
>                        bb.data.setVar(key, sdata[key], d)
>  }
>
> --
> 1.7.0
>
>

Re: [PATCH] package_ipk.bbclass DON'T PUSH THIS, but PLEASE read it

[Richard Purdie]

On Wed, 2010-03-03 at 13:54 +0100, Martin Jansa wrote:
> On Fri, Feb 26, 2010 at 10:15 PM, Martin Jansa <martin.jansa@gmail.com> wrote:
> > There is quite common error about non-existent lockfile path
> >
> > I've seen that everytime when recipe changes ARCH (ie when removed machine specific config)
> > and package is no longer in tmp/work/om-gta01.. but tmp/work/armv4t.., then whole build works
> > ok, but in the end it's looking for lockfile in right directory but in wrong arch dir.
> >
> > Today I've seen similar error from mwester:
> > 18:10:59 < mwester> ERROR: Error, lockfile path does not exist!: /u/slug/slugos/tmp/work/armv5teb-linux-gnueabi/gpsd-2.39-r0/packages-split
> 
> ping?
> 
> I think this is quite big bug as wrong vars can be used in more places
> and create bigger problems than missing lockfile path.
> 
> At least please point me to example why we need collapsed subpackage
> data in global scope.
> 
> RP: sorry for adding you to CC directly, but you would probably know,
> if the variable name collapsing fuction from Poky:
> http://git.openembedded.org/cgit.cgi/openembedded/commit/?id=634c76d02e5796f5e908b6d6a79dcfef6f8af3d1
> should be somewhere in this code path.

Its a serious bug I agree but a very hard one to fix. The correct fix is
to have staging under full control of a packaging system, then when you
clean packages, all the right bits are actually removed.

In that sense this is no better or worse than the other 101 ways you can
break staging.

So in summary, no easy fix, we need to fix the big general problem -
packaged staging needs to be on by default and used by everything. Only
then will we nail all these problems.

Cheers,

Richard

Re: [PATCH] package_ipk.bbclass DON'T PUSH THIS, but PLEASE read it

[Martin Jansa]

On Wed, Mar 03, 2010 at 01:07:11PM +0000, Richard Purdie wrote:
> Its a serious bug I agree but a very hard one to fix. The correct fix is
> to have staging under full control of a packaging system, then when you
> clean packages, all the right bits are actually removed.
> 
> In that sense this is no better or worse than the other 101 ways you can
> break staging.

Fair enough, thanks for reply.

> So in summary, no easy fix, we need to fix the big general problem -
> packaged staging needs to be on by default and used by everything. Only
> then will we nail all these problems.

So I'll continue to use 
find tmp/pkgdata -name ${PN}\* -exec rm -rf {} \;
when I see non-existent lockfile path.

Or maybe try this little patch and watch what will break:
diff --git a/classes/packagedata.bbclass b/classes/packagedata.bbclass
index 86f18a9..e38ff3c 100644
--- a/classes/packagedata.bbclass
+++ b/classes/packagedata.bbclass
@@ -56,7 +56,8 @@ python read_subpackage_metadata () {
        for pkg in bb.data.getVar('PACKAGES', d, 1).split():
                sdata = read_subpkgdata(pkg, d)
                for key in sdata.keys():
-                       bb.data.setVar(key, sdata[key], d)
+                       if bb.data.getVar(key, d, None) == None:
+                               bb.data.setVar(key, sdata[key], d)
 }
 
 



-- 
uin:136542059                jid:Martin.Jansa@gmail.com
Jansa Martin                 sip:jamasip@voip.wengo.fr 
JaMa                         

Re: [PATCH] package_ipk.bbclass DON'T PUSH THIS, but PLEASE read it

[Martin Jansa]

On Wed, Mar 03, 2010 at 08:19:37PM +0100, Martin Jansa wrote:
> On Wed, Mar 03, 2010 at 01:07:11PM +0000, Richard Purdie wrote:
> > Its a serious bug I agree but a very hard one to fix. The correct fix is
> > to have staging under full control of a packaging system, then when you
> > clean packages, all the right bits are actually removed.
> > 
> > In that sense this is no better or worse than the other 101 ways you can
> > break staging.
> 
> Fair enough, thanks for reply.
> 
> > So in summary, no easy fix, we need to fix the big general problem -
> > packaged staging needs to be on by default and used by everything. Only
> > then will we nail all these problems.
> 
> So I'll continue to use 
> find tmp/pkgdata -name ${PN}\* -exec rm -rf {} \;
> when I see non-existent lockfile path.
> 
> Or maybe try this little patch and watch what will break:
> diff --git a/classes/packagedata.bbclass b/classes/packagedata.bbclass
> index 86f18a9..e38ff3c 100644
> --- a/classes/packagedata.bbclass
> +++ b/classes/packagedata.bbclass
> @@ -56,7 +56,8 @@ python read_subpackage_metadata () {
>         for pkg in bb.data.getVar('PACKAGES', d, 1).split():
>                 sdata = read_subpkgdata(pkg, d)
>                 for key in sdata.keys():
> -                       bb.data.setVar(key, sdata[key], d)
> +                       if bb.data.getVar(key, d, None) == None:
> +                               bb.data.setVar(key, sdata[key], d)
>  }

First thing broken is 
inherit update-alternatives in sysvinit
which didn't set init link in postinst and my image didn't boot, so now
I agree that it's harder than it seems and that "patch" was really bad
idea.

Regards,

-- 
uin:136542059                jid:Martin.Jansa@gmail.com
Jansa Martin                 sip:jamasip@voip.wengo.fr 
JaMa