sync: kill git fetch process before SSH control master process

If the SSH control master process is killed while an active git
fetch is using its network socket, the underlying SSH client may
not realize the connection was broken.  This can lead to both the
client and the server waiting indefinitely for network messages
which will never be sent.

Work around the problem by keeping track of any processes that use
the tunnels we establish.  If we are about to kill any of the SSH
control masters that we started, ensure the clients using them are
successfully killed first.

Change-Id: Ida6c124dcb0c6a26bf7dd69cba2fbdc2ecd5b2fc
Signed-off-by: Shawn O. Pearce <sop@google.com>

docs/manifest-format.txt

@@ -22,6 +22,7 @@ following DTD:
   <!DOCTYPE manifest [
     <!ELEMENT manifest (remote*,
                         default?,
+                        manifest-server?,
                         remove-project*,
                         project*)>
   
@@ -33,6 +34,9 @@ following DTD:
     <!ELEMENT default (EMPTY)>
     <!ATTLIST default remote   IDREF #IMPLIED>
     <!ATTLIST default revision CDATA #IMPLIED>
+
+    <!ELEMENT manifest-server (EMPTY)>
+    <!ATTLIST url              CDATA #REQUIRED>
   
     <!ELEMENT project (EMPTY)>
     <!ATTLIST project name     CDATA #REQUIRED>
@@ -89,6 +93,27 @@ Attribute `revision`: Name of a Git branch (e.g. `master` or
 revision attribute will use this revision.
 
 
+Element manifest-server
+-----------------------
+
+At most one manifest-server may be specified. The url attribute
+is used to specify the URL of a manifest server, which is an
+XML RPC service that will return a manifest in which each project
+is pegged to a known good revision for the current branch and
+target.
+
+The manifest server should implement:
+
+  GetApprovedManifest(branch, target)
+
+The target to use is defined by environment variables TARGET_PRODUCT
+and TARGET_BUILD_VARIANT. These variables are used to create a string
+of the form $TARGET_PRODUCT-$TARGET_BUILD_VARIANT, e.g. passion-userdebug.
+If one of those variables or both are not present, the program will call
+GetApprovedManifest without the target paramater and the manifest server
+should choose a reasonable default target.
+
+
 Element project
 ---------------
 

git_command.py

@@ -17,6 +17,7 @@ import os
 import sys
 import subprocess
 import tempfile
+from signal import SIGTERM
 from error import GitError
 from trace import REPO_TRACE, IsTrace, Trace
 
@@ -29,8 +30,9 @@ LAST_CWD = None
 
 _ssh_proxy_path = None
 _ssh_sock_path = None
+_ssh_clients = []
 
-def _ssh_sock(create=True):
+def ssh_sock(create=True):
   global _ssh_sock_path
   if _ssh_sock_path is None:
     if not create:
@@ -51,6 +53,24 @@ def _ssh_proxy():
       'git_ssh')
   return _ssh_proxy_path
 
+def _add_ssh_client(p):
+  _ssh_clients.append(p)
+
+def _remove_ssh_client(p):
+  try:
+    _ssh_clients.remove(p)
+  except ValueError:
+    pass
+
+def terminate_ssh_clients():
+  global _ssh_clients
+  for p in _ssh_clients:
+    try:
+      os.kill(p.pid, SIGTERM)
+      p.wait()
+    except OSError:
+      pass
+  _ssh_clients = []
 
 class _GitCall(object):
   def version(self):
@@ -119,7 +139,7 @@ class GitCommand(object):
     if disable_editor:
       env['GIT_EDITOR'] = ':'
     if ssh_proxy:
-      env['REPO_SSH_SOCK'] = _ssh_sock()
+      env['REPO_SSH_SOCK'] = ssh_sock()
       env['GIT_SSH'] = _ssh_proxy()
 
     if project:
@@ -188,6 +208,9 @@ class GitCommand(object):
     except Exception, e:
       raise GitError('%s: %s' % (command[1], e))
 
+    if ssh_proxy:
+      _add_ssh_client(p)
+
     self.process = p
     self.stdin = p.stdin
 
@@ -210,4 +233,8 @@ class GitCommand(object):
     else:
       p.stderr = None
 
-    return self.process.wait()
+    try:
+      rc = p.wait()
+    finally:
+      _remove_ssh_client(p)
+    return rc

git_config.py

@@ -23,7 +23,10 @@ from signal import SIGTERM
 from urllib2 import urlopen, HTTPError
 from error import GitError, UploadError
 from trace import Trace
-from git_command import GitCommand, _ssh_sock
+
+from git_command import GitCommand
+from git_command import ssh_sock
+from git_command import terminate_ssh_clients
 
 R_HEADS = 'refs/heads/'
 R_TAGS  = 'refs/tags/'
@@ -371,7 +374,7 @@ def _open_ssh(host, port):
     return False
 
   command = ['ssh',
-             '-o','ControlPath %s' % _ssh_sock(),
+             '-o','ControlPath %s' % ssh_sock(),
              '-p',str(port),
              '-M',
              '-N',
@@ -391,6 +394,8 @@ def _open_ssh(host, port):
   return True
 
 def close_ssh():
+  terminate_ssh_clients()
+
   for key,p in _ssh_cache.iteritems():
     try:
       os.kill(p.pid, SIGTERM)
@@ -399,7 +404,7 @@ def close_ssh():
       pass
   _ssh_cache.clear()
 
-  d = _ssh_sock(create=False)
+  d = ssh_sock(create=False)
   if d:
     try:
       os.rmdir(os.path.dirname(d))
@@ -442,8 +447,30 @@ class Remote(object):
                      self._Get('fetch', all=True))
     self._review_protocol = None
 
+  def _InsteadOf(self):
+    globCfg = GitConfig.ForUser()
+    urlList = globCfg.GetSubSections('url')
+    longest = ""
+    longestUrl = ""
+
+    for url in urlList:
+      key = "url." + url + ".insteadOf"
+      insteadOfList = globCfg.GetString(key, all=True)
+
+      for insteadOf in insteadOfList:
+        if self.url.startswith(insteadOf) \
+        and len(insteadOf) > len(longest):
+          longest = insteadOf
+          longestUrl = url
+
+    if len(longest) == 0:
+      return self.url
+
+    return self.url.replace(longest, longestUrl, 1)
+
   def PreConnectFetch(self):
-    return _preconnect(self.url)
+    connectionUrl = self._InsteadOf()
+    return _preconnect(connectionUrl)
 
   @property
   def ReviewProtocol(self):

hooks/commit-msg

@@ -0,0 +1,101 @@
+#!/bin/sh
+# From Gerrit Code Review 2.1.2-rc2-33-g7e30c72
+#
+# Part of Gerrit Code Review (http://code.google.com/p/gerrit/)
+#
+# Copyright (C) 2009 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.
+#
+
+CHANGE_ID_AFTER="Bug|Issue"
+MSG="$1"
+
+# Check for, and add if missing, a unique Change-Id
+#
+add_ChangeId() {
+	clean_message=$(sed -e '
+		/^diff --git a\/.*/{
+			s///
+			q
+		}
+		/^Signed-off-by:/d
+		/^#/d
+	' "$MSG" | git stripspace)
+	if test -z "$clean_message"
+	then
+		return
+	fi
+
+	if grep -i '^Change-Id:' "$MSG" >/dev/null
+	then
+		return
+	fi
+
+	id=$(_gen_ChangeId)
+	perl -e '
+		$MSG = shift;
+		$id = shift;
+		$CHANGE_ID_AFTER = shift;
+
+		undef $/;
+		open(I, $MSG); $_ = <I>; close I;
+		s|^diff --git a/.*||ms;
+		s|^#.*$||mg;
+		exit unless $_;
+
+		@message = split /\n/;
+		$haveFooter = 0;
+		$startFooter = @message;
+		for($line = @message - 1; $line >= 0; $line--) {
+			$_ = $message[$line];
+
+			($haveFooter++, next) if /^[a-zA-Z0-9-]+:/;
+			next if /^[ []/;
+			$startFooter = $line if ($haveFooter && /^\r?$/);
+			last;
+		}
+
+		@footer = @message[$startFooter+1..@message];
+		@message = @message[0..$startFooter];
+		push(@footer, "") unless @footer;
+
+		for ($line = 0; $line < @footer; $line++) {
+			$_ = $footer[$line];
+			next if /^($CHANGE_ID_AFTER):/i;
+			last;
+		}
+		splice(@footer, $line, 0, "Change-Id: I$id");
+
+		$_ = join("\n", @message, @footer);
+		open(O, ">$MSG"); print O; close O;
+	' "$MSG" "$id" "$CHANGE_ID_AFTER"
+}
+_gen_ChangeIdInput() {
+	echo "tree $(git write-tree)"
+	if parent=$(git rev-parse HEAD^0 2>/dev/null)
+	then
+		echo "parent $parent"
+	fi
+	echo "author $(git var GIT_AUTHOR_IDENT)"
+	echo "committer $(git var GIT_COMMITTER_IDENT)"
+	echo
+	printf '%s' "$clean_message"
+}
+_gen_ChangeId() {
+	_gen_ChangeIdInput |
+	git hash-object -t commit --stdin
+}
+
+
+add_ChangeId

manifest_xml.py

@@ -65,8 +65,8 @@ class XmlManifest(object):
 
     self._Unload()
 
-  def Link(self, name):
+  def Override(self, name):
-    """Update the repo metadata to use a different manifest.
+    """Use a different manifest, just for the current instantiation.
     """
     path = os.path.join(self.manifestProject.worktree, name)
     if not os.path.isfile(path):
@@ -80,6 +80,11 @@ class XmlManifest(object):
     finally:
       self.manifestFile = old
 
+  def Link(self, name):
+    """Update the repo metadata to use a different manifest.
+    """
+    self.Override(name)
+
     try:
       if os.path.exists(self.manifestFile):
         os.remove(self.manifestFile)
@@ -123,6 +128,12 @@ class XmlManifest(object):
       root.appendChild(e)
       root.appendChild(doc.createTextNode(''))
 
+    if self._manifest_server:
+      e = doc.createElement('manifest-server')
+      e.setAttribute('url', self._manifest_server)
+      root.appendChild(e)
+      root.appendChild(doc.createTextNode(''))
+
     sort_projects = list(self.projects.keys())
     sort_projects.sort()
 
@@ -168,6 +179,11 @@ class XmlManifest(object):
     self._Load()
     return self._default
 
+  @property
+  def manifest_server(self):
+    self._Load()
+    return self._manifest_server
+
   @property
   def IsMirror(self):
     return self.manifestProject.config.GetBoolean('repo.mirror')
@@ -178,6 +194,7 @@ class XmlManifest(object):
     self._remotes = {}
     self._default = None
     self.branch = None
+    self._manifest_server = None
 
   def _Load(self):
     if not self._loaded:
@@ -246,6 +263,15 @@ class XmlManifest(object):
     if self._default is None:
       self._default = _Default()
 
+    for node in config.childNodes:
+      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)
+        self._manifest_server = url
+
     for node in config.childNodes:
       if node.nodeName == 'project':
         project = self._ParseProject(node)
@@ -315,7 +341,7 @@ class XmlManifest(object):
   def _ParseProject(self, node):
     """
     reads a <project> element from the manifest file
-    """ 
+    """
     name = self._reqatt(node, 'name')
 
     remote = self._get_remote(node)

project.py

@@ -706,10 +706,9 @@ class Project(object):
           # commits are not yet merged upstream.  We do not want
           # to rewrite the published commits so we punt.
           #
-          syncbuf.info(self,
+          syncbuf.fail(self,
-                       "branch %s is published but is now %d commits behind",
+                       "branch %s is published (but not merged) and is now %d commits behind"
-                       branch.name,
+                       % (branch.name, len(upstream_gain)))
-                       len(upstream_gain))
         return
       elif pub == head:
         # All published commits are merged, and thus we are a
@@ -728,7 +727,7 @@ class Project(object):
     last_mine = None
     cnt_mine = 0
     for commit in local_changes:
-      commit_id, committer_email = commit.split(' ', 2)
+      commit_id, committer_email = commit.split(' ', 1)
       if committer_email == self.UserEmail:
         last_mine = commit_id
         cnt_mine += 1
@@ -1056,13 +1055,27 @@ class Project(object):
     if not os.path.exists(hooks):
       os.makedirs(hooks)
     for stock_hook in repo_hooks():
-      dst = os.path.join(hooks, os.path.basename(stock_hook))
+      name = os.path.basename(stock_hook)
+
+      if name in ('commit-msg') and not self.remote.review:
+        # Don't install a Gerrit Code Review hook if this
+        # project does not appear to use it for reviews.
+        #
+        continue
+
+      dst = os.path.join(hooks, name)
+      if os.path.islink(dst):
+        continue
+      if os.path.exists(dst):
+        if filecmp.cmp(stock_hook, dst, shallow=False):
+          os.remove(dst)
+        else:
+          _error("%s: Not replacing %s hook", self.relpath, name)
+          continue
       try:
         os.symlink(relpath(stock_hook, dst), dst)
       except OSError, e:
-        if e.errno == errno.EEXIST:
+        if e.errno == errno.EPERM:
-          pass
-        elif e.errno == errno.EPERM:
           raise GitError('filesystem must support symlinks')
         else:
           raise
@@ -1120,7 +1133,10 @@ class Project(object):
         try:
           src = os.path.join(self.gitdir, name)
           dst = os.path.join(dotgit, name)
-          os.symlink(relpath(src, dst), dst)
+          if os.path.islink(dst) or not os.path.exists(dst):
+            os.symlink(relpath(src, dst), dst)
+          else:
+            raise GitError('cannot overwrite a local work tree')
         except OSError, e:
           if e.errno == errno.EPERM:
             raise GitError('filesystem must support symlinks')

subcmds/branches.py

@@ -136,7 +136,7 @@ is shown, then the branch appears in all projects.
       hdr('%c%c %-*s' % (current, published, width, name))
       out.write(' |')
 
-      if in_cnt < project_cnt and (in_cnt == 1):
+      if in_cnt < project_cnt:
         fmt = out.write
         paths = []
         if in_cnt < project_cnt - in_cnt: 
@@ -150,15 +150,17 @@ is shown, then the branch appears in all projects.
           for b in i.projects:
             have.add(b.project)
           for p in projects:
-            paths.append(p.relpath)
+            if not p in have:
+              paths.append(p.relpath)
 
         s = ' %s %s' % (type, ', '.join(paths))
         if width + 7 + len(s) < 80:
           fmt(s)
         else:
-          out.nl()
+          fmt(' %s:' % type)
-          fmt('    %s:' % type)
           for p in paths:
             out.nl()
-            fmt('      %s' % p)
+            fmt(width*' ' + '          %s' % p)
+      else:
+        out.write(' in all projects')
       out.nl()

subcmds/init.py

@@ -167,8 +167,9 @@ to update the working directory files.
 
       print ''
       print 'Your identity is: %s <%s>' % (name, email)
-      sys.stdout.write('is this correct [yes/no]? ')
+      sys.stdout.write('is this correct [y/n]? ')
-      if 'yes' == sys.stdin.readline().strip():
+      a = sys.stdin.readline().strip()
+      if a in ('yes', 'y', 't', 'true'):
         break
 
     if name != mp.UserName:

subcmds/sync.py

@@ -17,11 +17,14 @@ from optparse import SUPPRESS_HELP
 import os
 import re
 import shutil
+import socket
 import subprocess
 import sys
 import time
+import xmlrpclib
 
 from git_command import GIT
+from git_refs import R_HEADS
 from project import HEAD
 from project import Project
 from project import RemoteSpec
@@ -57,6 +60,10 @@ back to the manifest revision.  This option is especially helpful
 if the project is currently on a topic branch, but the manifest
 revision is temporarily needed.
 
+The -s/--smart-sync option can be used to sync to a known good
+build as specified by the manifest-server element in the current
+manifest.
+
 SSH Connections
 ---------------
 
@@ -97,6 +104,9 @@ later is required to fix a server side protocol bug.
     p.add_option('-d','--detach',
                  dest='detach_head', action='store_true',
                  help='detach projects back to manifest revision')
+    p.add_option('-s', '--smart-sync',
+                 dest='smart_sync', action='store_true',
+                 help='smart sync using manifest from a known good build')
 
     g = p.add_option_group('repo Version options')
     g.add_option('--no-repo-verify',
@@ -138,32 +148,36 @@ later is required to fix a server side protocol bug.
         if not path:
           continue
         if path not in new_project_paths:
-          project = Project(
+          """If the path has already been deleted, we don't need to do it
-                         manifest = self.manifest,
+          """
-                         name = path,
+          if os.path.exists(self.manifest.topdir + '/' + path):
-                         remote = RemoteSpec('origin'),
+              project = Project(
-                         gitdir = os.path.join(self.manifest.topdir,
+                             manifest = self.manifest,
-                                               path, '.git'),
+                             name = path,
-                         worktree = os.path.join(self.manifest.topdir, path),
+                             remote = RemoteSpec('origin'),
-                         relpath = path,
+                             gitdir = os.path.join(self.manifest.topdir,
-                         revisionExpr = 'HEAD',
+                                                   path, '.git'),
-                         revisionId = None)
+                             worktree = os.path.join(self.manifest.topdir, path),
-          if project.IsDirty():
+                             relpath = path,
-            print >>sys.stderr, 'error: Cannot remove project "%s": \
+                             revisionExpr = 'HEAD',
+                             revisionId = None)
+
+              if project.IsDirty():
+                print >>sys.stderr, 'error: Cannot remove project "%s": \
 uncommitted changes are present' % project.relpath
-            print >>sys.stderr, '       commit changes, then run sync again'
+                print >>sys.stderr, '       commit changes, then run sync again'
-            return -1
+                return -1
-          else:
+              else:
-            print >>sys.stderr, 'Deleting obsolete path %s' % project.worktree
+                print >>sys.stderr, 'Deleting obsolete path %s' % project.worktree
-            shutil.rmtree(project.worktree)
+                shutil.rmtree(project.worktree)
-            # Try deleting parent subdirs if they are empty
+                # Try deleting parent subdirs if they are empty
-            dir = os.path.dirname(project.worktree)
+                dir = os.path.dirname(project.worktree)
-            while dir != self.manifest.topdir:
+                while dir != self.manifest.topdir:
-              try:
+                  try:
-                os.rmdir(dir)
+                    os.rmdir(dir)
-              except OSError:
+                  except OSError:
-                break
+                    break
-              dir = os.path.dirname(dir)
+                  dir = os.path.dirname(dir)
 
     new_project_paths.sort()
     fd = open(file_path, 'w')
@@ -182,6 +196,51 @@ uncommitted changes are present' % project.relpath
       print >>sys.stderr, 'error: cannot combine -n and -l'
       sys.exit(1)
 
+    if opt.smart_sync:
+      if not self.manifest.manifest_server:
+        print >>sys.stderr, \
+            'error: cannot smart sync: no manifest server defined in manifest'
+        sys.exit(1)
+      try:
+        server = xmlrpclib.Server(self.manifest.manifest_server)
+        p = self.manifest.manifestProject
+        b = p.GetBranch(p.CurrentBranch)
+        branch = b.merge
+        if branch.startswith(R_HEADS):
+          branch = branch[len(R_HEADS):]
+
+        env = dict(os.environ)
+        if (env.has_key('TARGET_PRODUCT') and
+            env.has_key('TARGET_BUILD_VARIANT')):
+          target = '%s-%s' % (env['TARGET_PRODUCT'],
+                              env['TARGET_BUILD_VARIANT'])
+          [success, manifest_str] = server.GetApprovedManifest(branch, target)
+        else:
+          [success, manifest_str] = server.GetApprovedManifest(branch)
+
+        if success:
+          manifest_name = "smart_sync_override.xml"
+          manifest_path = os.path.join(self.manifest.manifestProject.worktree,
+                                       manifest_name)
+          try:
+            f = open(manifest_path, 'w')
+            try:
+              f.write(manifest_str)
+            finally:
+              f.close()
+          except IOError:
+            print >>sys.stderr, 'error: cannot write manifest to %s' % \
+                manifest_path
+            sys.exit(1)
+          self.manifest.Override(manifest_name)
+        else:
+          print >>sys.stderr, 'error: %s' % manifest_str
+          sys.exit(1)
+      except socket.error:
+        print >>sys.stderr, 'error: cannot connect to manifest server %s' % (
+            self.manifest.manifest_server)
+        sys.exit(1)
+
     rp = self.manifest.repoProject
     rp.PreSync()
 

subcmds/upload.py

@@ -20,6 +20,17 @@ from command import InteractiveCommand
 from editor import Editor
 from error import UploadError
 
+UNUSUAL_COMMIT_THRESHOLD = 5
+
+def _ConfirmManyUploads(multiple_branches=False):
+  if multiple_branches:
+    print "ATTENTION: One or more branches has an unusually high number of commits."
+  else:
+    print "ATTENTION: You are uploading an unusually high number of commits."
+  print "YOU PROBABLY DO NOT MEAN TO DO THIS. (Did you rebase across branches?)"
+  answer = raw_input("If you are sure you intend to do this, type 'yes': ").strip()
+  return answer == "yes"
+
 def _die(fmt, *args):
   msg = fmt % args
   print >>sys.stderr, 'error: %s' % msg
@@ -128,6 +139,10 @@ Gerrit Code Review:  http://code.google.com/p/gerrit/
       answer = sys.stdin.readline().strip()
       answer = answer in ('y', 'Y', 'yes', '1', 'true', 't')
 
+    if answer:
+      if len(branch.commits) > UNUSUAL_COMMIT_THRESHOLD:
+        answer = _ConfirmManyUploads()
+
     if answer:
       self._UploadAndReport([branch], people)
     else:
@@ -192,6 +207,16 @@ Gerrit Code Review:  http://code.google.com/p/gerrit/
         todo.append(branch)
     if not todo:
       _die("nothing uncommented for upload")
+
+    many_commits = False
+    for branch in todo:
+      if len(branch.commits) > UNUSUAL_COMMIT_THRESHOLD:
+        many_commits = True
+        break
+    if many_commits:
+      if not _ConfirmManyUploads(multiple_branches=True):
+        _die("upload aborted by user")
+
     self._UploadAndReport(todo, people)
 
   def _FindGerritChange(self, branch):
@@ -258,6 +283,10 @@ Gerrit Code Review:  http://code.google.com/p/gerrit/
       print >>sys.stderr, "       use 'repo upload' without --replace"
       sys.exit(1)
 
+    if len(branch.commits) > UNUSUAL_COMMIT_THRESHOLD:
+      if not _ConfirmManyUploads(multiple_branches=True):
+        _die("upload aborted by user")
+
     branch.replace_changes = to_replace
     self._UploadAndReport([branch], people)