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 BC3831381F4 for ; Sat, 8 Dec 2012 06:02:14 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 8B5DEE06F7 for ; Sat, 8 Dec 2012 06:02:13 +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 81595E06C1 for ; Sat, 8 Dec 2012 04:45:56 +0000 (UTC) Received: from mail-ye0-f181.google.com (mail-ye0-f181.google.com [209.85.213.181]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) (Authenticated sender: mattst88) by smtp.gentoo.org (Postfix) with ESMTPSA id 8BE3A33DDDB for ; Sat, 8 Dec 2012 04:45:55 +0000 (UTC) Received: by mail-ye0-f181.google.com with SMTP id m11so218509yen.40 for ; Fri, 07 Dec 2012 20:45:53 -0800 (PST) Received: by 10.236.118.17 with SMTP id k17mr11249105yhh.69.1354941953826; Fri, 07 Dec 2012 20:45:53 -0800 (PST) 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 Received: by 10.101.80.16 with HTTP; Fri, 7 Dec 2012 20:45:32 -0800 (PST) In-Reply-To: <50C26126.9060301@gentoo.org> References: <1354915982-29772-1-git-send-email-sidhayn@gmail.com> <50C26126.9060301@gentoo.org> From: Matt Turner Date: Fri, 7 Dec 2012 20:45:32 -0800 Message-ID: Subject: Re: [gentoo-catalyst] [PATCH] make bindist optional To: gentoo-catalyst@lists.gentoo.org Content-Type: text/plain; charset=ISO-8859-1 X-Archives-Salt: cc024ee4-bcf7-4b9b-b58a-545ed929cbce X-Archives-Hash: ec6502566edb723cb0090d0bc98227ab On Fri, Dec 7, 2012 at 1:35 PM, Rick "Zero_Chaos" Farina wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > > From: "Rick Farina (Zero_Chaos)" > > After the recent fixes which ensure the bindist use flag > is always set, users now have no way to disable this flag. > This patch introduces the new "bindist" feature, enabled by > default, which will allow users to turn off bindist if > they are not going to redistribute the builds (or for > tinderbox testing, etc). > - --- > catalyst | 7 +++++++ > files/catalyst.conf | 4 +++- > modules/generic_stage_target.py | 5 +++-- > modules/grp_target.py | 11 ++++++----- > modules/livecd_stage1_target.py | 6 ++++-- > targets/stage1/stage1-chroot.sh | 10 +++++++--- > 6 files changed, 30 insertions(+), 13 deletions(-) > > diff --git a/catalyst b/catalyst > index 3d31599..8485984 100755 > - --- a/catalyst > +++ b/catalyst > @@ -112,6 +112,13 @@ def parse_config(myconfig): > print "Autoresuming support enabled." > conf_values["AUTORESUME"]="1" > > + 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 is prohibited > by law." > + > if "ccache" in string.split(conf_values["options"]): > print "Compiler cache support enabled." > conf_values["CCACHE"]="1" > diff --git a/files/catalyst.conf b/files/catalyst.conf > index ea74eb1..18c6ab8 100644 > - --- a/files/catalyst.conf > +++ b/files/catalyst.conf > @@ -49,6 +49,8 @@ hash_function="crc32" > # the -a option to the catalyst cmdline. -p will clear the autoresume > flags > # as well as your pkgcache and kerncache > # ( This option is not fully tested, bug reports welcome ) > +# bindist = enable this option when the builds will be publicly > redistributed to prevent > + building of patent encumbered and redistribution restricted items. > # ccache = enables build time ccache support (highly recommended) > # distcc = enable distcc support for building. You have to set > distcc_hosts in > # your spec file. > @@ -64,7 +66,7 @@ hash_function="crc32" > # your cache. The cache is unlinked before any empty or rm processing, > though. > # > # (These options can be used together) > - -options="autoresume kerncache pkgcache seedcache snapcache" > +options="autoresume bindist kerncache pkgcache seedcache snapcache" > > # portdir specifies the source portage tree used by the snapshot target. > portdir="/usr/portage" > diff --git a/modules/generic_stage_target.py > b/modules/generic_stage_target.py > index 3597680..89082c5 100644 > - --- a/modules/generic_stage_target.py > +++ b/modules/generic_stage_target.py > @@ -490,8 +490,9 @@ class generic_stage_target(generic_target): > if type(self.settings["use"])==types.StringType: > self.settings["use"]=self.settings["use"].split() > > - - # Force bindist for all targets > - - self.settings["use"].append("bindist") > + # Force bindist when options ask for it > + if self.settings.has_key("BINDIST"): > + self.settings["use"].append("bindist") > > def set_stage_path(self): > self.settings["stage_path"]=normpath(self.settings["chroot_path"]) > diff --git a/modules/grp_target.py b/modules/grp_target.py > index 12eab08..5d3af2e 100644 > - --- a/modules/grp_target.py > +++ b/modules/grp_target.py > @@ -62,11 +62,12 @@ class grp_target(generic_stage_target): > raise CatalystError,"GRP build aborting due to error." > > def set_use(self): > - - generic_stage_target.set_use(self) > - - if self.settings.has_key("use"): > - - self.settings["use"].append("bindist") > - - else: > - - self.settings["use"]=["bindist"] > + generic_stage_target.set_use(self) > + if self.settings.has_key("BINDIST"): > + if self.settings.has_key("use"): > + self.settings["use"].append("bindist") > + else: > + self.settings["use"]=["bindist"] > > def set_mounts(self): > self.mounts.append("/tmp/grp") > diff --git a/modules/livecd_stage1_target.py > b/modules/livecd_stage1_target.py > index 17254bb..a27cfe4 100644 > - --- a/modules/livecd_stage1_target.py > +++ b/modules/livecd_stage1_target.py > @@ -48,10 +48,12 @@ class livecd_stage1_target(generic_stage_target): > generic_stage_target.set_use(self) > if self.settings.has_key("use"): > self.settings["use"].append("livecd") > - - self.settings["use"].append("bindist") > + if self.settings.has_key("BINDIST"): > + self.settings["use"].append("bindist") > else: > self.settings["use"]=["livecd"] > - - self.settings["use"].append("bindist") > + if self.settings.has_key("BINDIST"): > + self.settings["use"].append("bindist") This if has_key -> append otherwise create and append pattern is ugly. I wonder if there's a cleaner way to do this. I'll see what I can find. In any case, you can factor the bindist part out of the if has_key block. > def set_packages(self): > generic_stage_target.set_packages(self) > diff --git a/targets/stage1/stage1-chroot.sh > b/targets/stage1/stage1-chroot.sh > index e40982b..8e5a492 100644 > - --- a/targets/stage1/stage1-chroot.sh > +++ b/targets/stage1/stage1-chroot.sh > @@ -6,7 +6,11 @@ source /tmp/chroot-functions.sh > export clst_buildpkgs="$(/tmp/build.py)" > > # Setup our environment > - -BOOTSTRAP_USE="$(portageq envvar BOOTSTRAP_USE)" > +if [ -n "${clst_BINDIST}" ]; then > + BOOTSTRAP_USE="$(portageq envvar BOOTSTRAP_USE) bindist" > +else > + BOOTSTRAP_USE="$(portageq envvar BOOTSTRAP_USE)" > +fi I would change this to just BOOTSTRAP_USE="$(portageq envvar BOOTSTRAP_USE)" if [ -n "${clst_BINDIST}" ]; then BOOTSTRAP_USE="$BOOTSTRAP_USE bindist" fi > FEATURES="${clst_myfeatures} nodoc noman noinfo -news" > > ## Sanity check profile > @@ -50,8 +54,8 @@ sed -i '/USE="${USE} -build"/d' /etc/portage/make.conf > > # Now, we install our packages > [ -e /etc/portage/make.conf ] && \ > - - echo "USE=\"-* bindist build ${BOOTSTRAP_USE} ${clst_HOSTUSE}\"" \ > + echo "USE=\"-* build ${BOOTSTRAP_USE} ${clst_HOSTUSE}\"" \ > >> /etc/portage/make.conf > run_merge "--oneshot ${clst_buildpkgs}" > - -sed -i "/USE=\"-* bindist build ${BOOTSTRAP_USE} ${clst_HOSTUSE}\"/d" \ > +sed -i "/USE=\"-* build ${BOOTSTRAP_USE} ${clst_HOSTUSE}\"/d" \ > /etc/portage/make.conf > - -- > 1.7.8.6 Overall, the patch looks good. With the couple of little style changes and the wording change suggested by Mike & Jorge, this is Reviewed-by: Matt Turner