[PATCHES] Sprinting on rpkg during PyCon-Fr

[Mathieu Bridon]

> From Saturday to Tuesday was PyCon-Fr, here in Lyon, so I used the
> time to 
hack a bit on rpkg.

The result is a pretty big changeset:

 .gitignore                       |   1 +
 setup.cfg                        |   5 +
 setup.py                         |  38 ++-----
 src/pyrpkg/__init__.py           | 357
+++++++++++++++++++++++++++++++-------------------------------
 src/pyrpkg/cli.py                | 976
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------------------------------------------------------------
 src/rpkg_man_page.py             |  11 +-
 test/commands/__init__.py        | 110 ++++++++++++++++++++
 test/commands/test_add_tag.py    | 156 ++++++++++++++++++++++++++++
 test/commands/test_clone.py      |  88 ++++++++++++++++
 test/commands/test_delete_tag.py |  51 +++++++++
 test/commands/test_list_tag.py   | 159 ++++++++++++++++++++++++++++
 test/test_gitgnore.py            |  83 +++++++++++++++
 12 files changed, 1274 insertions(+), 761 deletions(-)

Now, out of these 1274 insertions, 647 are new unit tests. So if you remove
that from the count, the overall diff stat is rather:

  6 files changed,  627 insertions(+), 761 deletions(-)

That's right, keeping the same features, fixing a couple of bugs, and making
us almost entirely Flake8-compliant, I'm still removing more 100 lines of
code. Not bad. :)

Speaking of Flake8 compliance, the remaining problems are all stuff I'd rather
not touch right now, in the middle of what are mostly trivial changes. We'll
get there at some point, though.

To make review easier, here's an outline of what the patches do.

* [PATCH 01/20] Some more PEP8
* [PATCH 02/20] Remove unused import

Just some trivial changes to the setup.py script, to make the next commit
easier to review.

* [PATCH 03/20] tests: Use nose

This makes use of the awesome 'nose' testing library, removing some home-grown
code, and adding a pretty nice feature: a report of the code coverage of the
unit tests.

* [PATCH 04/20] tests: Ensure proper functioning of GitIgnore
* [PATCH 05/20] gitignore: We're not modified any more after we wrote
* [PATCH 06/20] gitignore: Make sure each line ends with a \n

These add some unit tests for our handling of .gitignore files, which actually
found 2 bugs in the code. Hurray for unit tests!

* [PATCH 07/20] tests: Ensure functioning of Commands.clone
* [PATCH 08/20] tests: Factor out some code

This sets up the necessary conditions for easily testing all the interactions
with Git repositories, and actually tests the clone command.

* [PATCH 09/20] tests: Ensure proper functioning of Commands.add_tag
* [PATCH 10/20] add_tag: Run the tag command in the right directory

These add some unit tests for the add_tag command, which actually found a bug
in the code. One more victory for unit tests!

* [PATCH 11/20] tests: Ensure functioning of Commands.delete_tag
* [PATCH 12/20] delete_tag: Stop executing a command

This adds some unit tests for the delete_tag command.

And once it is tested, it is easy to move away from subprocessing the git cli,
which makes the code nicer and avoids the overhead of forking child processes.
Plus, pyrpkg is a library, and it's generally a terrible idea to subprocess
command line tools inside a library.

* [PATCH 13/20] list_tags: Fix the docstring
* [PATCH 14/20] list_tags: Stop executing a command
* [PATCH 15/20] tests: Ensure functioning of Commands.list_tag

These greatly improve the list_tag command.

The second patch is in fact necessary to make the method testable.

* [PATCH 16/20] Fix typo
* [PATCH 17/20] Simplify some code

Just two trivial changes I came up with while idly reading the source code.

* [PATCH 18/20] Fix some flake8 issues
* [PATCH 19/20] Fix some more Flake8 issues
* [PATCH 20/20] Massive Flake8 fix

These are pretty massive patches, but they are worth it: they make the code
easier to read and "get into" for newcomers by following standard Python
conventions, removing unused variables and imports, etc...

All in all, they even remove lines of code. The diffstat for just these 3
changes is: 3 files changed, 577 insertions(+), 709 deletions(-)

[PATCH 01/20] Some more PEP8

[Mathieu Bridon]

From: Mathieu Bridon <bochecha(a)daitauha.fr&gt;

---
 setup.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/setup.py b/setup.py
index 108e691..79d7852 100755
--- a/setup.py
+++ b/setup.py
@@ -33,12 +33,12 @@ setup(
     description=("A python library and runtime script for managing RPM"
                  "package sources in a git repository"),
     license="GPLv2+",
-    url = "https://fedorahosted.org/rpkg",
-    package_dir = {'': 'src'},
-    packages = ['pyrpkg'],
-    scripts = ['src/rpkg'],
-    data_files = [('/etc/bash_completion.d', ['src/rpkg.bash']),
-                  ('/etc/rpkg', ['src/rpkg.conf'])],
+    url="https://fedorahosted.org/rpkg";,
+    package_dir={'': 'src'},
+    packages=['pyrpkg'],
+    scripts=['src/rpkg'],
+    data_files=[('/etc/bash_completion.d', ['src/rpkg.bash']),
+                ('/etc/rpkg', ['src/rpkg.conf'])],
     cmdclass={'test': DiscoverTest},
     classifiers=(
         'Development Status :: 5 - Production/Stable',
-- 
2.1.0

[PATCH 02/20] Remove unused import

[Mathieu Bridon]

From: Mathieu Bridon <bochecha(a)daitauha.fr&gt;

---
 setup.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/setup.py b/setup.py
index 79d7852..55b44a4 100755
--- a/setup.py
+++ b/setup.py
@@ -1,5 +1,4 @@
 #!/usr/bin/python
-import sys
 
 from setuptools import setup, Command
 try:
-- 
2.1.0

[PATCH 03/20] tests: Use nose

[Mathieu Bridon]

From: Mathieu Bridon <bochecha(a)daitauha.fr&gt;

This simplifies dramatically the setup.py file, and we get a code
coverage report for free.
---
 .gitignore |  1 +
 setup.cfg  |  5 +++++
 setup.py   | 25 ++-----------------------
 3 files changed, 8 insertions(+), 23 deletions(-)
 create mode 100644 setup.cfg

diff --git a/.gitignore b/.gitignore
index b111b8c..630fa85 100644
--- a/.gitignore
+++ b/.gitignore
@@ -9,3 +9,4 @@ py-compile
 dist/
 src/rpkg.egg-info/
 /sources
+.coverage
diff --git a/setup.cfg b/setup.cfg
new file mode 100644
index 0000000..fd43831
--- /dev/null
+++ b/setup.cfg
@@ -0,0 +1,5 @@
+[nosetests]
+verbosity = 2
+detailed-errors = 1
+with-coverage = 1
+cover-package = pyrpkg
diff --git a/setup.py b/setup.py
index 55b44a4..3d2f378 100755
--- a/setup.py
+++ b/setup.py
@@ -1,29 +1,8 @@
 #!/usr/bin/python
 
-from setuptools import setup, Command
-try:
-    from unittest import TestLoader, TextTestRunner
-except ImportError:
-    from unittest2 import TestLoader, TextTestRunner
+from setuptools import setup
 
 
-class DiscoverTest(Command):
-    user_options = []
-
-    def run(self):
-        loader = TestLoader()
-        suite = loader.discover(start_dir='test')
-        runner = TextTestRunner()
-        result = runner.run(suite)
-        if not result.wasSuccessful():
-            sys.exit(1)
-
-    def initialize_options(self):
-        pass
-
-    def finalize_options(self):
-        pass
-
 setup(
     name="rpkg",
     version="1.28",
@@ -38,7 +17,7 @@ setup(
     scripts=['src/rpkg'],
     data_files=[('/etc/bash_completion.d', ['src/rpkg.bash']),
                 ('/etc/rpkg', ['src/rpkg.conf'])],
-    cmdclass={'test': DiscoverTest},
+    test_suite='nose.collector',
     classifiers=(
         'Development Status :: 5 - Production/Stable',
         'Environment :: Console',
-- 
2.1.0

[PATCH 04/20] tests: Ensure proper functioning of GitIgnore

[Mathieu Bridon]

The tests fail!

That's because they just found 2 bugs in the code. :)
---
 test/test_gitgnore.py | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)
 create mode 100644 test/test_gitgnore.py

diff --git a/test/test_gitgnore.py b/test/test_gitgnore.py
new file mode 100644
index 0000000..938abfd
--- /dev/null
+++ b/test/test_gitgnore.py
@@ -0,0 +1,83 @@
+import os
+import shutil
+import tempfile
+import unittest
+
+
+class GitIgnoreTestCase(unittest.TestCase):
+    def setUp(self):
+        self.workdir = tempfile.mkdtemp(prefix='rpkg-tests.')
+
+    def tearDown(self):
+        shutil.rmtree(self.workdir)
+
+    def test_match_empty(self):
+        import pyrpkg
+        gi = pyrpkg.GitIgnore(os.path.join(self.workdir, 'gitignore'))
+
+        self.assertFalse(gi.match('this does not exist'))
+
+        # The empty string could match an empty file, but we don't want it to
+        self.assertFalse(gi.match(''))
+
+    def test_match_line_from_existing_file(self):
+        gi_path = os.path.join(self.workdir, 'gitignore')
+
+        with open(gi_path, 'w') as f:
+            f.write('this line exists\n')
+
+        import pyrpkg
+        gi = pyrpkg.GitIgnore(gi_path)
+
+        self.assertTrue(gi.match('this line exists'))
+        self.assertTrue(gi.match('this line exists\n'))
+        self.assertTrue(gi.match('/this line exists'))
+        self.assertTrue(gi.match('/this line exists\n'))
+
+        self.assertFalse(gi.match('but this line does not'))
+
+    def test_match_unwritten_line(self):
+        import pyrpkg
+        gi = pyrpkg.GitIgnore(os.path.join(self.workdir, 'gitignore'))
+        gi.add('here is a new line')
+
+        self.assertTrue(gi.modified)
+        self.assertTrue(gi.match('here is a new line'))
+
+    def test_match_glob(self):
+        import pyrpkg
+        gi = pyrpkg.GitIgnore(os.path.join(self.workdir, 'gitignore'))
+        gi.add('*')
+
+        self.assertTrue(gi.match('Surely this is matched by a wildcard?'))
+
+    def test_write_new_file(self):
+        gi_path = os.path.join(self.workdir, 'gitignore')
+
+        import pyrpkg
+        gi = pyrpkg.GitIgnore(gi_path)
+        gi.add('here is a new line')
+        gi.write()
+
+        self.assertFalse(gi.modified)
+
+        with open(gi_path) as f:
+            self.assertEqual(f.read(), 'here is a new line\n')
+
+    def test_write_append_to_existing_file(self):
+        gi_path = os.path.join(self.workdir, 'gitignore')
+
+        lines = ('this line exists', 'here is a new line')
+
+        with open(gi_path, 'w') as f:
+            f.write(lines[0])
+
+        import pyrpkg
+        gi = pyrpkg.GitIgnore(gi_path)
+        gi.add(lines[1])
+        gi.write()
+
+        self.assertFalse(gi.modified)
+
+        with open(gi_path) as f:
+            self.assertEqual(f.read(), '%s\n' % '\n'.join(lines))
-- 
2.1.0

[PATCH 05/20] gitignore: We're not modified any more after we wrote to disk

[Mathieu Bridon]

The GitIgnore.modified attribute is supposed to indicate whether the
file is in a "dirty" state, to prevent unneeded writes.

But if we just wrote it to disk, then it's not dirty any more.
---
 src/pyrpkg/__init__.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py
index 57d187e..1f097d3 100644
--- a/src/pyrpkg/__init__.py
+++ b/src/pyrpkg/__init__.py
@@ -2408,3 +2408,4 @@ class GitIgnore(object):
             for line in self.__lines:
                 gitignore_file.write(line)
             gitignore_file.close()
+            self.modified = False
-- 
2.1.0

[PATCH 06/20] gitignore: Make sure each line ends with a \n

[Mathieu Bridon]

When loading an existing file, if its last line didn't end with a \n
character, then we'd load it as it is.

Then, if we added a new line, it would be appended to the last existing
line, as one line, which is clearly not what we want.

So we must ensure every line ends with a \n character when we load an
existing file.

While we're at it, if we're going to check for \n in 2 places, we might
as well make that a new function.
---
 src/pyrpkg/__init__.py | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py
index 1f097d3..660d327 100644
--- a/src/pyrpkg/__init__.py
+++ b/src/pyrpkg/__init__.py
@@ -2372,21 +2372,27 @@ class GitIgnore(object):
         self.__lines = []
         if os.path.exists(self.path):
             gitignore_file = open(self.path, 'r')
-            self.__lines = gitignore_file.readlines()
+            for line in gitignore_file:
+                self.__lines.append(self.__ensure_newline(line))
+
             gitignore_file.close()
 
         # Set to True if we end up making any modifications, used to
         # prevent unnecessary writes.
         self.modified = False
 
+    def __ensure_newline(self, line):
+        """Append a newline character if the given line didn't have
one"""
+        if line.endswith('\n'):
+            return line
+
+        return '%s\n' % line
+
     def add(self, line):
         """
         Add a line to .gitignore, but check if it's a duplicate first.
         """
-
-        # Append a newline character if the given line didn't have one:
-        if line[-1] != '\n':
-            line = "%s\n" % line
+        line = self.__ensure_newline(line)
 
         # Add this line if it doesn't already exist:
         if not line in self.__lines:
-- 
2.1.0

[PATCH 07/20] tests: Ensure functioning of Commands.clone

[Mathieu Bridon]

A lot of stuff to figure out (TODO) in there, but this actually manages
to test the function quite nicely.

When adding more tests for other functions, some of this stuff will
eventually get factored out and figured out better, but for now... this
works and covers almost 100% of the function. :)
---
 test/commands/__init__.py   |   0
 test/commands/test_clone.py | 157 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 157 insertions(+)
 create mode 100644 test/commands/__init__.py
 create mode 100644 test/commands/test_clone.py

diff --git a/test/commands/__init__.py b/test/commands/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/test/commands/test_clone.py b/test/commands/test_clone.py
new file mode 100644
index 0000000..f5a8030
--- /dev/null
+++ b/test/commands/test_clone.py
@@ -0,0 +1,157 @@
+import os
+import shutil
+import subprocess
+import tempfile
+import unittest
+
+
+class CommandCloneTestCase(unittest.TestCase):
+    def setUp(self):
+        self.path = tempfile.mkdtemp(prefix='rpkg-tests.')
+        self.gitroot = os.path.join(self.path, 'gitroot')
+
+        self.module = 'module1'
+
+        self.anongiturl = 'file://%s/%%(module)s' % self.gitroot
+        self.branchre = r'master|rpkg-tests-.+'
+        self.quiet = False
+
+        # TODO: Figure out how to handle this
+        self.lookaside = 'TODO'
+        self.lookasidehash = 'md5'
+        self.lookaside_cgi = 'TODO'
+        self.gitbaseurl = 'TODO'
+        self.kojiconfig = 'TODO'
+        self.build_client = 'TODO'
+        self.user = 'TODO'
+        self.dist = 'TODO'
+        self.target = 'TODO'
+
+    def tearDown(self):
+        shutil.rmtree(self.path)
+
+    def make_new_git(self, module, branches=None):
+        """Make a new git repo, so that tests can clone it
+
+        This is not a test method.
+        """
+        if branches is None:
+            branches = []
+
+        # Create a bare Git repository
+        moduledir = os.path.join(self.gitroot, module)
+        os.makedirs(moduledir)
+        subprocess.check_call(['git', 'init', '--bare'],
cwd=moduledir,
+                              stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+
+        # Clone it, and do the minimal Dist Git setup
+        cloneroot = os.path.join(self.path, 'clonedir')
+        os.makedirs(cloneroot)
+        subprocess.check_call(['git', 'clone', 'file://%s' %
moduledir],
+                              cwd=cloneroot, stdout=subprocess.PIPE,
+                              stderr=subprocess.PIPE)
+        clonedir = os.path.join(cloneroot, module)
+        open(os.path.join(clonedir, '.gitignore'), 'w').close()
+        open(os.path.join(clonedir, 'sources'), 'w').close()
+        subprocess.check_call(['git', 'add', '.gitignore',
'sources'],
+                              cwd=clonedir, stdout=subprocess.PIPE,
+                              stderr=subprocess.PIPE)
+        subprocess.check_call(['git', 'commit', '-m',
+                               'Initial setup of the repo'], cwd=clonedir,
+                              stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        subprocess.check_call(['git', 'push', 'origin',
'master'],
+                              cwd=clonedir, stdout=subprocess.PIPE,
+                              stderr=subprocess.PIPE)
+
+        # Add the requested branches
+        for branch in branches:
+            subprocess.check_call(['git', 'branch', branch],
cwd=clonedir,
+                                  stdout=subprocess.PIPE,
+                                  stderr=subprocess.PIPE)
+            subprocess.check_call(['git', 'push', 'origin',
branch],
+                                  cwd=clonedir, stdout=subprocess.PIPE,
+                                  stderr=subprocess.PIPE)
+
+        # Drop the clone
+        shutil.rmtree(cloneroot)
+
+    def test_clone_anonymous(self):
+        self.make_new_git(self.module)
+
+        import pyrpkg
+        cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash,
+                              self.lookaside_cgi, self.gitbaseurl,
+                              self.anongiturl, self.branchre, self.kojiconfig,
+                              self.build_client, self.user, self.dist,
+                              self.target, self.quiet)
+        cmd.clone(self.module, anon=True)
+
+        moduledir = os.path.join(self.path, self.module)
+        self.assertTrue(os.path.isdir(os.path.join(moduledir, '.git')))
+
+    def test_clone_anonymous_with_path(self):
+        self.make_new_git(self.module)
+
+        altpath = tempfile.mkdtemp(prefix='rpkg-tests.')
+
+        import pyrpkg
+        cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash,
+                              self.lookaside_cgi, self.gitbaseurl,
+                              self.anongiturl, self.branchre, self.kojiconfig,
+                              self.build_client, self.user, self.dist,
+                              self.target, self.quiet)
+        cmd.clone(self.module, anon=True, path=altpath)
+
+        moduledir = os.path.join(altpath, self.module)
+        self.assertTrue(os.path.isdir(os.path.join(moduledir, '.git')))
+
+        notmoduledir = os.path.join(self.path, self.module)
+        self.assertFalse(os.path.isdir(os.path.join(notmoduledir, '.git')))
+
+        shutil.rmtree(altpath)
+
+    def test_clone_anonymous_with_branch(self):
+        self.make_new_git(self.module,
+                          branches=['rpkg-tests-1', 'rpkg-tests-2'])
+
+        import pyrpkg
+        cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash,
+                              self.lookaside_cgi, self.gitbaseurl,
+                              self.anongiturl, self.branchre, self.kojiconfig,
+                              self.build_client, self.user, self.dist,
+                              self.target, self.quiet)
+        cmd.clone(self.module, anon=True, branch='rpkg-tests-1')
+
+        with open(os.path.join(
+                self.path, self.module, '.git', 'HEAD')) as HEAD:
+            self.assertEqual(HEAD.read(), 'ref: refs/heads/rpkg-tests-1\n')
+
+    def test_clone_anonymous_with_bare_dir(self):
+        self.make_new_git(self.module)
+
+        import pyrpkg
+        cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash,
+                              self.lookaside_cgi, self.gitbaseurl,
+                              self.anongiturl, self.branchre, self.kojiconfig,
+                              self.build_client, self.user, self.dist,
+                              self.target, self.quiet)
+        cmd.clone(self.module, anon=True, bare_dir='%s.git' % self.module)
+
+        clonedir = os.path.join(self.path, '%s.git' % self.module)
+        self.assertTrue(os.path.isdir(clonedir))
+        self.assertFalse(os.path.exists(os.path.join(clonedir, 'index')))
+
+    def test_clone_fails_with_both_branch_and_bare_dir(self):
+        self.make_new_git(self.module,
+                          branches=['rpkg-tests-1', 'rpkg-tests-2'])
+
+        import pyrpkg
+        cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash,
+                              self.lookaside_cgi, self.gitbaseurl,
+                              self.anongiturl, self.branchre, self.kojiconfig,
+                              self.build_client, self.user, self.dist,
+                              self.target, self.quiet)
+
+        with self.assertRaises(pyrpkg.rpkgError):
+            cmd.clone(self.module, anon=True, branch='rpkg-tests-1',
+                      bare_dir='test.git')
-- 
2.1.0

[PATCH 08/20] tests: Factor out some code

[Mathieu Bridon]

This code will be reused by other test cases.
---
 test/commands/__init__.py   | 76 ++++++++++++++++++++++++++++++++++++++++++++
 test/commands/test_clone.py | 77 +++------------------------------------------
 2 files changed, 80 insertions(+), 73 deletions(-)

diff --git a/test/commands/__init__.py b/test/commands/__init__.py
index e69de29..e55545b 100644
--- a/test/commands/__init__.py
+++ b/test/commands/__init__.py
@@ -0,0 +1,76 @@
+import os
+import shutil
+import subprocess
+import tempfile
+import unittest
+
+
+class CommandTestCase(unittest.TestCase):
+    def setUp(self):
+        self.path = tempfile.mkdtemp(prefix='rpkg-tests.')
+        self.gitroot = os.path.join(self.path, 'gitroot')
+
+        self.module = 'module1'
+
+        self.anongiturl = 'file://%s/%%(module)s' % self.gitroot
+        self.branchre = r'master|rpkg-tests-.+'
+        self.quiet = False
+
+        # TODO: Figure out how to handle this
+        self.lookaside = 'TODO'
+        self.lookasidehash = 'md5'
+        self.lookaside_cgi = 'TODO'
+        self.gitbaseurl = 'TODO'
+        self.kojiconfig = 'TODO'
+        self.build_client = 'TODO'
+        self.user = 'TODO'
+        self.dist = 'TODO'
+        self.target = 'TODO'
+
+    def tearDown(self):
+        shutil.rmtree(self.path)
+
+    def make_new_git(self, module, branches=None):
+        """Make a new git repo, so that tests can clone it
+
+        This is not a test method.
+        """
+        if branches is None:
+            branches = []
+
+        # Create a bare Git repository
+        moduledir = os.path.join(self.gitroot, module)
+        os.makedirs(moduledir)
+        subprocess.check_call(['git', 'init', '--bare'],
cwd=moduledir,
+                              stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+
+        # Clone it, and do the minimal Dist Git setup
+        cloneroot = os.path.join(self.path, 'clonedir')
+        os.makedirs(cloneroot)
+        subprocess.check_call(['git', 'clone', 'file://%s' %
moduledir],
+                              cwd=cloneroot, stdout=subprocess.PIPE,
+                              stderr=subprocess.PIPE)
+        clonedir = os.path.join(cloneroot, module)
+        open(os.path.join(clonedir, '.gitignore'), 'w').close()
+        open(os.path.join(clonedir, 'sources'), 'w').close()
+        subprocess.check_call(['git', 'add', '.gitignore',
'sources'],
+                              cwd=clonedir, stdout=subprocess.PIPE,
+                              stderr=subprocess.PIPE)
+        subprocess.check_call(['git', 'commit', '-m',
+                               'Initial setup of the repo'], cwd=clonedir,
+                              stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        subprocess.check_call(['git', 'push', 'origin',
'master'],
+                              cwd=clonedir, stdout=subprocess.PIPE,
+                              stderr=subprocess.PIPE)
+
+        # Add the requested branches
+        for branch in branches:
+            subprocess.check_call(['git', 'branch', branch],
cwd=clonedir,
+                                  stdout=subprocess.PIPE,
+                                  stderr=subprocess.PIPE)
+            subprocess.check_call(['git', 'push', 'origin',
branch],
+                                  cwd=clonedir, stdout=subprocess.PIPE,
+                                  stderr=subprocess.PIPE)
+
+        # Drop the clone
+        shutil.rmtree(cloneroot)
diff --git a/test/commands/test_clone.py b/test/commands/test_clone.py
index f5a8030..0ed3ecf 100644
--- a/test/commands/test_clone.py
+++ b/test/commands/test_clone.py
@@ -1,80 +1,11 @@
 import os
 import shutil
-import subprocess
 import tempfile
-import unittest
-
-
-class CommandCloneTestCase(unittest.TestCase):
-    def setUp(self):
-        self.path = tempfile.mkdtemp(prefix='rpkg-tests.')
-        self.gitroot = os.path.join(self.path, 'gitroot')
-
-        self.module = 'module1'
-
-        self.anongiturl = 'file://%s/%%(module)s' % self.gitroot
-        self.branchre = r'master|rpkg-tests-.+'
-        self.quiet = False
-
-        # TODO: Figure out how to handle this
-        self.lookaside = 'TODO'
-        self.lookasidehash = 'md5'
-        self.lookaside_cgi = 'TODO'
-        self.gitbaseurl = 'TODO'
-        self.kojiconfig = 'TODO'
-        self.build_client = 'TODO'
-        self.user = 'TODO'
-        self.dist = 'TODO'
-        self.target = 'TODO'
-
-    def tearDown(self):
-        shutil.rmtree(self.path)
-
-    def make_new_git(self, module, branches=None):
-        """Make a new git repo, so that tests can clone it
-
-        This is not a test method.
-        """
-        if branches is None:
-            branches = []
-
-        # Create a bare Git repository
-        moduledir = os.path.join(self.gitroot, module)
-        os.makedirs(moduledir)
-        subprocess.check_call(['git', 'init', '--bare'],
cwd=moduledir,
-                              stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-
-        # Clone it, and do the minimal Dist Git setup
-        cloneroot = os.path.join(self.path, 'clonedir')
-        os.makedirs(cloneroot)
-        subprocess.check_call(['git', 'clone', 'file://%s' %
moduledir],
-                              cwd=cloneroot, stdout=subprocess.PIPE,
-                              stderr=subprocess.PIPE)
-        clonedir = os.path.join(cloneroot, module)
-        open(os.path.join(clonedir, '.gitignore'), 'w').close()
-        open(os.path.join(clonedir, 'sources'), 'w').close()
-        subprocess.check_call(['git', 'add', '.gitignore',
'sources'],
-                              cwd=clonedir, stdout=subprocess.PIPE,
-                              stderr=subprocess.PIPE)
-        subprocess.check_call(['git', 'commit', '-m',
-                               'Initial setup of the repo'], cwd=clonedir,
-                              stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-        subprocess.check_call(['git', 'push', 'origin',
'master'],
-                              cwd=clonedir, stdout=subprocess.PIPE,
-                              stderr=subprocess.PIPE)
-
-        # Add the requested branches
-        for branch in branches:
-            subprocess.check_call(['git', 'branch', branch],
cwd=clonedir,
-                                  stdout=subprocess.PIPE,
-                                  stderr=subprocess.PIPE)
-            subprocess.check_call(['git', 'push', 'origin',
branch],
-                                  cwd=clonedir, stdout=subprocess.PIPE,
-                                  stderr=subprocess.PIPE)
-
-        # Drop the clone
-        shutil.rmtree(cloneroot)
 
+from . import CommandTestCase
+
+
+class CommandCloneTestCase(CommandTestCase):
     def test_clone_anonymous(self):
         self.make_new_git(self.module)
 
-- 
2.1.0

[PATCH 09/20] tests: Ensure proper functioning of Commands.add_tag

[Mathieu Bridon]

The tests fail!

That's because they just found 1 bug in the code. :)

However, notice the game I'm playing with the EDITOR environment
variable. This is a gross hack, but it is made necessary by a bad
design decision in the pyrpkg API: a library should never do some
interactive stuff in subprocesses like that.
---
 test/commands/__init__.py     |  14 ++++
 test/commands/test_add_tag.py | 156 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 170 insertions(+)
 create mode 100644 test/commands/test_add_tag.py

diff --git a/test/commands/__init__.py b/test/commands/__init__.py
index e55545b..41d513f 100644
--- a/test/commands/__init__.py
+++ b/test/commands/__init__.py
@@ -74,3 +74,17 @@ class CommandTestCase(unittest.TestCase):
 
         # Drop the clone
         shutil.rmtree(cloneroot)
+
+    def get_tags(self, gitdir):
+        result = []
+
+        tags = subprocess.check_output(['git', 'tag', '-n1'],
cwd=gitdir)
+
+        for line in tags.split('\n'):
+            if not line:
+                continue
+
+            tokens = [x for x in line.split() if x]
+            result.append([tokens[0], ' '.join(tokens[1:])])
+
+        return result
diff --git a/test/commands/test_add_tag.py b/test/commands/test_add_tag.py
new file mode 100644
index 0000000..f780c40
--- /dev/null
+++ b/test/commands/test_add_tag.py
@@ -0,0 +1,156 @@
+import os
+
+from . import CommandTestCase
+
+
+class CommandAddTagTestCase(CommandTestCase):
+    def setUp(self):
+        super(CommandAddTagTestCase, self).setUp()
+        self.old_editor = os.environ['EDITOR']
+
+    def tearDown(self):
+        os.environ['EDITOR'] = self.old_editor
+        super(CommandAddTagTestCase, self).tearDown()
+
+    def test_add_tag(self):
+        self.make_new_git(self.module)
+
+        tag = 'v1.0'
+        message = 'This is a release'
+
+        import pyrpkg
+        cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash,
+                              self.lookaside_cgi, self.gitbaseurl,
+                              self.anongiturl, self.branchre, self.kojiconfig,
+                              self.build_client, self.user, self.dist,
+                              self.target, self.quiet)
+        cmd.clone(self.module, anon=True)
+
+        moduledir = os.path.join(self.path, self.module)
+        cmd.path = moduledir
+
+        # `git tag` will call $EDITOR to ask the user to write a message
+        os.environ['EDITOR'] = ('/usr/bin/python -c "import sys; '
+                                'open(sys.argv[1],
\'w\').write(\'%s\')"'
+                                % message)
+
+        cmd.add_tag(tag)
+
+        self.assertEqual(self.get_tags(moduledir), [[tag, message]])
+
+    def test_add_tag_with_message(self):
+        self.make_new_git(self.module)
+
+        tag = 'v1.0'
+        message = 'This is a release'
+
+        import pyrpkg
+        cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash,
+                              self.lookaside_cgi, self.gitbaseurl,
+                              self.anongiturl, self.branchre, self.kojiconfig,
+                              self.build_client, self.user, self.dist,
+                              self.target, self.quiet)
+        cmd.clone(self.module, anon=True)
+
+        moduledir = os.path.join(self.path, self.module)
+        cmd.path = moduledir
+
+        cmd.add_tag(tag, message=message)
+
+        self.assertEqual(self.get_tags(moduledir), [[tag, message]])
+
+    def test_add_tag_with_message_from_file(self):
+        self.make_new_git(self.module)
+
+        tag = 'v1.0'
+        message = 'This is a release'
+
+        import pyrpkg
+        cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash,
+                              self.lookaside_cgi, self.gitbaseurl,
+                              self.anongiturl, self.branchre, self.kojiconfig,
+                              self.build_client, self.user, self.dist,
+                              self.target, self.quiet)
+        cmd.clone(self.module, anon=True)
+
+        moduledir = os.path.join(self.path, self.module)
+        cmd.path = moduledir
+
+        message_file = os.path.join(moduledir, 'tag_message')
+
+        with open(message_file, 'w') as f:
+            f.write(message)
+
+        cmd.add_tag(tag, file=message_file)
+
+        self.assertEqual(self.get_tags(moduledir), [[tag, message]])
+
+    def test_add_tag_fails_with_existing(self):
+        self.make_new_git(self.module)
+
+        tag = 'v1.0'
+        message = 'This is a release'
+
+        import pyrpkg
+        cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash,
+                              self.lookaside_cgi, self.gitbaseurl,
+                              self.anongiturl, self.branchre, self.kojiconfig,
+                              self.build_client, self.user, self.dist,
+                              self.target, self.quiet)
+        cmd.clone(self.module, anon=True)
+
+        moduledir = os.path.join(self.path, self.module)
+        cmd.path = moduledir
+
+        cmd.add_tag(tag, message=message)
+
+        # Now add the same tag again
+        with self.assertRaises(pyrpkg.rpkgError):
+            cmd.add_tag(tag, message='No, THIS is a release')
+
+    def test_add_tag_force_replace_existing(self):
+        self.make_new_git(self.module)
+
+        tag = 'v1.0'
+        message = 'This is a release'
+
+        import pyrpkg
+        cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash,
+                              self.lookaside_cgi, self.gitbaseurl,
+                              self.anongiturl, self.branchre, self.kojiconfig,
+                              self.build_client, self.user, self.dist,
+                              self.target, self.quiet)
+        cmd.clone(self.module, anon=True)
+
+        moduledir = os.path.join(self.path, self.module)
+        cmd.path = moduledir
+
+        cmd.add_tag(tag, message=message)
+
+        # Now add the same tag again by force
+        newmessage = 'No, THIS is a release'
+        cmd.add_tag(tag, message=newmessage, force=True)
+
+        self.assertEqual(self.get_tags(moduledir), [[tag, newmessage]])
+
+    def test_add_tag_many(self):
+        self.make_new_git(self.module)
+
+        tags = [['v1.0', 'This is a release'],
+                ['v2.0', 'This is another release']]
+
+        import pyrpkg
+        cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash,
+                              self.lookaside_cgi, self.gitbaseurl,
+                              self.anongiturl, self.branchre, self.kojiconfig,
+                              self.build_client, self.user, self.dist,
+                              self.target, self.quiet)
+        cmd.clone(self.module, anon=True)
+
+        moduledir = os.path.join(self.path, self.module)
+        cmd.path = moduledir
+
+        for tag, message in tags:
+            cmd.add_tag(tag, message=message)
+
+        self.assertEqual(self.get_tags(moduledir), tags)
-- 
2.1.0

[PATCH 10/20] add_tag: Run the tag command in the right directory

[Mathieu Bridon]

We were running `git tag` in the current directory.

This doesn't cause any problem when we run the tool in the git directory
we want to tag, which is the most common case.

However, when using the API (for example in the unit tests), the current
directory might not be the git clone, we might really be anywhere.

As such, the `git tag` command would just fail.

Worse, when running the tests, we would add tags to the clone of
upstream rpkg!
---
 src/pyrpkg/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py
index 660d327..ed40654 100644
--- a/src/pyrpkg/__init__.py
+++ b/src/pyrpkg/__init__.py
@@ -1016,7 +1016,7 @@ class Commands(object):
             cmd.extend(['-F', os.path.abspath(file)])
         cmd.append(tagname)
         # make it so
-        self._run_command(cmd)
+        self._run_command(cmd, cwd=self.path)
         self.log.info('Tag \'%s\' was created' % tagname)
 
     def clean(self, dry=False, useignore=True):
-- 
2.1.0

[PATCH 11/20] tests: Ensure functioning of Commands.delete_tag

[Mathieu Bridon]

---
 test/commands/test_delete_tag.py | 51 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 test/commands/test_delete_tag.py

diff --git a/test/commands/test_delete_tag.py b/test/commands/test_delete_tag.py
new file mode 100644
index 0000000..621b1e4
--- /dev/null
+++ b/test/commands/test_delete_tag.py
@@ -0,0 +1,51 @@
+import os
+
+from . import CommandTestCase
+
+
+class CommandDeleteTagTestCase(CommandTestCase):
+    def test_delete_tag(self):
+        self.make_new_git(self.module)
+
+        tag = 'v1.0'
+        message = 'This is a release'
+
+        import pyrpkg
+        cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash,
+                              self.lookaside_cgi, self.gitbaseurl,
+                              self.anongiturl, self.branchre, self.kojiconfig,
+                              self.build_client, self.user, self.dist,
+                              self.target, self.quiet)
+        cmd.clone(self.module, anon=True)
+
+        moduledir = os.path.join(self.path, self.module)
+        cmd.path = moduledir
+
+        # First, add a tag
+        cmd.add_tag(tag, message=message)
+        self.assertEqual(self.get_tags(moduledir), [[tag, message]])
+
+        # Now delete it
+        cmd.delete_tag(tag)
+        tags = [t for (t, m) in self.get_tags(moduledir)]
+        self.assertNotIn(tag, tags)
+
+    def test_delete_tag_fails_inexistent(self):
+        self.make_new_git(self.module)
+
+        tag = 'v1.0'
+
+        import pyrpkg
+        cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash,
+                              self.lookaside_cgi, self.gitbaseurl,
+                              self.anongiturl, self.branchre, self.kojiconfig,
+                              self.build_client, self.user, self.dist,
+                              self.target, self.quiet)
+        cmd.clone(self.module, anon=True)
+
+        moduledir = os.path.join(self.path, self.module)
+        cmd.path = moduledir
+
+        # Try deleting an inexistent tag
+        with self.assertRaises(pyrpkg.rpkgError):
+            cmd.delete_tag(tag)
-- 
2.1.0

[PATCH 12/20] delete_tag: Stop executing a command

[Mathieu Bridon]

From: Mathieu Bridon <bochecha(a)daitauha.fr&gt;

We have a Git API, let's use it, instead of constantly running commands
in subprocesses.
---
 src/pyrpkg/__init__.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py
index ed40654..48edf6c 100644
--- a/src/pyrpkg/__init__.py
+++ b/src/pyrpkg/__init__.py
@@ -1206,8 +1206,12 @@ class Commands(object):
     def delete_tag(self, tagname):
         """Delete a git tag from the repository found at optional
path"""
 
-        cmd = ['git', 'tag', '-d', tagname]
-        self._run_command(cmd, cwd=self.path)
+        try:
+            self.repo.delete_tag(tagname)
+
+        except git.GitCommandError as e:
+            raise rpkgError(e)
+
         self.log.info ('Tag %s was deleted' % tagname)
 
     def diff(self, cached=False, files=[]):
-- 
2.1.0

[PATCH 13/20] list_tags: Fix the docstring

[Mathieu Bridon]

From: Mathieu Bridon <bochecha(a)daitauha.fr&gt;

The documentation didn't describe the full behaviour. For example,
passing 'foo*' as tagname would match the glob, whereas the docstring
just talks about globs in the '*' case.
---
 src/pyrpkg/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py
index 48edf6c..03fb493 100644
--- a/src/pyrpkg/__init__.py
+++ b/src/pyrpkg/__init__.py
@@ -1384,7 +1384,7 @@ class Commands(object):
     def list_tag(self, tagname=None):
         """Create a list of all tags in the repository which match a given
tagname.
 
-        if tagname == '*' all tags will been shown.
+        The optional `tagname` argument may contain a '*' glob.
 
         """
 
-- 
2.1.0

[PATCH 14/20] list_tags: Stop executing a command

[Mathieu Bridon]

From: Mathieu Bridon <bochecha(a)daitauha.fr&gt;

We have a Git API, let's use it, instead of constantly running commands
in subprocesses.

This even results in simpler code!

In addition, it makes the method actually testable. Without this, the
tag list was printed in the stdout **of the subprocess**, which is
unaccessible to the parent process, and as such to the tests. The tests
couldn't even verify what the library had done!
---
 src/pyrpkg/__init__.py | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py
index 03fb493..9070ac0 100644
--- a/src/pyrpkg/__init__.py
+++ b/src/pyrpkg/__init__.py
@@ -1381,19 +1381,21 @@ class Commands(object):
         os.chdir(oldpath)
         return(uploadfiles)
 
-    def list_tag(self, tagname=None):
+    def list_tag(self, tagname='*'):
         """Create a list of all tags in the repository which match a given
tagname.
 
-        The optional `tagname` argument may contain a '*' glob.
+        The optional `tagname` argument may be a shell glob (it is matched
+        with fnmatch).
 
         """
 
-        cmd = ['git', 'tag']
-        cmd.extend(['-l'])
-        if tagname and tagname != '*':
-            cmd.extend([tagname])
-        # make it so
-        self._run_command(cmd)
+        tags = map(lambda t: t.name, self.repo.tags)
+
+        if tagname is not '*':
+            tags = filter(lambda t: fnmatch.fnmatch(t, tagname), tags)
+
+        for tag in tags:
+            print(tag)
 
     def new(self):
         """Return changes in a repo since the last tag"""
-- 
2.1.0

[PATCH 15/20] tests: Ensure functioning of Commands.list_tag

[Mathieu Bridon]

From: Mathieu Bridon <bochecha(a)daitauha.fr&gt;

This is pretty horrible though. The library function is just a poorly
thought-out API. A proper API would return the results, and let the
caller handle them (for example, printing them to stdout, or in a web
interface, or...).

However, the rpkg API instead directly prints it to stdout, which not
only prevents consumers from easily doing something nice with it, but
also prevents **unit tests** (!!) from easily verifying the behaviour of
the function.

As a result, if we want unit tests, we need to do something pretty
horrible: hijack the stdout of the function, and parse it.
---
 test/commands/__init__.py      |  20 ++++++
 test/commands/test_list_tag.py | 159 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 179 insertions(+)
 create mode 100644 test/commands/test_list_tag.py

diff --git a/test/commands/__init__.py b/test/commands/__init__.py
index 41d513f..d199f31 100644
--- a/test/commands/__init__.py
+++ b/test/commands/__init__.py
@@ -1,6 +1,7 @@
 import os
 import shutil
 import subprocess
+import sys
 import tempfile
 import unittest
 
@@ -88,3 +89,22 @@ class CommandTestCase(unittest.TestCase):
             result.append([tokens[0], ' '.join(tokens[1:])])
 
         return result
+
+    def hijack_stdout(self):
+        class cm(object):
+            def __enter__(self):
+                from cStringIO import StringIO
+
+                self.old_stdout = sys.stdout
+                self.out = StringIO()
+                sys.stdout = self.out
+
+                return self.out
+
+            def __exit__(self, *args):
+                sys.stdout.flush()
+                sys.stdout = self.old_stdout
+
+                self.out.seek(0)
+
+        return cm()
diff --git a/test/commands/test_list_tag.py b/test/commands/test_list_tag.py
new file mode 100644
index 0000000..8135f5e
--- /dev/null
+++ b/test/commands/test_list_tag.py
@@ -0,0 +1,159 @@
+import os
+
+from . import CommandTestCase
+
+
+class CommandListTagTestCase(CommandTestCase):
+    def test_list_tag_no_tags(self):
+        self.make_new_git(self.module)
+
+        import pyrpkg
+        cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash,
+                              self.lookaside_cgi, self.gitbaseurl,
+                              self.anongiturl, self.branchre, self.kojiconfig,
+                              self.build_client, self.user, self.dist,
+                              self.target, self.quiet)
+        cmd.clone(self.module, anon=True)
+
+        moduledir = os.path.join(self.path, self.module)
+        cmd.path = moduledir
+
+        with self.hijack_stdout() as out:
+            cmd.list_tag()
+
+        self.assertEqual(out.read().strip(), '')
+
+    def test_list_tag_many(self):
+        self.make_new_git(self.module)
+
+        tags = [['v1.0', 'This is a release'],
+                ['v2.0', 'This is another release']]
+
+        import pyrpkg
+        cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash,
+                              self.lookaside_cgi, self.gitbaseurl,
+                              self.anongiturl, self.branchre, self.kojiconfig,
+                              self.build_client, self.user, self.dist,
+                              self.target, self.quiet)
+        cmd.clone(self.module, anon=True)
+
+        moduledir = os.path.join(self.path, self.module)
+        cmd.path = moduledir
+
+        for tag, message in tags:
+            cmd.add_tag(tag, message=message)
+
+        with self.hijack_stdout() as out:
+            cmd.list_tag()
+
+        result = out.read().strip().split('\n')
+
+        self.assertEqual(result, [t for (t, m) in tags])
+
+    def test_list_tag_specific(self):
+        self.make_new_git(self.module)
+
+        tags = [['v1.0', 'This is a release'],
+                ['v2.0', 'This is another release']]
+
+        import pyrpkg
+        cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash,
+                              self.lookaside_cgi, self.gitbaseurl,
+                              self.anongiturl, self.branchre, self.kojiconfig,
+                              self.build_client, self.user, self.dist,
+                              self.target, self.quiet)
+        cmd.clone(self.module, anon=True)
+
+        moduledir = os.path.join(self.path, self.module)
+        cmd.path = moduledir
+
+        for tag, message in tags:
+            cmd.add_tag(tag, message=message)
+
+        with self.hijack_stdout() as out:
+            cmd.list_tag(tagname='v1.0')
+
+        result = out.read().strip().split('\n')
+
+        self.assertEqual(result, ['v1.0'])
+
+    def test_list_tag_inexistent(self):
+        self.make_new_git(self.module)
+
+        tags = [['v1.0', 'This is a release'],
+                ['v2.0', 'This is another release']]
+
+        import pyrpkg
+        cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash,
+                              self.lookaside_cgi, self.gitbaseurl,
+                              self.anongiturl, self.branchre, self.kojiconfig,
+                              self.build_client, self.user, self.dist,
+                              self.target, self.quiet)
+        cmd.clone(self.module, anon=True)
+
+        moduledir = os.path.join(self.path, self.module)
+        cmd.path = moduledir
+
+        for tag, message in tags:
+            cmd.add_tag(tag, message=message)
+
+        with self.hijack_stdout() as out:
+            cmd.list_tag(tagname='v1.1')
+
+        result = out.read().strip().split('\n')
+
+        self.assertEqual(result, [''])
+
+    def test_list_tag_glob(self):
+        self.make_new_git(self.module)
+
+        tags = [['v1.0', 'This is a release'],
+                ['v2.0', 'This is another release']]
+
+        import pyrpkg
+        cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash,
+                              self.lookaside_cgi, self.gitbaseurl,
+                              self.anongiturl, self.branchre, self.kojiconfig,
+                              self.build_client, self.user, self.dist,
+                              self.target, self.quiet)
+        cmd.clone(self.module, anon=True)
+
+        moduledir = os.path.join(self.path, self.module)
+        cmd.path = moduledir
+
+        for tag, message in tags:
+            cmd.add_tag(tag, message=message)
+
+        with self.hijack_stdout() as out:
+            cmd.list_tag(tagname='v1*')
+
+        result = out.read().strip().split('\n')
+
+        self.assertEqual(result, ['v1.0'])
+
+    def test_list_tag_wildcard(self):
+        self.make_new_git(self.module)
+
+        tags = [['v1.0', 'This is a release'],
+                ['v2.0', 'This is another release']]
+
+        import pyrpkg
+        cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash,
+                              self.lookaside_cgi, self.gitbaseurl,
+                              self.anongiturl, self.branchre, self.kojiconfig,
+                              self.build_client, self.user, self.dist,
+                              self.target, self.quiet)
+        cmd.clone(self.module, anon=True)
+
+        moduledir = os.path.join(self.path, self.module)
+        cmd.path = moduledir
+
+        for tag, message in tags:
+            cmd.add_tag(tag, message=message)
+
+        with self.hijack_stdout() as out:
+            cmd.list_tag(tagname='*')
+
+        result = out.read().strip().split('\n')
+
+        self.assertEqual(result, [t for (t, m) in tags])
-- 
2.1.0

[PATCH 16/20] Fix typo

[Mathieu Bridon]

From: Mathieu Bridon <bochecha(a)daitauha.fr&gt;

---
 src/pyrpkg/cli.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/pyrpkg/cli.py b/src/pyrpkg/cli.py
index 80812e6..569bb6a 100755
--- a/src/pyrpkg/cli.py
+++ b/src/pyrpkg/cli.py
@@ -968,7 +968,7 @@ defined, packages will be built sequentially.""" %
                 build_set = []
                 sets = True
             else:
-                # Figure out the scm url to build from package namee
+                # Figure out the scm url to build from package name
                 hash = self.cmd.get_latest_commit(component,
                                                   self.cmd.branch_merge)
                 url = self.cmd.anongiturl % {'module':
-- 
2.1.0

[PATCH 17/20] Simplify some code

[Mathieu Bridon]

From: Mathieu Bridon <bochecha(a)daitauha.fr&gt;

---
 src/pyrpkg/cli.py | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/pyrpkg/cli.py b/src/pyrpkg/cli.py
index 569bb6a..65a1477 100755
--- a/src/pyrpkg/cli.py
+++ b/src/pyrpkg/cli.py
@@ -936,13 +936,14 @@ defined, packages will be built sequentially.""" %
         task_id = self.cmd.build(self.args.skip_tag, self.args.scratch,
                                  self.args.background, url, chain, arches,
                                  sets, nvr_check)
-        # Now that we have the task ID we need to deal with it.
+
+        # Log out of the koji session
+        self.cmd.kojisession.logout()
+
         if self.args.nowait:
-            # Log out of the koji session
-            self.cmd.kojisession.logout()
             return
-        # pass info off to our koji task watcher
-        self.cmd.kojisession.logout()
+
+        # Pass info off to our koji task watcher
         return self._watch_koji_tasks(self.cmd.kojisession,
                                       [task_id])
 
-- 
2.1.0

[PATCH 18/20] Fix some flake8 issues

[Mathieu Bridon]

From: Mathieu Bridon <bochecha(a)daitauha.fr&gt;

---
 src/rpkg_man_page.py | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/rpkg_man_page.py b/src/rpkg_man_page.py
index b1318d7..468be6e 100644
--- a/src/rpkg_man_page.py
+++ b/src/rpkg_man_page.py
@@ -10,7 +10,6 @@
 # the full text of the license.
 
 
-
 import sys
 import datetime
 
@@ -51,15 +50,14 @@ man_footer = """\
 .BR "https://fedorahosted.org/rpkg/"
 """
 
+
 class ManFormatter(object):
 
     def __init__(self, man):
         self.man = man
 
     def write(self, data):
-        #print "MF:", repr(data)
         for line in data.split('\n'):
-            #print 'MFL:', line
             self.man.write('  %s\n' % line)
 
 
@@ -102,8 +100,8 @@ def generate(parser, subparsers):
     helptext = parser.format_help()
     helptext = strip_usage(helptext)
     helptextsplit = helptext.split('\n')
-    helptextsplit = [ line for line in helptextsplit
-                      if not line.startswith('  -h, --help') ]
+    helptextsplit = [line for line in helptextsplit
+                     if not line.startswith('  -h, --help')]
 
     man_file.write('.SS "%s"\n' % ("Global Options",))
 
@@ -145,7 +143,8 @@ def generate(parser, subparsers):
 
         help = help_texts[command]
         if help and not cmdparser.description:
-            if not help.endswith('.'): help = "%s." % help
+            if not help.endswith('.'):
+                help = "%s." % help
             cmdparser.description = help
 
         h = cmdparser.format_help()
-- 
2.1.0

[PATCH 19/20] Fix some more Flake8 issues

[Mathieu Bridon]

From: Mathieu Bridon <bochecha(a)daitauha.fr&gt;

---
 src/pyrpkg/__init__.py | 312 +++++++++++++++++++++++--------------------------
 1 file changed, 147 insertions(+), 165 deletions(-)

diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py
index 9070ac0..d0e5ccd 100644
--- a/src/pyrpkg/__init__.py
+++ b/src/pyrpkg/__init__.py
@@ -30,7 +30,6 @@ import stat
 import StringIO
 import tempfile
 import fnmatch
-import cli
 import urlparse
 import posixpath
 # Try to import krb, it's OK if it fails
@@ -41,16 +40,19 @@ except ImportError:
 
 import pyrpkg.sources
 
+
 # Define our own error class
 class rpkgError(Exception):
     pass
 
+
 # Setup our logger
 # Null logger to avoid spurious messages, add a handler in app code
 class NullHandler(logging.Handler):
     def emit(self, record):
         pass
 
+
 h = NullHandler()
 # This is our log object, clients of this library can use this object to
 # define their own logging needs
@@ -58,6 +60,7 @@ log = logging.getLogger("rpkg")
 # Add the null handler
 log.addHandler(h)
 
+
 class Commands(object):
     """This is a class to hold all the commands that will be called
     by clients
@@ -164,8 +167,8 @@ class Commands(object):
     @path.setter
     def path(self, value):
         if self._path != value:
-            #Ensure all properties which depend on self.path will be
-            #freshly loaded next time
+            # Ensure all properties which depend on self.path will be
+            # freshly loaded next time
             self._push_url = None
             self._branch_remote = None
             self._repo = None
@@ -187,15 +190,14 @@ class Commands(object):
 
         # Stealing a bunch of code from /usr/bin/koji here, too bad it isn't
         # in a more usable library form
-        defaults = {
-                    'server' : 'http://localhost/kojihub',
-                    'weburl' : 'http://localhost/koji',
-                    'pkgurl' : 'http://localhost/packages',
-                    'topdir' : '/mnt/koji',
+        defaults = {'server': 'http://localhost/kojihub',
+                    'weburl': 'http://localhost/koji',
+                    'pkgurl': 'http://localhost/packages',
+                    'topdir': '/mnt/koji',
                     'cert': '~/.koji/client.crt',
                     'ca': '~/.koji/clientca.crt',
                     'serverca': '~/.koji/serverca.crt',
-                    'krbservice' : 'host',
+                    'krbservice': 'host',
                     'authtype': None,
                     'topurl': None
                     }
@@ -216,10 +218,11 @@ class Commands(object):
                 if config.has_section(os.path.basename(self.build_client)):
                     for name, value in config.items(os.path.basename(
                                                     self.build_client)):
-                        if defaults.has_key(name):
+                        if name in defaults:
                             defaults[name] = value
         # Expand out the directory options
-        for name in ('topdir', 'cert', 'ca', 'serverca',
'topurl', 'krbservice'):
+        for name in ('topdir', 'cert', 'ca', 'serverca',
'topurl',
+                     'krbservice'):
             if defaults[name]:
                 defaults[name] = os.path.expanduser(defaults[name])
         self.log.debug('Initiating a %s session to %s' %
@@ -239,26 +242,23 @@ class Commands(object):
         self._kojiweburl = defaults['weburl']
         self._topurl = defaults['topurl']
         if not anon:
-                # Default to ssl if not otherwise specified and we have
-                # the cert
-                if defaults['authtype'] == 'ssl' or \
-                os.path.isfile(defaults['cert']) and \
-                defaults['authtype'] is None:
-                    self._kojisession.ssl_login(defaults['cert'],
-                                                defaults['ca'],
-                                                defaults['serverca'])
-                # Or try password auth
-                elif defaults['authtype'] == 'password' or \
-                'user' in defaults and defaults['authtype'] is None:
-                    self._kojisession.login()
-                # Or try kerberos
-                elif defaults['authtype'] == 'kerberos' or \
-                self._has_krb_creds() and \
-                defaults['authtype'] is None:
-                    self._kojisession.krb_login()
-                if not self._kojisession.logged_in:
-                    raise rpkgError('Could not auth with koji as %s' %
-                                    self.user)
+            # Default to ssl if not otherwise specified and we have the cert
+            if defaults['authtype'] == 'ssl' or \
+                    os.path.isfile(defaults['cert']) and \
+                    defaults['authtype'] is None:
+                self._kojisession.ssl_login(defaults['cert'],
+                                            defaults['ca'],
+                                            defaults['serverca'])
+            # Or try password auth
+            elif defaults['authtype'] == 'password' or 'user' in
defaults \
+                    and defaults['authtype'] is None:
+                self._kojisession.login()
+            # Or try kerberos
+            elif defaults['authtype'] == 'kerberos' or
self._has_krb_creds() \
+                    and defaults['authtype'] is None:
+                self._kojisession.krb_login()
+            if not self._kojisession.logged_in:
+                raise rpkgError('Could not auth with koji as %s' % self.user)
 
     @property
     def branch_merge(self):
@@ -416,9 +416,9 @@ class Commands(object):
     def load_localarch(self):
         """Get the local arch as defined by rpm"""
 
-        self._localarch = subprocess.Popen(['rpm --eval %{_arch}'],
-                       shell=True,
-                       stdout=subprocess.PIPE).communicate()[0].strip('\n')
+        proc = subprocess.Popen(['rpm --eval %{_arch}'], shell=True,
+                                stdout=subprocess.PIPE)
+        self._localarch = proc.communicate()[0].strip('\n')
 
     @property
     def mockconfig(self):
@@ -460,7 +460,7 @@ class Commands(object):
                     module_name = module_name[:-len('.git')]
                 self._module_name = module_name
                 return
-        except rpkgError, error:
+        except rpkgError:
             self.log.info('Failed to get module name from Git url or pushurl')
 
         self.load_nameverrel()
@@ -503,17 +503,18 @@ class Commands(object):
                     '--specfile', os.path.join(self.path, self.spec)])
         joined_cmd = ' '.join(cmd)
         try:
-            output, err = subprocess.Popen(joined_cmd, shell=True,
-                                      stderr=subprocess.PIPE,
-                                      stdout=subprocess.PIPE).communicate()
+            proc = subprocess.Popen(joined_cmd, shell=True,
+                                    stdout=subprocess.PIPE,
+                                    stderr=subprocess.PIPE)
+            output, err = proc.communicate()
         except Exception, e:
             if err:
                 self.log.debug('Errors occoured while running following'
                                ' command to get N-V-R-E:')
                 self.log.debug(joined_cmd)
                 self.log.error(err)
-            raise rpkgError('Could not query n-v-r of %s: %s' %
(self.module_name,
-                                                                 e))
+            raise rpkgError('Could not query n-v-r of %s: %s'
+                            % (self.module_name, e))
         if err:
             self.log.debug('Errors occoured while running following command to'
                            ' get N-V-R-E:')
@@ -533,7 +534,7 @@ class Commands(object):
 
         # Most packages don't include a "Epoch: 0" line, in which case RPM
         # returns '(none)'
-        if self._epoch ==  "(none)":
+        if self._epoch == "(none)":
             self._epoch = "0"
 
     @property
@@ -683,7 +684,7 @@ class Commands(object):
 
     def _has_krb_creds(self):
         # This function is lifted from /usr/bin/koji
-        if not sys.modules.has_key('krbV'):
+        if 'krbV' not in sys.modules:
             return False
         try:
             ctx = krbV.default_context()
@@ -705,9 +706,10 @@ class Commands(object):
         # Loop through the file reading chunks at a time as to not
         # put the entire file in memory.  That would suck for DVDs
         while True:
-            chunk = input.read(8192) # magic number!  Taking suggestions
+            chunk = input.read(8192)  # magic number!  Taking suggestions
             if not chunk:
-                break # we're done with the file
+                # we're done with the file
+                break
             sum.update(chunk)
         input.close()
         return sum.hexdigest()
@@ -760,23 +762,20 @@ class Commands(object):
                     # We're piping the stderr over as well, which is probably a
                     # bad thing, but rpmbuild likes to put useful data on
                     # stderr, so....
-                    proc = subprocess.Popen(command, env=environ,
+                    proc = subprocess.Popen(command, env=environ, shell=shell,
                                             stdout=subprocess.PIPE,
-                                            stderr=subprocess.STDOUT, shell=shell,
-                                            cwd=cwd)
-                    subprocess.check_call(pipecmd, env=environ,
-                                          stdout=sys.stdout,
-                                          stderr=sys.stderr,
+                                            stderr=subprocess.STDOUT, cwd=cwd)
+                    subprocess.check_call(pipecmd, env=environ, shell=shell,
                                           stdin=proc.stdout,
-                                          shell=shell,
-                                          cwd=cwd)
+                                          stdout=sys.stdout,
+                                          stderr=sys.stderr, cwd=cwd)
                     (output, err) = proc.communicate()
                     if proc.returncode:
                         raise rpkgError('Non zero exit')
                 else:
-                    subprocess.check_call(command, env=environ, stdout=sys.stdout,
-                                          stderr=sys.stderr, shell=shell,
-                                          cwd=cwd)
+                    subprocess.check_call(command, env=environ, shell=shell,
+                                          stdout=sys.stdout,
+                                          stderr=sys.stderr, cwd=cwd)
             except (subprocess.CalledProcessError,
                     OSError), e:
                 raise rpkgError(e)
@@ -797,26 +796,23 @@ class Commands(object):
                                              stderr=subprocess.STDOUT,
                                              shell=shell,
                                              cwd=cwd)
-                    proc = subprocess.Popen(pipecmd, env=environ,
-                                             stdin=proc1.stdout,
-                                             stdout=subprocess.PIPE,
-                                             stderr=subprocess.PIPE, shell=shell,
-                                             cwd=cwd)
+                    proc = subprocess.Popen(pipecmd, env=environ, shell=shell,
+                                            stdin=proc1.stdout,
+                                            stdout=subprocess.PIPE,
+                                            stderr=subprocess.PIPE,
+                                            cwd=cwd)
                     output, error = proc.communicate()
                 else:
-                    proc = subprocess.Popen(command, env=environ,
+                    proc = subprocess.Popen(command, env=environ, shell=shell,
                                             stdout=subprocess.PIPE,
-                                            stderr=subprocess.PIPE, shell=shell,
-                                            cwd=cwd)
+                                            stderr=subprocess.PIPE, cwd=cwd)
                     output, error = proc.communicate()
             except OSError, e:
                 raise rpkgError(e)
             self.log.info(output)
             if proc.returncode:
-                raise rpkgError('Command %s returned code %s with error: %s' %
-                                  (' '.join(cmd),
-                                   proc.returncode,
-                                   error))
+                raise rpkgError('Command %s returned code %s with error: %s'
+                                % (' '.join(cmd), proc.returncode, error))
         return
 
     def _verify_file(self, file, hash, hashtype):
@@ -846,8 +842,8 @@ class Commands(object):
         """Use curl manually to upload a file"""
 
         cmd = ['curl', '--fail', '-o', '/dev/null',
'--show-error',
-        '--progress-bar', '-F', 'name=%s' % self.module_name,
'-F',
-        'md5sum=%s' % file_hash, '-F', 'file=@%s' % file]
+               '--progress-bar', '-F', 'name=%s' %
self.module_name,
+               '-F', 'md5sum=%s' % file_hash, '-F',
'file=@%s' % file]
         if self.quiet:
             cmd.append('-s')
         cmd.append(self.lookaside_cgi)
@@ -861,9 +857,9 @@ class Commands(object):
         spec = os.path.join(self.path, self.spec)
         try:
             hdr = rpm.spec(spec)
-        except Exception, er:
+        except Exception:
             raise rpkgError('%s is not a spec file' % spec)
-        archlist = [ pkg.header['arch'] for pkg in hdr.packages]
+        archlist = [pkg.header['arch'] for pkg in hdr.packages]
         if not archlist:
             raise rpkgError('No compatible build arches found in %s' % spec)
         return archlist
@@ -892,7 +888,7 @@ class Commands(object):
         if excludearch:
             archlist = [a for a in archlist if a not in excludearch]
         # do the noarch thing
-        if 'noarch' not in excludearch and ('noarch' in buildarchs or \
+        if 'noarch' not in excludearch and ('noarch' in buildarchs or
                                             'noarch' in exclusivearch):
             archlist.append('noarch')
         # See if we have anything compatible.  Should we raise here?
@@ -939,7 +935,7 @@ class Commands(object):
             elif type(ref) == git.RemoteReference:
                 if ref.remote_head == 'HEAD':
                     self.log.debug('Skipping remote branch alias HEAD')
-                    continue # Not useful in this context
+                    continue  # Not useful in this context
                 self.log.debug('Found remote branch %s' % ref.name)
                 remotes.append(ref.name)
         return (locals, remotes)
@@ -949,7 +945,7 @@ class Commands(object):
 
         # get the name
         cmd = ['rpm', '-qp', '--nosignature', '--qf',
'%{NAME}', srpm]
-                # Run the command
+        # Run the command
         self.log.debug('Running: %s' % ' '.join(cmd))
         try:
             proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
@@ -979,9 +975,8 @@ class Commands(object):
         # like:
         # warning: foo-0.0.src.rpm Header V3 RSA/SHA256 Signature, key ID
         # fd431d51: NOKEY
-        if error and not error.startswith("warning:") and not "NOKEY"
in error:
+        if error and not error.startswith("warning:") and "NOKEY" not
in error:
             raise rpkgError('Error querying srpm: %s' % error)
-        # Doing a strip and split here as splitting on \n gets me an extra entry
         contents = output.strip().split('\n')
         # Cycle through the stuff and sort correctly by its extension
         for file in contents:
@@ -1051,8 +1046,8 @@ class Commands(object):
 
         branch is the name of a branch to checkout instead of <remote>/master
 
-        bare_dir is the name of a directory to make a bare clone to, if this is a
-        bare clone. None otherwise.
+        bare_dir is the name of a directory to make a bare clone to, if this
+        is a bare clone. None otherwise.
 
         anon is whether or not to clone anonymously
 
@@ -1122,8 +1117,8 @@ class Commands(object):
         try:
             os.mkdir(top_path)
         except (OSError), e:
-            raise rpkgError('Could not create directory for module %s: %s' %
-                    (module, e))
+            raise rpkgError('Could not create directory for module %s: %s'
+                            % (module, e))
 
         # Create a bare clone first. This gives us a good list of branches
         try:
@@ -1136,8 +1131,8 @@ class Commands(object):
         repo_git = git.Git(repo_path)
 
         # Get a branch listing
-        branches = [x for x in repo_git.branch().split() if x != "*" and
-                re.search(self.branchre, x)]
+        branches = [x for x in repo_git.branch().split()
+                    if x != "*" and re.search(self.branchre, x)]
 
         for branch in branches:
             try:
@@ -1153,8 +1148,8 @@ class Commands(object):
                                   "remote.%s.url" %
self.default_branch_remote,
                                   giturl)
             except (git.GitCommandError, OSError), e:
-                raise rpkgError('Could not locally clone %s from %s: %s' %
-                        (branch, repo_path, e))
+                raise rpkgError('Could not locally clone %s from %s: %s'
+                                % (branch, repo_path, e))
 
         # We don't need this now. Ignore errors since keeping it does no harm
         shutil.rmtree(repo_path, ignore_errors=True)
@@ -1177,7 +1172,8 @@ class Commands(object):
         # First lets see if we got a message or we're on a real tty:
         if not sys.stdin.isatty():
             if not message and not file:
-                raise rpkgError('Must have a commit message or be on a real
tty.')
+                raise rpkgError('Must have a commit message or be on a real '
+                                'tty.')
 
         # construct the git command
         # We do this via subprocess because the git module is terrible.
@@ -1212,7 +1208,7 @@ class Commands(object):
         except git.GitCommandError as e:
             raise rpkgError(e)
 
-        self.log.info ('Tag %s was deleted' % tagname)
+        self.log.info('Tag %s was deleted' % tagname)
 
     def diff(self, cached=False, files=[]):
         """Execute a git diff
@@ -1250,19 +1246,19 @@ class Commands(object):
         # This cmd below only works to scratch build rawhide
         # We need something better for epel
         cmd = ['git', 'ls-remote', url, 'refs/heads/%s' %
branch]
-        try :
+        try:
             proc = subprocess.Popen(cmd, stderr=subprocess.PIPE,
                                     stdout=subprocess.PIPE)
             output, error = proc.communicate()
-        except OSError, e:
+        except OSError as e:
             raise rpkgError(e)
         if error:
-            raise rpkgError('Got an error finding %s head for %s: %s' %
-                              (branch, module, error))
+            raise rpkgError('Got an error finding %s head for %s: %s'
+                            % (branch, module, error))
         # Return the hash sum
         if not output:
-            raise rpkgError('Could not find remote branch %s for %s' %
-                            (branch, module))
+            raise rpkgError('Could not find remote branch %s for %s'
+                            % (branch, module))
         return output.split()[0]
 
     def gitbuildhash(self, build):
@@ -1352,7 +1348,7 @@ class Commands(object):
         for file in ourfiles:
             if file not in files:
                 self.log.info("Removing no longer used file: %s" % file)
-                rv = self.repo.index.remove([file])
+                self.repo.index.remove([file])
                 os.remove(file)
 
         # Extract new files
@@ -1376,13 +1372,13 @@ class Commands(object):
                 # Create the file
                 open(file, 'w').close()
             files.append(file)
-        rv = self.repo.index.add(files)
+        self.repo.index.add(files)
         # Return to the caller and let them take it from there.
         os.chdir(oldpath)
         return(uploadfiles)
 
     def list_tag(self, tagname='*'):
-        """Create a list of all tags in the repository which match a given
tagname.
+        """List all tags in the repository which match a given tagname.
 
         The optional `tagname` argument may be a shell glob (it is matched
         with fnmatch).
@@ -1475,7 +1471,7 @@ class Commands(object):
 
         # Add it to the index
         # Again this returns a blank line we want to keep quiet
-        rv = self.repo.index.add([outfile])
+        self.repo.index.add([outfile])
         log.info('Created %s and added it to the index' % outfile)
 
     def pull(self, rebase=False, norebase=False):
@@ -1532,28 +1528,10 @@ class Commands(object):
             url = '%s/%s/%s/%s/%s' % (self.lookaside, self.module_name,
                                       file.replace(' ', '%20'),
                                       csum, file.replace(' ', '%20'))
-            # There is some code here for using pycurl, but for now,
-            # just use subprocess
-            #output = open(file, 'wb')
-            #curl = pycurl.Curl()
-            #curl.setopt(pycurl.URL, url)
-            #curl.setopt(pycurl.FOLLOWLOCATION, 1)
-            #curl.setopt(pycurl.MAXREDIRS, 5)
-            #curl.setopt(pycurl.CONNECTTIMEOUT, 30)
-            #curl.setopt(pycurl.TIMEOUT, 300)
-            #curl.setopt(pycurl.WRITEDATA, output)
-            #try:
-            #    curl.perform()
-            #except:
-            #    print "Problems downloading %s" % url
-            #    curl.close()
-            #    output.close()
-            #    return 1
-            #curl.close()
-            #output.close()
             # These options came from Makefile.common.
             # Probably need to support wget as well
-            command = ['curl', '-H', 'Pragma:', '-o',
outfile, '-R', '-S', '--fail']
+            command = ['curl', '-H', 'Pragma:', '-o',
outfile, '-R', '-S',
+                       '--fail']
             if self.quiet:
                 command.append('-s')
             command.append(url)
@@ -1583,7 +1561,7 @@ class Commands(object):
         # Get our list of branches
         (locals, remotes) = self._list_branches(fetch)
 
-        if not branch in locals:
+        if branch not in locals:
             # We need to create a branch
             self.log.debug('No local branch found, creating a new one')
             totrack = None
@@ -1596,15 +1574,18 @@ class Commands(object):
                 raise rpkgError('Unknown remote branch %s' % full_branch)
             try:
                 self.log.info(self.repo.git.checkout('-b', branch,
'--track',
-                                                totrack))
-            except Exception, err: # this needs to be finer grained I think...
-                raise rpkgError('Could not create branch %s: %s' % (branch,
err))
+                                                     totrack))
+            except Exception, err:
+                # This needs to be finer grained I think...
+                raise rpkgError('Could not create branch %s: %s'
+                                % (branch, err))
         else:
             try:
-                output = self.repo.git.checkout(branch)
+                self.repo.git.checkout(branch)
                 # The above should have no output, but stash it anyway
                 self.log.info("Switched to branch '%s'" % branch)
-            except: # This needs to be finer grained I think...
+            except:
+                # This needs to be finer grained I think...
                 raise rpkgError('Could not check out %s' % branch)
         return
 
@@ -1624,10 +1605,9 @@ class Commands(object):
         # Setup the POST data for lookaside CGI request. The use of
         # 'filename' here appears to be what differentiates this
         # request from an actual file upload.
-        post_data = [
-                ('name', pkg_name),
-                ('md5sum', md5sum),
-                ('filename', filename)]
+        post_data = [('name', pkg_name),
+                     ('md5sum', md5sum),
+                     ('filename', filename)]
 
         curl = self._create_curl()
         curl.setopt(pycurl.WRITEFUNCTION, buf.write)
@@ -1650,18 +1630,17 @@ class Commands(object):
         # Something unexpected happened, will trigger if the lookaside URL
         # cannot be reached, the package named does not exist, and probably
         # some other scenarios as well.
-        raise rpkgError("Error checking for %s at: %s" %
-                (filename, self.lookaside_cgi))
+        raise rpkgError("Error checking for %s at: %s"
+                        % (filename, self.lookaside_cgi))
 
     def upload_file(self, pkg_name, filepath, md5sum):
         """ Upload a file to the lookaside cache. """
 
         # Setup the POST data for lookaside CGI request. The use of
         # 'file' here appears to trigger the actual upload:
-        post_data = [
-                ('name', pkg_name),
-                ('md5sum', md5sum),
-                ('file', (pycurl.FORM_FILE, filepath))]
+        post_data = [('name', pkg_name),
+                     ('md5sum', md5sum),
+                     ('file', (pycurl.FORM_FILE, filepath))]
 
         curl = self._create_curl()
         curl.setopt(pycurl.HTTPPOST, post_data)
@@ -1708,8 +1687,8 @@ class Commands(object):
             if self.repo.is_dirty():
                 raise rpkgError('%s has uncommitted changes.  Use git status '
                                 'to see details' % self.path)
-            # Need to check here to see if the local commit you want to build is
-            # pushed or not
+            # Need to check here to see if the local commit you want to build
+            # is pushed or not
             branch = self.repo.active_branch
             full_branch = '%s/%s' % (self.branch_remote, self.branch_merge)
             if self.repo.git.rev_list('%s...%s' % (full_branch, branch)):
@@ -1724,17 +1703,17 @@ class Commands(object):
         # see if the dest tag is locked
         dest_tag = self.kojisession.getTag(build_target['dest_tag_name'])
         if not dest_tag:
-            raise rpkgError('Unknown destination tag %s' %
-                              build_target['dest_tag_name'])
+            raise rpkgError('Unknown destination tag %s'
+                            % build_target['dest_tag_name'])
         if dest_tag['locked'] and not scratch:
             raise rpkgError('Destination tag %s is locked' %
dest_tag['name'])
         # If we're chain building, make sure inheritance works
         if chain:
             cmd.append('chain-build')
             ancestors = self.kojisession.getFullInheritance(
-                                                    build_target['build_tag'])
-            if dest_tag['id'] not in [build_target['build_tag']] + \
-            [ancestor['parent_id'] for ancestor in ancestors]:
+                build_target['build_tag'])
+            ancestors = [ancestor['parent_id'] for ancestor in ancestors]
+            if dest_tag['id'] not in [build_target['build_tag']] +
ancestors:
                 raise rpkgError('Packages in destination tag '
                                 '%(dest_tag_name)s are not inherited by'
                                 'build tag %(build_tag_name)s' %
@@ -1753,10 +1732,11 @@ class Commands(object):
             cmd.append('--scratch')
         if background:
             cmd.append('--background')
-            priority = 5 # magic koji number :/
+            priority = 5  # magic koji number :/
         if arches:
             if not scratch:
-                raise rpkgError('Cannot override arches for non-scratch builds')
+                raise rpkgError('Cannot override arches for non-scratch '
+                                'builds')
             for arch in arches:
                 if not re.match(r'^[0-9a-zA-Z_.]+$', arch):
                     raise rpkgError('Invalid architecture name: %s' % arch)
@@ -1982,9 +1962,10 @@ class Commands(object):
             if os.path.exists(os.path.join(self.path, arch)):
                 # For each available arch folder, lists file and keep
                 # those ending with .rpm
-                rpms.extend([os.path.join(self.path, arch, file) for file in
-                         os.listdir(os.path.join(self.path, arch))
-                         if file.endswith('.rpm')])
+                rpms.extend([os.path.join(self.path, arch, file)
+                             for file in os.listdir(os.path.join(self.path,
+                                                    arch))
+                             if file.endswith('.rpm')])
         if not rpms:
             log.warn('No rpm found')
         cmd = ['rpmlint']
@@ -2027,8 +2008,10 @@ class Commands(object):
             hashtype = self._guess_hashtype()
         # This may need to get updated if we ever change our checksum default
         if not hashtype == 'sha256':
-            cmd.extend(["--define '_source_filedigest_algorithm %s'" %
hashtype,
-                    "--define '_binary_filedigest_algorithm %s'" %
hashtype])
+            cmd.extend(["--define '_source_filedigest_algorithm %s'"
+                        % hashtype,
+                        "--define '_binary_filedigest_algorithm %s'"
+                        % hashtype])
         if arch:
             cmd.extend(['--target', arch])
         if self.quiet:
@@ -2059,16 +2042,16 @@ class Commands(object):
 
         try:
             repoid = self.anon_kojisession.getRepo(
-                                build_target['build_tag_name'])['id']
-        except Exception, e:
+                build_target['build_tag_name'])['id']
+        except Exception:
             raise rpkgError('Could not find a valid build repo')
 
         # Generate the config
         config = koji.genMockConfig('%s-%s' % (target, arch), arch,
-                                   distribution=self.disttag,
-                                   tag_name=build_target['build_tag_name'],
-                                   repoid=repoid,
-                                   topurl=self.topurl)
+                                    distribution=self.disttag,
+                                    tag_name=build_target['build_tag_name'],
+                                    repoid=repoid,
+                                    topurl=self.topurl)
 
         # Return the mess
         return(config)
@@ -2242,10 +2225,6 @@ class Commands(object):
             else:
                 # Ensure the new file is readable:
                 os.chmod(f, stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH)
-                #lookaside.upload_file(self.module, f, file_hash)
-                # For now don't use the pycurl upload function as it does
-                # not produce any progress output.  Cheat and use curl
-                # directly.
                 self._do_curl(file_hash, f)
                 uploaded.append(file_basename)
 
@@ -2254,7 +2233,7 @@ class Commands(object):
         # Write .gitignore with the new sources if anything changed:
         gitignore.write()
 
-        rv = self.repo.index.add(['sources', '.gitignore'])
+        self.repo.index.add(['sources', '.gitignore'])
 
         # Change back to original working dir:
         os.chdir(oldpath)
@@ -2296,8 +2275,8 @@ class Commands(object):
         """
 
         self.srpmname = os.path.join(self.path,
-                            "%s-%s-%s.src.rpm" % (self.module_name,
-                                                  self.ver, self.rel))
+                                     "%s-%s-%s.src.rpm"
+                                     % (self.module_name, self.ver, self.rel))
         # See if we need to build the srpm
         if os.path.exists(self.srpmname):
             self.log.debug('Srpm found, rewriting it.')
@@ -2312,8 +2291,10 @@ class Commands(object):
             hashtype = self._guess_hashtype()
         # This may need to get updated if we ever change our checksum default
         if not hashtype == 'sha256':
-            cmd.extend(["--define '_source_filedigest_algorithm %s'" %
hashtype,
-                    "--define '_binary_filedigest_algorithm %s'" %
hashtype])
+            cmd.extend(["--define '_source_filedigest_algorithm %s'"
+                        % hashtype,
+                        "--define '_binary_filedigest_algorithm %s'"
+                        % hashtype])
         cmd.extend(['--nodeps', '-bs', os.path.join(self.path,
self.spec)])
         self._run_command(cmd, shell=True)
 
@@ -2336,7 +2317,7 @@ class Commands(object):
         files = self.repo.git.ls_files('--exclude-standard').split()
         for file in files:
             # throw out non patches
-            if not file.endswith(('.patch','.diff')):
+            if not file.endswith(('.patch', '.diff')):
                 continue
             if file not in spec:
                 unused.append(file)
@@ -2361,6 +2342,7 @@ class Commands(object):
         # Run the command
         self._run_command(cmd, shell=True)
 
+
 class GitIgnore(object):
     """ Smaller wrapper for managing a .gitignore file and it's
entries. """
 
@@ -2373,8 +2355,8 @@ class GitIgnore(object):
         """
         self.path = path
 
-        # Lines of the .gitignore file, used to check if entries need to be added
-        # or already exist.
+        # Lines of the .gitignore file, used to check if entries need to be
+        # added or already exist.
         self.__lines = []
         if os.path.exists(self.path):
             gitignore_file = open(self.path, 'r')
@@ -2401,7 +2383,7 @@ class GitIgnore(object):
         line = self.__ensure_newline(line)
 
         # Add this line if it doesn't already exist:
-        if not line in self.__lines:
+        if line not in self.__lines:
             self.__lines.append(line)
             self.modified = True
 
-- 
2.1.0

[PATCH 20/20] Massive Flake8 fix

[Mathieu Bridon]

From: Mathieu Bridon <bochecha(a)daitauha.fr&gt;

---
 src/pyrpkg/cli.py | 963 ++++++++++++++++++++++++------------------------------
 1 file changed, 425 insertions(+), 538 deletions(-)

diff --git a/src/pyrpkg/cli.py b/src/pyrpkg/cli.py
index 65a1477..7b40c4b 100755
--- a/src/pyrpkg/cli.py
+++ b/src/pyrpkg/cli.py
@@ -23,6 +23,7 @@ import xmlrpclib
 import pwd
 import koji
 
+
 class cliClient(object):
     """This is a client class for rpkg clients."""
 
@@ -34,18 +35,18 @@ class cliClient(object):
 
         self.config = config
         self._name = name
-        #Define default name in child class
-        #self.DEFAULT_CLI_NAME = None
+        # Define default name in child class
+        # self.DEFAULT_CLI_NAME = None
         # Property holders, set to none
         self._cmd = None
         self._module = None
         # Setup the base argparser
         self.setup_argparser()
         # Add a subparser
-        self.subparsers = self.parser.add_subparsers(title = 'Targets',
-                                                 description = 'These are '
-                                                 'valid commands you can '
-                                                 'ask %s to do' % self.name)
+        self.subparsers = self.parser.add_subparsers(
+            title='Targets',
+            description='These are valid commands you can ask %s to do'
+                        % self.name)
         # Register all the commands
         self.setup_subparsers()
 
@@ -64,11 +65,11 @@ class cliClient(object):
             try:
                 name = self.DEFAULT_CLI_NAME
             except AttributeError:
-                #Ignore missing DEFAULT_CLI_NAME for backwards
-                #compatibility
+                # Ignore missing DEFAULT_CLI_NAME for backwards
+                # compatibility
                 pass
         if not name:
-            #We don't have logger available yet
+            # We don't have logger available yet
             sys.stderr.write('Could not determine CLI name\n')
             sys.exit(1)
         return name
@@ -135,9 +136,9 @@ class cliClient(object):
     def setup_argparser(self):
         """Setup the argument parser and register some basic
commands."""
 
-        self.parser = argparse.ArgumentParser(prog = self.name,
-                                              epilog = 'For detailed help '
-                                              'pass --help to a target')
+        self.parser = argparse.ArgumentParser(
+            prog=self.name,
+            epilog='For detailed help pass --help to a target')
         # Add some basic arguments that should be used by all.
         # Add a config file
         self.parser.add_argument('--config', '-C',
@@ -162,10 +163,10 @@ class cliClient(object):
                                  help='Define the directory to work in '
                                  '(defaults to cwd)')
         # Verbosity
-        self.parser.add_argument('-v', action = 'store_true',
-                                 help = 'Run with verbose debug output')
-        self.parser.add_argument('-q', action = 'store_true',
-                                 help = 'Run quietly only displaying errors')
+        self.parser.add_argument('-v', action='store_true',
+                                 help='Run with verbose debug output')
+        self.parser.add_argument('-q', action='store_true',
+                                 help='Run quietly only displaying errors')
 
     def setup_subparsers(self):
         """Setup basic subparsers that all clients should
use"""
@@ -217,84 +218,75 @@ class cliClient(object):
     def register_help(self):
         """Register the help command."""
 
-        help_parser = self.subparsers.add_parser('help', help = 'Show
usage')
-        help_parser.set_defaults(command = self.parser.print_help)
+        help_parser = self.subparsers.add_parser('help', help='Show
usage')
+        help_parser.set_defaults(command=self.parser.print_help)
 
     # Setup a couple common parsers to save code duplication
     def register_build_common(self):
         """Create a common build parser to use in other
commands"""
 
-        self.build_parser_common = argparse.ArgumentParser('build_common',
-                                                         add_help = False)
-        self.build_parser_common.add_argument('--arches', nargs = '*',
-                                         help = 'Build for specific arches')
-        self.build_parser_common.add_argument('--md5',
action='store_const',
-                              const='md5', default=None, dest='hash',
-                              help='Use md5 checksums (for older rpm hosts)')
-        self.build_parser_common.add_argument('--nowait',
-                                         action = 'store_true',
-                                         default = False,
-                                         help = "Don't wait on build")
-        self.build_parser_common.add_argument('--target',
-                                         default = None,
-                                         help = 'Define build target to build '
-                                         'into')
-        self.build_parser_common.add_argument('--background',
-                                         action = 'store_true',
-                                         default = False,
-                                         help = 'Run the build at a low '
-                                         'priority')
+        self.build_parser_common = argparse.ArgumentParser(
+            'build_common', add_help=False)
+        self.build_parser_common.add_argument(
+            '--arches', nargs='*', help='Build for specific
arches')
+        self.build_parser_common.add_argument(
+            '--md5', action='store_const', const='md5',
default=None,
+            dest='hash', help='Use md5 checksums (for older rpm hosts)')
+        self.build_parser_common.add_argument(
+            '--nowait', action='store_true', default=False,
+            help="Don't wait on build")
+        self.build_parser_common.add_argument(
+            '--target', default=None,
+            help='Define build target to build into')
+        self.build_parser_common.add_argument(
+            '--background', action='store_true', default=False,
+            help='Run the build at a low priority')
 
     def register_rpm_common(self):
         """Create a common parser for rpm commands"""
 
-        self.rpm_parser_common = argparse.ArgumentParser('rpm_common',
-                                                         add_help=False)
-        self.rpm_parser_common.add_argument('--builddir', default=None,
-                                        help='Define an alternate builddir')
-        self.rpm_parser_common.add_argument('--arch',
-                                            help='Prep for a specific arch')
+        self.rpm_parser_common = argparse.ArgumentParser(
+            'rpm_common', add_help=False)
+        self.rpm_parser_common.add_argument(
+            '--builddir', default=None, help='Define an alternate
builddir')
+        self.rpm_parser_common.add_argument(
+            '--arch', help='Prep for a specific arch')
 
     def register_build(self):
         """Register the build target"""
 
-        build_parser = self.subparsers.add_parser('build',
-                                         help = 'Request build',
-                                         parents = [self.build_parser_common],
-                                         description = 'This command \
-                                         requests a build of the package \
-                                         in the build system.  By default \
-                                         it discovers the target to build for \
-                                         based on branch data, and uses the \
-                                         latest commit as the build source.')
-        build_parser.add_argument('--skip-nvr-check',
action='store_false',
-                                  default=True, dest='nvr_check',
-                                  help=('Submit build to buildsystem without'
-                                        ' check if NVR was already build.'
-                                        ' NVR is constructed locally and may'
-                                        ' be different from NVR constructed'
-                                        ' during build on builder.')
-                                  )
-        build_parser.add_argument('--skip-tag', action = 'store_true',
-                                  default = False,
-                                  help = 'Do not attempt to tag package')
-        build_parser.add_argument('--scratch', action = 'store_true',
-                                  default = False,
-                                  help = 'Perform a scratch build')
-        build_parser.add_argument('--srpm', nargs = '?', const =
'CONSTRUCT',
-                                  help = 'Build from an srpm.  If no srpm \
-                                  is provided with this option an srpm will \
-                                  be generated from current module content.')
-        build_parser.set_defaults(command = self.build)
+        build_parser = self.subparsers.add_parser(
+            'build', help='Request build',
parents=[self.build_parser_common],
+            description='This command requests a build of the package in the '
+                        'build system. By default it discovers the target '
+                        'to build for based on branch data, and uses the '
+                        'latest commit as the build source.')
+        build_parser.add_argument(
+            '--skip-nvr-check', action='store_false', default=True,
+            dest='nvr_check',
+            help='Submit build to buildsystem without check if NVR was '
+                 'already build. NVR is constructed locally and may be '
+                 'different from NVR constructed during build on builder.')
+        build_parser.add_argument(
+            '--skip-tag', action='store_true', default=False,
+            help='Do not attempt to tag package')
+        build_parser.add_argument(
+            '--scratch', action='store_true', default=False,
+            help='Perform a scratch build')
+        build_parser.add_argument(
+            '--srpm', nargs='?', const='CONSTRUCT',
+            help='Build from an srpm. If no srpm is provided with this option'
+                 ' an srpm will be generated from current module content.')
+        build_parser.set_defaults(command=self.build)
 
     def register_chainbuild(self):
         """Register the chain build target"""
 
-        chainbuild_parser = self.subparsers.add_parser('chain-build',
-                    help = 'Build current package in order with other packages',
-                    parents = [self.build_parser_common],
-                    formatter_class=argparse.RawDescriptionHelpFormatter,
-                    description = """
+        chainbuild_parser = self.subparsers.add_parser(
+            'chain-build', parents=[self.build_parser_common],
+            help='Build current package in order with other packages',
+            formatter_class=argparse.RawDescriptionHelpFormatter,
+            description="""
 Build current package in order with other packages.
 
 example: %(name)s chain-build libwidget libgizmo
@@ -311,583 +303,475 @@ For example:
 
 will cause libwidget and libaselib to be built in parallel, followed
 by libgizmo and then the current directory package. If no groups are
-defined, packages will be built sequentially.""" %
-                    {'name': self.name})
-        chainbuild_parser.add_argument('package', nargs = '+',
-                                       help = 'List the packages and order you '
-                                       'want to build in')
-        chainbuild_parser.set_defaults(command = self.chainbuild)
+defined, packages will be built sequentially.""" % {'name':
self.name})
+        chainbuild_parser.add_argument(
+            'package', nargs='+',
+            help='List the packages and order you want to build in')
+        chainbuild_parser.set_defaults(command=self.chainbuild)
 
     def register_clean(self):
         """Register the clean target"""
-        clean_parser = self.subparsers.add_parser('clean',
-                                         help = 'Remove untracked files',
-                                         description = "This command can be \
-                                         used to clean up your working \
-                                         directory.  By default it will \
-                                         follow .gitignore rules.")
-        clean_parser.add_argument('--dry-run', '-n', action =
'store_true',
-                                  help = 'Perform a dry-run')
-        clean_parser.add_argument('-x', action = 'store_true',
-                                  help = 'Do not follow .gitignore rules')
-        clean_parser.set_defaults(command = self.clean)
+        clean_parser = self.subparsers.add_parser(
+            'clean', help='Remove untracked files',
+            description="This command can be used to clean up your working "
+                        "directory. By default it will follow .gitignore "
+                        "rules.")
+        clean_parser.add_argument(
+            '--dry-run', '-n', action='store_true',
help='Perform a dry-run')
+        clean_parser.add_argument(
+            '-x', action='store_true', help='Do not follow .gitignore
rules')
+        clean_parser.set_defaults(command=self.clean)
 
     def register_clog(self):
         """Register the clog target"""
 
-        clog_parser = self.subparsers.add_parser('clog',
-                                        help = 'Make a clog file containing '
-                                        'top changelog entry',
-                                        description = 'This will create a \
-                                        file named "clog" that contains the \
-                                        latest rpm changelog entry. The \
-                                        leading "- " text will be
stripped.')
-        clog_parser.add_argument('--raw', action='store_true',
-                                 default=False,
-                                 help='Generate a more "raw" clog without
\
-                                 twiddling the contents')
-        clog_parser.set_defaults(command = self.clog)
+        clog_parser = self.subparsers.add_parser(
+            'clog', help='Make a clog file containing top changelog
entry',
+            description='This will create a file named "clog" that contains
'
+                        'the latest rpm changelog entry. The leading "- "
'
+                        'text will be stripped.')
+        clog_parser.add_argument(
+            '--raw', action='store_true', default=False,
+            help='Generate a more "raw" clog without twiddling the
contents')
+        clog_parser.set_defaults(command=self.clog)
 
     def register_clone(self):
         """Register the clone target and co alias"""
 
-        clone_parser = self.subparsers.add_parser('clone',
-                                         help = 'Clone and checkout a module',
-                                         description = 'This command will \
-                                         clone the named module from the \
-                                         configured repository base URL.  \
-                                         By default it will also checkout \
-                                         the master branch for your working \
-                                         copy.')
+        clone_parser = self.subparsers.add_parser(
+            'clone', help='Clone and checkout a module',
+            description='This command will clone the named module from the '
+                        'configured repository base URL. By default it will '
+                        'also checkout the master branch for your working '
+                        'copy.')
         # Allow an old style clone with subdirs for branches
-        clone_parser.add_argument('--branches', '-B',
-                                  action = 'store_true',
-                                  help = 'Do an old style checkout with \
-                                  subdirs for branches')
+        clone_parser.add_argument(
+            '--branches', '-B', action='store_true',
+            help='Do an old style checkout with subdirs for branches')
         # provide a convenient way to get to a specific branch
-        clone_parser.add_argument('--branch', '-b',
-                                  help = 'Check out a specific branch')
+        clone_parser.add_argument(
+            '--branch', '-b', help='Check out a specific
branch')
         # allow to clone without needing a account on the scm server
-        clone_parser.add_argument('--anonymous', '-a',
-                                  action = 'store_true',
-                                  help = 'Check out a module anonymously')
+        clone_parser.add_argument(
+            '--anonymous', '-a', action='store_true',
+            help='Check out a module anonymously')
         # store the module to be cloned
-        clone_parser.add_argument('module', nargs = 1,
-                                  help = 'Name of the module to clone')
-        clone_parser.set_defaults(command = self.clone)
+        clone_parser.add_argument(
+            'module', nargs=1, help='Name of the module to clone')
+        clone_parser.set_defaults(command=self.clone)
 
         # Add an alias for historical reasons
-        co_parser = self.subparsers.add_parser('co', parents = [clone_parser],
-                                          conflict_handler = 'resolve',
-                                          help = 'Alias for clone',
-                                          description = 'This command will \
-                                          clone the named module from the \
-                                          configured repository base URL.  \
-                                          By default it will also checkout \
-                                          the master branch for your working \
-                                          copy.')
-        co_parser.set_defaults(command = self.clone)
+        co_parser = self.subparsers.add_parser(
+            'co', parents=[clone_parser], conflict_handler='resolve',
+            help='Alias for clone')
+        co_parser.set_defaults(command=self.clone)
 
     def register_commit(self):
         """Register the commit target and ci alias"""
 
-        commit_parser = self.subparsers.add_parser('commit',
-                                          help = 'Commit changes',
-                                          description = 'This invokes a git \
-                                          commit.  All tracked files with \
-                                          changes will be committed unless \
-                                          a specific file list is provided.  \
-                                          $EDITOR will be used to generate a \
-                                          changelog message unless one is \
-                                          given to the command.  A push \
-                                          can be done at the same time.')
-        commit_parser.add_argument('-c', '--clog',
-                                   default = False,
-                                   action = 'store_true',
-                                   help = 'Generate the commit message from \
-                                   the Changelog section')
-        commit_parser.add_argument('--raw', action='store_true',
-                                   default=False,
-                                   help='Make the clog raw')
-        commit_parser.add_argument('-t', '--tag',
-                                   default = False,
-                                   action = 'store_true',
-                                   help = 'Create a tag for this commit')
-        commit_parser.add_argument('-m', '--message',
-                                   default = None,
-                                   help = 'Use the given <msg> as the commit \
-                                   message')
-        commit_parser.add_argument('-F', '--file',
-                                   default = None,
-                                   help = 'Take the commit message from the \
-                                   given file')
+        commit_parser = self.subparsers.add_parser(
+            'commit', help='Commit changes',
+            description='This invokes a git commit. All tracked files with '
+                        'changes will be committed unless a specific file '
+                        'list is provided. $EDITOR will be used to generate a'
+                        ' changelog message unless one is given to the '
+                        'command. A push can be done at the same time.')
+        commit_parser.add_argument(
+            '-c', '--clog', default=False, action='store_true',
+            help='Generate the commit message from the Changelog section')
+        commit_parser.add_argument(
+            '--raw', action='store_true', default=False,
+            help='Make the clog raw')
+        commit_parser.add_argument(
+            '-t', '--tag', default=False, action='store_true',
+            help='Create a tag for this commit')
+        commit_parser.add_argument(
+            '-m', '--message', default=None,
+            help='Use the given <msg> as the commit message')
+        commit_parser.add_argument(
+            '-F', '--file', default=None,
+            help='Take the commit message from the given file')
         # allow one to commit /and/ push at the same time.
-        commit_parser.add_argument('-p', '--push',
-                                   default = False,
-                                   action = 'store_true',
-                                   help = 'Commit and push as one action')
+        commit_parser.add_argument(
+            '-p', '--push', default=False, action='store_true',
+            help='Commit and push as one action')
         # Allow a list of files to be committed instead of everything
-        commit_parser.add_argument('files', nargs = '*',
-                                   default = [],
-                                   help = 'Optional list of specific files to \
-                                   commit')
-        commit_parser.add_argument('-s', '--signoff',
-                                   default = False,
-                                   action = 'store_true',
-                                   help = 'Include a signed-off-by')
-        commit_parser.set_defaults(command = self.commit)
+        commit_parser.add_argument(
+            'files', nargs='*', default=[],
+            help='Optional list of specific files to commit')
+        commit_parser.add_argument(
+            '-s', '--signoff', default=False,
action='store_true',
+            help='Include a signed-off-by')
+        commit_parser.set_defaults(command=self.commit)
 
         # Add a ci alias
-        ci_parser = self.subparsers.add_parser('ci', parents = [commit_parser],
-                                          conflict_handler = 'resolve',
-                                          help = 'Alias for commit',
-                                          description = 'This invokes a git \
-                                          commit.  All tracked files with \
-                                          changes will be committed unless \
-                                          a specific file list is provided.  \
-                                          $EDITOR will be used to generate a \
-                                          changelog message unless one is \
-                                          given to the command.  A push \
-                                          can be done at the same time.')
-        ci_parser.set_defaults(command = self.commit)
+        ci_parser = self.subparsers.add_parser(
+            'ci', parents=[commit_parser], conflict_handler='resolve',
+            help='Alias for commit')
+        ci_parser.set_defaults(command=self.commit)
 
     def register_compile(self):
         """Register the compile target"""
 
-        compile_parser = self.subparsers.add_parser('compile',
-                                       parents=[self.rpm_parser_common],
-                                       help = 'Local test rpmbuild compile',
-                                       description = 'This command calls \
-                                       rpmbuild to compile the source.  \
-                                       By default the prep and configure \
-                                       stages will be done as well, \
-                                       unless the short-circuit option \
-                                       is used.')
-        compile_parser.add_argument('--short-circuit', action =
'store_true',
-                                    help = 'short-circuit compile')
-        compile_parser.set_defaults(command = self.compile)
+        compile_parser = self.subparsers.add_parser(
+            'compile', parents=[self.rpm_parser_common],
+            help='Local test rpmbuild compile',
+            description='This command calls rpmbuild to compile the source. '
+                        'By default the prep and configure stages will be '
+                        'done as well, unless the short-circuit option is '
+                        'used.')
+        compile_parser.add_argument(
+            '--short-circuit', action='store_true',
+            help='short-circuit compile')
+        compile_parser.set_defaults(command=self.compile)
 
     def register_diff(self):
         """Register the diff target"""
 
-        diff_parser = self.subparsers.add_parser('diff',
-                                        help = 'Show changes between commits, '
-                                        'commit and working tree, etc',
-                                        description = 'Use git diff to show \
-                                        changes that have been made to \
-                                        tracked files.  By default cached \
-                                        changes (changes that have been git \
-                                        added) will not be shown.')
-        diff_parser.add_argument('--cached', default = False,
-                                 action = 'store_true',
-                                 help = 'View staged changes')
-        diff_parser.add_argument('files', nargs = '*',
-                                 default = [],
-                                 help = 'Optionally diff specific files')
-        diff_parser.set_defaults(command = self.diff)
+        diff_parser = self.subparsers.add_parser(
+            'diff', help='Show changes between commits, commit and working
'
+                         'tree, etc',
+            description='Use git diff to show changes that have been made to '
+                        'tracked files. By default cached changes (changes '
+                        'that have been git added) will not be shown.')
+        diff_parser.add_argument(
+            '--cached', default=False, action='store_true',
+            help='View staged changes')
+        diff_parser.add_argument(
+            'files', nargs='*', default=[],
+            help='Optionally diff specific files')
+        diff_parser.set_defaults(command=self.diff)
 
     def register_gimmespec(self):
         """Register the gimmespec target"""
 
-        gimmespec_parser = self.subparsers.add_parser('gimmespec',
-                                         help = 'Print the spec file name')
-        gimmespec_parser.set_defaults(command = self.gimmespec)
+        gimmespec_parser = self.subparsers.add_parser(
+            'gimmespec', help='Print the spec file name')
+        gimmespec_parser.set_defaults(command=self.gimmespec)
 
     def register_gitbuildhash(self):
         """Register the gitbuildhash target"""
 
-        gitbuildhash_parser = self.subparsers.add_parser('gitbuildhash',
-                                          help = 'Print the git hash used '
-                                          'to build the provided n-v-r',
-                                          description = 'This will show you \
-                                          the commit hash string used to \
-                                          build the provided build n-v-r')
-        gitbuildhash_parser.add_argument('build',
-                                         help='name-version-release of the \
-                                         build to query.')
-        gitbuildhash_parser.set_defaults(command = self.gitbuildhash)
+        gitbuildhash_parser = self.subparsers.add_parser(
+            'gitbuildhash',
+            help='Print the git hash used to build the provided n-v-r',
+            description='This will show you the commit hash string used to '
+                        'build the provided build n-v-r')
+        gitbuildhash_parser.add_argument(
+            'build', help='name-version-release of the build to query.')
+        gitbuildhash_parser.set_defaults(command=self.gitbuildhash)
 
     def register_giturl(self):
         """Register the giturl target"""
 
-        giturl_parser = self.subparsers.add_parser('giturl',
-                                          help = 'Print the git url for '
-                                          'building',
-                                          description = 'This will show you \
-                                          which git URL would be used in a \
-                                          build command.  It uses the git \
-                                          hashsum of the HEAD of the current \
-                                          branch (which may not be pushed).')
-        giturl_parser.set_defaults(command = self.giturl)
+        giturl_parser = self.subparsers.add_parser(
+            'giturl', help='Print the git url for building',
+            description='This will show you which git URL would be used in a '
+                        'build command. It uses the git hashsum of the HEAD '
+                        'of the current branch (which may not be pushed).')
+        giturl_parser.set_defaults(command=self.giturl)
 
     def register_import_srpm(self):
         """Register the import-srpm target"""
 
-        import_srpm_parser = self.subparsers.add_parser('import',
-                                               help = 'Import srpm content '
-                                               'into a module',
-                                               description = 'This will \
-                                               extract sources, patches, and \
-                                               the spec file from an srpm and \
-                                               update the current module \
-                                               accordingly.  It will import \
-                                               to the current branch by \
-                                               default.')
-        #import_srpm_parser.add_argument('--branch', '-b',
-        #                                help = 'Branch to import onto',
-        #                                default = 'devel')
-        #import_srpm_parser.add_argument('--create', '-c',
-        #                                help = 'Create a new local repo',
-        #                                action = 'store_true')
-        import_srpm_parser.add_argument('--skip-diffs',
-                                        help = "Don't show diffs when \
-                                        import srpms",
-                                        action = 'store_true')
-        import_srpm_parser.add_argument('srpm',
-                                        help = 'Source rpm to import')
-        import_srpm_parser.set_defaults(command = self.import_srpm)
+        import_srpm_parser = self.subparsers.add_parser(
+            'import', help='Import srpm content into a module',
+            description='This will extract sources, patches, and the spec '
+                        'file from an srpm and update the current module '
+                        'accordingly. It will import to the current branch by'
+                        'default.')
+        import_srpm_parser.add_argument(
+            '--skip-diffs', help="Don't show diffs when import
srpms",
+            action='store_true')
+        import_srpm_parser.add_argument('srpm', help='Source rpm to
import')
+        import_srpm_parser.set_defaults(command=self.import_srpm)
 
     def register_install(self):
         """Register the install target"""
 
-        install_parser = self.subparsers.add_parser('install',
-                                       parents=[self.rpm_parser_common],
-                                       help = 'Local test rpmbuild install',
-                                       description = 'This will call \
-                                       rpmbuild to run the install \
-                                       section.  All leading sections \
-                                       will be processed as well, unless \
-                                       the short-circuit option is used.')
-        install_parser.add_argument('--short-circuit', action =
'store_true',
-                                    help = 'short-circuit install',
-                                    default = False)
-        install_parser.set_defaults(command = self.install)
+        install_parser = self.subparsers.add_parser(
+            'install', parents=[self.rpm_parser_common],
+            help='Local test rpmbuild install',
+            description='This will call rpmbuild to run the install section. '
+                        'All leading sections will be processed as well, '
+                        'unless the short-circuit option is used.')
+        install_parser.add_argument(
+            '--short-circuit', action='store_true', default=False,
+            help='short-circuit install')
+        install_parser.set_defaults(command=self.install)
 
     def register_lint(self):
         """Register the lint target"""
 
-        lint_parser = self.subparsers.add_parser('lint',
-                                            help = 'Run rpmlint against local '
-                                            'spec and build output if present.',
-                                            description = 'Rpmlint can be \
-                                            configured using the \
-                                            --rpmlintconf/-r option or by setting \
-                                            a .rpmlint file in the working \
-                                            directory')
-        lint_parser.add_argument('--info', '-i',
-                                 default = False,
-                                 action = 'store_true',
-                                 help = 'Display explanations for reported \
-                                 messages')
-        lint_parser.add_argument('--rpmlintconf', '-r',
-                                 default = None,
-                                 help = 'Use a specific configuration file \
-                                 for rpmlint')
-        lint_parser.set_defaults(command = self.lint)
+        lint_parser = self.subparsers.add_parser(
+            'lint', help='Run rpmlint against local spec and build output if
'
+                         'present.',
+            description='Rpmlint can be configured using the --rpmlintconf/-r'
+                         ' option or by setting a .rpmlint file in the '
+                         'working directory')
+        lint_parser.add_argument(
+            '--info', '-i', default=False, action='store_true',
+            help='Display explanations for reported messages')
+        lint_parser.add_argument(
+            '--rpmlintconf', '-r', default=None,
+            help='Use a specific configuration file for rpmlint')
+        lint_parser.set_defaults(command=self.lint)
 
     def register_local(self):
         """Register the local target"""
 
-        local_parser = self.subparsers.add_parser('local',
-                                     parents=[self.rpm_parser_common],
-                                     help = 'Local test rpmbuild binary',
-                                     description = 'Locally test run of \
-                                     rpmbuild producing binary RPMs. The \
-                                     rpmbuild output will be logged into a \
-                                     file named \
-                                     .build-%{version}-%{release}.log')
+        local_parser = self.subparsers.add_parser(
+            'local', parents=[self.rpm_parser_common],
+            help='Local test rpmbuild binary',
+            description='Locally test run of rpmbuild producing binary RPMs. '
+                        'The rpmbuild output will be logged into a file named'
+                        ' .build-%{version}-%{release}.log')
         # Allow the user to just pass "--md5" which will set md5 as the
         # hash, otherwise use the default of sha256
-        local_parser.add_argument('--md5', action='store_const',
-                              const='md5', default=None, dest='hash',
-                              help='Use md5 checksums (for older rpm hosts)')
-        local_parser.set_defaults(command = self.local)
+        local_parser.add_argument(
+            '--md5', action='store_const', const='md5',
default=None,
+            dest='hash', help='Use md5 checksums (for older rpm hosts)')
+        local_parser.set_defaults(command=self.local)
 
     def register_new(self):
         """Register the new target"""
 
-        new_parser = self.subparsers.add_parser('new',
-                                       help = 'Diff against last tag',
-                                       description = 'This will use git to \
-                                       show a diff of all the changes \
-                                       (even uncommitted changes) since the \
-                                       last git tag was applied.')
-        new_parser.set_defaults(command = self.new)
+        new_parser = self.subparsers.add_parser(
+            'new', help='Diff against last tag',
+            description='This will use git to show a diff of all the changes '
+                        '(even uncommitted changes) since the last git tag '
+                        'was applied.')
+        new_parser.set_defaults(command=self.new)
 
     def register_mockbuild(self):
         """Register the mockbuild target"""
 
-        mockbuild_parser = self.subparsers.add_parser('mockbuild',
-                               help='Local test build using mock',
-                               description='This will use the mock \
-                               utility to build the package for the \
-                               distribution detected from branch \
-                               information.  This can be overridden \
-                               using the global --dist option. Your \
-                               user must be in the local "mock" group.',
-                               epilog='If config file for mock isn\'t found in \
-                               /etc/mock directory, temporary config directory \
-                               for mock is created and populated with config \
-                               file created with mock-config.')
+        mockbuild_parser = self.subparsers.add_parser(
+            'mockbuild', help='Local test build using mock',
+            description='This will use the mock utility to build the package '
+                        'for the distribution detected from branch '
+                        'information. This can be overridden using the global'
+                        ' --dist option. Your user must be in the local '
+                        '"mock" group.',
+                        epilog="If config file for mock isn't found in the
"
+                               "/etc/mock directory, a temporary config "
+                               "directory for mock is created and populated "
+                               "with a config file created with mock-config.")
         mockbuild_parser.add_argument('--root', help='Override mock
root')
         # Allow the user to just pass "--md5" which will set md5 as the
         # hash, otherwise use the default of sha256
-        mockbuild_parser.add_argument('--md5', action='store_const',
-                              const='md5', default=None, dest='hash',
-                              help='Use md5 checksums (for older rpm hosts)')
+        mockbuild_parser.add_argument(
+            '--md5', action='store_const', const='md5',
default=None,
+            dest='hash', help='Use md5 checksums (for older rpm hosts)')
         mockbuild_parser.set_defaults(command=self.mockbuild)
 
     def register_mock_config(self):
         """Register the mock-config target"""
 
-        mock_config_parser = self.subparsers.add_parser('mock-config',
-                                        help='Generate a mock config',
-                                        description='This will generate a \
-                                        mock config based on the buildsystem \
-                                        target')
-        mock_config_parser.add_argument('--target',
-                                        help='Override target used for config',
-                                        default=None)
-        mock_config_parser.add_argument('--arch',
-                                        help='Override local arch')
+        mock_config_parser = self.subparsers.add_parser(
+            'mock-config', help='Generate a mock config',
+            description='This will generate a mock config based on the '
+                        'buildsystem target')
+        mock_config_parser.add_argument(
+            '--target', help='Override target used for config',
default=None)
+        mock_config_parser.add_argument('--arch', help='Override local
arch')
         mock_config_parser.set_defaults(command=self.mock_config)
 
     def register_new_sources(self):
         """Register the new-sources target"""
 
         # Make it part of self to be used later
-        self.new_sources_parser = self.subparsers.add_parser('new-sources',
-                                              help = 'Upload new source files',
-                                              description = 'This will upload \
-                                              new source files to the \
-                                              lookaside cache and remove \
-                                              any existing files.  The \
-                                              "sources" and .gitignore file \
-                                              will be updated for the new \
-                                              file(s).')
-        self.new_sources_parser.add_argument('files', nargs = '+')
-        self.new_sources_parser.set_defaults(command = self.new_sources,
-                                             replace = True)
+        self.new_sources_parser = self.subparsers.add_parser(
+            'new-sources', help='Upload new source files',
+            description='This will upload new source files to the lookaside '
+                        'cache and remove any existing ones. The "sources"
'
+                        'and .gitignore files will be updated with the new '
+                        'uploaded file(s).')
+        self.new_sources_parser.add_argument('files', nargs='+')
+        self.new_sources_parser.set_defaults(
+            command=self.new_sources, replace=True)
 
     def register_patch(self):
         """Register the patch target"""
 
-        patch_parser = self.subparsers.add_parser('patch',
-                                         help = 'Create and add a gendiff '
-                                         'patch file',
-                                         epilog = 'Patch file will be '
-                                         'named: package-version-suffix.patch '
-                                         'and the file will be added to the '
-                                         'repo index')
-        patch_parser.add_argument('--rediff',
-                          help = 'Recreate gendiff file retaining comments \
-                          Saves old patch file with a suffix of ~',
-                          action = 'store_true',
-                          default = False)
-        patch_parser.add_argument('suffix',
-                                  help = 'Look for files with this suffix \
-                                  to diff')
-        patch_parser.set_defaults(command = self.patch)
+        patch_parser = self.subparsers.add_parser(
+            'patch', help='Create and add a gendiff patch file',
+            epilog='Patch file will be named: package-version-suffix.patch '
+                   'and the file will be added to the repo index')
+        patch_parser.add_argument(
+            '--rediff', action='store_true', default=False,
+            help='Recreate gendiff file retaining comments Saves old patch '
+                 'file with a suffix of ~')
+        patch_parser.add_argument(
+            'suffix', help='Look for files with this suffix to diff')
+        patch_parser.set_defaults(command=self.patch)
 
     def register_prep(self):
         """Register the prep target"""
 
-        prep_parser = self.subparsers.add_parser('prep',
-                                        parents=[self.rpm_parser_common],
-                                        help = 'Local test rpmbuild prep',
-                                        description = 'Use rpmbuild to
"prep" \
-                                        the sources (unpack the source \
-                                        archive(s) and apply any patches.)')
-        prep_parser.set_defaults(command = self.prep)
+        prep_parser = self.subparsers.add_parser(
+            'prep', parents=[self.rpm_parser_common],
+            help='Local test rpmbuild prep',
+            description='Use rpmbuild to "prep" the sources (unpack the
'
+                        'source archive(s) and apply any patches.)')
+        prep_parser.set_defaults(command=self.prep)
 
     def register_pull(self):
         """Register the pull target"""
 
-        pull_parser = self.subparsers.add_parser('pull',
-                                        help = 'Pull changes from remote '
-                                        'repository and update working copy.',
-                                        description = 'This command uses git \
-                                        to fetch remote changes and apply \
-                                        them to the current working copy.  A \
-                                        rebase option is available which can \
-                                        be used to avoid merges.',
-                                        epilog = 'See git pull --help for \
-                                        more details')
-        pull_parser.add_argument('--rebase', action = 'store_true',
-                             help = 'Rebase the locally committed changes on \
-                             top of the remote changes after fetching.  This \
-                             can avoid a merge commit, but does rewrite local \
-                             history.')
-        pull_parser.add_argument('--no-rebase', action = 'store_true',
-                             help = 'Do not rebase, override .git settings to \
-                             automatically rebase')
-        pull_parser.set_defaults(command = self.pull)
+        pull_parser = self.subparsers.add_parser(
+            'pull', help='Pull changes from the remote repository and update
'
+                         'the working copy.',
+            description='This command uses git to fetch remote changes and '
+                        'apply them to the current working copy. A rebase '
+                        'option is available which can be used to avoid '
+                        'merges.',
+            epilog='See git pull --help for more details')
+        pull_parser.add_argument(
+            '--rebase', action='store_true',
+            help='Rebase the locally committed changes on top of the remote '
+                 'changes after fetching. This can avoid a merge commit, but '
+                 'does rewrite local history.')
+        pull_parser.add_argument(
+            '--no-rebase', action='store_true',
+            help='Do not rebase, overriding .git settings to the contrary')
+        pull_parser.set_defaults(command=self.pull)
 
     def register_push(self):
         """Register the push target"""
 
-        push_parser = self.subparsers.add_parser('push',
-                                            help = 'Push changes to remote '
-                                            'repository')
-        push_parser.set_defaults(command = self.push)
+        push_parser = self.subparsers.add_parser(
+            'push', help='Push changes to remote repository')
+        push_parser.set_defaults(command=self.push)
 
     def register_scratch_build(self):
         """Register the scratch-build target"""
 
-        scratch_build_parser = self.subparsers.add_parser('scratch-build',
-                                        help = 'Request scratch build',
-                                        parents = [self.build_parser_common],
-                                        description = 'This command \
-                                        will request a scratch build \
-                                        of the package.  Without \
-                                        providing an srpm, it will \
-                                        attempt to build the latest \
-                                        commit, which must have been \
-                                        pushed.  By default all \
-                                        appropriate arches will be \
-                                        built.')
-        scratch_build_parser.add_argument('--srpm', nargs = '?',
-                                  const = 'CONSTRUCT',
-                                  help = 'Build from an srpm.  If no srpm \
-                                  is provided with this option an srpm will \
-                                  be generated from current module content.')
-        scratch_build_parser.set_defaults(command = self.scratch_build)
+        scratch_build_parser = self.subparsers.add_parser(
+            'scratch-build', help='Request scratch build',
+            parents=[self.build_parser_common],
+            description='This command will request a scratch build of the '
+                        'package. Without providing an srpm, it will attempt '
+                        'to build the latest commit, which must have been '
+                        'pushed. By default all appropriate arches will be '
+                        'built.')
+        scratch_build_parser.add_argument(
+            '--srpm', nargs='?', const='CONSTRUCT',
+            help='Build from an srpm. If no srpm is provided with this '
+                 'option an srpm will be generated from the current module '
+                 'content.')
+        scratch_build_parser.set_defaults(command=self.scratch_build)
 
     def register_sources(self):
         """Register the sources target"""
 
-        sources_parser = self.subparsers.add_parser('sources',
-                                               help = 'Download source files')
-        sources_parser.add_argument('--outdir',
-                                    default = os.curdir,
-                                    help = 'Directory to download files into \
-                                    (defaults to pwd)')
-        sources_parser.set_defaults(command = self.sources)
+        sources_parser = self.subparsers.add_parser(
+            'sources', help='Download source files')
+        sources_parser.add_argument(
+            '--outdir', default=os.curdir,
+            help='Directory to download files into (defaults to pwd)')
+        sources_parser.set_defaults(command=self.sources)
 
     def register_srpm(self):
         """Register the srpm target"""
 
-        srpm_parser = self.subparsers.add_parser('srpm',
-                                                 help = 'Create a source rpm')
+        srpm_parser = self.subparsers.add_parser(
+            'srpm', help='Create a source rpm')
         # optionally define old style hashsums
-        srpm_parser.add_argument('--md5', action='store_const',
-                              const='md5', default=None, dest='hash',
-                              help='Use md5 checksums (for older rpm hosts)')
-        srpm_parser.set_defaults(command = self.srpm)
+        srpm_parser.add_argument(
+            '--md5', action='store_const', const='md5',
default=None,
+            dest='hash', help='Use md5 checksums (for older rpm hosts)')
+        srpm_parser.set_defaults(command=self.srpm)
 
     def register_switch_branch(self):
         """Register the switch-branch target"""
 
-        switch_branch_parser = self.subparsers.add_parser('switch-branch',
-                                                help = 'Work with branches',
-                                                description = 'This command \
-                                                can switch to a local git \
-                                                branch.  If provided with a \
-                                                remote branch name that does \
-                                                not have a local match it \
-                                                will create one.  It can \
-                                                also be used to list the \
-                                                existing local and remote \
-                                                branches.')
-        switch_branch_parser.add_argument('branch',  nargs = '?',
-                                          help = 'Branch name to switch to')
-        switch_branch_parser.add_argument('-l', '--list',
-                                          help = 'List both remote-tracking \
-                                          branches and local branches',
-                                          action = 'store_true')
-        switch_branch_parser.add_argument('--fetch',
-                                          help = 'Fetch new data from remote'
-                                                 ' before switch',
-                                          action = 'store_true',
-                                          dest = 'fetch')
-        switch_branch_parser.set_defaults(command = self.switch_branch)
+        switch_branch_parser = self.subparsers.add_parser(
+            'switch-branch', help='Work with branches',
+            description='This command can switch to a local git branch. If '
+                        'provided with a remote branch name that does not '
+                        'have a local match it will create one.  It can also '
+                        'be used to list the existing local and remote '
+                        'branches.')
+        switch_branch_parser.add_argument(
+            'branch', nargs='?', help='Branch name to switch
to')
+        switch_branch_parser.add_argument(
+            '-l', '--list', action='store_true',
+            help='List both remote-tracking branches and local branches')
+        switch_branch_parser.add_argument(
+            '--fetch', help='Fetch new data from remote before switch',
+            action='store_true', dest='fetch')
+        switch_branch_parser.set_defaults(command=self.switch_branch)
 
     def register_tag(self):
         """Register the tag target"""
 
-        tag_parser = self.subparsers.add_parser('tag',
-                                       help = 'Management of git tags',
-                                       description = 'This command uses git \
-                                       to create, list, or delete tags.')
-        tag_parser.add_argument('-f', '--force',
-                                default = False,
-                                action = 'store_true',
-                                help = 'Force the creation of the tag')
-        tag_parser.add_argument('-m', '--message',
-                                default = None,
-                                help = 'Use the given <msg> as the tag \
-                                message')
-        tag_parser.add_argument('-c', '--clog',
-                                default = False,
-                                action = 'store_true',
-                                help = 'Generate the tag message from the \
-                                spec changelog section')
-        tag_parser.add_argument('--raw', action='store_true',
-                                default=False,
-                                help='Make the clog raw')
-        tag_parser.add_argument('-F', '--file',
-                                default = None,
-                                help = 'Take the tag message from the given \
-                                file')
-        tag_parser.add_argument('-l', '--list',
-                                default = False,
-                                action = 'store_true',
-                                help = 'List all tags with a given pattern, \
-                                or all if not pattern is given')
-        tag_parser.add_argument('-d', '--delete',
-                                default = False,
-                                action = 'store_true',
-                                help = 'Delete a tag')
-        tag_parser.add_argument('tag',
-                                nargs = '?',
-                                default = None,
-                                help = 'Name of the tag')
-        tag_parser.set_defaults(command = self.tag)
+        tag_parser = self.subparsers.add_parser(
+            'tag', help='Management of git tags',
+            description='This command uses git to create, list, or delete '
+                        'tags.')
+        tag_parser.add_argument(
+            '-f', '--force', default=False,
+            action='store_true', help='Force the creation of the tag')
+        tag_parser.add_argument(
+            '-m', '--message', default=None,
+            help='Use the given <msg> as the tag message')
+        tag_parser.add_argument(
+            '-c', '--clog', default=False, action='store_true',
+            help='Generate the tag message from the spec changelog section')
+        tag_parser.add_argument(
+            '--raw', action='store_true', default=False,
+            help='Make the clog raw')
+        tag_parser.add_argument(
+            '-F', '--file', default=None,
+            help='Take the tag message from the given file')
+        tag_parser.add_argument(
+            '-l', '--list', default=False, action='store_true',
+            help='List all tags with a given pattern, or all if not pattern '
+                 'is given')
+        tag_parser.add_argument(
+            '-d', '--delete', default=False,
action='store_true',
+            help='Delete a tag')
+        tag_parser.add_argument(
+            'tag', nargs='?', default=None, help='Name of the
tag')
+        tag_parser.set_defaults(command=self.tag)
 
     def register_unused_patches(self):
         """Register the unused-patches target"""
 
-        unused_patches_parser = self.subparsers.add_parser('unused-patches',
-                                             help = 'Print list of patches '
-                                             'not referenced by name in '
-                                             'the specfile')
-        unused_patches_parser.set_defaults(command = self.unused_patches)
+        unused_patches_parser = self.subparsers.add_parser(
+            'unused-patches',
+            help='Print list of patches not referenced by name in the '
+                 'specfile')
+        unused_patches_parser.set_defaults(command=self.unused_patches)
 
     def register_upload(self):
         """Register the upload target"""
 
-        upload_parser = self.subparsers.add_parser('upload',
-                                          parents = [self.new_sources_parser],
-                                          conflict_handler = 'resolve',
-                                          help = 'Upload source files',
-                                          description = 'This command will \
-                                          add a new source archive to the \
-                                          lookaside cache.  The sources and \
-                                          .gitignore file will be updated \
-                                          with the new file(s).')
-        upload_parser.set_defaults(command = self.new_sources,
-                                   replace = False)
+        upload_parser = self.subparsers.add_parser(
+            'upload', parents=[self.new_sources_parser],
+            conflict_handler='resolve', help='Upload source files',
+            description='This command will add a new source archive to the '
+                        'lookaside cache. The sources and .gitignore file '
+                        'will be updated with the new file(s).')
+        upload_parser.set_defaults(command=self.new_sources, replace=False)
 
     def register_verify_files(self):
         """Register the verify-files target"""
 
-        verify_files_parser = self.subparsers.add_parser('verify-files',
-                                            parents=[self.rpm_parser_common],
-                                            help='Locally verify %%files '
-                                            'section',
-                                            description="Locally run \
-                                            'rpmbuild -bl' to verify the \
-                                            spec file's %files sections. \
-                                            This requires a successful run \
-                                            of the 'compile' target.")
-        verify_files_parser.set_defaults(command = self.verify_files)
+        verify_files_parser = self.subparsers.add_parser(
+            'verify-files', parents=[self.rpm_parser_common],
+            help='Locally verify %%files section',
+            description="Locally run 'rpmbuild -bl' to verify the spec
file's"
+                        " %files sections. This requires a successful run of "
+                        "the 'compile' target.")
+        verify_files_parser.set_defaults(command=self.verify_files)
 
     def register_verrel(self):
 
-        verrel_parser = self.subparsers.add_parser('verrel',
-                                                   help = 'Print the '
-                                                   'name-version-release')
-        verrel_parser.set_defaults(command = self.verrel)
+        verrel_parser = self.subparsers.add_parser(
+            'verrel', help='Print the name-version-release')
+        verrel_parser.set_defaults(command=self.verrel)
 
     # All the command functions go here
     def usage(self):
@@ -919,9 +803,10 @@ defined, packages will be built sequentially.""" %
             if not self.args.q:
                 callback = self._progress_callback
             # define a unique path for this upload.  Stolen from /usr/bin/koji
-            uniquepath = 'cli-build/%r.%s' % (time.time(),
-                                 ''.join([random.choice(string.ascii_letters)
-                                          for i in range(8)]))
+            uniquepath = ('cli-build/%r.%s'
+                          % (time.time(),
+                             ''.join([random.choice(string.ascii_letters)
+                                      for i in range(8)])))
             # Should have a try here, not sure what errors we'll get yet though
             self.cmd.koji_upload(self.args.srpm, uniquepath, callback=callback)
             if not self.args.q:
@@ -974,8 +859,8 @@ defined, packages will be built sequentially.""" %
                                                   self.cmd.branch_merge)
                 url = self.cmd.anongiturl % {'module':
                                              component} + '#%s' % hash
-                # If there are no ':' in the chain list, treat each object as an
-                # individual chain
+                # If there are no ':' in the chain list, treat each object as
+                # an individual chain
                 if ':' in self.args.package:
                     build_set.append(url)
                 else:
@@ -1210,11 +1095,11 @@ defined, packages will be built sequentially.""" %
             status = tasks[task_id].info['state']
             if status == koji.TASK_STATES['FAILED']:
                 failed += 1
-            elif status == koji.TASK_STATES['CLOSED'] or \
-            status == koji.TASK_STATES['CANCELED']:
+            elif status in (koji.TASK_STATES['CLOSED'],
+                            koji.TASK_STATES['CANCELED']):
                 done += 1
-            elif status == koji.TASK_STATES['OPEN'] or \
-            status == koji.TASK_STATES['ASSIGNED']:
+            elif status in (koji.TASK_STATES['OPEN'],
+                            koji.TASK_STATES['ASSIGNED']):
                 open += 1
             elif status == koji.TASK_STATES['FREE']:
                 free += 1
@@ -1249,7 +1134,7 @@ defined, packages will be built sequentially.""" %
                                              quiet=self.args.q)
             while True:
                 all_done = True
-                for task_id,task in tasks.items():
+                for task_id, task in tasks.items():
                     changed = task.update()
                     if not task.is_done():
                         all_done = False
@@ -1262,7 +1147,7 @@ defined, packages will be built sequentially.""" %
                             rv = 1
                     for child in session.getTaskChildren(task_id):
                         child_id = child['id']
-                        if not child_id in tasks.keys():
+                        if child_id not in tasks.keys():
                             tasks[child_id] = TaskWatcher(child_id,
                                                           session,
                                                           self.log,
@@ -1281,14 +1166,15 @@ defined, packages will be built sequentially.""" %
                 time.sleep(1)
         except (KeyboardInterrupt):
             if tasks:
-                self.log.info(
-    """
+                self.log.info("""
 Tasks still running. You can continue to watch with the '%s watch-task' command.
     Running Tasks:
-    %s""" % (self.config.get(self.name, 'build_client'),
-                             '\n'.join(['%s: %s' % (t.str(),
-                                                    t.display_state(t.info))
-                       for t in tasks.values() if not t.is_done()])))
+    %s"""
+                              % (self.config.get(self.name, 'build_client'),
+                                 '\n'.join(['%s: %s' % (t.str(),
+                                                        t.display_state(t.info))
+                                            for t in tasks.values()
+                                            if not t.is_done()])))
             # A ^c should return non-zero so that it doesn't continue
             # on to any && commands.
             rv = 1
@@ -1300,7 +1186,7 @@ Tasks still running. You can continue to watch with the '%s
watch-task' command.
             return "%0.2f GiB" % (size / 1073741824.0)
         if (size / 1048576 >= 1):
             return "%0.2f MiB" % (size / 1048576.0)
-        if (size / 1024 >=1):
+        if (size / 1024 >= 1):
             return "%0.2f KiB" % (size / 1024.0)
         return "%0.2f B" % (size)
 
@@ -1323,7 +1209,7 @@ Tasks still running. You can continue to watch with the '%s
watch-task' command.
                 speed = self._format_size(float(piece)/float(time)) + "/sec"
             else:
                 speed = self._format_size(float(total)/float(total_time)) + \
-                "/sec"
+                    "/sec"
 
         # write formatted string and flush
         sys.stdout.write("[% -36s] % 4s % 8s % 10s % 14s\r" %
@@ -1363,7 +1249,7 @@ Tasks still running. You can continue to watch with the '%s
watch-task' command.
         This also sets up self.user
         """
 
-        if  manpage:
+        if manpage:
             # Generate the man page
             man_name = self.name
             if man_name.endswith('.py'):
@@ -1475,6 +1361,7 @@ class TaskWatcher(object):
         else:
             return koji.TASK_STATES[info['state']].lower()
 
+
 if __name__ == '__main__':
     client = cliClient()
     client.do_imports()
@@ -1482,7 +1369,7 @@ if __name__ == '__main__':
 
     if not client.args.path:
         try:
-            client.args.path=os.getcwd()
+            client.args.path = os.getcwd()
         except:
             print('Could not get current path, have you deleted it?')
             sys.exit(1)
-- 
2.1.0

[Pavol Babincak]

On 10/29/2014 01:56 PM, Mathieu Bridon wrote:
>  From Saturday to Tuesday was PyCon-Fr, here in Lyon, so I used the
> time to
>  hack a bit on rpkg. 
Thanks for great set of tests! Pushed upstream.
-- 
Pavol Babincak
Release Engineering, Red Hat