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 7E88C138247 for ; Fri, 3 Jan 2014 06:52:08 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 9E885E0A9D; Fri, 3 Jan 2014 06:52:06 +0000 (UTC) Received: from smtp.gentoo.org (smtp.gentoo.org [140.211.166.183]) (using TLSv1 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id 05313E0A9D for ; Fri, 3 Jan 2014 06:52:05 +0000 (UTC) Received: from [192.168.1.210] (S010600222de111ff.vc.shawcable.net [96.49.5.156]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: dolsen) by smtp.gentoo.org (Postfix) with ESMTPSA id BA4C733F608 for ; Fri, 3 Jan 2014 06:52:04 +0000 (UTC) Message-ID: <1388731920.3753.14.camel@big_daddy.dol-sen.ca> Subject: Re: [gentoo-catalyst] [PATCH 1/2] modules/generic_stage_target.py, Create SOURCE_MOUNTS_DEFAULTS From: Brian Dolbec To: gentoo-catalyst@lists.gentoo.org Date: Thu, 02 Jan 2014 22:52:00 -0800 In-Reply-To: <20140103062052.GJ4680@odin.tremily.us> References: <1388728844-2142-1-git-send-email-dolsen@gentoo.org> <1388728844-2142-2-git-send-email-dolsen@gentoo.org> <20140103062052.GJ4680@odin.tremily.us> Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-/b713NG/znYptsHWWDBZ" X-Mailer: Evolution 3.6.4 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 X-Archives-Salt: be55ec69-518b-4b14-a417-0520c9032894 X-Archives-Hash: 884dc471696b0756054df22811e0a623 --=-/b713NG/znYptsHWWDBZ Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2014-01-02 at 22:20 -0800, W. Trevor King wrote: > I shift quoted lines around for easier comparison. >=20 > On Thu, Jan 02, 2014 at 10:00:43PM -0800, Brian Dolbec wrote: > > +SOURCE_MOUNTS_DEFAULTS =3D { > > =E2=80=A6 > > + "distdir": "/usr/portage/distfiles", > > + "portdir": "/usr/portage", > > =E2=80=A6 > > - "distdir": self.settings["distdir"], > > - "portdir": normpath("/".join([ > > - self.settings["snapshot_cache_path"], > > - self.settings["repo_name"], > > - ])), > > =E2=80=A6 > > + # initialize our source mounts > > + self.mountmap =3D SOURCE_MOUNTS_DEFAULTS.copy() > > + # update them from settings > > + self.mountmap["distdir"] =3D self.settings["distdir"] > > + self.mountmap["portdir"] =3D normpath("/".join([ > > + self.settings["snapshot_cache_path"], > > + self.settings["repo_name"], > > + ])) >=20 > Why create dummy initial values and then blow them away? Wouldn't: >=20 > SOURCE_MOUNTS_DEFAULTS =3D { > =E2=80=A6 > 'distdir': None, # initialized from settings > 'portdir': None, # initialized from settings > } >=20 > make more sense? We'll blow away this default dict once we're loading > it via ConfigParser, so this doesn't have to be super elegant, but > adding values just to clobber them seems misleading. >=20 We probably could, they are defaulted in config_defaults. But at the time I was just thinking that catalyst.conf or a spec file may not have the value declared, so was better to give it a default here, just in case. > > - if "SNAPCACHE" in self.settings: > > - self.mounts =3D ["proc", "dev", "portdir", "distdir", "port_tmpdir"= ] > > =E2=80=A6 > > - else: > > - self.mounts =3D ["proc", "dev", "distdir", "port_tmpdir"] > > =E2=80=A6 > > + self.mounts =3D ["proc", "dev", "portdir", "distdir", "port_tmpdir"] > > =E2=80=A6 > > + if "SNAPCACHE" not in self.settings: > > + self.mounts.remove("portdir") >=20 > I'd prefer: >=20 > self.mounts =3D ["proc", "dev", "distdir", "port_tmpdir"] > if "SNAPCACHE" in self.settings: > self.mounts.append("portdir") >=20 > Cheers, > Trevor >=20 I didn't do that because I didn't know if the mount order was important, so erred on the side of the current order. --=-/b713NG/znYptsHWWDBZ Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQF8BAABCgBmBQJSxl4QXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXQ4Njg4RkQxQ0M3MUMxQzA0RUFFQTQyMzcy MjE0RDkwQTAxNEYxN0NCAAoJECIU2QoBTxfLkSsIAIQL11eAdQDyl0Rh7EYBgeAm +mS4blGZosGz2KHh0zE4ce1vf7SYbpo4tYQGB6sVn24HBeOo3UemN77vxWw271OT XuGXH3QDMbw2/dsf21eebOEcZ1IUHhCUo8vARS3l4A1heeFl8RGK9sSfrdjBqBzc V5vguhvrT+UI8xSRfdNGLMGbiyiRLkh/wcEuKqccg0pdQ0nGcsQFEvisOprhSy05 Edx6jxQo4VeJu0EAFL4g4rX8Au0lKWqrlpGDichy5PmwTLF5tIf+F2vTVOTRCO7y Akl3suEEc6qfYJcXu+x3OcuprujXlXtJbJspvx1ER1iiDL1//0sDV5cqlyc0HCE= =czP+ -----END PGP SIGNATURE----- --=-/b713NG/znYptsHWWDBZ--