Patchwork Ensuring a task is re-ran when local.conf is updated

login
register
mail settings
Submitter Paul Barker
Date May 2, 2014, 4:43 p.m.
Message ID <CANyK_8f74ZEqzX1ve=93GnZ7fg1NEiZUVeOKmmHuY+kHX-UseA@mail.gmail.com>
Download mbox | patch
Permalink /patch/71419/
State New
Headers show

Comments

Paul Barker - May 2, 2014, 4:43 p.m.
Hi all, I'm wondering if someone can help me debug this problem a
little as it's getting into the parts of bitbake I don't know very
well, in particular the determination of whether a task needs to
re-run or not.

I've attached a patch which adds code to do_package_write_ipk in
package_ipk.bbclass to split up the package feed if
IPK_HIERARCHICAL_FEED is set to "1". The commit message in the patch
explains why I'm doing this and if it all works I'll submit this to
master. The attached patch isn't ready for submission yet, it needs
more testing first.

As expected, if IPK_HIERARCHICAL_FEED is not set in local.conf, the
package feed is created in the old way. If IPK_HIERARCHICAL_FEED is
set to "1" in local.conf, the package feed is created in my modified
way. This was tested by building from scratch in each case, with no
sstate or tmp directory present.

However if IPK_HIERARCHICAL_FEED is changed in local.conf, the package
feed is not recreated. I can force do_write_package_ipk and the new
value is taken into account, but if I just do my usual 'bitbake
kitchen-sink' where kitchen-sink is a packagegroup which depends on
everything I want in the package feed, do_write_package_ipk is not
re-ran for any recipe.

Am I missing something here? Is it expected that this variable change
is detected and the relevant tasks re-executed? I know changing
variables like PREFERRED_PROVIDER_* in local.conf causes things to be
rebuilt, so why does that work and this not?

Cheers,
Richard Purdie - May 2, 2014, 9:34 p.m.
On Fri, 2014-05-02 at 17:43 +0100, Paul Barker wrote:
> Hi all, I'm wondering if someone can help me debug this problem a
> little as it's getting into the parts of bitbake I don't know very
> well, in particular the determination of whether a task needs to
> re-run or not.
> 
> I've attached a patch which adds code to do_package_write_ipk in
> package_ipk.bbclass to split up the package feed if
> IPK_HIERARCHICAL_FEED is set to "1". The commit message in the patch
> explains why I'm doing this and if it all works I'll submit this to
> master. The attached patch isn't ready for submission yet, it needs
> more testing first.
> 
> As expected, if IPK_HIERARCHICAL_FEED is not set in local.conf, the
> package feed is created in the old way. If IPK_HIERARCHICAL_FEED is
> set to "1" in local.conf, the package feed is created in my modified
> way. This was tested by building from scratch in each case, with no
> sstate or tmp directory present.
> 
> However if IPK_HIERARCHICAL_FEED is changed in local.conf, the package
> feed is not recreated. I can force do_write_package_ipk and the new
> value is taken into account, but if I just do my usual 'bitbake
> kitchen-sink' where kitchen-sink is a packagegroup which depends on
> everything I want in the package feed, do_write_package_ipk is not
> re-ran for any recipe.
> 
> Am I missing something here? Is it expected that this variable change
> is detected and the relevant tasks re-executed? I know changing
> variables like PREFERRED_PROVIDER_* in local.conf causes things to be
> rebuilt, so why does that work and this not?

This should work, I'm not sure why it wouldn't. The code should be able
to detect the dependency on the IPK_HIERARCHICAL_FEED variable and
notice when the value changes.

I'd have to experiment with some builds to figure out anything further
but its probably worth opening a bug report as it sounds broken.

bitbake-dffsigs on a siginfo file of a do_package_write_ipk is where to
start with debugging, it should show the dependency on the variable.

Cheers,

Richard
Paul Barker - May 2, 2014, 9:51 p.m.
On 2 May 2014 22:34, Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
> On Fri, 2014-05-02 at 17:43 +0100, Paul Barker wrote:
>>
>> Am I missing something here? Is it expected that this variable change
>> is detected and the relevant tasks re-executed? I know changing
>> variables like PREFERRED_PROVIDER_* in local.conf causes things to be
>> rebuilt, so why does that work and this not?
>
> This should work, I'm not sure why it wouldn't. The code should be able
> to detect the dependency on the IPK_HIERARCHICAL_FEED variable and
> notice when the value changes.
>
> I'd have to experiment with some builds to figure out anything further
> but its probably worth opening a bug report as it sounds broken.
>
> bitbake-dffsigs on a siginfo file of a do_package_write_ipk is where to
> start with debugging, it should show the dependency on the variable.

Looks like I found the issue quickly using bitbake-diffsigs.
do_package_write_ipk does:
    localdata = bb.data.createCopy(d)
Then uses localdata.

If I check localdata.getVar(), the variable name doesn't appear in the
list of dependencies.

If instead I check d.getVar(), it does appear in the list of dependencies.

It looks like localdata is used so that certain variables can be set
without the changes affecting anything else (PKG, OVERRIDES, ROOT,
etc). But there are also some calls to localdata.getVar() for
PACKAGE_ARCH and ALLOW_EMPTY.

With my check changed to d.getVar(), the dependencies are: (['PKGE',
'IPK_HIERARCHICAL_FEED', 'get_package_additional_metadata',
'PKGWRITEDIRIPK', 'OPKGBUILDCMD', 'mapping_rename_hook', 'PKGDEST',
'PACKAGES', 'HOMEPAGE'])

Does this mean changes to PACKAGE_ARCH and ALLOW_EMPTY may go unnoticed?

Thanks,
Richard Purdie - May 2, 2014, 10:07 p.m.
On Fri, 2014-05-02 at 22:51 +0100, Paul Barker wrote:
> On 2 May 2014 22:34, Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
> > On Fri, 2014-05-02 at 17:43 +0100, Paul Barker wrote:
> >>
> >> Am I missing something here? Is it expected that this variable change
> >> is detected and the relevant tasks re-executed? I know changing
> >> variables like PREFERRED_PROVIDER_* in local.conf causes things to be
> >> rebuilt, so why does that work and this not?
> >
> > This should work, I'm not sure why it wouldn't. The code should be able
> > to detect the dependency on the IPK_HIERARCHICAL_FEED variable and
> > notice when the value changes.
> >
> > I'd have to experiment with some builds to figure out anything further
> > but its probably worth opening a bug report as it sounds broken.
> >
> > bitbake-dffsigs on a siginfo file of a do_package_write_ipk is where to
> > start with debugging, it should show the dependency on the variable.
> 
> Looks like I found the issue quickly using bitbake-diffsigs.
> do_package_write_ipk does:
>     localdata = bb.data.createCopy(d)
> Then uses localdata.
> 
> If I check localdata.getVar(), the variable name doesn't appear in the
> list of dependencies.
> 
> If instead I check d.getVar(), it does appear in the list of dependencies.
> 
> It looks like localdata is used so that certain variables can be set
> without the changes affecting anything else (PKG, OVERRIDES, ROOT,
> etc). But there are also some calls to localdata.getVar() for
> PACKAGE_ARCH and ALLOW_EMPTY.
> 
> With my check changed to d.getVar(), the dependencies are: (['PKGE',
> 'IPK_HIERARCHICAL_FEED', 'get_package_additional_metadata',
> 'PKGWRITEDIRIPK', 'OPKGBUILDCMD', 'mapping_rename_hook', 'PKGDEST',
> 'PACKAGES', 'HOMEPAGE'])
> 
> Does this mean changes to PACKAGE_ARCH and ALLOW_EMPTY may go unnoticed?

It may well do and I've realised the issue:

codeparser.py:
class PythonParser():
    getvars = ("d.getVar", "bb.data.getVar", "data.getVar", "d.appendVar", "d.prependVar")

we probably need to change this to an .endswith(".getVar", ".appendVar",
".prependVar") type check...

Cheers,

Richard
Ross Burton - May 3, 2014, 9:03 a.m.
On 2 May 2014 23:07, Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
> It may well do and I've realised the issue:
>
> codeparser.py:
> class PythonParser():
>     getvars = ("d.getVar", "bb.data.getVar", "data.getVar", "d.appendVar", "d.prependVar")
>
> we probably need to change this to an .endswith(".getVar", ".appendVar",
> ".prependVar") type check...

Could this be the root-cause of so many instances where we've had to
add explicit hints that variables should be added to the hash?

Ross
Richard Purdie - May 3, 2014, 9:08 a.m.
On Sat, 2014-05-03 at 10:03 +0100, Burton, Ross wrote:
> On 2 May 2014 23:07, Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
> > It may well do and I've realised the issue:
> >
> > codeparser.py:
> > class PythonParser():
> >     getvars = ("d.getVar", "bb.data.getVar", "data.getVar", "d.appendVar", "d.prependVar")
> >
> > we probably need to change this to an .endswith(".getVar", ".appendVar",
> > ".prependVar") type check...
> 
> Could this be the root-cause of so many instances where we've had to
> add explicit hints that variables should be added to the hash?

Is there really "so many"?

In each case we've seen its been due to the code being python that the
parser can't expand. A simple example that trips it up:

a = "FOO"
d.getVar(a)

We've only added in fixups where the missing variables were adequately
explained. The other set of things we manually tune are exclusions since
there are some variables like the build path that we want to actively
exclude.

However the majority of the accesses we make are simple and the parser
can find them...

Cheers,

Richard
Ross Burton - May 3, 2014, 8:46 p.m.
On 3 May 2014 10:08, Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
>> Could this be the root-cause of so many instances where we've had to
>> add explicit hints that variables should be added to the hash?
>
> Is there really "so many"?
>
> In each case we've seen its been due to the code being python that the
> parser can't expand. A simple example that trips it up:

Maybe I'm biased from my experience with the systemd class but
thinking about the situation which caused me pain, it was through a
localdata.getVar so this would solve it.

Ross

Patch

From 75e80008b2f6bb702bd49c47eca6e61b6a047373 Mon Sep 17 00:00:00 2001
From: Paul Barker <paul@paulbarker.me.uk>
Date: Fri, 2 May 2014 14:51:35 +0000
Subject: [PATCH] package_ipk.bbclass: Support hierarchical feed
To: openembedded-core@lists.openembedded.org

This patch allows for an optional new layout for ipk feed directories which I've
called a 'hierarchical feed' and is based on how Debian pools package files. It
is disabled by default and is enabled by setting IPK_HIERARCHICAL_FEED to "1".

In the traditional feed layout, package files are placed in <outdir>/<arch>/.
This can lead to several thousand files existing in a single directory which is
often a problem if developers want to upload a package feed to a shared web
hosting provider. For example, in my case, listing files via FTP only shows the
first 2000 files, breaking my scripts which attempt to upload only new and
changed files via FTP.

In the hierarchical feed, package files are written to
<outdir>/<arch>/<pkg_prefix>/<pkg_subdir>, where pkg_prefix is the first letter
of the package file name for non-lib packages or "lib" plus the 4th letter of
the pacakge file name for lib pacakges (eg, 'l' for less, 'libc' for libc6).
pkg_subdir is the root of the pacakge file name, discarding the version and
architecture parts and the common suffixes '-dbg', '-dev', '-doc', '-staticdev'
and '-locale-*'.

This change relies on recent patches to opkg-utils which support hierarchical
package feeds.

Signed-off-by: Paul Barker <paul@paulbarker.me.uk>
---
 meta/classes/package_ipk.bbclass | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/meta/classes/package_ipk.bbclass b/meta/classes/package_ipk.bbclass
index 2949d1d..680b9bf 100644
--- a/meta/classes/package_ipk.bbclass
+++ b/meta/classes/package_ipk.bbclass
@@ -63,7 +63,26 @@  python do_package_ipk () {
         bb.data.update_data(localdata)
         basedir = os.path.join(os.path.dirname(root))
         arch = localdata.getVar('PACKAGE_ARCH', True)
-        pkgoutdir = "%s/%s" % (outdir, arch)
+        if localdata.getVar('IPK_HIERARCHICAL_FEED') == "1":
+            # Spread packages across subdirectories so each isn't too crowded
+            if pkgname.startswith('lib'):
+                pkg_prefix = 'lib' + pkgname[3]
+            else:
+                pkg_prefix = pkgname[0]
+
+            # Keep -dbg, -dev, -doc, -staticdev and -locale-* packages together
+            if pkgname[-4:] in ('-dbg', '-dev', '-doc'):
+                pkg_subdir = pkgname[:-4]
+            elif pkgname.endswith('-staticdev'):
+                pkg_subdir = pkgname[:-10]
+            elif '-locale-' in pkgname:
+                pkg_subdir = pkgname[:pkgname.find('-locale-')]
+            else:
+                pkg_subdir = pkgname
+
+            pkgoutdir = "%s/%s/%s/%s" % (outdir, arch, pkg_prefix, pkg_subdir)
+        else:
+            pkgoutdir = "%s/%s" % (outdir, arch)
         bb.utils.mkdirhier(pkgoutdir)
         os.chdir(root)
         cleanupcontrol(root)
-- 
1.9.2