[gentoo-catalyst] [PATCH 1/4] catalyst: Specify python2 rather than the generic python

[gentoo-catalyst] [PATCH 1/4] catalyst: Specify python2 rather than the generic python

[Dylan Baker]

This allows catalyst to work regardless of whether a user prefers that
usr/bin/python be python 2.x or 3.x.
---
 catalyst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/catalyst b/catalyst
index 4550d05..11560fb 100755
--- a/catalyst
+++ b/catalyst
@@ -1,4 +1,4 @@
-#!/usr/bin/python -OO
+#!/usr/bin/python2 -OO
 
 # Maintained in full by:
 # Catalyst Team <catalyst@gentoo.org>
-- 
1.8.1.5

[gentoo-catalyst] [PATCH 2/4] Catalyst: use a more pythonic method to import modules

[Dylan Baker]

Rather than appending a directory to the system path, this patch adds a
__init__.py file to modules, which allows python to search it, and it's
children for python modules. This with the import...as syntax allows for
a cleaner import of python modules without changing any other parts of
the catalyst file.
---
 catalyst            | 8 ++------
 modules/__init__.py | 0
 2 files changed, 2 insertions(+), 6 deletions(-)
 create mode 100644 modules/__init__.py

diff --git a/catalyst b/catalyst
index 11560fb..be1548f 100755
--- a/catalyst
+++ b/catalyst
@@ -11,12 +11,8 @@ import os, sys, imp, string, getopt
 import pdb
 import os.path
 
-__selfpath__ = os.path.abspath(os.path.dirname(__file__))
-
-sys.path.append(__selfpath__ + "/modules")
-
-import catalyst.config
-import catalyst.util
+import modules.catalyst.config as catalyst.config
+import modules.catalyst.util as catalyst.util
 
 __maintainer__="Catalyst <catalyst@gentoo.org>"
 __version__="2.0.14"
diff --git a/modules/__init__.py b/modules/__init__.py
new file mode 100644
index 0000000..e69de29
-- 
1.8.1.5

[gentoo-catalyst] Re: [PATCH 2/4] Catalyst: use a more pythonic method to import modules

[Dylan Baker]

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

On Friday, October 11, 2013 10:38:25 AM Dylan Baker wrote:
> Rather than appending a directory to the system path, this patch adds a
> __init__.py file to modules, which allows python to search it, and it's
> children for python modules. This with the import...as syntax allows for
> a cleaner import of python modules without changing any other parts of
> the catalyst file.
> ---
>  catalyst            | 8 ++------
>  modules/__init__.py | 0
>  2 files changed, 2 insertions(+), 6 deletions(-)
>  create mode 100644 modules/__init__.py
> 
> diff --git a/catalyst b/catalyst
> index 11560fb..be1548f 100755
> --- a/catalyst
> +++ b/catalyst
> @@ -11,12 +11,8 @@ import os, sys, imp, string, getopt
>  import pdb
>  import os.path
> 
> -__selfpath__ = os.path.abspath(os.path.dirname(__file__))
> -
> -sys.path.append(__selfpath__ + "/modules")
> -
> -import catalyst.config
> -import catalyst.util
> +import modules.catalyst.config as catalyst.config
> +import modules.catalyst.util as catalyst.util
> 
>  __maintainer__="Catalyst <catalyst@gentoo.org>"
>  __version__="2.0.14"
> diff --git a/modules/__init__.py b/modules/__init__.py
> new file mode 100644
> index 0000000..e69de29

Have you ever wondered why you had patches sitting in a branch for months 
without sending them to the list? It's probably because one in the middle 
doesn't work :(. That happens to be this one.

Expect a v2 with this patch fixed.

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

[gentoo-catalyst] [PATCH 3/4] catalyst: split combined import

[Dylan Baker]

Combining multiple modules into a single import is discouraged in
python's PEP8 style guide:

"""
Imports should usually be on separate lines, e.g.:

Yes: import os
     import sys

     No:  import sys, os
"""
---
 catalyst | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/catalyst b/catalyst
index be1548f..406250c 100755
--- a/catalyst
+++ b/catalyst
@@ -7,7 +7,11 @@
 # Chris Gianelloni <wolf31o2@wolf31o2.org>
 # $Id$
 
-import os, sys, imp, string, getopt
+import os
+import sys
+import imp
+import string
+import getopt
 import pdb
 import os.path
 
-- 
1.8.1.5

[gentoo-catalyst] [PATCH 4/4] catalyst: Remove commented sections of code

[Dylan Baker]

Code in the upstream tree should either run or it shouldn't be there.
---
 catalyst | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/catalyst b/catalyst
index 406250c..7d78e46 100755
--- a/catalyst
+++ b/catalyst
@@ -128,10 +128,6 @@ def parse_config(myconfig):
 		print "Cleaning autoresume flags support enabled."
 		conf_values["CLEAR_AUTORESUME"]="1"
 
-#	if "compress" in string.split(conf_values["options"]):
-#		print "Compression enabled."
-#		conf_values["COMPRESS"]="1"
-
 	if "distcc" in string.split(conf_values["options"]):
 		print "Distcc support enabled."
 		conf_values["DISTCC"]="1"
@@ -164,10 +160,6 @@ def parse_config(myconfig):
 		print "Snapshot cache support enabled."
 		conf_values["SNAPCACHE"]="1"
 
-#	if "tarball" in string.split(conf_values["options"]):
-#		print "Tarball creation enabled."
-#		conf_values["TARBALL"]="1"
-
 	if "digests" in myconf:
 		conf_values["digests"]=myconf["digests"]
 	if "contents" in myconf:
@@ -414,20 +406,3 @@ if __name__ == "__main__":
 		print "Catalyst aborting...."
 		raise
 		sys.exit(2)
-
-	#except CatalystError:
-	#	print
-	#	print "Catalyst aborting...."
-	#	sys.exit(2)
-	#except KeyError:
-	#	print "\nproblem with command line or spec file ( Key Error )"
-	#	print "Key: "+str(sys.exc_value)+" was not found"
-	#	print "Catalyst aborting...."
-	#	sys.exit(2)
-	#except UnboundLocalError:
-	#	print
-	#	print "UnboundLocalError: "+str(sys.exc_value)+" was not found"
-	#	raise
-	#	print
-	#	print "Catalyst aborting...."
-	#	sys.exit(2)
-- 
1.8.1.5

Re: [gentoo-catalyst] [PATCH 1/4] catalyst: Specify python2 rather than the generic python

[Brian Dolbec]

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

On Fri, 2013-10-11 at 10:38 -0700, Dylan Baker wrote:
> This allows catalyst to work regardless of whether a user prefers that
> usr/bin/python be python 2.x or 3.x.
> ---
>  catalyst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/catalyst b/catalyst
> index 4550d05..11560fb 100755
> --- a/catalyst
> +++ b/catalyst
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python -OO
> +#!/usr/bin/python2 -OO
>  
>  # Maintained in full by:
>  # Catalyst Team <catalyst@gentoo.org>


I'll reply to all 4 patches from this one.

This is not needed due to the way python apps are wrapped during
install.  bin/catalyst is renamed to bin/catalyst-python2.x and a
python-exec script replaces the original bin/catalyst.  This is only
good for running the code directly from the git checkout.  Also, we plan
to make the code python-3 capable as soon as a few more main code
changes are done.

As for the other patches, that work is already done in my rewrite and
will be taking over the master branch soon.  It is the 3.0 branch which
has been rebased to the rewrite-on-master branch.  There are a couple
rebase errors to fix on it still.  

http://git.overlays.gentoo.org/gitweb/?p=proj/catalyst.git

I have also been working on rewriting the compress/decompress code in
catalyst.  This new code, the compression type will be easily
configurable.  Decompression will autodetect the type needed by
extension (also configurable).  I still need to add the configure
portions of the code before merging that in.

my working code is at:
http://dev.gentoo.org/~dolsen/catalyst/


I have been unable to get any work done on it for awhile.  I had a work
accident making things difficult.  The we sold our house and moved.  I'm
still getting settled in and unpacked.  

I'll be getting back to it again very soon.

Join us in the #gentoo-releng channel.  We can discuss what else can be
worked on.  I need to update the todo list for the rewrite.



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

Re: [gentoo-catalyst] [PATCH 1/4] catalyst: Specify python2 rather than the generic python

[Matt Turner]

On Fri, Oct 11, 2013 at 11:15 AM, Brian Dolbec <dolsen@gentoo.org> wrote:
> On Fri, 2013-10-11 at 10:38 -0700, Dylan Baker wrote:
>> This allows catalyst to work regardless of whether a user prefers that
>> usr/bin/python be python 2.x or 3.x.
>> ---
>>  catalyst | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/catalyst b/catalyst
>> index 4550d05..11560fb 100755
>> --- a/catalyst
>> +++ b/catalyst
>> @@ -1,4 +1,4 @@
>> -#!/usr/bin/python -OO
>> +#!/usr/bin/python2 -OO
>>
>>  # Maintained in full by:
>>  # Catalyst Team <catalyst@gentoo.org>
>
>
> I'll reply to all 4 patches from this one.
>
> This is not needed due to the way python apps are wrapped during
> install.  bin/catalyst is renamed to bin/catalyst-python2.x and a
> python-exec script replaces the original bin/catalyst.

What do we gain by doing this in the ebuild's src_install() over doing
it in? Because...

> This is only
> good for running the code directly from the git checkout.

actually seems useful. We've had clearly broken commits go upstream,
and if the author had been able to test from a git checkout we
probably could have avoided that.

> Also, we plan
> to make the code python-3 capable as soon as a few more main code
> changes are done.

This isn't a reason to not specify the correct version of python.

> As for the other patches, that work is already done in my rewrite and
> will be taking over the master branch soon.  It is the 3.0 branch which
> has been rebased to the rewrite-on-master branch.  There are a couple
> rebase errors to fix on it still.
>
> http://git.overlays.gentoo.org/gitweb/?p=proj/catalyst.git

Sounds like it shouldn't be any trouble to rebase on top of Dylan's
patches, which are (pending the v2 of patch 2) read to be applied to
master.

What I mean is that I don't want to turn down contributions from new
developers because there's a big backlog of work that hasn't gone
upstream.

Re: [gentoo-catalyst] [PATCH 1/4] catalyst: Specify python2 rather than the generic python

[Brian Dolbec]

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

On Fri, 2013-10-11 at 12:28 -0700, Matt Turner wrote:
> On Fri, Oct 11, 2013 at 11:15 AM, Brian Dolbec <dolsen@gentoo.org> wrote:

> > This is only
> > good for running the code directly from the git checkout.
> 
> actually seems useful. We've had clearly broken commits go upstream,
> and if the author had been able to test from a git checkout we
> probably could have avoided that.

Which is why I made the rewrite code able to run from the checkout fully
and properly.  Just cd into the directory, run "source ./testpath" and
it's will run completely from the checkout.


> What I mean is that I don't want to turn down contributions from new
> developers because there's a big backlog of work that hasn't gone
> upstream.
> 

I don't want to discourage others either.  It is just much better to
encourage some help on the rewrite in my opinion.  Especially since
patches 2 & 3 have already been done in the rewrite branch.  Some of
patch 4 might have been done already, but likely not all.  If the
rewrite is to take over from the master branch...

The rewrite is not far from being able to take over as master.  There
are a few rebase errors in the rewrite-on-master you did.  There is some
cleanup work to do on the autoresume operation.  Then a little more
testing with the tree defaults relocated to ensure I haven't missed any
hard coding.

The default tree location move is waiting on the catalyst rewrite code
to go live producing stages, etc..

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

Re: [gentoo-catalyst] [PATCH 1/4] catalyst: Specify python2 rather than the generic python

[Matt Turner]

On Fri, Oct 11, 2013 at 2:11 PM, Brian Dolbec <dolsen@gentoo.org> wrote:
> On Fri, 2013-10-11 at 12:28 -0700, Matt Turner wrote:
>> On Fri, Oct 11, 2013 at 11:15 AM, Brian Dolbec <dolsen@gentoo.org> wrote:
>
>> > This is only
>> > good for running the code directly from the git checkout.
>>
>> actually seems useful. We've had clearly broken commits go upstream,
>> and if the author had been able to test from a git checkout we
>> probably could have avoided that.
>
> Which is why I made the rewrite code able to run from the checkout fully
> and properly.  Just cd into the directory, run "source ./testpath" and
> it's will run completely from the checkout.
>
>
>> What I mean is that I don't want to turn down contributions from new
>> developers because there's a big backlog of work that hasn't gone
>> upstream.
>>
>
> I don't want to discourage others either.  It is just much better to
> encourage some help on the rewrite in my opinion.

No, you should be moving patches that are reviewed and tested to
master (which means rebasing on master and sending patches to the
mailing list).

> Especially since
> patches 2 & 3 have already been done in the rewrite branch.  Some of
> patch 4 might have been done already, but likely not all.  If the
> rewrite is to take over from the master branch...
>
> The rewrite is not far from being able to take over as master.  There
> are a few rebase errors in the rewrite-on-master you did.  There is some
> cleanup work to do on the autoresume operation.  Then a little more
> testing with the tree defaults relocated to ensure I haven't missed any
> hard coding.

The development model on git is to make incremental changes that do
not break things. I've been saying this for a while.

The code needs to be reviewed as well. Maybe Dylan, who has been
cleaning up a lot of python code in another project, would be willing
to help review as well.

> The default tree location move is waiting on the catalyst rewrite code
> to go live producing stages, etc..

I'm exactly sure what this means, but I think you might mean something
like renaming master to old-master and your branch to master. That's
not the right way to do it, and that's not how git works.

Re: [gentoo-catalyst] [PATCH 1/4] catalyst: Specify python2 rather than the generic python

[Dylan Baker]

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

On Friday, October 11, 2013 02:58:22 PM Matt Turner wrote:
> On Fri, Oct 11, 2013 at 2:11 PM, Brian Dolbec <dolsen@gentoo.org> wrote:
> > On Fri, 2013-10-11 at 12:28 -0700, Matt Turner wrote:
> >> On Fri, Oct 11, 2013 at 11:15 AM, Brian Dolbec <dolsen@gentoo.org> wrote:
> >> > This is only
> >> > good for running the code directly from the git checkout.
> >> 
> >> actually seems useful. We've had clearly broken commits go upstream,
> >> and if the author had been able to test from a git checkout we
> >> probably could have avoided that.
> > 
> > Which is why I made the rewrite code able to run from the checkout fully
> > and properly.  Just cd into the directory, run "source ./testpath" and
> > it's will run completely from the checkout.
> > 
> >> What I mean is that I don't want to turn down contributions from new
> >> developers because there's a big backlog of work that hasn't gone
> >> upstream.
> > 
> > I don't want to discourage others either.  It is just much better to
> > encourage some help on the rewrite in my opinion.
> 
> No, you should be moving patches that are reviewed and tested to
> master (which means rebasing on master and sending patches to the
> mailing list).
> 
> > Especially since
> > patches 2 & 3 have already been done in the rewrite branch.  Some of
> > patch 4 might have been done already, but likely not all.  If the
> > rewrite is to take over from the master branch...
> > 
> > The rewrite is not far from being able to take over as master.  There
> > are a few rebase errors in the rewrite-on-master you did.  There is some
> > cleanup work to do on the autoresume operation.  Then a little more
> > testing with the tree defaults relocated to ensure I haven't missed any
> > hard coding.
> 
> The development model on git is to make incremental changes that do
> not break things. I've been saying this for a while.
> 
> The code needs to be reviewed as well. Maybe Dylan, who has been
> cleaning up a lot of python code in another project, would be willing
> to help review as well.

I'd be more than happy to help review things. I've paged through the rewrite-
on-master branch a bit, and there's definately some good work going on there, 
but there are also so things that could be polished up a bit.

> 
> > The default tree location move is waiting on the catalyst rewrite code
> > to go live producing stages, etc..
> 
> I'm exactly sure what this means, but I think you might mean something
> like renaming master to old-master and your branch to master. That's
> not the right way to do it, and that's not how git works.

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

Re: [gentoo-catalyst] [PATCH 1/4] catalyst: Specify python2 rather than the generic python

[Brian Dolbec]

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

On Fri, 2013-10-11 at 14:58 -0700, Matt Turner wrote:
> On Fri, Oct 11, 2013 at 2:11 PM, Brian Dolbec <dolsen@gentoo.org> wrote:

> > The default tree location move is waiting on the catalyst rewrite code
> > to go live producing stages, etc..
> 
> I'm exactly sure what this means, but I think you might mean something
> like renaming master to old-master and your branch to master. That's
> not the right way to do it, and that's not how git works.
> 

No, completely wrong.  The default tree location is going to be moved
from /usr/portage to somewhere in /var (they never could agree where)
and distfiles and packages directories are being moved out of the
PORTDIR tree directory.

Current catalyst master code has tree paths hardcoded everywhere.  Not
only that, but the paths are often used as both a path and a variable
name in many places.  

That makes it impossible for the tree move to happen and have catalyst
work correctly to produce stages, etc. with the new default locations.

That is why I started to work on catalyst in the first place.  But the
code is such a mess, I couldn't stop there with the encouragement of
many from the releng team.

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

Re: [gentoo-catalyst] [PATCH 1/4] catalyst: Specify python2 rather than the generic python

[Dylan Baker]

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

On Sunday, October 13, 2013 09:33:53 AM Brian Dolbec wrote:
> On Fri, 2013-10-11 at 14:58 -0700, Matt Turner wrote:
> > On Fri, Oct 11, 2013 at 2:11 PM, Brian Dolbec <dolsen@gentoo.org> wrote:
> > > The default tree location move is waiting on the catalyst rewrite code
> > > to go live producing stages, etc..
> > 
> > I'm exactly sure what this means, but I think you might mean something
> > like renaming master to old-master and your branch to master. That's
> > not the right way to do it, and that's not how git works.
> 
> No, completely wrong.  The default tree location is going to be moved
> from /usr/portage to somewhere in /var (they never could agree where)
> and distfiles and packages directories are being moved out of the
> PORTDIR tree directory.
> 
> Current catalyst master code has tree paths hardcoded everywhere.  Not
> only that, but the paths are often used as both a path and a variable
> name in many places.
> 
> That makes it impossible for the tree move to happen and have catalyst
> work correctly to produce stages, etc. with the new default locations.
> 
> That is why I started to work on catalyst in the first place.  But the
> code is such a mess, I couldn't stop there with the encouragement of
> many from the releng team.

I think what Matt is suggesting is that you split your rebase-on-master branch 
into logical series and send them to the list for review and merger into 
master. A Git workflow is one of incremental, reviewed changes being merged 
over time.

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