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

[Richard Purdie <richard.purdie@linuxfoundation.org>]

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.

> +        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.

> +        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?

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

Cheers,

Richard