Re: [PATCH 3/6] command|cooker: Add reparseFiles command

[Joshua Lock <josh@linux.intel.com>]

On Fri, 2011-07-15 at 23:31 +0100, Richard Purdie wrote:
> On Fri, 2011-07-15 at 13:20 -0700, Joshua Lock wrote:
> > Add command reparseFiles to reparse bb files in a running cooker instance.
> > 
> > Fixes [YOCTO #1249]
> > 
> > Signed-off-by: Joshua Lock <josh@linux.intel.com>
> > ---
> >  lib/bb/command.py |    8 +++++++
> >  lib/bb/cooker.py  |   54 +++++++++++++++++++++++++++++++++-------------------
> >  2 files changed, 42 insertions(+), 20 deletions(-)
> > 
> 
> > diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
> > index 23f2c76..e4daaef 100644
> > --- a/lib/bb/cooker.py
> > +++ b/lib/bb/cooker.py
> > @@ -122,21 +122,8 @@ class BBCooker:
> >                  logger.critical("Unable to import extra RecipeInfo '%s' from '%s': %s" % (cache_name, module_name, exc))
> >                  sys.exit("FATAL: Failed to import extra cache class '%s'." % cache_name)
> >  
> > -        self.configuration.data = bb.data.init()
> > -
> > -        bb.data.inheritFromOS(self.configuration.data)
> > -
> > -        try:
> > -            self.parseConfigurationFiles(self.configuration.prefile,
> > -                                         self.configuration.postfile)
> > -        except SyntaxError:
> > -            sys.exit(1)
> > -        except Exception:
> > -            logger.exception("Error parsing configuration files")
> > -            sys.exit(1)
> > -
> > -        if not self.configuration.cmd:
> > -            self.configuration.cmd = bb.data.getVar("BB_DEFAULT_TASK", self.configuration.data, True) or "build"
> > +        self.configuration.data = None
> > +        self.loadConfigurationData()
> >  
> >          bbpkgs = bb.data.getVar('BBPKGS', self.configuration.data, True)
> >          if bbpkgs and len(self.configuration.pkgs_to_build) == 0:
> > @@ -163,6 +150,26 @@ class BBCooker:
> >  
> >          self.parser = None
> >  
> > +    def loadConfigurationData(self):
> > +        self.configuration.data = bb.data.init()
> > +
> > +        if not self.server_registration_cb:
> > +            bb.data.setVar("BB_WORKERCONTEXT", "1", self.configuration.data)
> 
> This looks like one of the last remaining differences between the
> bitbake in poky and the upstream. See:
> 
> http://git.openembedded.net/cgit.cgi/bitbake/commit/?h=master-poky&id=a67671bcdd92f2ae3cd364b2addefef70a3c2398
> 
> but bottom line is this shouldn't be here.

Whoops.

> 
> > +        bb.data.inheritFromOS(self.configuration.data)
> 
> and this line is the one that worries me since it pokes around the
> environment. If we do this later, we're likely using a different
> environment and I'm not happy this is going to give good results.
> 
> This actually ties into the terminal bbclass issues Chris has been
> proposing recently. I think we need to store off the original
> environment into a datastore and then leave it somewhere fixed we can
> recall it later for the likes of this function and the terminal class
> code.

Ah, that's a better idea. I'll take a stab at something to do that.

> 
> > +        try:
> > +            self.parseConfigurationFiles(self.configuration.prefile,
> > +                                         self.configuration.postfile)
> > +        except SyntaxError:
> > +            sys.exit(1)
> > +        except Exception:
> > +            logger.exception("Error parsing configuration files")
> > +            sys.exit(1)
> > +
> > +        if not self.configuration.cmd:
> > +            self.configuration.cmd = bb.data.getVar("BB_DEFAULT_TASK", self.configuration.data, True) or "build"
> > +
> >      def parseConfiguration(self):
> >  
> > 
> > @@ -508,7 +515,7 @@ class BBCooker:
> >          bb.data.expandKeys(localdata)
> >  
> >          # Handle PREFERRED_PROVIDERS
> > -        for p in (bb.data.getVar('PREFERRED_PROVIDERS', localdata, 1) or "").split():
> > +        for p in (bb.data.getVar('PREFERRED_PROVIDERS', localdata, True) or "").split():
> >              try:
> >                  (providee, provider) = p.split(':')
> >              except:
> > @@ -999,8 +1006,8 @@ class BBCooker:
> >  
> >          self.server_registration_cb(buildTargetsIdle, rq)
> >  
> > -    def updateCache(self):
> > -        if self.state == state.running:
> > +    def updateCache(self, force=False):
> > +        if self.state == state.running and not force:
> >              return
> >  
> >          if self.state in (state.shutdown, state.stop):
> > @@ -1010,6 +1017,8 @@ class BBCooker:
> >          if self.state != state.parsing:
> >              self.parseConfiguration ()
> >  
> > +            if self.status:
> > +                del self.status
> >              self.status = bb.cache.CacheData(self.caches_array)
> >  
> >              ignore = bb.data.getVar("ASSUME_PROVIDED", self.configuration.data, 1) or ""
> > @@ -1083,7 +1092,7 @@ class BBCooker:
> >  
> >          collectlog.debug(1, "collecting .bb files")
> >  
> > -        files = (data.getVar( "BBFILES", self.configuration.data, 1 ) or "").split()
> > +        files = (data.getVar( "BBFILES", self.configuration.data, True) or "").split()
> >          data.setVar("BBFILES", " ".join(files), self.configuration.data)
> >  
> >          # Sort files by priority
> > @@ -1140,7 +1149,8 @@ class BBCooker:
> >              base = os.path.basename(f).replace('.bbappend', '.bb')
> >              if not base in self.appendlist:
> >                 self.appendlist[base] = []
> > -            self.appendlist[base].append(f)
> > +            if f not in self.appendlist[base]:
> > +                self.appendlist[base].append(f)
> 
> append files only get appended once. Shouldn't that be a separate patch?

Quite possibly. I have a bad habit of spotting things and changing them
when I should be thinking about something else. I'll split this change
out next time. Must have missed it this try.

> 
> Otherwise the patch and the idea are ok, we just need to be careful to
> get this right...

Thanks for the detailed review. I'll work on a new revision to address
your concerns.

Regards,
Joshua
-- 
Joshua Lock
        Yocto Project "Johannes factotum"
        Intel Open Source Technology Centre