Refactor git_common config and die

R=iannucci@chromium.org
BUG=376099

Review-Url: https://codereview.chromium.org/2075603002

git_common.py

@@ -281,6 +281,10 @@ def once(function):
 
 ## Git functions
 
+def die(message, *args):
+  print >> sys.stderr, textwrap.dedent(message % args)
+  sys.exit(1)
+
 
 def blame(filename, revision=None, porcelain=False, *_args):
   command = ['blame']
@@ -293,23 +297,14 @@ def blame(filename, revision=None, porcelain=False, *_args):
 
 
 def branch_config(branch, option, default=None):
-  return config('branch.%s.%s' % (branch, option), default=default)
+  return get_config('branch.%s.%s' % (branch, option), default=default)
-
-
-def config_regexp(pattern):
-  if IS_WIN: # pragma: no cover
-    # this madness is because we call git.bat which calls git.exe which calls
-    # bash.exe (or something to that effect). Each layer divides the number of
-    # ^'s by 2.
-    pattern = pattern.replace('^', '^' * 8)
-  return run('config', '--get-regexp', pattern).splitlines()
 
 
 def branch_config_map(option):
   """Return {branch: <|option| value>} for all branches."""
   try:
     reg = re.compile(r'^branch\.(.*)\.%s$' % option)
-    lines = config_regexp(reg.pattern)
+    lines = get_config_regexp(reg.pattern)
     return {reg.match(k).group(1): v for k, v in (l.split() for l in lines)}
   except subprocess2.CalledProcessError:
     return {}
@@ -319,23 +314,22 @@ def branches(*args):
   NO_BRANCH = ('* (no branch', '* (detached', '* (HEAD detached')
 
   key = 'depot-tools.branch-limit'
-  limit = 20
+  limit = get_config_int(key, 20)
-  try:
-    limit = int(config(key, limit))
-  except ValueError:
-    pass
 
   raw_branches = run('branch', *args).splitlines()
 
   num = len(raw_branches)
-  if num > limit:
-    print >> sys.stderr, textwrap.dedent("""\
-    Your git repo has too many branches (%d/%d) for this tool to work well.
 
-    You may adjust this limit by running:
+  if num > limit:
+    die("""\
+      Your git repo has too many branches (%d/%d) for this tool to work well.
+
+      You may adjust this limit by running:
       git config %s <new_limit>
-    """ % (num, limit, key))
+
-    sys.exit(1)
+      You may also try cleaning up your old branches by running:
+      git cl archive
+      """, num, limit, key)
 
   for line in raw_branches:
     if line.startswith(NO_BRANCH):
@@ -343,20 +337,37 @@ def branches(*args):
     yield line.split()[-1]
 
 
-def config(option, default=None):
+def get_config(option, default=None):
   try:
     return run('config', '--get', option) or default
   except subprocess2.CalledProcessError:
     return default
 
 
-def config_list(option):
+def get_config_int(option, default=0):
+  assert isinstance(default, int)
+  try:
+    return int(get_config(option, default))
+  except ValueError:
+    return default
+
+
+def get_config_list(option):
   try:
     return run('config', '--get-all', option).split()
   except subprocess2.CalledProcessError:
     return []
 
 
+def get_config_regexp(pattern):
+  if IS_WIN: # pragma: no cover
+    # this madness is because we call git.bat which calls git.exe which calls
+    # bash.exe (or something to that effect). Each layer divides the number of
+    # ^'s by 2.
+    pattern = pattern.replace('^', '^' * 8)
+  return run('config', '--get-regexp', pattern).splitlines()
+
+
 def current_branch():
   try:
     return run('rev-parse', '--abbrev-ref', 'HEAD')
@@ -567,7 +578,7 @@ def repo_root():
 
 
 def root():
-  return config('depot-tools.upstream', 'origin/master')
+  return get_config('depot-tools.upstream', 'origin/master')
 
 
 @contextlib.contextmanager

git_map.py

@@ -18,7 +18,7 @@ import sys
 
 import subprocess2
 
-from git_common import current_branch, branches, tags, config_list, GIT_EXE
+from git_common import current_branch, branches, tags, get_config_list, GIT_EXE
 from git_common import get_or_create_merge_base, root
 
 from third_party import colorama
@@ -38,7 +38,7 @@ RESET = colorama.Fore.RESET + colorama.Back.RESET + colorama.Style.RESET_ALL
 BRIGHT_RED = '\x1b[1;31m'
 
 def main(argv):
-  map_extra = config_list('depot_tools.map_extra')
+  map_extra = get_config_list('depot_tools.map_extra')
   fmt = '%C(red bold)%h%x09%Creset%C(green)%d%Creset %C(yellow)%ad%Creset ~ %s'
   log_proc = subprocess2.Popen(
     [GIT_EXE, 'log', '--graph', '--branches', '--tags', root(),

git_rebase_update.py

@@ -31,8 +31,8 @@ def find_return_branch_workdir():
   These values may persist across multiple invocations of rebase-update, if
   rebase-update runs into a conflict mid-way.
   """
-  return_branch = git.config(STARTING_BRANCH_KEY)
+  return_branch = git.get_config(STARTING_BRANCH_KEY)
-  workdir = git.config(STARTING_WORKDIR_KEY)
+  workdir = git.get_config(STARTING_WORKDIR_KEY)
   if not return_branch:
     workdir = os.getcwd()
     git.set_config(STARTING_WORKDIR_KEY, workdir)
@@ -49,7 +49,7 @@ def fetch_remotes(branch_tree):
   remotes = set()
   tag_set = git.tags()
   fetchspec_map = {}
-  all_fetchspec_configs = git.config_regexp(r'^remote\..*\.fetch')
+  all_fetchspec_configs = git.get_config_regexp(r'^remote\..*\.fetch')
   for fetchspec_config in all_fetchspec_configs:
     key, _, fetchspec = fetchspec_config.partition(' ')
     dest_spec = fetchspec.partition(':')[2]

git_upstream_diff.py

@@ -11,7 +11,7 @@ import subprocess2
 import git_common as git
 
 def main(args):
-  default_args = git.config_list('depot-tools.upstream-diff.default-args')
+  default_args = git.get_config_list('depot-tools.upstream-diff.default-args')
   args = default_args + args
 
   parser = argparse.ArgumentParser()

tests/git_common_test.py

@@ -415,26 +415,29 @@ class GitMutableFunctionsTest(git_test_utils.GitRepoReadWriteTestBase,
 
   def testConfig(self):
     self.repo.git('config', '--add', 'happy.derpies', 'food')
-    self.assertEquals(self.repo.run(self.gc.config_list, 'happy.derpies'),
+    self.assertEquals(self.repo.run(self.gc.get_config_list, 'happy.derpies'),
                       ['food'])
-    self.assertEquals(self.repo.run(self.gc.config_list, 'sad.derpies'), [])
+    self.assertEquals(self.repo.run(self.gc.get_config_list, 'sad.derpies'), [])
 
     self.repo.git('config', '--add', 'happy.derpies', 'cat')
-    self.assertEquals(self.repo.run(self.gc.config_list, 'happy.derpies'),
+    self.assertEquals(self.repo.run(self.gc.get_config_list, 'happy.derpies'),
                       ['food', 'cat'])
 
-    self.assertEquals('cat', self.repo.run(self.gc.config, 'dude.bob', 'cat'))
+    self.assertEquals('cat', self.repo.run(self.gc.get_config, 'dude.bob',
+                                           'cat'))
 
     self.repo.run(self.gc.set_config, 'dude.bob', 'dog')
 
-    self.assertEquals('dog', self.repo.run(self.gc.config, 'dude.bob', 'cat'))
+    self.assertEquals('dog', self.repo.run(self.gc.get_config, 'dude.bob',
+                                           'cat'))
 
     self.repo.run(self.gc.del_config, 'dude.bob')
 
     # This should work without raising an exception
     self.repo.run(self.gc.del_config, 'dude.bob')
 
-    self.assertEquals('cat', self.repo.run(self.gc.config, 'dude.bob', 'cat'))
+    self.assertEquals('cat', self.repo.run(self.gc.get_config, 'dude.bob',
+                                           'cat'))
 
     self.assertEquals('origin/master', self.repo.run(self.gc.root))
 
@@ -559,10 +562,11 @@ class GitMutableStructuredTest(git_test_utils.GitRepoReadWriteTestBase,
     )
 
     self.assertEqual(
-      self.repo['B'], self.repo.run(self.gc.config, 'branch.branch_K.base')
+      self.repo['B'], self.repo.run(self.gc.get_config, 'branch.branch_K.base')
     )
     self.assertEqual(
-      'branch_G', self.repo.run(self.gc.config, 'branch.branch_K.base-upstream')
+      'branch_G', self.repo.run(self.gc.get_config,
+                                'branch.branch_K.base-upstream')
     )
 
     # deadbeef is a bad hash, so this will result in repo['B']
@@ -588,8 +592,8 @@ class GitMutableStructuredTest(git_test_utils.GitRepoReadWriteTestBase,
     self.repo.run(self.gc.remove_merge_base, 'branch_K')
     self.repo.run(self.gc.remove_merge_base, 'branch_L')
 
-    self.assertEqual(None,
+    self.assertEqual(None, self.repo.run(self.gc.get_config,
-                     self.repo.run(self.gc.config, 'branch.branch_K.base'))
+                                         'branch.branch_K.base'))
 
     self.assertEqual({}, self.repo.run(self.gc.branch_config_map, 'base'))