[gentoo-catalyst] [PATCH] The string module is deprecated, stop using it.

[gentoo-catalyst] [PATCH] The string module is deprecated, stop using it.

[antarus]

From: Alec Warner <antarus@gentoo.org>

Also the old code called string.split() on the same input
a dozen times. Just split the string once (its immutable).
We can re-use the result for all the checking.

TESTS=I ran catalyst -f examples/stage4_template.spec and it
didn't until it was supposed to.
---
 catalyst | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/catalyst b/catalyst
index 4ea4248..edc0b24 100755
--- a/catalyst
+++ b/catalyst
@@ -10,7 +10,6 @@
 import os
 import sys
 import imp
-import string
 import getopt
 import pdb
 import os.path
@@ -116,12 +115,13 @@ def parse_config(myconfig):
 			print "Setting",x,"to default value \""+confdefaults[x]+"\""
 			conf_values[x]=confdefaults[x]
 
+	options = conf_values["options"].split()
 	# parse out the rest of the options from the config file
-	if "autoresume" in string.split(conf_values["options"]):
+	if "autoresume" in options:
 		print "Autoresuming support enabled."
 		conf_values["AUTORESUME"]="1"
 
-	if "bindist" in string.split(conf_values["options"]):
+	if "bindist" in options:
 		print "Binary redistribution enabled"
 		conf_values["BINDIST"]="1"
 	else:
@@ -129,43 +129,43 @@ def parse_config(myconfig):
 		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."
 
-	if "ccache" in string.split(conf_values["options"]):
+	if "ccache" in options:
 		print "Compiler cache support enabled."
 		conf_values["CCACHE"]="1"
 
-	if "clear-autoresume" in string.split(conf_values["options"]):
+	if "clear-autoresume" in options:
 		print "Cleaning autoresume flags support enabled."
 		conf_values["CLEAR_AUTORESUME"]="1"
 
-	if "distcc" in string.split(conf_values["options"]):
+	if "distcc" in options:
 		print "Distcc support enabled."
 		conf_values["DISTCC"]="1"
 
-	if "icecream" in string.split(conf_values["options"]):
+	if "icecream" in options:
 		print "Icecream compiler cluster support enabled."
 		conf_values["ICECREAM"]="1"
 
-	if "kerncache" in string.split(conf_values["options"]):
+	if "kerncache" in options:
 		print "Kernel cache support enabled."
 		conf_values["KERNCACHE"]="1"
 
-	if "pkgcache" in string.split(conf_values["options"]):
+	if "pkgcache" in options:
 		print "Package cache support enabled."
 		conf_values["PKGCACHE"]="1"
 
-	if "preserve_libs" in string.split(conf_values["options"]):
+	if "preserve_libs" in options:
 		print "Preserving libs during unmerge."
 		conf_values["PRESERVE_LIBS"]="1"
 
-	if "purge" in string.split(conf_values["options"]):
+	if "purge" in options:
 		print "Purge support enabled."
 		conf_values["PURGE"]="1"
 
-	if "seedcache" in string.split(conf_values["options"]):
+	if "seedcache" in options:
 		print "Seed cache support enabled."
 		conf_values["SEEDCACHE"]="1"
 
-	if "snapcache" in string.split(conf_values["options"]):
+	if "snapcache" in options:
 		print "Snapshot cache support enabled."
 		conf_values["SNAPCACHE"]="1"
 
-- 
1.8.1.2

Re: [gentoo-catalyst] [PATCH] The string module is deprecated, stop using it.

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]

On Wed, Dec 18, 2013 at 09:18:06PM -0800, antarus@gentoo.org wrote:
> @@ -116,12 +115,13 @@ def parse_config(myconfig):
>  			print "Setting",x,"to default value \""+confdefaults[x]+"\""
>  			conf_values[x]=confdefaults[x]
>  
> +	options = conf_values["options"].split()
>  	# parse out the rest of the options from the config file
> -	if "autoresume" in string.split(conf_values["options"]):
> +	if "autoresume" in options:
>  		print "Autoresuming support enabled."
>  		conf_values["AUTORESUME"]="1"
>  
> -	if "bindist" in string.split(conf_values["options"]):
> +	if "bindist" in options:
>  		print "Binary redistribution enabled"
>  		conf_values["BINDIST"]="1"
>  	else:

These changes look fine to me, but I'd rather just convert to the
stdlib's ConfigParser.  That's going to blow away this whole function,
so I don't see the point in fixing it up now.  Of course, if the
powers that be think a ConfigParser conversion is in the far-distant
future, then I'm ok with this as a temporary cleanup.

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [gentoo-catalyst] [PATCH] The string module is deprecated, stop using it.

[Brian Dolbec]

[-- Attachment #1: Type: text/plain, Size: 1531 bytes --]

On Wed, 2013-12-18 at 22:39 -0800, W. Trevor King wrote:
> On Wed, Dec 18, 2013 at 09:18:06PM -0800, antarus@gentoo.org wrote:
> > @@ -116,12 +115,13 @@ def parse_config(myconfig):
> >  			print "Setting",x,"to default value \""+confdefaults[x]+"\""
> >  			conf_values[x]=confdefaults[x]
> >  
> > +	options = conf_values["options"].split()
> >  	# parse out the rest of the options from the config file
> > -	if "autoresume" in string.split(conf_values["options"]):
> > +	if "autoresume" in options:
> >  		print "Autoresuming support enabled."
> >  		conf_values["AUTORESUME"]="1"
> >  
> > -	if "bindist" in string.split(conf_values["options"]):
> > +	if "bindist" in options:
> >  		print "Binary redistribution enabled"
> >  		conf_values["BINDIST"]="1"
> >  	else:
> 
> These changes look fine to me, but I'd rather just convert to the
> stdlib's ConfigParser.  That's going to blow away this whole function,
> so I don't see the point in fixing it up now.  Of course, if the
> powers that be think a ConfigParser conversion is in the far-distant
> future, then I'm ok with this as a temporary cleanup.
> 
> Cheers,
> Trevor
> 

There's more, I have a commit in the rewrite that removes the
duplication of options and the resulting standalone boolean variables.
Instead, I turn options into a set which eliminates the possibility of
duplicate entries, plus inclusion automatically implies  a True value,
absence a False value.

So even if this was merged, it'd be wiped away soon anyway.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 620 bytes --]