From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) by finch.gentoo.org (Postfix) with ESMTP id B1878138A1F for ; Thu, 18 Sep 2014 13:01:18 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 3C50BE0980; Thu, 18 Sep 2014 13:01:17 +0000 (UTC) Received: from resqmta-po-04v.sys.comcast.net (resqmta-po-04v.sys.comcast.net [96.114.154.163]) (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id D059BE0980 for ; Thu, 18 Sep 2014 13:01:16 +0000 (UTC) Received: from resomta-po-16v.sys.comcast.net ([96.114.154.240]) by resqmta-po-04v.sys.comcast.net with comcast id sczn1o0055BUCh401d0uSx; Thu, 18 Sep 2014 13:00:54 +0000 Received: from odin.tremily.us ([24.18.63.50]) by resomta-po-16v.sys.comcast.net with comcast id sd0Y1o004152l3L01d0YKB; Thu, 18 Sep 2014 13:00:54 +0000 Received: by odin.tremily.us (Postfix, from userid 1000) id 0720B13A2DC5; Thu, 18 Sep 2014 06:00:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tremily.us; s=odin; t=1411045231; bh=3aG+yDYas0Gqq1byO3s3EyGGx2LpxGgzO/Ltvv1f2zo=; h=Date:From:To:Subject:References:In-Reply-To; b=ca1pUQEH9wDCzsRbjms6nzMkGt5ydFM8x6xF19gIf0K1RVCh2NTqVcNd8hQGRJgQ1 UwohfH9KyHkYxWm68AUlbHb3LPcM15Pe/ZoLl4VGObEsQ/edHOcp05EcW4sTqDgp88 7nB3O0zcLmP91MsZHhxgXPBcPTTOMLYgSVSpjXNQ= Date: Thu, 18 Sep 2014 06:00:30 -0700 From: "W. Trevor King" To: gentoo-catalyst@lists.gentoo.org Subject: Re: [gentoo-catalyst] [PATCH 1/5] Some options cleanup, unifying their use, reducing redundancy. Message-ID: <20140918130030.GN5139@odin.tremily.us> References: <1410407024-2445-1-git-send-email-dolsen@gentoo.org> <1410407024-2445-2-git-send-email-dolsen@gentoo.org> Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-catalyst@lists.gentoo.org Reply-to: gentoo-catalyst@lists.gentoo.org MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="/JIF1IJL1ITjxcV4" Content-Disposition: inline In-Reply-To: <1410407024-2445-2-git-send-email-dolsen@gentoo.org> OpenPGP: id=39A2F3FA2AB17E5D8764F388FC29BDCDF15F5BE8; url=http://tremily.us/pubkey.txt User-Agent: Mutt/1.5.23 (2014-03-12) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=comcast.net; s=q20140121; t=1411045254; bh=LprDiJ79yYDpUFU6Ys4ajfFKg75FqlPf2Ic0J/xaDzQ=; h=Received:Received:Received:Date:From:To:Subject:Message-ID: MIME-Version:Content-Type; b=VlCjbCHx7fXLGK19EUakQa0Z5d2lmuCTAbBmmtqGUGAa5HC6X/iQSHpzXBoefIZ+G PeNiQUyZDKMIy4AeOZd1t+ha6JOvK+5F6BSh7yFLvQ6tPI6wBMVaTuQSEdW/4z6Xsv I/EZVhWfUBIZFFTjV5iD3Zp/hSTXGXOiPE/f21Xn6/OuY+VFMTRaOvJElWdOgmuVmT zv2ZopeEKfmTeQtdFW9zNp/JHOykLWwvWpsn90BbA/dgaUSg/uXpJvnDz+7lOkICEi PtHoZSyKzDg6lDxezU59UkMnGDmyY/vz3CVUb2m+mSiHYB3beWeuetOZooaScdbI9z Z+1eNb2MlOT6Q== X-Archives-Salt: 7a867e4e-7ea0-4a7f-a13b-a1b868525f05 X-Archives-Hash: 7e0207bae355d48741b384321a200363 --/JIF1IJL1ITjxcV4 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Sep 10, 2014 at 08:43:40PM -0700, Brian Dolbec wrote: > -valid_config_file_values.extend(["PKGCACHE", "KERNCACHE", "CCACHE", "DIS= TCC", > - "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 =3D { > + "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"]=3D"1" > - else: > - print "Bindist is not enabled in catalyst.conf" > - print "Binary redistribution of generated stages/isos may be prohibite= d by law." > - print "Please see the use description for bindist on any package you a= re 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 =3D", options You don't need to add this ;). > + conf_values["options"].update(options) > + #print "MAIN: conf_values['options'] =3D", 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). =20 > @@ -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 =3D", list(self.settings) > + for x in list(self.settings): > + #print "setup_environment(); processing:", x > + if x =3D=3D "options": > + #self.env['clst_' + x] =3D ' '.join(self.settings[x]) > + for opt in self.settings[x]: > + self.env['clst_' + opt.upper()] =3D "true" > + continue > """ Sanitize var names by doing "s|/-.|_|g" """ > varname=3D"clst_"+string.replace(x,"/","_") > varname=3Dstring.replace(varname,"-","_") No need to add the commented lines. I don't understand the switch =66rom 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 --=20 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 --/JIF1IJL1ITjxcV4 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJUGtdqAAoJEG8/JgBt8ol8cFYQAKmbZlrNKYoBtD6dRifuZhgb V6pyxuWTR9mbUmyPwXJh2Og7HeDqIlqYMpes3zSCaiOVdhaoRyGT9icbKBK/SLQp gLkMh45ZLV5C54LGeb+5JNQsAslG1JMbDPjfyuwTxBodznT5LwRgmcgP9p5upeHV LeV8/t6s2P7/KdKJe9cxzuT+M6FHh2/BNvpBg09WE+bEchnSTUGfGc0S2NnPcaYf TlyloBEZcPKINcW4YdMpuGj+ZQud0zc8/vjUqrlUAkM0lbHy3IsQSJlWoojvUFkj MRrax+zn6GainG3pZ56Xxd6jM+J8mHrmX+po+mH//+htF+I7nJc+0MpZdx+LF2S6 6JdVKHj1VkIqeqLk14Zulw5bBPfZeudLTfjz4THcnuuZLqNnJfOVTUuRiodjEXdF PVg+dMmw9XILxUA5n8bMJ8j6atzWtrBvuVL8+3jHKDtxzheMzYZSp4Jh5FLmxcd7 ZyX2xfPSxPBNVyRP194k09ldOJJJVRVCSDn5roDMPddu6/iMLFwQ72VJlLSrixE0 BbkkJYvltgJoFSlUpK6OJhVzZaqymIyxrhZbg8vQlkXTiQO3Jp3f7ClzXP3+k4J1 CPe1+PpZi+oky5t6vNyaul3mWqDpgHSlFeKAhPKxYXkD1+PhqywhDypBQFRhAK7C 4V2Koh5yW4Fovw8ufS+E =GRn7 -----END PGP SIGNATURE----- --/JIF1IJL1ITjxcV4--