On Wed, Sep 10, 2014 at 08:43:40PM -0700, Brian Dolbec wrote: > -valid_config_file_values.extend(["PKGCACHE", "KERNCACHE", "CCACHE", "DISTCC", > - "ICECREAM", "ENVSCRIPT", "AUTORESUME", "FETCH", "CLEAR_AUTORESUME", > - "options", "DEBUG", "VERBOSE", "PURGE", "PURGEONLY", "SNAPCACHE", > - "snapshot_cache", "hash_function", "digests", "contents", "SEEDCACHE" > +valid_config_file_values.extend([ "distcc", "envscript", > + "options", "DEBUG", "VERBOSE", > + "snapshot_cache", "hash_function", "digests", "contents"
I think you meant to drop 'distcc' from valid_config_file_values,
since it's just a boolean, and you do have it in the new
option_messages:
> +# legend: key: message
> +option_messages = {
> + "autoresume": "Autoresuming support enabled.",
> + "ccache": "Compiler cache support enabled.",
> + "clear-autoresume": "Cleaning autoresume flags support enabled.",
> + #"compress": "Compression enabled.",
> + "distcc": "Distcc support enabled.",
> + "icecream": "Icecream compiler cluster support enabled.",
> + "kerncache": "Kernel cache support enabled.",
> + "pkgcache": "Package cache support enabled.",
> + "purge": "Purge support enabled.",
> + "seedcache": "Seed cache support enabled.",
> + "snapcache": "Snapshot cache support enabled.",
> + #"tarball": "Tarball creation enabled.",
> + }
I'd also remove the tarball entry, since we haven't used TARBALL since
9be28ed1 (Fixed up action_sequence when using --fetchonly to not
create tarballs or IS O images for bug #143392, 2006-11-22).
> - if "bindist" in string.split(conf_values["options"]):
> - print "Binary redistribution enabled"
> - conf_values["BINDIST"]="1"
> - else:
> - print "Bindist is not enabled in catalyst.conf"
> - print "Binary redistribution of generated stages/isos may be
> prohibited by law."
> - print "Please see the use description for bindist on any
> package you are including."
Hmm, I think you need a 'bindist' entry in option_messages. And I'd
probably tack on the legal warning to the enabled message.
> + #print "MAIN: cli options =", options
You don't need to add this ;).
> + conf_values["options"].update(options)
> + #print "MAIN: conf_values['options'] =", conf_values["options"]
Or this #print.
> @@ -501,8 +501,8 @@ class generic_stage_target(generic_target):
>
> "base_dirs","bind","chroot_setup","setup_environment",\
> "run_local","preclean","unbind","clean"]
> # if "TARBALL" in self.settings or \
> -# "FETCH" not in self.settings:
> - if "FETCH" not in self.settings:
> +# "fetch" not in self.settings["options"]:
> + if "fetch" not in self.settings["options"]:
> self.settings["action_sequence"].append("capture")
> self.settings["action_sequence"].append("clear_autoresume")
I'd just remove the whole comment, since we don't use TARBALL (see my
earlier comment).
> @@ -1285,7 +1292,14 @@ class generic_stage_target(generic_target):
> fixed. We need this to use the os.system() call since we can't
> specify our own environ
> """
> - for x in self.settings.keys():
> + #print "setup_environment(); settings =", list(self.settings)
> + for x in list(self.settings):
> + #print "setup_environment(); processing:", x
> + if x == "options":
> + #self.env['clst_' + x] = '
> '.join(self.settings[x])
> + for opt in self.settings[x]:
> + self.env['clst_' + opt.upper()] = "true"
> + continue
> """ Sanitize var names by doing "s|/-.|_|g" """
> varname="clst_"+string.replace(x,"/","_")
> varname=string.replace(varname,"-","_")
No need to add the commented lines. I don't understand the switch
from self.settings.keys() to list(self.settings) either, is that just
churn?
> def build_kernel(self):
> - "Build all configured kernels"
> + '''Build all configured kernels'''
This seems like unnecessary churn ;).
> # if "TARBALL" in self.settings or \
> -# "FETCH" not in self.settings:
> - if "FETCH" not in self.settings:
> +# "fetch" not in self.settings['options']:
> + if "fetch" not in self.settings['options']:
> self.settings["action_sequence"].append("capture")
> self.settings["action_sequence"].append("clear_autoresume")
Goodbye comment (no more TARBALL).
I haven't tested this one yet (and it would be hard to do thoroughly
with so many options on the line), but it looks good other than the
things I mentioned ;). I'm very positive on the options set for
boolean configs.
Cheers,
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
signature.asc
Description: OpenPGP digital signature
