On Fri, Dec 7, 2012 at 1:35 PM, Rick "Zero_Chaos" Farina
<[email protected]> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>
> From: "Rick Farina (Zero_Chaos)" <[email protected]>
>
> 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 <[email protected]>
