Merge lp:~ev/ubuntu-ci-services-itself/wider-pyflakes-coverage into lp:ubuntu-ci-services-itself

[Evan]

Fix a large number of pep8 issues in cupstream2distro. Cover run_worker and other out of module scripts with the pep8 and pyflakes tests.

I noticed that pyflakes wasn't running against watch_ppa.py, which exists a level above the bsbuilder module.

While fixing this and running the tests, I was having a hard time pulling useful failures from the noise generated by cupstream2distro. So I fixed all of its pep8 and pyflakes issues.

We should be very careful in landing this branch. cupstream2distro is completely untested code and I made some logic changes to it.

You'll notice that there are symlinks to the run_worker script in a few modules. This exists for three reasons:
 1. pep8 and pyflakes are hardcoded in uci-tests to look for *.py files.
 2. Importing `run_worker` using imp creates a run_workerc file unless you tell Python to give the compiled code a different name (by providing an open file and path argument).
 3. Our test harness imports all the run_worker scripts under the same namespace, so you end up using whichever was imported last. Providing a uniquely named symlink fixes this.

Note that there is still a pyflakes failure with this branch. That is bug 1283449.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:280
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~ev/ubuntu-ci-services-itself/wider-pyflakes-coverage/+merge/207801/+edit-commit-message

http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/217/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/217/rebuild

review: Needs Fixing (continuous-integration)

[lp:~ev/ubuntu-ci-services-itself/wider-pyflakes-coverage]

281. By Evan on 2014-02-23

Expand the pyflakes/pep8 coverage for the test_runner as well.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:281
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/219/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/219/rebuild

review: Needs Fixing (continuous-integration)

[Ursula Junque (ursinha)]

I like how elegant this is handling the huge messages (the part that was worrying me most). I've been working on adding tests to this code before actually making such changes, because they are already part of upstream and "only" need to be ported (at least basic unit tests), and that would avoid headaches in case things started to fail because we accidentally changed the logic somewhere. If you agree to hold this for a bit I think I can land tests first, then you can land this branch safely.

[Evan (ev)]

Yup, I think that's entirely reasonable. Let's put this on hold until your branch lands.

Unmerged revisions

281. By Evan on 2014-02-23

Expand the pyflakes/pep8 coverage for the test_runner as well.

280. By Evan on 2014-02-23

Fix the style tests. Specifying more than one module under the same directory causes it to be tested twice by pep8 and pyflakes. Modules imported with the same name with override each other.

279. By Evan on 2014-02-22

Fix pep8 issues in cupstream2distro packageinppa module.

278. By Evan on 2014-02-22

Fix pep8 issues in cupstream2distro packageinppamanager module.

277. By Evan on 2014-02-22

Fix pep8 issues in cupstream2distro packagemanager module.

276. By Evan on 2014-02-22

Fix pep8 issues in cupstream2distro silomanager module.

275. By Evan on 2014-02-22

Fix pep8 issues in cupstream2distro settings module.

274. By Evan on 2014-02-22

Fix pep8 issues in cupstream2distro stacks module.

273. By Evan on 2014-02-22

Oops. One more in tools.py.

272. By Evan on 2014-02-22

Fix pep8 issues in cupstream2distro tools module.

=== modified file 'branch-source-builder/bsbuilder/tests/test_style.py'
--- branch-source-builder/bsbuilder/tests/test_style.py	2014-02-15 12:06:40 +0000
+++ branch-source-builder/bsbuilder/tests/test_style.py	2014-02-23 16:37:01 +0000
@@ -14,15 +14,19 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 from ucitests import styles
-
-import bsbuilder
+# If we just import run_worker, we'll end up testing whichever run_worker was
+# last imported, so import it under a different name.
+import bsbuilder_run_worker
 
 
 class TestPep8(styles.TestPep8):
 
-    packages = [bsbuilder]
+    # uci-tests will scan all subdirectories that this module is found in, so
+    # we do not need to explicitly provide bsbuilder. Doing so would cause the
+    # tests to be run twice.
+    packages = [bsbuilder_run_worker]
 
 
 class TestPyflakes(styles.TestPyflakes):
 
-    packages = [bsbuilder]
+    packages = [bsbuilder_run_worker]
 
=== added symlink 'branch-source-builder/bsbuilder_run_worker.py'
=== target is u'run_worker'
=== modified file 'branch-source-builder/cupstream2distro/branchhandling.py'
--- branch-source-builder/cupstream2distro/branchhandling.py	2014-02-19 16:34:46 +0000
+++ branch-source-builder/cupstream2distro/branchhandling.py	2014-02-23 16:37:01 +0000
@@ -23,13 +23,17 @@
 import os
 import re
 import subprocess
+from subprocess import PIPE
 
-from .settings import BRANCH_URL, IGNORECHANGELOG_COMMIT, PACKAGING_MERGE_COMMIT_MESSAGE, PROJECT_CONFIG_SUFFIX, SILO_PACKAGING_RELEASE_COMMIT_MESSAGE
+from .settings import (BRANCH_URL, IGNORECHANGELOG_COMMIT,
+                       PACKAGING_MERGE_COMMIT_MESSAGE, PROJECT_CONFIG_SUFFIX,
+                       SILO_PACKAGING_RELEASE_COMMIT_MESSAGE)
 
 
 def get_branch(branch_url, dest_dir):
     '''Grab a branch'''
-    instance = subprocess.Popen(["bzr", "branch", branch_url, dest_dir], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    cmd = ["bzr", "branch", branch_url, dest_dir]
+    instance = subprocess.Popen(cmd, stdout=PIPE, stderr=PIPE)
     (stdout, stderr) = instance.communicate()
     if instance.returncode != 0:
         raise Exception(stderr.decode("utf-8").strip())
@@ -37,15 +41,18 @@
 
 def get_tip_bzr_revision():
     '''Get latest revision in bzr'''
-    instance = subprocess.Popen(["bzr", "log", "-c", "-1", "--line"], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    cmd = ["bzr", "log", "-c", "-1", "--line"]
+    instance = subprocess.Popen(cmd, stdout=PIPE, stderr=PIPE)
     (stdout, stderr) = instance.communicate()
     if instance.returncode != 0:
         raise Exception(stderr.decode("utf-8").strip())
     return (int(stdout.split(':')[0]))
 
 
-def collect_author_commits(content_to_parse, bugs_to_skip, additional_stamp=""):
-    '''return a tuple of a dict with authors and commits message from the content to parse
+def collect_author_commits(content_to_parse, bugs_to_skip,
+                           additional_stamp=""):
+    '''Return a tuple of a dict with authors and commits message from the
+    content to parse.
 
     bugs_to_skip is a set of bugs we need to skip
 
@@ -60,15 +67,17 @@
     commit_message_stenza = False
     for line in content_to_parse.splitlines():
         # new revision, collect what we have found
-        if line.startswith("------------------------------------------------------------"):
-            # try to decipher a special case: we have some commits which were already in bugs_to_skip,
-            # so we eliminate them.
+        dash = "------------------------------------------------------------"
+        if line.startswith(dash):
+            # try to decipher a special case: we have some commits which were
+            # already in bugs_to_skip, so we eliminate them.
             # Also ignore when having IGNORECHANGELOG_COMMIT
-            if (current_bugs and not (current_bugs - bugs_to_skip)) or IGNORECHANGELOG_COMMIT in current_commit:
-                current_authors = set()
-                current_commit = ""
-                current_bugs = set()
-                continue
+            if ((current_bugs and not (current_bugs - bugs_to_skip))
+               or IGNORECHANGELOG_COMMIT in current_commit):
+                    current_authors = set()
+                    current_commit = ""
+                    current_bugs = set()
+                    continue
             current_bugs -= bugs_to_skip
             commit_message = current_commit + _format_bugs(current_bugs)
             for author in current_authors:
@@ -79,7 +88,8 @@
             current_commit = ""
             current_bugs = set()
 
-        # we ignore this commit if we have a changelog provided as part of the diff
+        # we ignore this commit if we have a changelog provided as part of the
+        # diff
         if line.startswith("=== modified file 'debian/changelog'"):
             current_authors = set()
             current_commit = ""
@@ -94,14 +104,15 @@
         elif commit_message_stenza:
             if line.startswith("diff:"):
                 commit_message_stenza = False
-                current_commit, current_bugs = _extract_commit_bugs(current_commit, additional_stamp)
+                args = (current_commit, additional_stamp)
+                current_commit, current_bugs = _extract_commit_bugs(*args)
             else:
-                line = line[2:] # Dedent the message provided by bzr
-                if line[0:2] in ('* ', '- '): # paragraph line.
-                    line = line[2:] # Remove bullet
-                    if line[-1] != '.': # Grammar nazi...
-                        line += '.' # ... or the lines will be merged.
-                line = line + ' ' # Add a space to preserve lines
+                line = line[2:]  # Dedent the message provided by bzr
+                if line[0:2] in ('* ', '- '):  # paragraph line.
+                    line = line[2:]  # Remove bullet
+                    if line[-1] != '.':  # Grammar nazi...
+                        line += '.'  # ... or the lines will be merged.
+                line = line + ' '  # Add a space to preserve lines
                 current_commit += line
                 #  Maybe add something like that
                 #for content in mp.commit_message.split('\n'):
@@ -127,10 +138,13 @@
 
 
 def _extract_commit_bugs(commit_message, additional_stamp=""):
-    '''extract relevant commit message part and bugs number from a commit message'''
+    '''extract relevant commit message part and bugs number from a commit
+    message'''
 
     current_bugs = _return_bugs(commit_message)
-    changelog_content = " ".join(commit_message.rsplit('Fixes: ')[0].rsplit('Approved by ')[0].split())
+    changelog_content = commit_message.rsplit('Fixes: ')[0]
+    changelog_content = changelog_content.rsplit('Approved by ')[0].split()
+    changelog_content = " ".join(changelog_content)
     if additional_stamp:
         changelog_content = changelog_content + " " + additional_stamp
     return (changelog_content, current_bugs)
@@ -149,7 +163,8 @@
     # #12345 (but not 12345 for false positive)
     # Support multiple bugs per commit
     bug_numbers = set()
-    bug_regexp = re.compile("((lp|bug|fix(es)?)[: #]*|#|launchpad.net/bugs/)(\d{5,})", re.IGNORECASE)
+    pattern = "((lp|bug|fix(es)?)[: #]*|#|launchpad.net/bugs/)(\d{5,})"
+    bug_regexp = re.compile(pattern, re.IGNORECASE)
     for match in bug_regexp.findall(string):
         logging.debug("Bug regexp match: {}".format(match[-1]))
         bug_numbers.add(int(match[-1]))
@@ -170,7 +185,9 @@
 def return_log_diff(starting_rev):
     '''Return the relevant part of the cvs log since starting_rev'''
 
-    instance = subprocess.Popen(["bzr", "log", "-r", "{}..".format(starting_rev), "--show-diff", "--forward"], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    rev = "{}..".format(starting_rev)
+    cmd = ["bzr", "log", "-r", rev, "--show-diff", "--forward"]
+    instance = subprocess.Popen(cmd, stdout=PIPE, stderr=PIPE)
     (stdout, stderr) = instance.communicate()
     if instance.returncode != 0:
         raise Exception(stderr.decode("utf-8").strip())
@@ -178,8 +195,10 @@
 
 
 def return_log_diff_since_last_release(content_to_parse):
-    '''From a bzr log content, return only the log diff since the latest release'''
-    after_release = content_to_parse.split(SILO_PACKAGING_RELEASE_COMMIT_MESSAGE.format(''))[-1]
+    '''From a bzr log content, return only the log diff since the latest
+    release'''
+    formatted = SILO_PACKAGING_RELEASE_COMMIT_MESSAGE.format('')
+    after_release = content_to_parse.split(formatted)[-1]
     sep = '------------------------------------------------------------'
     sep_index = after_release.find(sep)
     if sep_index != 1:
@@ -190,10 +209,11 @@
 def commit_release(new_package_version, tip_bzr_rev=None):
     '''Commit latest release'''
     if not tip_bzr_rev:
-        message = SILO_PACKAGING_RELEASE_COMMIT_MESSAGE.format(new_package_version)
+        msg = SILO_PACKAGING_RELEASE_COMMIT_MESSAGE.format(new_package_version)
     else:
-        message = "Releasing {}, based on r{}".format(new_package_version, tip_bzr_rev)
-    if subprocess.call(["bzr", "commit", "-m", message]) != 0:
+        msg = "Releasing {}, based on r{}"
+        msg = msg.format(new_package_version, tip_bzr_rev)
+    if subprocess.call(["bzr", "commit", "-m", msg]) != 0:
         raise Exception("The above command returned an error.")
 
 
@@ -214,20 +234,26 @@
     env["BZR_EDITOR"] = "echo"
 
     os.chdir(source_package_name)
-    if subprocess.call(["bzr", "push", BRANCH_URL.format(source_package_name, version.replace("~", "").replace(":", "")), "--overwrite"]) != 0:
+    args = (source_package_name, version.replace("~", "").replace(":", ""))
+    loc = BRANCH_URL.format(*args)
+    if subprocess.call(["bzr", "push", loc, "--overwrite"]) != 0:
         raise Exception("The push command returned an error.")
-    mergeinstance = subprocess.Popen(["bzr", "lp-propose-merge", parent_branch, "-m", PACKAGING_MERGE_COMMIT_MESSAGE.format(version, tip_rev, branch), "--approve"], stdin=subprocess.PIPE, env=env)
+    msg = PACKAGING_MERGE_COMMIT_MESSAGE.format(version, tip_rev, branch)
+    cmd = ["bzr", "lp-propose-merge", parent_branch, "-m", msg, "--approve"]
+    mergeinstance = subprocess.Popen(cmd, stdin=PIPE, env=env)
     mergeinstance.communicate(input="y")
     if mergeinstance.returncode != 0:
         raise Exception("The lp-propose command returned an error.")
     os.chdir('..')
 
 
-def merge_branch_with_parent_into(local_branch_uri, lp_parent_branch, dest_uri, commit_message, revision):
+def merge_branch_with_parent_into(local_branch_uri, lp_parent_branch, dest_uri,
+                                  commit_message, revision):
     """Merge local branch into lp_parent_branch at revision"""
     success = False
     cur_dir = os.path.abspath('.')
-    subprocess.call(["bzr", "branch", "-r", str(revision), lp_parent_branch, dest_uri])
+    cmd = ["bzr", "branch", "-r", str(revision), lp_parent_branch, dest_uri]
+    subprocess.call(cmd)
     os.chdir(dest_uri)
     if subprocess.call(["bzr", "merge", local_branch_uri]) == 0:
         subprocess.call(["bzr", "commit", "-m", commit_message])
@@ -236,12 +262,14 @@
     return success
 
 
-def merge_branch(uri_to_merge, lp_parent_branch, commit_message, authors=set()):
+def merge_branch(uri_to_merge, lp_parent_branch, commit_message,
+                 authors=set()):
     """Resync with targeted branch if possible"""
     success = False
     cur_dir = os.path.abspath('.')
     os.chdir(uri_to_merge)
-    lp_parent_branch = lp_parent_branch.replace("https://code.launchpad.net/", "lp:")
+    args = ("https://code.launchpad.net/", "lp:")
+    lp_parent_branch = lp_parent_branch.replace(*args)
     if subprocess.call(["bzr", "merge", lp_parent_branch]) == 0:
         cmd = ["bzr", "commit", "-m", commit_message, "--unchanged"]
         for author in authors:
@@ -251,12 +279,14 @@
     os.chdir(cur_dir)
     return success
 
+
 def push_to_branch(source_uri, lp_parent_branch, overwrite=False):
     """Push source to parent branch"""
     success = False
     cur_dir = os.path.abspath('.')
     os.chdir(source_uri)
-    lp_parent_branch = lp_parent_branch.replace("https://code.launchpad.net/", "lp:")
+    args = ("https://code.launchpad.net/", "lp:")
+    lp_parent_branch = lp_parent_branch.replace(*args)
     command = ["bzr", "push", lp_parent_branch]
     if overwrite:
         command.append("--overwrite")
@@ -265,16 +295,21 @@
     os.chdir(cur_dir)
     return success
 
+
 def grab_committers_compared_to(source_uri, lp_branch_to_scan):
     """Return unique list of committers for a given branch"""
     committers = set()
     cur_dir = os.path.abspath('.')
     os.chdir(source_uri)
-    lp_branch_to_scan = lp_branch_to_scan.replace("https://code.launchpad.net/", "lp:")
-    instance = subprocess.Popen(["bzr", "missing", lp_branch_to_scan, "--other"], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    args = ("https://code.launchpad.net/", "lp:")
+    lp_branch_to_scan = lp_branch_to_scan.replace(*args)
+    cmd = ["bzr", "missing", lp_branch_to_scan, "--other"]
+    instance = subprocess.Popen(cmd, stdout=PIPE, stderr=PIPE)
     (stdout, stderr) = instance.communicate()
     if stderr != "":
-        raise Exception("bzr missing on {} returned a failure: {}".format(lp_branch_to_scan, stderr.decode("utf-8").strip()))
+        msg = "bzr missing on {} returned a failure: {}"
+        msg = msg.format(lp_branch_to_scan, stderr.decode("utf-8").strip())
+        raise Exception(msg)
     committer_regexp = re.compile("\ncommitter: (.*)\n")
     for match in committer_regexp.findall(stdout):
         for committer in match.split(', '):
 
=== modified file 'branch-source-builder/cupstream2distro/launchpadmanager.py'
--- branch-source-builder/cupstream2distro/launchpadmanager.py	2014-02-19 16:34:46 +0000
+++ branch-source-builder/cupstream2distro/launchpadmanager.py	2014-02-23 16:37:01 +0000
@@ -25,10 +25,13 @@
 import os
 launchpad = None
 
-from .settings import ARCHS_TO_EVENTUALLY_IGNORE, ARCHS_TO_UNCONDITIONALLY_IGNORE, VIRTUALIZED_PPA_ARCH, CRED_FILE_PATH, COMMON_LAUNCHPAD_CACHE_DIR
-
-
-def get_launchpad(use_staging=False, use_cred_file=os.path.expanduser(CRED_FILE_PATH)):
+from .settings import (ARCHS_TO_EVENTUALLY_IGNORE,
+                       ARCHS_TO_UNCONDITIONALLY_IGNORE, VIRTUALIZED_PPA_ARCH,
+                       CRED_FILE_PATH, COMMON_LAUNCHPAD_CACHE_DIR)
+
+
+def get_launchpad(use_staging=False,
+                  use_cred_file=os.path.expanduser(CRED_FILE_PATH)):
     '''Get THE Launchpad'''
     global launchpad
     if not launchpad:
@@ -42,14 +45,20 @@
             os.makdedirs(launchpadlib_dir)
 
         if use_cred_file:
-            launchpad = Launchpad.login_with('cupstream2distro', server, allow_access_levels=["WRITE_PRIVATE"],
-                                             version='devel',  # devel because copyPackage is only available there
-                                             credentials_file=use_cred_file,
-                                             launchpadlib_dir=launchpadlib_dir)
+            launchpad = Launchpad.login_with(
+                'cupstream2distro', server,
+                allow_access_levels=["WRITE_PRIVATE"],
+                # devel because copyPackage is only available there
+                version='devel',
+                credentials_file=use_cred_file,
+                launchpadlib_dir=launchpadlib_dir)
         else:
-            launchpad = Launchpad.login_with('cupstream2distro', server, allow_access_levels=["WRITE_PRIVATE"],
-                                             version='devel',  # devel because copyPackage is only available there
-                                             launchpadlib_dir=launchpadlib_dir)
+            launchpad = Launchpad.login_with(
+                'cupstream2distro', server,
+                allow_access_levels=["WRITE_PRIVATE"],
+                # devel because copyPackage is only available there
+                version='devel',
+                launchpadlib_dir=launchpadlib_dir)
 
     return launchpad
 
@@ -77,7 +86,8 @@
         bug_title_sets = set()
         for bug in author_bugs[author]:
             try:
-                bug_title_sets.add("{} (LP: #{})".format(lp.bugs[bug].title, bug))
+                title = "{} (LP: #{})".format(lp.bugs[bug].title, bug)
+                bug_title_sets.add(title)
             except KeyError:
                 # still list non existing or if launchpad timeouts bugs
                 bug_title_sets.add(u"Fix LP: #{}".format(bug))
@@ -102,9 +112,12 @@
             bug = lp.bugs[bug_num]
             bug.addTask(target=package)
             bug.lp_save()
-        except (KeyError, lazr.restfulclient.errors.BadRequest, lazr.restfulclient.errors.ServerError):
-            # ignore non existing or available bugs
-            logging.info("Can't synchronize upstream/downstream bugs for bug #{}. Not blocking on that.".format(bug_num))
+        except (KeyError, lazr.restfulclient.errors.BadRequest,
+                lazr.restfulclient.errors.ServerError):
+                    # ignore non existing or available bugs
+                    m = ("Can't synchronize upstream/downstream bugs "
+                         "for bug #{}. Not blocking on that.".format(bug_num))
+                    logging.info(m)
 
 
 def get_available_and_all_archs(series, ppa=None):
@@ -115,7 +128,8 @@
         arch_all_arch = VIRTUALIZED_PPA_ARCH[0]
     else:
         for arch in series.architectures:
-            # HACK: filters armel as it's still seen as available on raring: https://launchpad.net/bugs/1077257
+            # HACK: filters armel as it's still seen as available on raring:
+            # https://launchpad.net/bugs/1077257
             if arch.architecture_tag == "armel":
                 continue
             available_arch.add(arch.architecture_tag)
@@ -135,23 +149,29 @@
     if ppa_name.startswith("ppa:"):
         ppa_name = ppa_name[4:]
     ppa_dispatch = ppa_name.split("/")
-    return get_launchpad().people[ppa_dispatch[0]].getPPAByName(name=ppa_dispatch[1])
+    person = get_launchpad().people[ppa_dispatch[0]]
+    return person.getPPAByName(name=ppa_dispatch[1])
+
 
 def is_series_current(series_name):
     '''Return if series_name is the edge development version'''
     return get_ubuntu().current_series.name == series_name
 
+
 def get_resource_from_url(url):
     '''Return a lp resource from a launchpad url'''
     lp = get_launchpad()
-    url = lp.resource_type_link.replace("/#service-root", "") + url.split("launchpad.net")[1]
+    url = lp.resource_type_link.replace("/#service-root", "")
+    url = url + url.split("launchpad.net")[1]
     return lp.load(url)
 
+
 def get_resource_from_token(url):
     '''Return a lp resource from a launchpad token'''
     lp = get_launchpad()
     return lp.load(url)
 
+
 def is_dest_ubuntu_archive(series_link):
     '''return if series_link is the ubuntu archive'''
     return series_link == get_ubuntu_archive().self_link
 
=== modified file 'branch-source-builder/cupstream2distro/packageinppa.py'
--- branch-source-builder/cupstream2distro/packageinppa.py	2014-02-19 16:34:46 +0000
+++ branch-source-builder/cupstream2distro/packageinppa.py	2014-02-23 16:37:01 +0000
@@ -27,8 +27,9 @@
     (BUILDING, FAILED, PUBLISHED) = range(3)
 
     def __init__(self, source_name, version, ppa, destarchive, series,
-                 available_archs_in_ppa, arch_all_arch, archs_to_eventually_ignore,
-                 archs_to_unconditionually_ignore, package_archs=None):
+                 available_archs_in_ppa, arch_all_arch,
+                 archs_to_eventually_ignore, archs_to_unconditionually_ignore,
+                 package_archs=None):
         self.source_name = source_name
         self.version = version
         self.series = series
@@ -39,7 +40,8 @@
         # Get archs we should look at
         version_for_source_file = version.split(':')[-1]
         if not package_archs:
-            dsc_filename = "{}_{}.dsc".format(source_name, version_for_source_file)
+            tmpl = "{}_{}.dsc"
+            dsc_filename = tmpl.format(source_name, version_for_source_file)
             regexp = re.compile("^Architecture: (.*)\n")
             for line in open(dsc_filename):
                 arch_lists = regexp.findall(line)
@@ -57,21 +59,28 @@
             archs_supported_by_package = set()
             for arch in package_archs.split():
                 archs_supported_by_package.add(arch)
-            self.archs = archs_supported_by_package.intersection(available_archs_in_ppa)
-        # ignore some eventual archs if doesn't exist in latest published version in dest
+            intersection = archs_supported_by_package.intersection
+            self.archs = intersection(available_archs_in_ppa)
+        # ignore some eventual archs if doesn't exist in latest published
+        # version in dest
         archs_to_eventually_ignore = archs_to_eventually_ignore.copy()
         if archs_to_eventually_ignore:
             try:
-                previous_source = destarchive.getPublishedSources(exact_match=True, source_name=self.source_name,
-                                                                  distro_series=self.series, status="Published")[0]
+                kw = {'exact_match': True, 'source_name': self.source_name,
+                      'distro_series': self.series, 'status': "Published"}
+                previous_source = destarchive.getPublishedSources(**kw)[0]
                 for binary in previous_source.getPublishedBinaries():
-                    if binary.architecture_specific and binary.distro_arch_series.architecture_tag in archs_to_eventually_ignore:
-                        archs_to_eventually_ignore -= set([binary.distro_arch_series.architecture_tag])
+                    if (binary.architecture_specific and
+                       binary.distro_arch_series.architecture_tag in
+                       archs_to_eventually_ignore):
+                            rem = [binary.distro_arch_series.architecture_tag]
+                            archs_to_eventually_ignore -= set(rem)
                     if not archs_to_eventually_ignore:
                         break
 
             except IndexError:
-                # no package in dest, don't wait on any archs_to_eventually_ignore
+                # no package in dest, don't wait on any
+                # archs_to_eventually_ignore
                 pass
             # remove from the inspection remaining archs to ignore
             if archs_to_eventually_ignore:
@@ -79,11 +88,9 @@
             if archs_to_unconditionually_ignore:
                 self.archs -= archs_to_unconditionually_ignore
 
-
     def __repr__(self):
         return '{} - {}'.format(self.source_name, self.version)
 
-
     def get_status(self, on_particular_arch=None):
         '''Look at the package status in the ppa
 
@@ -120,11 +127,15 @@
 
         for arch in self.archs.copy():
             if os.path.isfile("{}.{}.ignore".format(self.source_name, arch)):
-                logging.warning("Request to ignore {} on {}.".format(self.source_name, arch))
+                tmpl = "Request to ignore {} on {}."
+                logging.warning(tmpl.format(self.source_name, arch))
                 try:
                     self.archs.remove(arch)
                 except ValueError:
-                    logging.warning("Request to ignore {} on {} has been proceeded, but this one wasn't in the list we were monitor for.".format(self.source_name, arch))
+                    tmpl = ("Request to ignore {} on {} has been proceeded, "
+                            "but this one wasn't in the list we were monitor "
+                            "for.")
+                    logging.warning(tmpl.format(self.source_name, arch))
                 try:
                     self.current_status.pop(arch)
                 except KeyError:
@@ -137,10 +148,12 @@
 
         # first step, get the source published
         if not self.current_status:
-            (self.current_status, self.source) = self._get_status_for_source_package_in_ppa()
+            status = self._get_status_for_source_package_in_ppa()
+            (self.current_status, self.source) = status
         # check the binary status
         if self.current_status:
-            self.current_status = self._get_status_for_binary_packages_in_ppa()
+            status = self._get_status_for_binary_packages_in_ppa()
+            self.current_status = status
 
     def _get_status_for_source_package_in_ppa(self):
         '''Return current_status for source package in ppa.
@@ -149,13 +162,16 @@
             - None -> not visible yet
             - BUILDING -> currently Building (or waiting to build)
             - FAILED -> Build failed for this arch or has been canceled
-            - PUBLISHED -> All packages (including arch:all from other archs) published.
+            - PUBLISHED -> All packages (including arch:all from other archs)
+              published.
 
-            Only the 2 first status are returned by this call. See _get_status_for_binary_packages_in_ppa
-            for the others.'''
+            Only the 2 first status are returned by this call. See
+            _get_status_for_binary_packages_in_ppa for the others.'''
 
         try:
-            source = self.ppa.getPublishedSources(exact_match=True, source_name=self.source_name, version=self.version, distro_series=self.series)[0]
+            kw = {'exact_match': True, 'source_name': self.source_name,
+                  'version': self.version, 'distro_series': self.series}
+            source = self.ppa.getPublishedSources(**kw)[0]
             logging.info("Source available in ppa")
             current_status = {}
             for arch in self.archs:
@@ -172,26 +188,36 @@
             - None -> not visible yet
             - BUILDING -> currently Building (or waiting to build)
             - FAILED -> Build failed for this arch or has been canceled
-            - PUBLISHED -> All packages (including arch:all from other archs) published.
-
-            Only the 3 last statuses are returned by this call. See _get_status_for_source_package_in_ppa
-            for the other.'''
-
-        # Try to see if all binaries availables for this arch are built, including arch:all on other archs
+            - PUBLISHED -> All packages (including arch:all from other archs)
+              published.
+
+            Only the 3 last statuses are returned by this call. See
+            _get_status_for_source_package_in_ppa for the other.'''
+
+        # Try to see if all binaries availables for this arch are built,
+        # including arch:all on other archs
         status = self.current_status
-        at_least_one_published_binary = False
         for binary in self.source.getPublishedBinaries():
-            at_least_one_published_binary = True
             # all binaries for an arch are published at the same time
-            # launchpad is lying, it's telling that archs not in the ppa are built (for arch:all). Even for non supported arch!
-            # for instance, we can have the case of self.arch_all_arch (arch:all), built before the others and amd64 will be built for it
-            if binary.status == "Published" and (binary.distro_arch_series.architecture_tag == self.arch_all_arch or
-               (binary.distro_arch_series.architecture_tag != self.arch_all_arch and binary.architecture_specific)):
-                status[binary.distro_arch_series.architecture_tag] = self.PUBLISHED
+            # launchpad is lying, it's telling that archs not in the ppa are
+            # built (for arch:all). Even for non supported arch!
+            # for instance, we can have the case of self.arch_all_arch
+            # (arch:all), built before the others and amd64 will be built for
+            # it
+            published = binary.status == "Published"
+            arch_tag = binary.distro_arch_series.architecture_tag
+            not_arch_all = arch_tag != self.arch_all_arch
+            arch_all = arch_tag == self.arch_all_arch
+            arch_specific = binary.architecture_specific
+            if published and (arch_all or (not_arch_all and arch_specific)):
+                status[arch_tag] = self.PUBLISHED
 
-        # Looking for builds on archs still BUILDING (just loop on builds once to avoid too many lp requests)
+        # Looking for builds on archs still BUILDING (just loop on builds once
+        # to avoid too many lp requests)
         needs_checking_build = False
-        build_state_failed = ('Failed to build', 'Chroot problem', 'Failed to upload', 'Cancelled build', 'Build for superseded Source')
+        build_state_failed = ('Failed to build', 'Chroot problem',
+                              'Failed to upload', 'Cancelled build',
+                              'Build for superseded Source')
         for arch in self.archs:
             if self.current_status[arch] == self.BUILDING:
                 needs_checking_build = True
@@ -202,24 +228,35 @@
                     continue
                 if self.current_status[build.arch_tag] == self.BUILDING:
                     if build.buildstate in build_state_failed:
-                        logging.error("{}: Build {} ({}) failed because of {}".format(build.arch_tag, build.title,
-                                                                                       build.web_link, build.buildstate))
+                        tmpl = "{}: Build {} ({}) failed because of {}"
+                        logging.error(tmpl.format(build.arch_tag, build.title,
+                                      build.web_link, build.buildstate))
                         status[build.arch_tag] = self.FAILED
-                    # Another launchpad trick: if a binary arch was published, but then is superseeded, getPublishedBinaries() won't list
-                    # those binaries anymore. So it's seen as BUILDING again.
-                    # If there is a successful build record of it and the source is superseded, it means that it built fine at some point,
-                    # Another arch will fail as superseeded.
-                    # We don't just retain the old state of "PUBLISHED" because maybe we started the script with that situation already
-                    elif build.buildstate not in build_state_failed and self.source.status == "Superseded":
-                        status[build.arch_tag] = self.PUBLISHED
+                    # Another launchpad trick: if a binary arch was published,
+                    # but then is superseeded, getPublishedBinaries() won't
+                    # list those binaries anymore. So it's seen as BUILDING
+                    # again.  If there is a successful build record of it and
+                    # the source is superseded, it means that it built fine at
+                    # some point, Another arch will fail as superseeded.
+                    # We don't just retain the old state of "PUBLISHED" because
+                    # maybe we started the script with that situation already
+                    elif (build.buildstate not in build_state_failed
+                          and self.source.status == "Superseded"):
+                                status[build.arch_tag] = self.PUBLISHED
 
-        # There is no way to know if there are some arch:all packages (and there are not in publishedBinaries for this arch until
-        # it's built on arch_all_arch). So mark all arch to BUILDING if self.arch_all_arch is building or FAILED if it failed.
-        if self.arch_all_arch in status and status[self.arch_all_arch] != self.PUBLISHED:
+        # There is no way to know if there are some arch:all packages (and
+        # there are not in publishedBinaries for this arch until
+        # it's built on arch_all_arch). So mark all arch to BUILDING if
+        # self.arch_all_arch is building or FAILED if it failed.
+        not_published = status[self.arch_all_arch] != self.PUBLISHED
+        if self.arch_all_arch in status and not_published:
             for arch in self.archs:
                 if status[arch] == self.PUBLISHED:
                     status[arch] = status[self.arch_all_arch]
-                    if arch != self.arch_all_arch and status[arch] == self.FAILED:
-                        logging.error("{} marked as FAILED because {} build FAILED and we may miss arch:all packages".format(arch, self.arch_all_arch))
+                    failed = status[arch] == self.FAILED
+                    if arch != self.arch_all_arch and failed:
+                        msg = ("{} marked as FAILED because {} build FAILED "
+                               "and we may miss arch:all packages")
+                        logging.error(msg.format(arch, self.arch_all_arch))
 
         return status
 
=== modified file 'branch-source-builder/cupstream2distro/packageinppamanager.py'
--- branch-source-builder/cupstream2distro/packageinppamanager.py	2014-02-19 16:34:46 +0000
+++ branch-source-builder/cupstream2distro/packageinppamanager.py	2014-02-23 16:37:01 +0000
@@ -41,7 +41,8 @@
     # we do not rely on the .changes files but in the config file
     # because we need the exact version (which can have an epoch)
     result = set()
-    source_package_regexp = re.compile("(.*).{}$".format(PROJECT_CONFIG_SUFFIX))
+    tmpl = "(.*).{}$"
+    source_package_regexp = re.compile(tmpl.format(PROJECT_CONFIG_SUFFIX))
     for file in os.listdir('.'):
         substract = source_package_regexp.findall(file)
         if substract:
@@ -59,40 +60,51 @@
 
 
 def get_packages_and_versions_uploaded():
-    '''Get (package, version) of all packages uploaded. We can have duplicates'''
+    '''Get (package, version) of all packages uploaded. We can have
+    duplicates'''
 
     # we do not rely on the .changes files but in the config file
     # because we need the exact version (which can have an epoch)
     result = set()
-    source_package_regexp = re.compile("(.*).{}.*$".format(PROJECT_CONFIG_SUFFIX))
+    tmpl = "(.*).{}.*$"
+    source_package_regexp = re.compile(tmpl.format(PROJECT_CONFIG_SUFFIX))
     for file in os.listdir('.'):
         substract = source_package_regexp.findall(file)
         if substract:
             config = ConfigParser.RawConfigParser()
             config.read(file)
-            result.add((substract[0], config.get('Package', 'packaging_version')))
+            pkg = config.get('Package', 'packaging_version')
+            result.add((substract[0], pkg))
     return result
 
 
-def update_all_packages_status(packages_not_in_ppa, packages_building, packages_failed, particular_arch=None):
+def update_all_packages_status(packages_not_in_ppa, packages_building,
+                               packages_failed, particular_arch=None):
     '''Update all packages status, checking in the ppa'''
 
     for current_package in (packages_not_in_ppa.union(packages_building)):
-        logging.info("current_package: " + current_package.source_name + " " + current_package.version)
+        logging.info("current_package: " + current_package.source_name + " " +
+                     current_package.version)
         package_status = current_package.get_status(particular_arch)
-        if package_status != None:  # global package_status can be 0 (building), 1 (failed), 2 (published)
+        # global package_status can be 0 (building), 1 (failed), 2 (published)
+        if package_status is None:
             # if one arch building, still considered as building
             if package_status == PackageInPPA.BUILDING:
-                _ensure_removed_from_set(packages_not_in_ppa, current_package)  # maybe already removed
+                # maybe already removed
+                _ensure_removed_from_set(packages_not_in_ppa, current_package)
                 packages_building.add(current_package)
             # if one arch failed, considered as failed
             elif package_status == PackageInPPA.FAILED:
-                _ensure_removed_from_set(packages_building, current_package)  # in case we missed the "build" step
-                _ensure_removed_from_set(packages_not_in_ppa, current_package)  # in case we missed the "wait" step
+                # in case we missed the "build" step
+                _ensure_removed_from_set(packages_building, current_package)
+                # in case we missed the "wait" step
+                _ensure_removed_from_set(packages_not_in_ppa, current_package)
                 packages_failed.add(current_package)
             elif package_status == PackageInPPA.PUBLISHED:
-                _ensure_removed_from_set(packages_building, current_package)  # in case we missed the "build" step
-                _ensure_removed_from_set(packages_not_in_ppa, current_package)  # in case we missed the "wait" step
+                # in case we missed the "build" step
+                _ensure_removed_from_set(packages_building, current_package)
+                # in case we missed the "wait" step
+                _ensure_removed_from_set(packages_not_in_ppa, current_package)
 
 
 def _get_current_packaging_version_from_config(source_package_name):
 
=== modified file 'branch-source-builder/cupstream2distro/packagemanager.py'
--- branch-source-builder/cupstream2distro/packagemanager.py	2014-02-19 16:34:46 +0000
+++ branch-source-builder/cupstream2distro/packagemanager.py	2014-02-23 16:37:01 +0000
@@ -31,9 +31,11 @@
 import launchpadmanager
 import settings
 from .utils import ignored
-
-
-def get_current_version_for_series(source_package_name, series_name, ppa_name=None, dest=None):
+from subprocess import Popen, PIPE
+
+
+def get_current_version_for_series(source_package_name, series_name,
+                                   ppa_name=None, dest=None):
     '''Get current version for a package name in that series'''
     series = launchpadmanager.get_series(series_name)
     if not dest:
@@ -41,29 +43,37 @@
             dest = launchpadmanager.get_ppa(ppa_name)
         else:
             dest = launchpadmanager.get_ubuntu_archive()
-    source_collection = dest.getPublishedSources(exact_match=True, source_name=source_package_name, distro_series=series)
+    kw = {'exact_match': True, 'source_name': source_package_name,
+          'distro_series': series}
+    source_collection = dest.getPublishedSources(**kw)
     try:
-        # cjwatson told that list always have the more recently published first (even if removed)
+        # cjwatson told that list always have the more recently published first
+        # (even if removed)
         return source_collection[0].source_package_version
     # was never in the dest, set the lowest possible version
     except IndexError:
         return "0"
 
 
-def is_version_for_series_in_dest(source_package_name, version, series, dest, pocket="Release"):
+def is_version_for_series_in_dest(source_package_name, version, series, dest,
+                                  pocket="Release"):
     '''Return if version for a package name in that series is in dest'''
-    return dest.getPublishedSources(exact_match=True, source_name=source_package_name, version=version,
-                                    distro_series=series, pocket=pocket).total_size > 0
+    kw = {'exact_match': True, 'source_name': source_package_name,
+          'version': version, 'distro_series': series, 'pocket': pocket}
+    return dest.getPublishedSources(**kw).total_size > 0
+
 
 def is_version_in_queue(source_package_name, version, dest_serie, queue):
     '''Return if version for a package name in that series is in dest'''
-    return dest_serie.getPackageUploads(exact_match=True, name=source_package_name, version=version,
-                                        status=queue).total_size > 0
+    kw = {'exact_match': True, 'name': source_package_name, 'version': version,
+          'status': queue}
+    return dest_serie.getPackageUploads(**kw).total_size > 0
 
 
 def is_version1_higher_than_version2(version1, version2):
     '''return if version1 is higher than version2'''
-    return (subprocess.call(["dpkg", "--compare-versions", version1, 'gt', version2], stdout=subprocess.PIPE, stderr=subprocess.PIPE) == 0)
+    cmd = ["dpkg", "--compare-versions", version1, 'gt', version2]
+    return (subprocess.call(cmd, stdout=PIPE, stderr=PIPE) == 0)
 
 
 def is_version_in_changelog(version, f):
@@ -72,7 +82,8 @@
     if version == "0":
         return True
 
-    desired_changelog_line = re.compile("\({}\) (?!UNRELEASED).*\; urgency=".format(version.replace('+', '\+')))
+    t = "\({}\) (?!UNRELEASED).*\; urgency="
+    desired_changelog_line = re.compile(t.format(version.replace('+', '\+')))
     for line in f.readlines():
         if desired_changelog_line.search(line):
             return True
@@ -83,9 +94,12 @@
 def get_latest_upstream_bzr_rev(f, dest_ppa=None):
     '''Report latest bzr rev in the file
 
-    If dest_ppa, first try to fetch the dest ppa tag. Otherwise, fallback to first distro version'''
+    If dest_ppa, first try to fetch the dest ppa tag. Otherwise, fallback to
+    first distro version'''
     distro_regex = re.compile("{} (\d+)".format(settings.REV_STRING_FORMAT))
-    destppa_regexp = re.compile("{} (\d+) \(ppa:{}\)".format(settings.REV_STRING_FORMAT, dest_ppa))
+    tmpl = "{} (\d+) \(ppa:{}\)"
+    destppa_regexp = re.compile(
+        tmpl.format(settings.REV_STRING_FORMAT, dest_ppa))
     distro_rev = None
     candidate_destppa_rev = None
     candidate_distro_rev = None
@@ -111,15 +125,19 @@
                 destppa_element_found = True
             except IndexError:
                 destppa_element_found = False
-        if not distro_rev and not destppa_element_found and not "(ppa:" in line:
+        ppa_line = "(ppa:" in line
+        if not distro_rev and not destppa_element_found and not ppa_line:
             try:
                 candidate_distro_rev = int(distro_regex.findall(line)[0])
                 distro_element_found = True
             except IndexError:
                 distro_element_found = False
 
-        # try to catchup next line if we have a marker start without anything found
-        if settings.REV_STRING_FORMAT in line and (dest_ppa and not destppa_element_found) and not distro_element_found:
+        # try to catchup next line if we have a marker start without anything
+        # found
+        rev_str_found = settings.REV_STRING_FORMAT in line
+        ppa_found = dest_ppa and not destppa_element_found
+        if rev_str_found and ppa_found and not distro_element_found:
             previous_line = line
 
         if line.startswith(" -- "):
@@ -150,7 +168,8 @@
 
 def get_packaging_version():
     '''Get current packaging rev'''
-    instance = subprocess.Popen(["dpkg-parsechangelog"], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    instance = Popen(["dpkg-parsechangelog"],
+                     stdout=PIPE, stderr=PIPE)
     (stdout, stderr) = instance.communicate()
     if instance.returncode != 0:
         raise Exception(stderr.decode("utf-8").strip())
@@ -160,19 +179,24 @@
         if packaging_version:
             return packaging_version[0]
 
-    raise Exception("Didn't find any Version in the package: {}".format(stdout))
-
-
-def get_source_package_from_dest(source_package_name, dest_archive, dest_current_version, series_name):
-    '''Download and return a path containing a checkout of the current dest version.
+    tmpl = "Didn't find any Version in the package: {}"
+    raise Exception(tmpl.format(stdout))
+
+
+def get_source_package_from_dest(source_package_name, dest_archive,
+                                 dest_current_version, series_name):
+    '''Download and return a path containing a checkout of the current dest
+    version.
 
     None if this package was never published to dest archive'''
 
     if dest_current_version == "0":
-        logging.info("This package was never released to the destination archive, don't return downloaded source")
+        logging.info("This package was never released to the destination "
+                     "archive, don't return downloaded source")
         return None
 
-    logging.info("Grab code for {} ({}) from {}".format(source_package_name, dest_current_version, series_name))
+    args = (source_package_name, dest_current_version, series_name)
+    logging.info("Grab code for {} ({}) from {}".format(*args))
     source_package_download_dir = os.path.join('ubuntu', source_package_name)
     series = launchpadmanager.get_series(series_name)
     with ignored(OSError):
@@ -180,27 +204,39 @@
     os.chdir(source_package_download_dir)
 
     try:
-        sourcepkg = dest_archive.getPublishedSources(status="Published", exact_match=True, source_name=source_package_name, distro_series=series, version=dest_current_version)[0]
+        kw = {'status': "Published", 'exact_match': True,
+              'source_name': source_package_name, 'distro_series': series,
+              'version': dest_current_version}
+        sourcepkg = dest_archive.getPublishedSources(**kw)[0]
     except IndexError:
         raise Exception("Couldn't get in the destination the expected version")
-    logging.info('Downloading %s version %s', source_package_name, dest_current_version)
+    tmpl = 'Downloading %s version %s'
+    logging.info(tmpl, source_package_name, dest_current_version)
     for url in sourcepkg.sourceFileUrls():
         urllib.urlretrieve(url, urllib.unquote(url.split('/')[-1]))
-    instance = subprocess.Popen("dpkg-source -x *dsc", shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    cmd = "dpkg-source -x *dsc"
+    instance = Popen(cmd, shell=True, stdout=PIPE, stderr=PIPE)
     (stdout, stderr) = instance.communicate()
     if instance.returncode != 0:
         raise Exception(stderr.decode("utf-8").strip())
 
     # check the dir exist
-    splitted_version = dest_current_version.split(':')[-1].split('-')  # remove epoch is there is one
+    # remove epoch is there is one
+    splitted_version = dest_current_version.split(':')[-1].split('-')
     # TODO: debian version (like -3) is not handled here.
-    # We do handle 42ubuntu1 though (as splitted_version[0] can contain "ubuntu")
-    if "ubuntu" in splitted_version[-1] and len(splitted_version) > 1:  # don't remove last item for the case where we had a native version (-0.35.2) without ubuntu in it
+    # We do handle 42ubuntu1 though (as splitted_version[0] can contain
+    # "ubuntu")
+    if "ubuntu" in splitted_version[-1] and len(splitted_version) > 1:
+        # don't remove last item for the case where we had a native version
+        # (-0.35.2) without ubuntu in it
         splitted_version = splitted_version[:-1]
     version_for_source_file = '-'.join(splitted_version)
-    source_directory_name = "{}-{}".format(source_package_name, version_for_source_file)
+    args = (source_package_name, version_for_source_file)
+    source_directory_name = "{}-{}".format(*args)
     if not os.path.isdir(source_directory_name):
-        raise Exception("We tried to download and check that the directory {} is present, but it's not the case".format(source_directory_name))
+        tmpl = ("We tried to download and check that the directory {} is "
+                "present, but it's not the case")
+        raise Exception(tmpl.format(source_directory_name))
     os.chdir('../..')
     return (os.path.join(source_package_download_dir, source_directory_name))
 
@@ -210,24 +246,40 @@
 
     dest_version_source can be None if no released version was done before.'''
 
-    # we always released something not yet in ubuntu, no matter criterias are not met.
+    # we always released something not yet in ubuntu, no matter criterias are
+    # not met.
     if not dest_version_source:
         return True
 
-    # now check the relevance of the committed changes compared to the version in the repository (if any)
-    diffinstance = subprocess.Popen(['diff', '-Nrup', '.', dest_version_source], stdout=subprocess.PIPE)
-    filterinstance = subprocess.Popen(['filterdiff', '--clean', '-x', '*po', '-x', '*pot', '-x', '*local-options'], stdin=diffinstance.stdout, stdout=subprocess.PIPE)
-    lsdiffinstance = subprocess.Popen(['lsdiff'], stdin=filterinstance.stdout, stdout=subprocess.PIPE)
-    (relevant_changes, err) = subprocess.Popen(['grep', '-Ev', '.bzr|.pc'], stdin=lsdiffinstance.stdout, stdout=subprocess.PIPE).communicate()
-
-    # detect if the only change is a Vcs* target changes (with or without changelog edit). We won't release in that case
+    # now check the relevance of the committed changes compared to the version
+    # in the repository (if any)
+    cmd = ['diff', '-Nrup', '.', dest_version_source]
+    diffinstance = Popen(cmd, stdout=PIPE)
+
+    cmd = ['filterdiff', '--clean', '-x', '*po', '-x', '*pot', '-x',
+           '*local-options']
+    filterinstance = Popen(cmd, stdin=diffinstance.stdout, stdout=PIPE)
+    cmd = ['lsdiff']
+    lsdiffinstance = Popen(cmd, stdin=filterinstance.stdout, stdout=PIPE)
+    cmd = ['grep', '-Ev', '.bzr|.pc']
+    p = Popen(cmd, stdin=lsdiffinstance.stdout, stdout=PIPE)
+    (relevant_changes, err) = p.communicate()
+
+    # detect if the only change is a Vcs* target changes (with or without
+    # changelog edit). We won't release in that case
     number_of_changed_files = relevant_changes.count("\n")
-    if ((number_of_changed_files == 1 and "debian/control" in relevant_changes) or
-       (number_of_changed_files == 2 and "debian/control" in relevant_changes and "debian/changelog" in relevant_changes)):
-        (results, err) = subprocess.Popen(['diff', os.path.join('debian', 'control'), os.path.join(dest_version_source, "debian", "control")], stdout=subprocess.PIPE).communicate()
+
+    control_changed = "debian/control" in relevant_changes
+    if control_changed and number_of_changed_files in (1, 2):
+        cmd = ['diff', os.path.join('debian', 'control'),
+               os.path.join(dest_version_source, "debian", "control")]
+
+        (results, err) = Popen(cmd, stdout=PIPE).communicate()
         for diff_line in results.split('\n'):
             if diff_line.startswith("< ") or diff_line.startswith("> "):
-                if not diff_line[2:].startswith("Vcs-") and not diff_line[2:].startswith("#"):
+                vcs = diff_line[2:].startswith("Vcs-")
+                comment = diff_line[2:].startswith("#")
+                if not vcs and not comment:
                     return True
         return False
 
@@ -237,24 +289,34 @@
     return (relevant_changes != '')
 
 
-def is_relevant_source_diff_from_previous_dest_version(newdsc_path, dest_version_source):
-    '''Extract and check if the generated source diff different from previous one'''
+def is_relevant_source_diff_from_previous_dest_version(newdsc_path,
+                                                       dest_version_source):
+    '''Extract and check if the generated source diff different from previous
+    one'''
 
     with ignored(OSError):
         os.makedirs("generated")
-    extracted_generated_source = os.path.join("generated", newdsc_path.split('_')[0])
+    prefix = newdsc_path.split('_')[0]
+    extracted_generated_source = os.path.join("generated", prefix)
     with ignored(OSError):
         shutil.rmtree(extracted_generated_source)
 
     # remove epoch is there is one
-    if subprocess.call(["dpkg-source", "-x", newdsc_path, extracted_generated_source]) != 0:
+    cmd = ["dpkg-source", "-x", newdsc_path, extracted_generated_source]
+    if subprocess.call(cmd) != 0:
         raise Exception("dpkg-source command returned an error.")
 
-    # now check the relevance of the committed changes compared to the version in the repository (if any)
-    diffinstance = subprocess.Popen(['diff', '-Nrup', extracted_generated_source, dest_version_source], stdout=subprocess.PIPE)
-    (diff, err) = subprocess.Popen(['filterdiff', '--clean', '-x', '*po', '-x', '*pot', '-x', '*local-options'], stdin=diffinstance.stdout, stdout=subprocess.PIPE).communicate()
+    # now check the relevance of the committed changes compared to the version
+    # in the repository (if any)
+    cmd = ['diff', '-Nrup', extracted_generated_source, dest_version_source]
+    diffinstance = Popen(cmd, stdout=PIPE)
+    cmd = ['filterdiff', '--clean', '-x', '*po', '-x', '*pot', '-x',
+           '*local-options']
+    p = Popen(cmd, stdin=diffinstance.stdout, stdout=PIPE)
+    (diff, err) = p.communicate()
 
-    # there is no important diff if the diff only contains 12 lines, corresponding to "Automatic daily release" marker in debian/changelog
+    # there is no important diff if the diff only contains 12 lines,
+    # corresponding to "Automatic daily release" marker in debian/changelog
     if (diff.count('\n') <= 12):
         return False
     return True
@@ -267,47 +329,65 @@
     if not oldsource_dsc:
         return True
     if not os.path.isfile(oldsource_dsc) or not os.path.isfile(newsource_dsc):
-        raise Exception("{} or {} doesn't not exist, can't create a diff".format(oldsource_dsc, newsource_dsc))
-    diffinstance = subprocess.Popen(['debdiff', oldsource_dsc, newsource_dsc], stdout=subprocess.PIPE)
-    filterinstance = subprocess.Popen(['filterdiff', '--clean', '-i', '*debian/*', '-x', '*changelog'], stdin=diffinstance.stdout, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        tmpl = "{} or {} doesn't not exist, can't create a diff"
+        raise Exception(tmpl.format(oldsource_dsc, newsource_dsc))
+    diffinstance = Popen(['debdiff', oldsource_dsc, newsource_dsc],
+                         stdout=PIPE)
+    cmd = ['filterdiff', '--clean', '-i', '*debian/*', '-x', '*changelog']
+    filterinstance = Popen(cmd, stdin=diffinstance.stdout, stdout=PIPE,
+                           stderr=PIPE)
     (change_in_debian, filter_err) = filterinstance.communicate()
-    # we can't rely on diffinstance returncode as the signature key is maybe not present and it will exit with 1
+    # we can't rely on diffinstance returncode as the signature key is maybe
+    # not present and it will exit with 1
     if filterinstance.returncode != 0:
-        raise Exception("Error in diff: {}".format(filter_err.decode("utf-8").strip()))
+        tmpl = "Error in diff: {}"
+        raise Exception(tmpl.format(filter_err.decode("utf-8").strip()))
     return(change_in_debian != "")
 
 
 def generate_diff_between_dsc(diff_filepath, oldsource_dsc, newsource_dsc):
-    '''Generate a diff file in diff_filepath if there is a relevant packaging diff between 2 sources
+    '''Generate a diff file in diff_filepath if there is a relevant packaging
+    diff between 2 sources
 
     The diff contains autotools files and cmakeries'''
     if _packaging_changes_between_dsc(oldsource_dsc, newsource_dsc):
         with open(diff_filepath, "w") as f:
             if not oldsource_dsc:
-                f.writelines("This source is a new package, if the destination is ubuntu, please ensure it has been preNEWed by an archive admin before publishing that stack.")
+                f.writelines("This source is a new package, if the destination"
+                             " is ubuntu, please ensure it has been preNEWed"
+                             " by an archive admin before publishing that"
+                             " stack.")
                 return
-            f.write("/!\ Remember that this diff only represents packaging changes and build tools diff, not the whole content diff!\n\n")
-            diffinstance = subprocess.Popen(['debdiff', oldsource_dsc, newsource_dsc], stdout=subprocess.PIPE)
-            (changes_to_publish, err) = subprocess.Popen(['filterdiff', '--remove-timestamps', '--clean', '-i', '*setup.py',
-                                                          '-i', '*Makefile.am', '-i', '*configure.*', '-i', '*debian/*',
-                                                          '-i', '*CMakeLists.txt'], stdin=diffinstance.stdout, stdout=subprocess.PIPE).communicate()
+            f.write("/!\ Remember that this diff only represents packaging "
+                    "changes and build tools diff, not the whole content "
+                    "diff!\n\n")
+            diffinstance = Popen(['debdiff', oldsource_dsc, newsource_dsc],
+                                 stdout=PIPE)
+            cmd = ['filterdiff', '--remove-timestamps', '--clean', '-i',
+                   '*setup.py', '-i', '*Makefile.am', '-i', '*configure.*',
+                   '-i', '*debian/*', '-i', '*CMakeLists.txt']
+            (changes_to_publish, err) = Popen(cmd, stdin=diffinstance.stdout,
+                                              stdout=PIPE).communicate()
             f.write(changes_to_publish)
 
 
-def create_new_packaging_version(base_package_version, series_version, destppa=''):
+def create_new_packaging_version(base_package_version, series_version,
+                                 destppa=''):
     '''Deliver a new packaging version, based on simple rules:
 
-    Version would be <upstream_version>.<series>.<yyyymmdd(.minor)>-0ubuntu1
-    if we already have something delivered today, it will be .minor, then, .minor+1…
+    Version would be <upstream_version>.<series>.<yyyymmdd(.minor)>-0ubuntu1 if
+    we already have something delivered today, it will be .minor, then,
+    .minor+1…
 
-    We append the destination ppa name if we target a dest ppa and not distro'''
+    We append the destination ppa name if we target a dest ppa and not
+    distro'''
     # to keep track of whether the package is native or not
     native_pkg = False
 
     today_version = datetime.date.today().strftime('%Y%m%d')
     destppa = destppa.replace("-", '.').replace("_", ".").replace("/", ".")
-    # bootstrapping mode or direct upload or UNRELEASED for bumping to a new series
-    # TRANSITION
+    # bootstrapping mode or direct upload or UNRELEASED for bumping to a new
+    # series TRANSITION
     if not ("daily" in base_package_version or "+" in base_package_version):
         # support both 42, 42-0ubuntu1
         upstream_version = base_package_version.split('-')[0]
@@ -322,17 +402,18 @@
             upstream_version = previous_day[0]
             native_pkg = True
             if (previous_day[1] == series_version and
-                previous_day[2] == today_version):
-                minor = 1
-                if previous_day[3]:  # second upload of the day
-                    minor = int(previous_day[3]) + 1
-                today_version = "{}.{}".format(today_version, minor)
+               previous_day[2] == today_version):
+                    minor = 1
+                    if previous_day[3]:  # second upload of the day
+                        minor = int(previous_day[3]) + 1
+                    today_version = "{}.{}".format(today_version, minor)
         except IndexError:
             raise Exception(
                 "Unable to get previous day from native version: %s"
                 % base_package_version)
     else:
-        # extract the day of previous daily upload and bump if already uploaded today
+        # extract the day of previous daily upload and bump if already uploaded
+        # today
         regexp = re.compile("(.*)\+([\d\.]{5})\.(\d{8})\.?([\d]*).*-.*")
         try:
             previous_day = regexp.findall(base_package_version)[0]
@@ -342,11 +423,17 @@
                 regexp = re.compile("(.*)(daily)([\d\.]{8})\.?([\d]*).*-.*")
                 previous_day = regexp.findall(base_package_version)[0]
                 # make the version compatible with the new version
-                previous_day = (previous_day[0], previous_day[1], "20" + previous_day[2].replace(".", ""), previous_day[3])
+                previous_day = (previous_day[0], previous_day[1], "20" +
+                                previous_day[2].replace(".", ""),
+                                previous_day[3])
             except IndexError:
-                raise Exception("Didn't find a correct versioning in the current package: {}".format(base_package_version))
+                tmpl = ("Didn't find a correct versioning in the current "
+                        "package: {}")
+                raise Exception(tmpl.format(base_package_version))
         upstream_version = previous_day[0]
-        if previous_day[1] == series_version and previous_day[2] == today_version:
+        is_series = previous_day[1] == series_version
+        is_today = previous_day[2] == today_version
+        if is_series and is_today:
             minor = 1
             if previous_day[3]:  # second upload of the day
                 minor = int(previous_day[3]) + 1
@@ -363,7 +450,7 @@
 
 def get_packaging_sourcename():
     '''Get current packaging source name'''
-    instance = subprocess.Popen(["dpkg-parsechangelog"], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    instance = Popen(["dpkg-parsechangelog"], stdout=PIPE, stderr=PIPE)
     (stdout, stderr) = instance.communicate()
     if instance.returncode != 0:
         raise Exception(stderr.decode("utf-8").strip())
@@ -373,7 +460,8 @@
         if source_name:
             return source_name[0]
 
-    raise Exception("Didn't find any source name in the package: {}".format(stdout))
+    tmpl = "Didn't find any source name in the package: {}"
+    raise Exception(tmpl.format(stdout))
 
 
 def collect_bugs_in_changelog_until_latest_snapshot(f, source_package_name):
@@ -382,20 +470,24 @@
     # matching only bug format that launchpad accepts
     group_bugs_regexp = re.compile("lp: ?(.*\d{5,})", re.IGNORECASE)
     bug_decipher_regexp = re.compile("(#\d{5,})+")
-    new_upload_changelog_regexp = re.compile(settings.NEW_CHANGELOG_PATTERN.format(source_package_name))
+    pattern = settings.NEW_CHANGELOG_PATTERN.format(source_package_name)
+    new_upload_changelog_regexp = re.compile(pattern)
     for line in f:
         grouped_bugs_list = group_bugs_regexp.findall(line)
         for grouped_bugs in grouped_bugs_list:
-            for bug in map(lambda bug_with_hash: bug_with_hash.replace('#', ''), bug_decipher_regexp.findall(grouped_bugs)):
+            func = lambda bug_with_hash: bug_with_hash.replace('#', '')
+            for bug in map(func, bug_decipher_regexp.findall(grouped_bugs)):
                 bugs.add(bug)
-        #  a released upload to distro (automated or manual)n exit as bugs before were already covered
+        # a released upload to distro (automated or manual)n exit as bugs
+        # before were already covered
         if new_upload_changelog_regexp.match(line):
             return bugs
 
     return bugs
 
 
-def update_changelog(new_package_version, series, tip_bzr_rev, authors_commits, dest_ppa=None):
+def update_changelog(new_package_version, series, tip_bzr_rev, authors_commits,
+                     dest_ppa=None):
     '''Update the changelog for the incoming upload'''
 
     dch_env = os.environ.copy()
@@ -403,18 +495,20 @@
         dch_env["DEBFULLNAME"] = author
         for bug_desc in authors_commits[author]:
             if bug_desc.startswith('-'):
-                # Remove leading '-' or dch thinks (rightly) that it's an option
+                # Remove leading '-' or dch thinks (rightly) that it's an
+                # option
                 bug_desc = bug_desc[1:]
             if bug_desc.startswith(' '):
                 # Remove leading spaces, there are useless and the result is
                 # prettier without them anyway ;)
                 bug_desc = bug_desc.strip()
-            cmd = ["dch", "--multimaint-merge", "--release-heuristic", "changelog",
-                   "-v{}".format(new_package_version), bug_desc]
-            subprocess.Popen(cmd, env=dch_env).communicate()
+            cmd = ["dch", "--multimaint-merge", "--release-heuristic",
+                   "changelog", "-v{}".format(new_package_version), bug_desc]
+            Popen(cmd, env=dch_env).communicate()
 
-    if tip_bzr_rev != None:
-        commit_message = "{} {}".format(settings.REV_STRING_FORMAT, tip_bzr_rev)
+    if tip_bzr_rev is None:
+        tmpl = "{} {}"
+        commit_message = tmpl.format(settings.REV_STRING_FORMAT, tip_bzr_rev)
         if dest_ppa:
             commit_message += " ({})".format(dest_ppa)
     else:
@@ -422,17 +516,21 @@
 
     dch_env["DEBFULLNAME"] = settings.BOT_DEBFULLNAME
     dch_env["DEBEMAIL"] = settings.BOT_DEBEMAIL
-    instance = subprocess.Popen(["dch", "--release-heuristic", "changelog",
-                                 "-v{}".format(new_package_version), commit_message],
-                                stderr=subprocess.PIPE, env=dch_env)
+    instance = Popen(["dch", "--release-heuristic", "changelog",
+                     "-v{}".format(new_package_version), commit_message],
+                     stderr=PIPE, env=dch_env)
     (stdout, stderr) = instance.communicate()
     if instance.returncode != 0:
         raise Exception(stderr.decode("utf-8").strip())
-    subprocess.call(["dch", "-r", "--distribution", series, "--force-distribution", ""], env=dch_env)
+    cmd = ["dch", "-r", "--distribution", series, "--force-distribution", ""]
+    subprocess.call(cmd, env=dch_env)
 
-    # in the case of no commit_message and no symbols file change, we have an addition [ DEBFULLNAME ] follow by an empty line
+    # in the case of no commit_message and no symbols file change, we have an
+    # addition [ DEBFULLNAME ] follow by an empty line
     # better to remove both lines
-    subprocess.call(["sed", "-i", "/ \[ " + settings.BOT_DEBFULLNAME + " \]/{$q; N; /\\n$/d;}", "debian/changelog"])
+    pattern = "/ \[ " + settings.BOT_DEBFULLNAME + " \]/{$q; N; /\\n$/d;}"
+    cmd = ["sed", "-i", pattern, "debian/changelog"]
+    subprocess.call(cmd)
 
 
 def build_source_package(series, distro_version, ppa=None):
@@ -457,7 +555,7 @@
            "--distro-version", distro_version]
     if ppa:
         cmd.extend(["--ppa", ppa])
-    instance = subprocess.Popen(cmd, env=cowbuilder_env)
+    instance = Popen(cmd, env=cowbuilder_env)
     instance.communicate()
     if instance.returncode != 0:
         raise Exception("%r returned: %s." % (cmd, instance.returncode))
@@ -490,15 +588,19 @@
     replacement_done = False
     for filename in os.listdir("debian"):
         if filename.endswith("symbols"):
-            for line in fileinput.input(os.path.join('debian', filename), inplace=1):
+            dfile = os.path.join('debian', filename)
+            for line in fileinput.input(dfile, inplace=1):
                 if settings.REPLACEME_TAG in line:
                     replacement_done = True
-                    line = line.replace(settings.REPLACEME_TAG, new_upstream_version)
+                    line = line.replace(settings.REPLACEME_TAG,
+                                        new_upstream_version)
                 sys.stdout.write(line)
 
     if replacement_done:
         dch_env = os.environ.copy()
         dch_env["DEBFULLNAME"] = settings.BOT_DEBFULLNAME
         dch_env["DEBEMAIL"] = settings.BOT_DEBEMAIL
-        subprocess.Popen(["dch", "debian/*symbols: auto-update new symbols to released version"], env=dch_env).communicate()
+        cmd = ["dch",
+               "debian/*symbols: auto-update new symbols to released version"]
+        Popen(cmd, env=dch_env).communicate()
         subprocess.call(["bzr", "commit", "-m", "Update symbols"])
 
=== modified file 'branch-source-builder/cupstream2distro/settings.py'
--- branch-source-builder/cupstream2distro/settings.py	2014-02-19 16:34:46 +0000
+++ branch-source-builder/cupstream2distro/settings.py	2014-02-23 16:37:01 +0000
@@ -39,6 +39,7 @@
     GNUPG_DIR = home_dir
 CRED_FILE_PATH = os.path.join("/tmp", "launchpad.credentials")
 
+
 # TODO refactor into a ci-utils module
 def _unit_config():
     path = os.path.join(home_dir, 'unit_config')
@@ -73,7 +74,8 @@
 
 # selected arch for building arch:all packages
 VIRTUALIZED_PPA_ARCH = ["i386", "amd64"]
-# an arch we will ignore for publication if latest published version in dest doesn't build it
+# an arch we will ignore for publication if latest published version in dest
+# doesn't build it
 ARCHS_TO_EVENTUALLY_IGNORE = set(['powerpc', 'arm64', 'ppc64el'])
 ARCHS_TO_UNCONDITIONALLY_IGNORE = set(['arm64', 'ppc64el'])
 SRU_PPA = "ubuntu-unity/sru-staging"
@@ -87,11 +89,15 @@
 
 OLD_STACK_DIR = 'old'
 PACKAGE_LIST_RSYNC_FILENAME_PREFIX = 'packagelist_rsync'
-PACKAGE_LIST_RSYNC_FILENAME_FORMAT = PACKAGE_LIST_RSYNC_FILENAME_PREFIX + '_{}-{}'
-RSYNC_PATTERN = "rsync://RSYNCSVR/cu2d_out/{}*".format(PACKAGE_LIST_RSYNC_FILENAME_PREFIX)
+PACKAGE_LIST_RSYNC_FILENAME_FORMAT = (PACKAGE_LIST_RSYNC_FILENAME_PREFIX
+                                      + '_{}-{}')
+RSYNC_PATTERN = "rsync://RSYNCSVR/cu2d_out/{}*"
+RSYNC_PATTERN = RSYNC_PATTERN.format(PACKAGE_LIST_RSYNC_FILENAME_PREFIX)
 
-ROOT_CU2D = os.path.join(os.path.dirname(os.path.dirname(os.path.realpath(__file__))))
-DEFAULT_CONFIG_STACKS_DIR = os.path.join(os.path.dirname(ROOT_CU2D), 'cupstream2distro-config', 'stacks')
+ROOT_CU2D = os.path.join(
+    os.path.dirname(os.path.dirname(os.path.realpath(__file__))))
+DEFAULT_CONFIG_STACKS_DIR = os.path.join(
+    os.path.dirname(ROOT_CU2D), 'cupstream2distro-config', 'stacks')
 STACK_STATUS_FILENAME = "stack.status"
 STACK_STARTED_FILENAME = "stack.started"
 STACK_BUILDING_FILENAME = "stack.building"
 
=== modified file 'branch-source-builder/cupstream2distro/silomanager.py'
--- branch-source-builder/cupstream2distro/silomanager.py	2014-01-31 17:06:36 +0000
+++ branch-source-builder/cupstream2distro/silomanager.py	2014-02-23 16:37:01 +0000
@@ -22,7 +22,8 @@
 import os
 import shutil
 
-from cupstream2distro.settings import SILO_CONFIG_FILENAME, SILO_NAME_LIST, SILO_STATUS_RSYNCDIR
+from cupstream2distro.settings import (SILO_CONFIG_FILENAME, SILO_NAME_LIST,
+                                       SILO_STATUS_RSYNCDIR)
 from cupstream2distro.utils import ignored
 
 
@@ -42,10 +43,13 @@
         os.makedirs(SILO_STATUS_RSYNCDIR)
     silo_name = os.path.dirname(silo_config_path).split(os.path.sep)[-1]
     dest = os.path.join(SILO_STATUS_RSYNCDIR, silo_name)
-    logging.debug("Copying configuration from {} to {}".format(silo_config_path, dest))
-    shutil.copy2(silo_config_path, os.path.join(SILO_STATUS_RSYNCDIR, silo_name))
+    tmpl = "Copying configuration from {} to {}"
+    logging.debug(tmpl.format(silo_config_path, dest))
+    dest = os.path.join(SILO_STATUS_RSYNCDIR, silo_name)
+    shutil.copy2(silo_config_path, dest)
     return True
 
+
 def load_config(uri=None):
     """return a loaded config
 
@@ -62,23 +66,33 @@
         logging.warning("Can't load configuration: " + e.message)
     return None
 
+
 def remove_status_file(silo_name):
     """Remove status file"""
     os.remove(os.path.join(SILO_STATUS_RSYNCDIR, silo_name))
 
 
-def is_project_not_in_any_configs(project_name, series, dest, base_silo_uri, ignore_silo):
-    """Return true if the project for that serie in that dest is not in any configuration"""
-    logging.info("Checking if {} is already configured for {} ({}) in another silo".format(project_name, dest.name, series.name))
+def is_project_not_in_any_configs(project_name, series, dest, base_silo_uri,
+                                  ignore_silo):
+    """Return true if the project for that serie in that dest is not in any
+    configuration"""
+    tmpl = "Checking if {} is already configured for {} ({}) in another silo"
+    logging.info(tmpl.format(project_name, dest.name, series.name))
     for silo_name in SILO_NAME_LIST:
         # we are reconfiguring current silo, ignoring it
         if ignore_silo == silo_name:
             continue
         config = load_config(os.path.join(base_silo_uri, silo_name))
         if config:
-            if (config["global"]["dest"] == dest.self_link and config["global"]["series"] == series.self_link and
-                (project_name in config["mps"] or project_name in config["sources"])):
-                logging.error("{} is already prepared for the same serie and destination in {}".format(project_name, silo_name))
+            project_match = (project_name in config["mps"]
+                             or project_name in config["sources"])
+            series_match = config["global"]["series"] == series.self_link
+            dest_match = config["global"]["dest"] == dest.self_link
+
+            if (dest_match and series_match and project_match):
+                tmpl = ("{} is already prepared for the same series "
+                        "and destination in {}")
+                logging.error(tmpl.format(project_name, silo_name))
                 return False
     return True
 
@@ -86,27 +100,32 @@
 def return_first_available_silo(base_silo_uri):
     """Check which silos are free and return the first one"""
     for silo_name in SILO_NAME_LIST:
-        if not os.path.isfile(os.path.join(base_silo_uri, silo_name, SILO_CONFIG_FILENAME)):
+        p = os.path.join(base_silo_uri, silo_name, SILO_CONFIG_FILENAME)
+        if not os.path.isfile(p):
             return silo_name
     return None
 
+
 def get_config_step(config):
     """Get configuration step"""
     return config["global"]["step"]
 
+
 def set_config_step(config, new_step, uri=''):
     """Set configuration step to new_step"""
     config["global"]["step"] = new_step
     return save_config(config, uri)
 
+
 def set_config_status(config, status, uri='', add_url=True):
     """Change status to reflect latest status"""
     build_url = os.getenv('BUILD_URL')
     if add_url and build_url:
-        status = "{} ({}console)".format(status , build_url)
+        status = "{} ({}console)".format(status, build_url)
     config["global"]["status"] = status
     return save_config(config, uri)
 
+
 def get_all_projects(config):
     """Get a list of all projets"""
     projects = []
 
=== modified file 'branch-source-builder/cupstream2distro/stack.py'
--- branch-source-builder/cupstream2distro/stack.py	2014-02-19 16:34:46 +0000
+++ branch-source-builder/cupstream2distro/stack.py	2014-02-23 16:37:01 +0000
@@ -21,11 +21,75 @@
 import os
 import yaml
 
-from .settings import DEFAULT_CONFIG_STACKS_DIR, STACK_STATUS_FILENAME, STACK_STARTED_FILENAME
+from .settings import (DEFAULT_CONFIG_STACKS_DIR, STACK_STATUS_FILENAME,
+                       STACK_STARTED_FILENAME)
 from .utils import ignored
 
 _stacks_ref = {}
 
+CANT_FIND_STATUS = '''Can't find status for {depstack} ({deprel}). This
+shouldn't happen unless the stack is currently running. If this is the case, it
+means that the current stack shouldn't be uploaded as the state is unknown.'''
+
+FAILED_TO_PUBLISH = '''{depstack} ({deprel}) failed to publish. Possible causes
+are:
+        * the stack really didn't build/can't be prepared at all.
+        * the stack has integration tests not working with this previous stack.
+
+    What needs to be done:
+    Either:
+        * If we want to publish both stacks: retry the integration tests for
+          {depstack} ({deprel}), including components from this stack (check
+          with the whole PPA). If that works, both stacks should be published
+          at the same time.
+    Or:
+        * If we only want to publish this stack: check that we can safely
+          publish it by itself (e.g. without the stacks it depends on). The
+          trick there is to make sure that the stack is not relying on, or
+          affected by, a change that happened in one of its dependencies.
+          Example: if the {depstack} ({deprel}) API changed in a way that
+          affects any component of the current stack, and both stacks got
+          updated in trunk, we need to make sure we don't land only one of the
+          two stacks which would result in a broken state. Also think about
+          potential ABI changes.'''
+
+MANUAL_PUBLISH = '''{depstack} ({deprel}) is in manually publish mode. Possible
+causes are:
+        * Some part of the stack has packaging changes
+        * This stack is depending on another stack not being published
+
+    What needs to be done:
+    Either:
+        * If {depstack} ({deprel}) can be published, we should publish both
+          stacks at the same time.
+    Or:
+        * If we only want to publish this stack: check that we can safely
+          publish it by itself (e.g. without the stacks it depends on). The
+          trick there is to make sure that the stack is not relying on, or
+          affected by, a change that happened in one of its dependencies.
+          Example: if the {depstack} ({deprel}) API changed in a way that
+          affects any component of the current stack, and both stacks got
+          updated in trunk, we need to make sure we don't land only one of the
+          two stacks which would result in a broken state. Also think about
+          potential ABI changes.'''
+
+MANUALLY_ABORTED = '''{depstack} ({deprel}) has been manually aborted or failed
+for an unknown reason. Possible causes are:
+        * A job of this stack was stopped manually
+        * Jenkins had an internal error/shutdown
+
+    What needs to be done:
+        * If we only want to publish this stack: check that we can safely
+          publish it by itself (e.g. without the stacks it depends on). The
+          trick there is to make sure that the stack is not relying on, or
+          affected by, a change that happened in one of its dependencies.
+          Example: if the {depstack} ({deprel}) API changed in a way that
+          affects any component of the current stack, and both stacks got
+          updated in trunk, we need to make sure we don't land only one of the
+          two stacks which would result in a broken state. Also think about
+          potential ABI changes.'''
+
+
 # TODO: should be used by a metaclass
 def get_stack(release, stack_name):
     try:
@@ -33,22 +97,28 @@
     except KeyError:
         return Stack(release, stack_name)
 
+
 class Stack():
 
     def __init__(self, release, stack_name):
-    	self.stack_name = stack_name
-    	self.release = release
-        self.statusfile = os.path.join('..', '..', release, stack_name, STACK_STATUS_FILENAME)
-        self.startedfile = os.path.join('..', '..', release, stack_name, STACK_STARTED_FILENAME)
+        self.stack_name = stack_name
+        self.release = release
+        args = ('..', '..', release, stack_name, STACK_STATUS_FILENAME)
+        self.statusfile = os.path.join(*args)
+        args = ('..', '..', release, stack_name, STACK_STARTED_FILENAME)
+        self.startedfile = os.path.join(*args)
         self.stack_file_path = None
         self._dependencies = None
         self._rdependencies = None
-    	for stack_file_path in Stack.get_stacks_file_path(release):
-            if stack_file_path.split(os.path.sep)[-1] == "{}.cfg".format(stack_name):
+        for stack_file_path in Stack.get_stacks_file_path(release):
+            formatted = "{}.cfg".format(stack_name)
+            if stack_file_path.split(os.path.sep)[-1] == formatted:
                 self.stack_file_path = stack_file_path
                 break
         if not self.stack_file_path:
-            raise Exception("{}.cfg for {} doesn't exist anywhere in {}".format(stack_name, release, self.get_root_stacks_dir()))
+            msg = "{}.cfg for {} doesn't exist anywhere in {}"
+            msg = msg.format(stack_name, release, self.get_root_stacks_dir())
+            raise Exception(msg)
 
         with open(self.stack_file_path, 'r') as f:
             cfg = yaml.load(f)
@@ -118,7 +188,11 @@
                     else:
                         (stackname, release) = (item, self.release)
                     self._dependencies.append(get_stack(release, stackname))
-                logging.info("{} ({}) dependency list is: {}".format(self.stack_name, self.release, ["{} ({})".format(stack.stack_name, stack.release) for stack in self._dependencies]))
+                deps = ["{} ({})".format(stack.stack_name, stack.release)
+                        for stack in self._dependencies]
+                msg = "{} ({}) dependency list is: {}"
+                msg = msg.format(self.stack_name, self.release, deps)
+                logging.info(msg)
                 return self._dependencies
             except (TypeError, KeyError):
                 return []
@@ -137,7 +211,8 @@
         return self._rdependencies
 
     def generate_dep_status_message(self):
-        '''Return a list of potential problems from others stack which should block current publication'''
+        '''Return a list of potential problems from others stack which should
+        block current publication'''
 
         # TODO: get the first Stack object
         # iterate over all stacks objects from dep chain
@@ -145,39 +220,24 @@
 
         global_dep_status_info = []
         for stack in self.get_direct_depending_stacks():
-            logging.info("Check status for {} ({})".format(stack.stack_name, stack.release))
+            msg = "Check status for {} ({})"
+            msg = msg.format(stack.stack_name, stack.release)
+            logging.info(msg)
             status = stack.get_status()
             message = None
             # We should have a status for every stack
             if status is None:
-                message = "Can't find status for {depstack} ({deprel}). This shouldn't happen unless the stack is currently running. If this is the case, it means that the current stack shouldn't be uploaded as the state is unknown.".format(depstack=stack, deprel=stack.release)
+                kw = {'depstack': stack, 'deprel': stack.release}
+                message = CANT_FIND_STATUS.format(**kw)
             elif status == 1:
-                message = '''{depstack} ({deprel}) failed to publish. Possible causes are:
-        * the stack really didn't build/can't be prepared at all.
-        * the stack has integration tests not working with this previous stack.
-
-    What needs to be done:
-    Either:
-        * If we want to publish both stacks: retry the integration tests for {depstack} ({deprel}), including components from this stack (check with the whole PPA). If that works, both stacks should be published at the same time.
-    Or:
-        * If we only want to publish this stack: check that we can safely publish it by itself (e.g. without the stacks it depends on). The trick there is to make sure that the stack is not relying on, or affected by, a change that happened in one of its dependencies. Example: if the {depstack} ({deprel}) API changed in a way that affects any component of the current stack, and both stacks got updated in trunk, we need to make sure we don't land only one of the two stacks which would result in a broken state. Also think about potential ABI changes.'''.format(depstack=stack.stack_name, deprel=stack.release)
+                kw = {'depstack': stack.stack_name, 'deprel': stack.release}
+                message = FAILED_TO_PUBLISH.format(**kw)
             elif status == 2:
-                message = '''{depstack} ({deprel}) is in manually publish mode. Possible causes are:
-        * Some part of the stack has packaging changes
-        * This stack is depending on another stack not being published
-
-    What needs to be done:
-    Either:
-        * If {depstack} ({deprel}) can be published, we should publish both stacks at the same time.
-    Or:
-        * If we only want to publish this stack: check that we can safely publish it by itself (e.g. without the stacks it depends on). The trick there is to make sure that the stack is not relying on, or affected by, a change that happened in one of its dependencies. Example: if the {depstack} ({deprel}) API changed in a way that affects any component of the current stack, and both stacks got updated in trunk, we need to make sure we don't land only one of the two stacks which would result in a broken state. Also think about potential ABI changes.'''.format(depstack=stack.stack_name, deprel=stack.release)
+                kw = {'depstack': stack.stack_name, 'deprel': stack.release}
+                message = MANUAL_PUBLISH.format(**kw)
             elif status == 3 or status == -1:
-                message = '''{depstack} ({deprel}) has been manually aborted or failed for an unknown reason. Possible causes are:
-        * A job of this stack was stopped manually
-        * Jenkins had an internal error/shutdown
-
-    What needs to be done:
-        * If we only want to publish this stack: check that we can safely publish it by itself (e.g. without the stacks it depends on). The trick there is to make sure that the stack is not relying on, or affected by, a change that happened in one of its dependencies. Example: if the {depstack} ({deprel}) API changed in a way that affects any component of the current stack, and both stacks got updated in trunk, we need to make sure we don't land only one of the two stacks which would result in a broken state. Also think about potential ABI changes.'''.format(depstack=stack.stack_name, deprel=stack.release)
+                kw = {'depstack': stack.stack_name, 'deprel': stack.release}
+                message = MANUALLY_ABORTED.format(**kw)
 
             if message:
                 logging.warning(message)
@@ -186,19 +246,21 @@
 
     @staticmethod
     def get_root_stacks_dir():
-	    '''Get root stack dir'''
-	    return os.environ.get('CONFIG_STACKS_DIR', DEFAULT_CONFIG_STACKS_DIR)
+        '''Get root stack dir'''
+        return os.environ.get('CONFIG_STACKS_DIR', DEFAULT_CONFIG_STACKS_DIR)
 
     @staticmethod
     def get_stacks_file_path(release):
         '''Return an iterator with all path for every discovered stack files'''
-        for root, dirs, files in os.walk(os.path.join(Stack.get_root_stacks_dir(), release)):
+        walking = os.walk(os.path.join(Stack.get_root_stacks_dir(), release))
+        for root, dirs, files in walking:
             for candidate in files:
                 if candidate.endswith('.cfg'):
                     yield os.path.join(root, candidate)
 
     @staticmethod
     def get_current_stack():
-        '''Return current stack object based on current path (release/stackname)'''
+        '''Return current stack object based on current path
+        (release/stackname)'''
         path = os.getcwd().split(os.path.sep)
         return get_stack(path[-2], path[-1])
 
=== modified file 'branch-source-builder/cupstream2distro/stacks.py'
--- branch-source-builder/cupstream2distro/stacks.py	2014-02-19 16:34:46 +0000
+++ branch-source-builder/cupstream2distro/stacks.py	2014-02-23 16:37:01 +0000
@@ -21,6 +21,7 @@
 import os
 import yaml
 import subprocess
+from subprocess import PIPE
 
 from .settings import PACKAGE_LIST_RSYNC_FILENAME_PREFIX, RSYNC_PATTERN
 from .tools import get_packaging_diff_filename
@@ -38,7 +39,7 @@
         raise Exception('Please set environment variable CU2D_RSYNCSVR')
 
     cmd = ["rsync", '--remove-source-files', '--timeout=60', remoteaddr, '.']
-    instance = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    instance = subprocess.Popen(cmd, stdout=PIPE, stderr=PIPE)
     (stdout, stderr) = instance.communicate()
     if instance.returncode not in (0, 23):
         raise Exception(stderr.decode("utf-8").strip())
@@ -62,10 +63,12 @@
             try:
                 projects_list = cfg['stack']['projects']
             except (TypeError, KeyError):
-                logging.warning("{} seems broken in not having stack or projects keys".format(file_path))
+                tmpl = "{} seems broken in not having stack or projects keys"
+                logging.warning(tmpl.format(file_path))
                 continue
             if not projects_list:
-                logging.warning("{} don't have any project list".format(file_path))
+                tmpl = "{} don't have any project list"
+                logging.warning(tmpl.format(file_path))
                 continue
             for project in projects_list:
                 if isinstance(project, dict):
@@ -74,11 +77,13 @@
                     projects.append(project)
     return set(projects)
 
+
 def get_stack_packaging_change_status(source_version_list):
     '''Return global package change status list
 
     # FIXME: added too many infos now, should only be: (source, version)
-    source_version_list is a list of couples (source, version, tip_rev, target_branch)'''
+    source_version_list is a list of couples (source, version, tip_rev,
+    target_branch)'''
 
     packaging_change_status = []
     for (source, version, tip_rev, target_branch) in source_version_list:
 
=== modified file 'branch-source-builder/cupstream2distro/tools.py'
--- branch-source-builder/cupstream2distro/tools.py	2014-02-19 16:34:46 +0000
+++ branch-source-builder/cupstream2distro/tools.py	2014-02-23 16:37:01 +0000
@@ -26,18 +26,21 @@
 from .settings import PROJECT_CONFIG_SUFFIX
 from .utils import ignored
 
-WRAPPER_STRING = '''<testsuite errors="0" failures="{}" name="" tests="1" time="0.1">
+WRAPPER_STRING = '''<testsuite errors="0" failures="{}" name="" tests="1"
+time="0.1">
   <testcase classname="MarkUnstable" name={} time="0.0">{}</testcase>
 </testsuite>'''
 
 
 def generate_xml_artefacts(test_name, details, filename):
-    '''Generate a fake test name xml result for marking the build as unstable'''
+    '''Generate a fake test name xml result for marking the build as
+    unstable'''
     failure = ""
     errnum = 0
     for detail in details:
         errnum = 1
-        failure += '    <failure type="exception">{}</failure>\n'.format(escape(detail))
+        ex = escape(detail)
+        failure += '    <failure type="exception">{}</failure>\n'.format(ex)
     if failure:
         failure = '\n{}'.format(failure)
 
@@ -52,7 +55,8 @@
     return config.get('Package', 'dest_current_version')
 
 
-def save_project_config(source_package_name, branch, revision, dest_current_version, current_packaging_version):
+def save_project_config(source_package_name, branch, revision,
+                        dest_current_version, current_packaging_version):
     '''Save branch and package configuration'''
     config = ConfigParser.RawConfigParser()
     config.add_section('Branch')
@@ -61,21 +65,27 @@
     config.add_section('Package')
     config.set('Package', 'dest_current_version', dest_current_version)
     config.set('Package', 'packaging_version', current_packaging_version)
-    with open("{}.{}".format(source_package_name, PROJECT_CONFIG_SUFFIX), 'wb') as configfile:
+    path = "{}.{}".format(source_package_name, PROJECT_CONFIG_SUFFIX)
+    with open(path, 'wb') as configfile:
         config.write(configfile)
 
 
 def get_packaging_diff_filename(source_package_name, packaging_version):
     '''Return the packaging diff filename'''
 
-    return "packaging_changes_{}_{}.diff".format(source_package_name, packaging_version)
+    ret = "packaging_changes_{}_{}.diff"
+    return ret.format(source_package_name, packaging_version)
 
 
 def mark_project_as_published(source_package_name, packaging_version):
-    '''Rename .project and eventual diff files so that if we do a partial rebuild, we don't try to republish them'''
-    project_filename = "{}.{}".format(source_package_name, PROJECT_CONFIG_SUFFIX)
-    os.rename(project_filename, "{}_{}".format(project_filename, packaging_version))
-    diff_filename = get_packaging_diff_filename(source_package_name, packaging_version)
+    '''Rename .project and eventual diff files so that if we do a partial
+    rebuild, we don't try to republish them'''
+    tmpl = "{}.{}"
+    project_filename = tmpl.format(source_package_name, PROJECT_CONFIG_SUFFIX)
+    new_name = tmpl.format(project_filename, packaging_version)
+    os.rename(project_filename, new_name)
+    diff_filename = get_packaging_diff_filename(source_package_name,
+                                                packaging_version)
     if os.path.isfile(diff_filename):
         os.rename(diff_filename, "{}.published".format(diff_filename))
 
 
=== modified file 'charms/precise/python-django/hooks/hooks.py'
--- charms/precise/python-django/hooks/hooks.py	2014-02-18 20:11:41 +0000
+++ charms/precise/python-django/hooks/hooks.py	2014-02-23 16:37:01 +0000
@@ -562,7 +562,6 @@
 
 def config_changed(config_data):
     os.environ['DJANGO_SETTINGS_MODULE'] = django_settings_modules
-    django_admin_cmd = find_django_admin_cmd()
 
     site_secret_key = config_data['site_secret_key']
     if not site_secret_key:
@@ -610,7 +609,7 @@
         else:
             run('git pull %s %s' % (repos_url, vcs_clone_dir))
     elif vcs == 'bzr' or vcs == 'bazaar':
-        run('cd %s; bzr pull %s %s' % (vcs_clonse_dir, repos_url))
+        run('cd %s; bzr pull %s %s' % (vcs_clone_dir, repos_url))
     elif vcs == 'svn' or vcs == 'subversion':
         run('svn up %s %s' % (repos_url, vcs_clone_dir))
     else:
 
=== modified file 'image-builder/imagebuilder/tests/test_style.py'
--- image-builder/imagebuilder/tests/test_style.py	2014-02-15 12:06:40 +0000
+++ image-builder/imagebuilder/tests/test_style.py	2014-02-23 16:37:01 +0000
@@ -14,15 +14,19 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 from ucitests import styles
-
-import imagebuilder
+# If we just import run_worker, we'll end up testing whichever run_worker was
+# last imported, so import it under a different name.
+import imagebuilder_run_worker
 
 
 class TestPep8(styles.TestPep8):
 
-    packages = [imagebuilder]
+    # uci-tests will scan all subdirectories that this module is found in, so
+    # we do not need to explicitly provide imagebuilder. Doing so would cause
+    # the tests to be run twice.
+    packages = [imagebuilder_run_worker]
 
 
 class TestPyflakes(styles.TestPyflakes):
 
-    packages = [imagebuilder]
+    packages = [imagebuilder_run_worker]
 
=== added symlink 'image-builder/imagebuilder_run_worker.py'
=== target is u'run_worker'
=== modified file 'image-builder/run_worker'
--- image-builder/run_worker	2014-01-28 13:50:43 +0000
+++ image-builder/run_worker	2014-02-23 16:37:01 +0000
@@ -50,7 +50,7 @@
         if amqp_utils.progress_completed(trigger, {'image_id': image_id}):
             log.error(
                 'Unable to notify progress-trigger completition of action')
-    except Exception as e:
+    except Exception:
         type, val, tb = sys.exc_info()
         amqp_utils.progress_failed(trigger, {'message': val.message})
         log.exception('Image build failed:')
 
=== modified file 'lander/lander/tests/test_style.py'
--- lander/lander/tests/test_style.py	2014-02-15 12:06:40 +0000
+++ lander/lander/tests/test_style.py	2014-02-23 16:37:01 +0000
@@ -14,15 +14,19 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 from ucitests import styles
-
-import lander
+# If we just import run_worker, we'll end up testing whichever run_worker was
+# last imported, so import it under a different name.
+import lander_run_worker
 
 
 class TestPep8(styles.TestPep8):
 
-    packages = [lander]
+    # uci-tests will scan all subdirectories that this module is found in, so
+    # we do not need to explicitly provide lander. Doing so would cause the
+    # tests to be run twice.
+    packages = [lander_run_worker]
 
 
 class TestPyflakes(styles.TestPyflakes):
 
-    packages = [lander]
+    packages = [lander_run_worker]
 
=== added symlink 'lander/lander_run_worker.py'
=== target is u'run_worker'
=== modified file 'test_runner/tstrun/tests/test_style.py'
--- test_runner/tstrun/tests/test_style.py	2014-02-15 12:06:40 +0000
+++ test_runner/tstrun/tests/test_style.py	2014-02-23 16:37:01 +0000
@@ -16,13 +16,19 @@
 from ucitests import styles
 
 import tstrun
+# If we just import run_worker, we'll end up testing whichever run_worker was
+# last imported, so import it under a different name.
+import tstrun_run_worker
 
 
 class TestPep8(styles.TestPep8):
 
-    packages = [tstrun]
+    # uci-tests will scan all subdirectories that this module is found in, so
+    # we do not need to explicitly provide tstrun. Doing so would cause
+    # the tests to be run twice.
+    packages = [tstrun_run_worker]
 
 
 class TestPyflakes(styles.TestPyflakes):
 
-    packages = [tstrun]
+    packages = [tstrun_run_worker]
 
=== added symlink 'test_runner/tstrun_run_worker.py'
=== target is u'run_worker'