Merge "Simplify error handling in subcommand execution"
diff --git a/.pylintrc b/.pylintrc
index 9f81ee1..9e8882e 100644
--- a/.pylintrc
+++ b/.pylintrc
@@ -53,7 +53,7 @@
 enable=RP0004
 
 # Disable the message(s) with the given id(s).
-disable=R0903,R0912,R0913,R0914,R0915,W0141,C0111,C0103,C0323,C0322,C0324,W0603,W0703,R0911,C0301,C0302,R0902,R0904,W0142,W0212,E1101,E1103,R0201,W0201,W0122,W0232,W0311,RP0001,RP0003,RP0101,RP0002,RP0401,RP0701,RP0801
+disable=R0903,R0912,R0913,R0914,R0915,W0141,C0111,C0103,W0603,W0703,R0911,C0301,C0302,R0902,R0904,W0142,W0212,E1101,E1103,R0201,W0201,W0122,W0232,RP0001,RP0003,RP0101,RP0002,RP0401,RP0701,RP0801
 
 [REPORTS]
 
diff --git a/color.py b/color.py
index 33bc877..d856313 100644
--- a/color.py
+++ b/color.py
@@ -40,47 +40,47 @@
                   # backslash is not anomalous
 
 def is_color(s):
-    return s in COLORS
+  return s in COLORS
 
 def is_attr(s):
-    return s in ATTRS
+  return s in ATTRS
 
 def _Color(fg = None, bg = None, attr = None):
-    fg = COLORS[fg]
-    bg = COLORS[bg]
-    attr = ATTRS[attr]
+  fg = COLORS[fg]
+  bg = COLORS[bg]
+  attr = ATTRS[attr]
 
-    if attr >= 0 or fg >= 0 or bg >= 0:
-      need_sep = False
-      code = "\033["  #pylint: disable=W1401
+  if attr >= 0 or fg >= 0 or bg >= 0:
+    need_sep = False
+    code = "\033["  #pylint: disable=W1401
 
-      if attr >= 0:
-        code += chr(ord('0') + attr)
-        need_sep = True
+    if attr >= 0:
+      code += chr(ord('0') + attr)
+      need_sep = True
 
-      if fg >= 0:
-        if need_sep:
-          code += ';'
-        need_sep = True
+    if fg >= 0:
+      if need_sep:
+        code += ';'
+      need_sep = True
 
-        if fg < 8:
-          code += '3%c' % (ord('0') + fg)
-        else:
-          code += '38;5;%d' % fg
+      if fg < 8:
+        code += '3%c' % (ord('0') + fg)
+      else:
+        code += '38;5;%d' % fg
 
-      if bg >= 0:
-        if need_sep:
-          code += ';'
-        need_sep = True
+    if bg >= 0:
+      if need_sep:
+        code += ';'
+      need_sep = True
 
-        if bg < 8:
-          code += '4%c' % (ord('0') + bg)
-        else:
-          code += '48;5;%d' % bg
-      code += 'm'
-    else:
-      code = ''
-    return code
+      if bg < 8:
+        code += '4%c' % (ord('0') + bg)
+      else:
+        code += '48;5;%d' % bg
+    code += 'm'
+  else:
+    code = ''
+  return code
 
 
 class Coloring(object):
diff --git a/git_config.py b/git_config.py
index 6bb9200..56cc6a2 100644
--- a/git_config.py
+++ b/git_config.py
@@ -303,10 +303,10 @@
     for line in d.rstrip('\0').split('\0'):  # pylint: disable=W1401
                                              # Backslash is not anomalous
       if '\n' in line:
-          key, val = line.split('\n', 1)
+        key, val = line.split('\n', 1)
       else:
-          key = line
-          val = None
+        key = line
+        val = None
 
       if key in c:
         c[key].append(val)
@@ -431,7 +431,7 @@
                      '-o','ControlPath %s' % ssh_sock(),
                      host]
     if port is not None:
-      command_base[1:1] = ['-p',str(port)]
+      command_base[1:1] = ['-p', str(port)]
 
     # Since the key wasn't in _master_keys, we think that master isn't running.
     # ...but before actually starting a master, we'll double-check.  This can
diff --git a/main.py b/main.py
index 35622ce..9594454 100755
--- a/main.py
+++ b/main.py
@@ -274,17 +274,17 @@
     return req
 
 def _AddPasswordFromUserInput(handler, msg, req):
-    # If repo could not find auth info from netrc, try to get it from user input
-    url = req.get_full_url()
-    user, password = handler.passwd.find_user_password(None, url)
-    if user is None:
-      print(msg)
-      try:
-        user = raw_input('User: ')
-        password = getpass.getpass()
-      except KeyboardInterrupt:
-        return
-      handler.passwd.add_password(None, url, user, password)
+  # If repo could not find auth info from netrc, try to get it from user input
+  url = req.get_full_url()
+  user, password = handler.passwd.find_user_password(None, url)
+  if user is None:
+    print(msg)
+    try:
+      user = raw_input('User: ')
+      password = getpass.getpass()
+    except KeyboardInterrupt:
+      return
+    handler.passwd.add_password(None, url, user, password)
 
 class _BasicAuthHandler(urllib.request.HTTPBasicAuthHandler):
   def http_error_401(self, req, fp, code, msg, headers):
diff --git a/manifest_xml.py b/manifest_xml.py
index bb93bca..1b95456 100644
--- a/manifest_xml.py
+++ b/manifest_xml.py
@@ -67,7 +67,7 @@
     # urljoin will get confused if there is no scheme in the base url
     # ie, if manifestUrl is of the form <hostname:port>
     if manifestUrl.find(':') != manifestUrl.find('/') - 1:
-        manifestUrl = 'gopher://' + manifestUrl
+      manifestUrl = 'gopher://' + manifestUrl
     url = urlparse.urljoin(manifestUrl, url)
     return re.sub(r'^gopher://', '', url)
 
@@ -349,24 +349,24 @@
     nodes = []
     for node in manifest.childNodes:  # pylint:disable=W0631
                                       # We only get here if manifest is initialised
-        if node.nodeName == 'include':
-            name = self._reqatt(node, 'name')
-            fp = os.path.join(include_root, name)
-            if not os.path.isfile(fp):
-                raise ManifestParseError, \
-                    "include %s doesn't exist or isn't a file" % \
-                    (name,)
-            try:
-                nodes.extend(self._ParseManifestXml(fp, include_root))
-            # should isolate this to the exact exception, but that's
-            # tricky.  actual parsing implementation may vary.
-            except (KeyboardInterrupt, RuntimeError, SystemExit):
-                raise
-            except Exception as e:
-                raise ManifestParseError(
-                    "failed parsing included manifest %s: %s", (name, e))
-        else:
-          nodes.append(node)
+      if node.nodeName == 'include':
+        name = self._reqatt(node, 'name')
+        fp = os.path.join(include_root, name)
+        if not os.path.isfile(fp):
+          raise ManifestParseError, \
+              "include %s doesn't exist or isn't a file" % \
+              (name,)
+        try:
+          nodes.extend(self._ParseManifestXml(fp, include_root))
+        # should isolate this to the exact exception, but that's
+        # tricky.  actual parsing implementation may vary.
+        except (KeyboardInterrupt, RuntimeError, SystemExit):
+          raise
+        except Exception as e:
+          raise ManifestParseError(
+              "failed parsing included manifest %s: %s", (name, e))
+      else:
+        nodes.append(node)
     return nodes
 
   def _ParseManifest(self, node_list):
@@ -404,9 +404,9 @@
       if node.nodeName == 'manifest-server':
         url = self._reqatt(node, 'url')
         if self._manifest_server is not None:
-            raise ManifestParseError(
-                'duplicate manifest-server in %s' %
-                (self.manifestFile))
+          raise ManifestParseError(
+              'duplicate manifest-server in %s' %
+              (self.manifestFile))
         self._manifest_server = url
 
     for node in itertools.chain(*node_list):
diff --git a/project.py b/project.py
index 22ac9ef..75c5e5e 100644
--- a/project.py
+++ b/project.py
@@ -556,7 +556,7 @@
                                '--unmerged',
                                '--ignore-missing',
                                '--refresh')
-    if self.work_git.DiffZ('diff-index','-M','--cached',HEAD):
+    if self.work_git.DiffZ('diff-index', '-M', '--cached', HEAD):
       return True
     if self.work_git.DiffZ('diff-files'):
       return True
@@ -585,14 +585,14 @@
     return self._userident_email
 
   def _LoadUserIdentity(self):
-      u = self.bare_git.var('GIT_COMMITTER_IDENT')
-      m = re.compile("^(.*) <([^>]*)> ").match(u)
-      if m:
-        self._userident_name = m.group(1)
-        self._userident_email = m.group(2)
-      else:
-        self._userident_name = ''
-        self._userident_email = ''
+    u = self.bare_git.var('GIT_COMMITTER_IDENT')
+    m = re.compile("^(.*) <([^>]*)> ").match(u)
+    if m:
+      self._userident_name = m.group(1)
+      self._userident_email = m.group(2)
+    else:
+      self._userident_name = ''
+      self._userident_email = ''
 
   def GetRemote(self, name):
     """Get the configuration for a single remote.
@@ -1381,14 +1381,14 @@
     tag_name = None
 
     def CheckForSha1():
-        try:
-          # if revision (sha or tag) is not present then following function
-          # throws an error.
-          self.bare_git.rev_parse('--verify', '%s^0' % self.revisionExpr)
-          return True
-        except GitError:
-          # There is no such persistent revision. We have to fetch it.
-          return False
+      try:
+        # if revision (sha or tag) is not present then following function
+        # throws an error.
+        self.bare_git.rev_parse('--verify', '%s^0' % self.revisionExpr)
+        return True
+      except GitError:
+        # There is no such persistent revision. We have to fetch it.
+        return False
 
     if current_branch_only:
       if ID_RE.match(self.revisionExpr) is not None:
@@ -1880,7 +1880,7 @@
                     self.level = self.level[1:]
 
             info = info[1:].split(' ')
-            info =_Info(path, *info)
+            info = _Info(path, *info)
             if info.status in ('R', 'C'):
               info.src_path = info.path
               info.path = out.next()
diff --git a/repo b/repo
index 4d8e8dc..ac1eca8 100755
--- a/repo
+++ b/repo
@@ -3,8 +3,8 @@
 ## repo default configuration
 ##
 from __future__ import print_function
-REPO_URL='https://gerrit.googlesource.com/git-repo'
-REPO_REV='stable'
+REPO_URL = 'https://gerrit.googlesource.com/git-repo'
+REPO_REV = 'stable'
 
 # Copyright (C) 2008 Google Inc.
 #
@@ -24,7 +24,7 @@
 VERSION = (1, 19)
 
 # increment this if the MAINTAINER_KEYS block is modified
-KEYRING_VERSION = (1,1)
+KEYRING_VERSION = (1, 1)
 MAINTAINER_KEYS = """
 
      Repo Maintainer <repo@android.kernel.org>
@@ -154,7 +154,8 @@
                  help='initial manifest file', metavar='NAME.xml')
 group.add_option('--mirror',
                  dest='mirror', action='store_true',
-                 help='mirror the forrest')
+                 help='create a replica of the remote repositories '
+                      'rather than a client working directory')
 group.add_option('--reference',
                  dest='reference',
                  help='location of mirror directory', metavar='DIR')
@@ -225,7 +226,7 @@
     except OSError as e:
       print('fatal: cannot make %s directory: %s'
             % (repodir, e.strerror), file=sys.stderr)
-      # Don't faise CloneFailure; that would delete the
+      # Don't raise CloneFailure; that would delete the
       # name. Instead exit immediately.
       #
       sys.exit(1)
diff --git a/subcmds/download.py b/subcmds/download.py
index 6aa54af..471e88b 100644
--- a/subcmds/download.py
+++ b/subcmds/download.py
@@ -33,13 +33,13 @@
 """
 
   def _Options(self, p):
-    p.add_option('-c','--cherry-pick',
+    p.add_option('-c', '--cherry-pick',
                  dest='cherrypick', action='store_true',
                  help="cherry-pick instead of checkout")
-    p.add_option('-r','--revert',
+    p.add_option('-r', '--revert',
                  dest='revert', action='store_true',
                  help="revert instead of checkout")
-    p.add_option('-f','--ff-only',
+    p.add_option('-f', '--ff-only',
                  dest='ffonly', action='store_true',
                  help="force fast-forward merge")
 
diff --git a/subcmds/grep.py b/subcmds/grep.py
index fa5f876..dd391cf 100644
--- a/subcmds/grep.py
+++ b/subcmds/grep.py
@@ -85,7 +85,7 @@
     g.add_option('--cached',
                  action='callback', callback=carry,
                  help='Search the index, instead of the work tree')
-    g.add_option('-r','--revision',
+    g.add_option('-r', '--revision',
                  dest='revision', action='append', metavar='TREEish',
                  help='Search TREEish, instead of the work tree')
 
@@ -97,7 +97,7 @@
     g.add_option('-i', '--ignore-case',
                  action='callback', callback=carry,
                  help='Ignore case differences')
-    g.add_option('-a','--text',
+    g.add_option('-a', '--text',
                  action='callback', callback=carry,
                  help="Process binary files as if they were text")
     g.add_option('-I',
@@ -126,7 +126,7 @@
     g.add_option('--and', '--or', '--not',
                  action='callback', callback=carry,
                  help='Boolean operators to combine patterns')
-    g.add_option('-(','-)',
+    g.add_option('-(', '-)',
                  action='callback', callback=carry,
                  help='Boolean operator grouping')
 
@@ -146,10 +146,10 @@
                  action='callback', callback=carry,
                  metavar='CONTEXT', type='str',
                  help='Show CONTEXT lines after match')
-    g.add_option('-l','--name-only','--files-with-matches',
+    g.add_option('-l', '--name-only', '--files-with-matches',
                  action='callback', callback=carry,
                  help='Show only file names containing matching lines')
-    g.add_option('-L','--files-without-match',
+    g.add_option('-L', '--files-without-match',
                  action='callback', callback=carry,
                  help='Show only file names not containing matching lines')
 
@@ -158,9 +158,9 @@
     out = GrepColoring(self.manifest.manifestProject.config)
 
     cmd_argv = ['grep']
-    if out.is_on and git_require((1,6,3)):
+    if out.is_on and git_require((1, 6, 3)):
       cmd_argv.append('--color')
-    cmd_argv.extend(getattr(opt,'cmd_argv',[]))
+    cmd_argv.extend(getattr(opt, 'cmd_argv', []))
 
     if '-e' not in cmd_argv:
       if not args:
diff --git a/subcmds/help.py b/subcmds/help.py
index 57fb3cc..15aab7f 100644
--- a/subcmds/help.py
+++ b/subcmds/help.py
@@ -126,7 +126,7 @@
 
             p('%s', title)
             self.nl()
-            p('%s', ''.ljust(len(title),section_type[0]))
+            p('%s', ''.ljust(len(title), section_type[0]))
             self.nl()
             continue
 
diff --git a/subcmds/info.py b/subcmds/info.py
new file mode 100644
index 0000000..3a25e3b
--- /dev/null
+++ b/subcmds/info.py
@@ -0,0 +1,195 @@
+#
+# Copyright (C) 2012 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from command import PagedCommand
+from color import Coloring
+from error import NoSuchProjectError
+from git_refs import R_M
+
+class _Coloring(Coloring):
+  def __init__(self, config):
+    Coloring.__init__(self, config, "status")
+
+class Info(PagedCommand):
+  common = True
+  helpSummary = "Get info on the manifest branch, current branch or unmerged branches"
+  helpUsage = "%prog [-dl] [-o [-b]] [<project>...]"
+
+  def _Options(self, p, show_smart=True):
+    p.add_option('-d', '--diff',
+                 dest='all', action='store_true',
+                 help="show full info and commit diff including remote branches")
+    p.add_option('-o', '--overview',
+                 dest='overview', action='store_true',
+                 help='show overview of all local commits')
+    p.add_option('-b', '--current-branch',
+                 dest="current_branch", action="store_true",
+                 help="consider only checked out branches")
+    p.add_option('-l', '--local-only',
+                 dest="local", action="store_true",
+                 help="Disable all remote operations")
+
+
+  def Execute(self, opt, args):
+    self.out = _Coloring(self.manifest.globalConfig)
+    self.heading = self.out.printer('heading', attr = 'bold')
+    self.headtext = self.out.printer('headtext', fg = 'yellow')
+    self.redtext = self.out.printer('redtext', fg = 'red')
+    self.sha = self.out.printer("sha", fg = 'yellow')
+    self.text = self.out.printer('text')
+    self.dimtext = self.out.printer('dimtext', attr = 'dim')
+
+    self.opt = opt
+
+    mergeBranch = self.manifest.manifestProject.config.GetBranch("default").merge
+
+    self.heading("Manifest branch: ")
+    self.headtext(self.manifest.default.revisionExpr)
+    self.out.nl()
+    self.heading("Manifest merge branch: ")
+    self.headtext(mergeBranch)
+    self.out.nl()
+
+    self.printSeparator()
+
+    if not opt.overview:
+      self.printDiffInfo(args)
+    else:
+      self.printCommitOverview(args)
+
+  def printSeparator(self):
+    self.text("----------------------------")
+    self.out.nl()
+
+  def printDiffInfo(self, args):
+    try:
+      projs = self.GetProjects(args)
+    except NoSuchProjectError:
+      return
+
+    for p in projs:
+      self.heading("Project: ")
+      self.headtext(p.name)
+      self.out.nl()
+
+      self.heading("Mount path: ")
+      self.headtext(p.worktree)
+      self.out.nl()
+
+      self.heading("Current revision: ")
+      self.headtext(p.revisionExpr)
+      self.out.nl()
+
+      localBranches = p.GetBranches().keys()
+      self.heading("Local Branches: ")
+      self.redtext(str(len(localBranches)))
+      if len(localBranches) > 0:
+        self.text(" [")
+        self.text(", ".join(localBranches))
+        self.text("]")
+      self.out.nl()
+
+      if self.opt.all:
+        self.findRemoteLocalDiff(p)
+
+      self.printSeparator()
+
+  def findRemoteLocalDiff(self, project):
+    #Fetch all the latest commits
+    if not self.opt.local:
+      project.Sync_NetworkHalf(quiet=True, current_branch_only=True)
+
+    logTarget = R_M + self.manifest.default.revisionExpr
+
+    bareTmp = project.bare_git._bare
+    project.bare_git._bare = False
+    localCommits = project.bare_git.rev_list(
+        '--abbrev=8',
+        '--abbrev-commit',
+        '--pretty=oneline',
+        logTarget + "..",
+        '--')
+
+    originCommits = project.bare_git.rev_list(
+        '--abbrev=8',
+        '--abbrev-commit',
+        '--pretty=oneline',
+        ".." + logTarget,
+        '--')
+    project.bare_git._bare = bareTmp
+
+    self.heading("Local Commits: ")
+    self.redtext(str(len(localCommits)))
+    self.dimtext(" (on current branch)")
+    self.out.nl()
+
+    for c in localCommits:
+      split = c.split()
+      self.sha(split[0] + " ")
+      self.text("".join(split[1:]))
+      self.out.nl()
+
+    self.printSeparator()
+
+    self.heading("Remote Commits: ")
+    self.redtext(str(len(originCommits)))
+    self.out.nl()
+
+    for c in originCommits:
+      split = c.split()
+      self.sha(split[0] + " ")
+      self.text("".join(split[1:]))
+      self.out.nl()
+
+  def printCommitOverview(self, args):
+    all_branches = []
+    for project in self.GetProjects(args):
+      br = [project.GetUploadableBranch(x)
+            for x in project.GetBranches().keys()]
+      br = [x for x in br if x]
+      if self.opt.current_branch:
+        br = [x for x in br if x.name == project.CurrentBranch]
+      all_branches.extend(br)
+
+    if not all_branches:
+      return
+
+    self.out.nl()
+    self.heading('Projects Overview')
+    project = None
+
+    for branch in all_branches:
+      if project != branch.project:
+        project = branch.project
+        self.out.nl()
+        self.headtext(project.relpath)
+        self.out.nl()
+
+      commits = branch.commits
+      date = branch.date
+      self.text('%s %-33s (%2d commit%s, %s)' % (
+        branch.name == project.CurrentBranch and '*' or ' ',
+        branch.name,
+        len(commits),
+        len(commits) != 1 and 's' or '',
+        date))
+      self.out.nl()
+
+      for commit in commits:
+        split = commit.split()
+        self.text('{0:38}{1} '.format('','-'))
+        self.sha(split[0] + " ")
+        self.text("".join(split[1:]))
+        self.out.nl()
diff --git a/subcmds/init.py b/subcmds/init.py
index 7aaa7f1..eeadc70 100644
--- a/subcmds/init.py
+++ b/subcmds/init.py
@@ -279,14 +279,14 @@
     print()
     print("Testing colorized output (for 'repo diff', 'repo status'):")
 
-    for c in ['black','red','green','yellow','blue','magenta','cyan']:
+    for c in ['black', 'red', 'green', 'yellow', 'blue', 'magenta', 'cyan']:
       out.write(' ')
       out.printer(fg=c)(' %-6s ', c)
     out.write(' ')
     out.printer(fg='white', bg='black')(' %s ' % 'white')
     out.nl()
 
-    for c in ['bold','dim','ul','reverse']:
+    for c in ['bold', 'dim', 'ul', 'reverse']:
       out.write(' ')
       out.printer(fg='black', attr=c)(' %-6s ', c)
     out.nl()
diff --git a/subcmds/overview.py b/subcmds/overview.py
index 9e6100b..418459a 100644
--- a/subcmds/overview.py
+++ b/subcmds/overview.py
@@ -55,8 +55,11 @@
       def __init__(self, config):
         Coloring.__init__(self, config, 'status')
         self.project = self.printer('header', attr='bold')
+        self.text = self.printer('text')
 
     out = Report(all_branches[0].project.config)
+    out.text("Deprecated. See repo info -o.")
+    out.nl()
     out.project('Projects Overview')
     out.nl()
 
diff --git a/subcmds/sync.py b/subcmds/sync.py
index a64f2c4..5b3dca7 100644
--- a/subcmds/sync.py
+++ b/subcmds/sync.py
@@ -150,22 +150,22 @@
     p.add_option('-f', '--force-broken',
                  dest='force_broken', action='store_true',
                  help="continue sync even if a project fails to sync")
-    p.add_option('-l','--local-only',
+    p.add_option('-l', '--local-only',
                  dest='local_only', action='store_true',
                  help="only update working tree, don't fetch")
-    p.add_option('-n','--network-only',
+    p.add_option('-n', '--network-only',
                  dest='network_only', action='store_true',
                  help="fetch only, don't update working tree")
-    p.add_option('-d','--detach',
+    p.add_option('-d', '--detach',
                  dest='detach_head', action='store_true',
                  help='detach projects back to manifest revision')
-    p.add_option('-c','--current-branch',
+    p.add_option('-c', '--current-branch',
                  dest='current_branch_only', action='store_true',
                  help='fetch only current branch from server')
-    p.add_option('-q','--quiet',
+    p.add_option('-q', '--quiet',
                  dest='quiet', action='store_true',
                  help='be more quiet')
-    p.add_option('-j','--jobs',
+    p.add_option('-j', '--jobs',
                  dest='jobs', action='store', type='int',
                  help="projects to fetch simultaneously (default %d)" % self.jobs)
     p.add_option('-m', '--manifest-name',
@@ -197,62 +197,62 @@
                  help=SUPPRESS_HELP)
 
   def _FetchHelper(self, opt, project, lock, fetched, pm, sem, err_event):
-      """Main function of the fetch threads when jobs are > 1.
+    """Main function of the fetch threads when jobs are > 1.
 
-      Args:
-        opt: Program options returned from optparse.  See _Options().
-        project: Project object for the project to fetch.
-        lock: Lock for accessing objects that are shared amongst multiple
-            _FetchHelper() threads.
-        fetched: set object that we will add project.gitdir to when we're done
-            (with our lock held).
-        pm: Instance of a Project object.  We will call pm.update() (with our
-            lock held).
-        sem: We'll release() this semaphore when we exit so that another thread
-            can be started up.
-        err_event: We'll set this event in the case of an error (after printing
-            out info about the error).
-      """
-      # We'll set to true once we've locked the lock.
-      did_lock = False
+    Args:
+      opt: Program options returned from optparse.  See _Options().
+      project: Project object for the project to fetch.
+      lock: Lock for accessing objects that are shared amongst multiple
+          _FetchHelper() threads.
+      fetched: set object that we will add project.gitdir to when we're done
+          (with our lock held).
+      pm: Instance of a Project object.  We will call pm.update() (with our
+          lock held).
+      sem: We'll release() this semaphore when we exit so that another thread
+          can be started up.
+      err_event: We'll set this event in the case of an error (after printing
+          out info about the error).
+    """
+    # We'll set to true once we've locked the lock.
+    did_lock = False
 
-      # Encapsulate everything in a try/except/finally so that:
-      # - We always set err_event in the case of an exception.
-      # - We always make sure we call sem.release().
-      # - We always make sure we unlock the lock if we locked it.
+    # Encapsulate everything in a try/except/finally so that:
+    # - We always set err_event in the case of an exception.
+    # - We always make sure we call sem.release().
+    # - We always make sure we unlock the lock if we locked it.
+    try:
       try:
-        try:
-          start = time.time()
-          success = project.Sync_NetworkHalf(
-            quiet=opt.quiet,
-            current_branch_only=opt.current_branch_only,
-            clone_bundle=not opt.no_clone_bundle)
-          self._fetch_times.Set(project, time.time() - start)
+        start = time.time()
+        success = project.Sync_NetworkHalf(
+          quiet=opt.quiet,
+          current_branch_only=opt.current_branch_only,
+          clone_bundle=not opt.no_clone_bundle)
+        self._fetch_times.Set(project, time.time() - start)
 
-          # Lock around all the rest of the code, since printing, updating a set
-          # and Progress.update() are not thread safe.
-          lock.acquire()
-          did_lock = True
+        # Lock around all the rest of the code, since printing, updating a set
+        # and Progress.update() are not thread safe.
+        lock.acquire()
+        did_lock = True
 
-          if not success:
-            print('error: Cannot fetch %s' % project.name, file=sys.stderr)
-            if opt.force_broken:
-              print('warn: --force-broken, continuing to sync',
-                    file=sys.stderr)
-            else:
-              raise _FetchError()
+        if not success:
+          print('error: Cannot fetch %s' % project.name, file=sys.stderr)
+          if opt.force_broken:
+            print('warn: --force-broken, continuing to sync',
+                  file=sys.stderr)
+          else:
+            raise _FetchError()
 
-          fetched.add(project.gitdir)
-          pm.update()
-        except _FetchError:
-          err_event.set()
-        except:
-          err_event.set()
-          raise
-      finally:
-        if did_lock:
-          lock.release()
-        sem.release()
+        fetched.add(project.gitdir)
+        pm.update()
+      except _FetchError:
+        err_event.set()
+      except:
+        err_event.set()
+        raise
+    finally:
+      if did_lock:
+        lock.release()
+      sem.release()
 
   def _Fetch(self, projects, opt):
     fetched = set()
@@ -379,36 +379,36 @@
         if path not in new_project_paths:
           # If the path has already been deleted, we don't need to do it
           if os.path.exists(self.manifest.topdir + '/' + path):
-              project = Project(
-                             manifest = self.manifest,
-                             name = path,
-                             remote = RemoteSpec('origin'),
-                             gitdir = os.path.join(self.manifest.topdir,
-                                                   path, '.git'),
-                             worktree = os.path.join(self.manifest.topdir, path),
-                             relpath = path,
-                             revisionExpr = 'HEAD',
-                             revisionId = None,
-                             groups = None)
+            project = Project(
+                           manifest = self.manifest,
+                           name = path,
+                           remote = RemoteSpec('origin'),
+                           gitdir = os.path.join(self.manifest.topdir,
+                                                 path, '.git'),
+                           worktree = os.path.join(self.manifest.topdir, path),
+                           relpath = path,
+                           revisionExpr = 'HEAD',
+                           revisionId = None,
+                           groups = None)
 
-              if project.IsDirty():
-                print('error: Cannot remove project "%s": uncommitted changes'
-                      'are present' % project.relpath, file=sys.stderr)
-                print('       commit changes, then run sync again',
-                      file=sys.stderr)
-                return -1
-              else:
-                print('Deleting obsolete path %s' % project.worktree,
-                      file=sys.stderr)
-                shutil.rmtree(project.worktree)
-                # Try deleting parent subdirs if they are empty
-                project_dir = os.path.dirname(project.worktree)
-                while project_dir != self.manifest.topdir:
-                  try:
-                    os.rmdir(project_dir)
-                  except OSError:
-                    break
-                  project_dir = os.path.dirname(project_dir)
+            if project.IsDirty():
+              print('error: Cannot remove project "%s": uncommitted changes'
+                    'are present' % project.relpath, file=sys.stderr)
+              print('       commit changes, then run sync again',
+                    file=sys.stderr)
+              return -1
+            else:
+              print('Deleting obsolete path %s' % project.worktree,
+                    file=sys.stderr)
+              shutil.rmtree(project.worktree)
+              # Try deleting parent subdirs if they are empty
+              project_dir = os.path.dirname(project.worktree)
+              while project_dir != self.manifest.topdir:
+                try:
+                  os.rmdir(project_dir)
+                except OSError:
+                  break
+                project_dir = os.path.dirname(project_dir)
 
     new_project_paths.sort()
     fd = open(file_path, 'w')
diff --git a/subcmds/upload.py b/subcmds/upload.py
index a6ada33..e314032 100644
--- a/subcmds/upload.py
+++ b/subcmds/upload.py
@@ -50,7 +50,7 @@
 class Upload(InteractiveCommand):
   common = True
   helpSummary = "Upload changes for code review"
-  helpUsage="""
+  helpUsage = """
 %prog [--re --cc] [<project>]...
 """
   helpDescription = """
@@ -312,23 +312,23 @@
 
         # Check if there are local changes that may have been forgotten
         if branch.project.HasChanges():
-            key = 'review.%s.autoupload' % branch.project.remote.review
-            answer = branch.project.config.GetBoolean(key)
+          key = 'review.%s.autoupload' % branch.project.remote.review
+          answer = branch.project.config.GetBoolean(key)
 
-            # if they want to auto upload, let's not ask because it could be automated
-            if answer is None:
-                sys.stdout.write('Uncommitted changes in ' + branch.project.name + ' (did you forget to amend?). Continue uploading? (y/N) ')
-                a = sys.stdin.readline().strip().lower()
-                if a not in ('y', 'yes', 't', 'true', 'on'):
-                    print("skipping upload", file=sys.stderr)
-                    branch.uploaded = False
-                    branch.error = 'User aborted'
-                    continue
+          # if they want to auto upload, let's not ask because it could be automated
+          if answer is None:
+            sys.stdout.write('Uncommitted changes in ' + branch.project.name + ' (did you forget to amend?). Continue uploading? (y/N) ')
+            a = sys.stdin.readline().strip().lower()
+            if a not in ('y', 'yes', 't', 'true', 'on'):
+              print("skipping upload", file=sys.stderr)
+              branch.uploaded = False
+              branch.error = 'User aborted'
+              continue
 
         # Check if topic branches should be sent to the server during upload
         if opt.auto_topic is not True:
-           key = 'review.%s.uploadtopic' % branch.project.remote.review
-           opt.auto_topic = branch.project.config.GetBoolean(key)
+          key = 'review.%s.uploadtopic' % branch.project.remote.review
+          opt.auto_topic = branch.project.config.GetBoolean(key)
 
         branch.UploadForReview(people, auto_topic=opt.auto_topic, draft=opt.draft)
         branch.uploaded = True
@@ -355,11 +355,11 @@
       print()
 
     for branch in todo:
-        if branch.uploaded:
-          print('[OK    ] %-15s %s' % (
-                 branch.project.relpath + '/',
-                 branch.name),
-                 file=sys.stderr)
+      if branch.uploaded:
+        print('[OK    ] %-15s %s' % (
+               branch.project.relpath + '/',
+               branch.name),
+               file=sys.stderr)
 
     if have_errors:
       sys.exit(1)
@@ -397,7 +397,7 @@
       reviewers = _SplitEmails(opt.reviewers)
     if opt.cc:
       cc = _SplitEmails(opt.cc)
-    people = (reviewers,cc)
+    people = (reviewers, cc)
 
     if not pending:
       print("no branches ready for upload", file=sys.stderr)
diff --git a/tests/test_git_config.py b/tests/test_git_config.py
index 5b1770e..3d4b997 100644
--- a/tests/test_git_config.py
+++ b/tests/test_git_config.py
@@ -4,49 +4,49 @@
 import git_config
 
 def fixture(*paths):
-    """Return a path relative to test/fixtures.
-    """
-    return os.path.join(os.path.dirname(__file__), 'fixtures', *paths)
+  """Return a path relative to test/fixtures.
+  """
+  return os.path.join(os.path.dirname(__file__), 'fixtures', *paths)
 
 class GitConfigUnitTest(unittest.TestCase):
-    """Tests the GitConfig class.
+  """Tests the GitConfig class.
+  """
+  def setUp(self):
+    """Create a GitConfig object using the test.gitconfig fixture.
     """
-    def setUp(self):
-        """Create a GitConfig object using the test.gitconfig fixture.
-        """
-        config_fixture = fixture('test.gitconfig')
-        self.config = git_config.GitConfig(config_fixture)
+    config_fixture = fixture('test.gitconfig')
+    self.config = git_config.GitConfig(config_fixture)
 
-    def test_GetString_with_empty_config_values(self):
-        """
-        Test config entries with no value.
+  def test_GetString_with_empty_config_values(self):
+    """
+    Test config entries with no value.
 
-        [section]
-            empty
+    [section]
+        empty
 
-        """
-        val = self.config.GetString('section.empty')
-        self.assertEqual(val, None)
+    """
+    val = self.config.GetString('section.empty')
+    self.assertEqual(val, None)
 
-    def test_GetString_with_true_value(self):
-        """
-        Test config entries with a string value.
+  def test_GetString_with_true_value(self):
+    """
+    Test config entries with a string value.
 
-        [section]
-            nonempty = true
+    [section]
+        nonempty = true
 
-        """
-        val = self.config.GetString('section.nonempty')
-        self.assertEqual(val, 'true')
+    """
+    val = self.config.GetString('section.nonempty')
+    self.assertEqual(val, 'true')
 
-    def test_GetString_from_missing_file(self):
-        """
-        Test missing config file
-        """
-        config_fixture = fixture('not.present.gitconfig')
-        config = git_config.GitConfig(config_fixture)
-        val = config.GetString('empty')
-        self.assertEqual(val, None)
+  def test_GetString_from_missing_file(self):
+    """
+    Test missing config file
+    """
+    config_fixture = fixture('not.present.gitconfig')
+    config = git_config.GitConfig(config_fixture)
+    val = config.GetString('empty')
+    self.assertEqual(val, None)
 
 if __name__ == '__main__':
-    unittest.main()
+  unittest.main()