Fixing project renaming bug.

This bug happens when a project gets added to the manifest, and
then is renamed. Users who happened to have run "repo sync" after
the project was added but before the rename happened will try to
read the data from the old project, as the manifest was only updated
after all projects were updated successfully.

.gitignore

@@ -1 +1,2 @@
 *.pyc
+.repopickle_*

SUBMITTING_PATCHES

@@ -0,0 +1,80 @@
+Short Version:
+
+ - Make small logical changes.
+ - Provide a meaningful commit message.
+ - Make sure all code is under the Apache License, 2.0.
+ - Publish your changes for review:
+
+   git push ssh://review.source.android.com:29418/tools/repo.git HEAD:refs/for/master
+
+
+Long Version:
+
+I wanted a file describing how to submit patches for repo,
+so I started with the one found in the core Git distribution
+(Documentation/SubmittingPatches), which itself was based on the
+patch submission guidelines for the Linux kernel.
+
+However there are some differences, so please review and familiarize
+yourself with the following relevant bits:
+
+
+(1) Make separate commits for logically separate changes.
+
+Unless your patch is really trivial, you should not be sending
+out a patch that was generated between your working tree and your
+commit head.  Instead, always make a commit with complete commit
+message and generate a series of patches from your repository.
+It is a good discipline.
+
+Describe the technical detail of the change(s).
+
+If your description starts to get too long, that's a sign that you
+probably need to split up your commit to finer grained pieces.
+
+
+(2) Check the license
+
+repo is licensed under the Apache License, 2.0.
+
+Because of this licensing model *every* file within the project
+*must* list the license that covers it in the header of the file.
+Any new contributions to an existing file *must* be submitted under
+the current license of that file.  Any new files *must* clearly
+indicate which license they are provided under in the file header.
+
+Please verify that you are legally allowed and willing to submit your
+changes under the license covering each file *prior* to submitting
+your patch.  It is virtually impossible to remove a patch once it
+has been applied and pushed out.
+
+
+(3) Sending your patches.
+
+Do not email your patches to anyone.
+
+Instead, login to the Gerrit Code Review tool at:
+
+  https://review.source.android.com/
+
+Ensure you have completed one of the necessary contributor
+agreements, providing documentation to the project maintainers that
+they have right to redistribute your work under the Apache License:
+
+  https://review.source.android.com/#settings,agreements
+
+Ensure you have registered one or more SSH public keys, so you can
+push your commits directly over SSH:
+
+  https://review.source.android.com/#settings,ssh-keys
+
+Push your patches over SSH to the review server, possibly through
+a remembered remote to make this easier in the future:
+
+   git config remote.review.url ssh://review.source.android.com:29418/tools/repo.git
+   git config remote.review.push HEAD:refs/for/master
+
+   git push review
+
+You will be automatically emailed a copy of your commits, and any
+comments made by the project maintainers.

editor.py

@@ -14,6 +14,7 @@
 # limitations under the License.
 
 import os
+import re
 import sys
 import subprocess
 import tempfile
@@ -38,9 +39,10 @@ class Editor(object):
     if e:
       return e
 
-    e = cls.globalConfig.GetString('core.editor')
-    if e:
-      return e
+    if cls.globalConfig:
+      e = cls.globalConfig.GetString('core.editor')
+      if e:
+        return e
 
     e = os.getenv('VISUAL')
     if e:
@@ -69,15 +71,33 @@ least one of these before using this command."""
       Returns:
         new value of edited text; None if editing did not succeed
     """
-    editor = cls._GetEditor().split()
+    editor = cls._GetEditor()
+    if editor == ':':
+      return data
+
     fd, path = tempfile.mkstemp()
     try:
       os.write(fd, data)
       os.close(fd)
       fd = None
 
-      if subprocess.Popen(editor + [path]).wait() != 0:
-        raise EditorError()
+      if re.compile("^.*[$ \t'].*$").match(editor):
+        args = [editor + ' "$@"']
+        shell = True
+      else:
+        args = [editor]
+        shell = False
+      args.append(path)
+
+      try:
+        rc = subprocess.Popen(args, shell=shell).wait()
+      except OSError, e:
+        raise EditorError('editor failed, %s: %s %s'
+          % (str(e), editor, path))
+      if rc != 0:
+        raise EditorError('editor failed with exit status %d: %s %s'
+          % (rc, editor, path))
+
       fd2 = open(path)
       try:
         return fd2.read()

error.py

@@ -24,6 +24,11 @@ class ManifestInvalidRevisionError(Exception):
 class EditorError(Exception):
   """Unspecified error from the user's text editor.
   """
+  def __init__(self, reason):
+    self.reason = reason
+
+  def __str__(self):
+    return self.reason
 
 class GitError(Exception):
   """Unspecified internal error from git.

git_command.py

@@ -68,6 +68,30 @@ class _GitCall(object):
     return fun
 git = _GitCall()
 
+_git_version = None
+
+def git_require(min_version, fail=False):
+  global _git_version
+
+  if _git_version is None:
+    ver_str = git.version()
+    if ver_str.startswith('git version '):
+      _git_version = tuple(
+        map(lambda x: int(x),
+          ver_str[len('git version '):].strip().split('.')[0:3]
+        ))
+    else:
+      print >>sys.stderr, 'fatal: "%s" unsupported' % ver_str
+      sys.exit(1)
+
+  if min_version <= _git_version:
+    return True
+  if fail:
+    need = '.'.join(map(lambda x: str(x), min_version))
+    print >>sys.stderr, 'fatal: git %s or later required' % need
+    sys.exit(1)
+  return False
+
 class GitCommand(object):
   def __init__(self,
                project,

git_config.py

@@ -236,6 +236,9 @@ class GitConfig(object):
         return cPickle.load(fd)
       finally:
         fd.close()
+    except EOFError:
+      os.remove(self._pickle)
+      return None
     except IOError:
       os.remove(self._pickle)
       return None
@@ -256,21 +259,28 @@ class GitConfig(object):
       os.remove(self._pickle)
 
   def _ReadGit(self):
-    d = self._do('--null', '--list')
-    c = {}
-    while d:
-      lf = d.index('\n')
-      nul = d.index('\0', lf + 1)
+    """
+    Read configuration data from git.
 
-      key = _key(d[0:lf])
-      val = d[lf + 1:nul]
+    This internal method populates the GitConfig cache.
+
+    """
+    c = {}
+    d = self._do('--null', '--list')
+    if d is None:
+      return c
+    for line in d.rstrip('\0').split('\0'):
+      if '\n' in line:
+          key, val = line.split('\n', 1)
+      else:
+          key = line
+          val = None
 
       if key in c:
         c[key].append(val)
       else:
         c[key] = [val]
 
-      d = d[nul + 1:]
     return c
 
   def _do(self, *args):
@@ -382,8 +392,11 @@ def _open_ssh(host, port):
 
 def close_ssh():
   for key,p in _ssh_cache.iteritems():
-    os.kill(p.pid, SIGTERM)
-    p.wait()
+    try:
+      os.kill(p.pid, SIGTERM)
+      p.wait()
+    except OSError:
+      pass
   _ssh_cache.clear()
 
   d = _ssh_sock(create=False)
@@ -414,6 +427,7 @@ def _preconnect(url):
     host = m.group(1)
     return _open_ssh(host, 22)
 
+  return False
 
 class Remote(object):
   """Configuration options related to a remote.

manifest_xml.py

@@ -183,7 +183,7 @@ class XmlManifest(object):
     if not self._loaded:
       m = self.manifestProject
       b = m.GetBranch(m.CurrentBranch).merge
-      if b.startswith(R_HEADS):
+      if b is not None and b.startswith(R_HEADS):
         b = b[len(R_HEADS):]
       self.branch = b
 

subcmds/branches.py

@@ -61,6 +61,33 @@ class Branches(Command):
 %prog [<project>...]
 
 Summarizes the currently available topic branches.
+
+Branch Display
+--------------
+
+The branch display output by this command is organized into four
+columns of information; for example:
+
+ *P nocolor                   | in repo
+    repo2                     |
+
+The first column contains a * if the branch is the currently
+checked out branch in any of the specified projects, or a blank
+if no project has the branch checked out.
+
+The second column contains either blank, p or P, depending upon
+the upload status of the branch.
+
+ (blank): branch not yet published by repo upload
+       P: all commits were published by repo upload
+       p: only some commits were published by repo upload
+
+The third column contains the branch name.
+
+The fourth column (after the | separator) lists the projects that
+the branch appears in, or does not appear in.  If no project list
+is shown, then the branch appears in all projects.
+
 """
 
   def Execute(self, opt, args):

subcmds/grep.py

@@ -17,7 +17,7 @@ import sys
 from optparse import SUPPRESS_HELP
 from color import Coloring
 from command import PagedCommand
-from git_command import GitCommand
+from git_command import git_require, GitCommand
 
 class GrepColoring(Coloring):
   def __init__(self, config):
@@ -158,7 +158,7 @@ contain a line that matches both expressions:
     out = GrepColoring(self.manifest.manifestProject.config)
 
     cmd_argv = ['grep']
-    if out.is_on:
+    if out.is_on and git_require((1,6,3)):
       cmd_argv.append('--color')
     cmd_argv.extend(getattr(opt,'cmd_argv',[]))
 

subcmds/init.py

@@ -20,7 +20,7 @@ from color import Coloring
 from command import InteractiveCommand, MirrorSafeCommand
 from error import ManifestParseError
 from project import SyncBuffer
-from git_command import git, MIN_GIT_VERSION
+from git_command import git_require, MIN_GIT_VERSION
 
 class Init(InteractiveCommand, MirrorSafeCommand):
   common = True
@@ -85,19 +85,6 @@ to update the working directory files.
                  dest='no_repo_verify', action='store_true',
                  help='do not verify repo source code')
 
-  def _CheckGitVersion(self):
-    ver_str = git.version()
-    if not ver_str.startswith('git version '):
-      print >>sys.stderr, 'error: "%s" unsupported' % ver_str
-      sys.exit(1)
-
-    ver_str = ver_str[len('git version '):].strip()
-    ver_act = tuple(map(lambda x: int(x), ver_str.split('.')[0:3]))
-    if ver_act < MIN_GIT_VERSION:
-      need = '.'.join(map(lambda x: str(x), MIN_GIT_VERSION))
-      print >>sys.stderr, 'fatal: git %s or later required' % need
-      sys.exit(1)
-
   def _SyncManifest(self, opt):
     m = self.manifest.manifestProject
     is_new = not m.Exists
@@ -161,20 +148,33 @@ to update the working directory files.
       print >>sys.stderr, 'fatal: %s' % str(e)
       sys.exit(1)
 
-  def _PromptKey(self, prompt, key, value):
+  def _Prompt(self, prompt, value):
     mp = self.manifest.manifestProject
 
     sys.stdout.write('%-10s [%s]: ' % (prompt, value))
     a = sys.stdin.readline().strip()
-    if a != '' and a != value:
-      mp.config.SetString(key, a)
+    if a == '':
+      return value
+    return a
 
   def _ConfigureUser(self):
     mp = self.manifest.manifestProject
 
-    print ''
-    self._PromptKey('Your Name', 'user.name', mp.UserName)
-    self._PromptKey('Your Email', 'user.email', mp.UserEmail)
+    while True:
+      print ''
+      name  = self._Prompt('Your Name', mp.UserName)
+      email = self._Prompt('Your Email', mp.UserEmail)
+
+      print ''
+      print 'Your identity is: %s <%s>' % (name, email)
+      sys.stdout.write('is this correct [yes/no]? ')
+      if 'yes' == sys.stdin.readline().strip():
+        break
+
+    if name != mp.UserName:
+      mp.config.SetString('user.name', name)
+    if email != mp.UserEmail:
+      mp.config.SetString('user.email', email)
 
   def _HasColorSet(self, gc):
     for n in ['ui', 'diff', 'status']:
@@ -214,7 +214,7 @@ to update the working directory files.
       gc.SetString('color.ui', 'auto')
 
   def Execute(self, opt, args):
-    self._CheckGitVersion()
+    git_require(MIN_GIT_VERSION, fail=True)
     self._SyncManifest(opt)
     self._LinkManifest(opt.manifest_name)
 

subcmds/sync.py

@@ -111,7 +111,6 @@ later is required to fix a server side protocol bug.
     pm = Progress('Fetching projects', len(projects))
     for project in projects:
       pm.update()
-
       if project.Sync_NetworkHalf():
         fetched.add(project.gitdir)
       else:
@@ -166,6 +165,7 @@ uncommitted changes are present' % project.relpath
                 break
               dir = os.path.dirname(dir)
 
+    new_project_paths.sort()
     fd = open(file_path, 'w')
     try:
       fd.write('\n'.join(new_project_paths))
@@ -191,6 +191,15 @@ uncommitted changes are present' % project.relpath
     if opt.repo_upgraded:
       _PostRepoUpgrade(self.manifest)
 
+    if not opt.local_only:
+      mp.Sync_NetworkHalf()
+
+    if mp.HasChanges:
+      syncbuf = SyncBuffer(mp.config)
+      mp.Sync_LocalHalf(syncbuf)
+      if not syncbuf.Finish():
+        sys.exit(1)
+      self.manifest._Unload()
     all = self.GetProjects(args, missing_ok=True)
 
     if not opt.local_only:
@@ -198,7 +207,6 @@ uncommitted changes are present' % project.relpath
       now = time.time()
       if (24 * 60 * 60) <= (now - rp.LastFetch):
         to_fetch.append(rp)
-      to_fetch.append(mp)
       to_fetch.extend(all)
 
       fetched = self._Fetch(to_fetch)
@@ -207,12 +215,6 @@ uncommitted changes are present' % project.relpath
         # bail out now; the rest touches the working tree
         return
 
-      if mp.HasChanges:
-        syncbuf = SyncBuffer(mp.config)
-        mp.Sync_LocalHalf(syncbuf)
-        if not syncbuf.Finish():
-          sys.exit(1)
-
         self.manifest._Unload()
         all = self.GetProjects(args, missing_ok=True)
         missing = []
@@ -240,7 +242,6 @@ uncommitted changes are present' % project.relpath
     if not syncbuf.Finish():
       sys.exit(1)
 
-
 def _PostRepoUpgrade(manifest):
   for project in manifest.projects.values():
     if project.Exists:

tests/fixtures/test.gitconfig

@@ -0,0 +1,3 @@
+[section]
+	empty
+	nonempty = true

tests/test_git_config.py

@@ -0,0 +1,52 @@
+import os
+import unittest
+
+import git_config
+
+def fixture(*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.
+    """
+    def setUp(self):
+        """Create a GitConfig object using the test.gitconfig 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.
+
+        [section]
+            empty
+
+        """
+        val = self.config.GetString('section.empty')
+        self.assertEqual(val, None)
+
+    def test_GetString_with_true_value(self):
+        """
+        Test config entries with a string value.
+
+        [section]
+            nonempty = 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)
+
+if __name__ == '__main__':
+    unittest.main()