project: fix error display when output_redir is disabled

We always pass in output_redir when syncing, but that's the common case:
there are a few situations (like `repo init`) where we don't pass in a
buffer, and if any errors show up in that case, we'd crash.  Rely on the
print function to handle this logic for us.

Bug: https://crbug.com/gerrit/14568
Change-Id: I8cd47e82329797ffc42534418a3dfbd8429205be
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/307222
Reviewed-by: Raman Tenneti <rtenneti@google.com>
Tested-by: Mike Frysinger <vapier@google.com>

docs/internal-fs-layout.md

@@ -110,6 +110,8 @@ Instead, you should use standard Git workflows like [git worktree] or
 [gitsubmodules] with [superprojects].
 ***
 
+*   `copy-link-files.json`: Tracking file used by `repo sync` to determine when
+    copyfile or linkfile are added or removed and need corresponding updates.
 *   `project.list`: Tracking file used by `repo sync` to determine when projects
     are added or removed and need corresponding updates in the checkout.
 *   `projects/`: Bare checkouts of every project synced by the manifest.  The

docs/manifest-format.md

@@ -31,6 +31,7 @@ following DTD:
                       extend-project*,
                       repo-hooks?,
                       superproject?,
+                      contactinfo?,
                       include*)>
 
   <!ELEMENT notice (#PCDATA)>
@@ -100,10 +101,13 @@ following DTD:
   <!ATTLIST repo-hooks in-project CDATA #REQUIRED>
   <!ATTLIST repo-hooks enabled-list CDATA #REQUIRED>
 
-  <!ELEMENT superproject (EMPTY)>
+  <!ELEMENT superproject EMPTY>
   <!ATTLIST superproject name    CDATA #REQUIRED>
   <!ATTLIST superproject remote  IDREF #IMPLIED>
 
+  <!ELEMENT contactinfo EMPTY>
+  <!ATTLIST contactinfo bugurl  CDATA #REQUIRED>
+
   <!ELEMENT include EMPTY>
   <!ATTLIST include name   CDATA #REQUIRED>
   <!ATTLIST include groups CDATA #IMPLIED>
@@ -405,7 +409,7 @@ Attribute `enabled-list`: List of hooks to use, whitespace or comma separated.
 ### Element superproject
 
 ***
- *Note*: This is currently a WIP.
+*Note*: This is currently a WIP.
 ***
 
 NB: See the [git superprojects documentation](
@@ -424,6 +428,19 @@ same meaning as project's name attribute. See the
 Attribute `remote`: Name of a previously defined remote element.
 If not supplied the remote given by the default element is used.
 
+### Element contactinfo
+
+***
+*Note*: This is currently a WIP.
+***
+
+This element is used to let manifest authors self-register contact info.
+It has "bugurl" as a required atrribute. This element can be repeated,
+and any later entries will clobber earlier ones. This would allow manifest
+authors who extend manifests to specify their own contact info.
+
+Attribute `bugurl`: The URL to file a bug against the manifest owner.
+
 ### Element include
 
 This element provides the capability of including another manifest

docs/release-process.md

@@ -83,7 +83,8 @@ control how repo finds updates:
 *   `--repo-rev`: This tells repo which branch to use for the full project.
     It defaults to the `stable` branch (`REPO_REV` in the launcher script).
 
-Whenever `repo sync` is run, repo will check to see if an update is available.
+Whenever `repo sync` is run, repo will, once every 24 hours, see if an update
+is available.
 It fetches the latest repo-rev from the repo-url.
 Then it verifies that the latest commit in the branch has a valid signed tag
 using `git tag -v` (which uses gpg).
@@ -95,6 +96,11 @@ If that tag is valid, then repo will warn and use that commit instead.
 
 If that tag cannot be verified, it gives up and forces the user to resolve.
 
+### Force an update
+
+The `repo selfupdate` command can be used to force an immediate update.
+It is not subject to the 24 hour limitation.
+
 ## Branch management
 
 All development happens on the `main` branch and should generally be stable.

error.py

@@ -13,10 +13,6 @@
 # limitations under the License.
 
 
-# URL to file bug reports for repo tool issues.
-BUG_REPORT_URL = 'https://bugs.chromium.org/p/gerrit/issues/entry?template=Repo+tool+issue'
-
-
 class ManifestParseError(Exception):
   """Failed to parse the manifest file.
   """

git_command.py

@@ -12,12 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import functools
 import os
-import re
 import sys
 import subprocess
-import tempfile
-from signal import SIGTERM
 
 from error import GitError
 from git_refs import HEAD
@@ -42,101 +40,15 @@ GIT_DIR = 'GIT_DIR'
 LAST_GITDIR = None
 LAST_CWD = None
 
-_ssh_proxy_path = None
-_ssh_sock_path = None
-_ssh_clients = []
-_ssh_version = None
-
-
-def _run_ssh_version():
-  """run ssh -V to display the version number"""
-  return subprocess.check_output(['ssh', '-V'], stderr=subprocess.STDOUT).decode()
-
-
-def _parse_ssh_version(ver_str=None):
-  """parse a ssh version string into a tuple"""
-  if ver_str is None:
-    ver_str = _run_ssh_version()
-  m = re.match(r'^OpenSSH_([0-9.]+)(p[0-9]+)?\s', ver_str)
-  if m:
-    return tuple(int(x) for x in m.group(1).split('.'))
-  else:
-    return ()
-
-
-def ssh_version():
-  """return ssh version as a tuple"""
-  global _ssh_version
-  if _ssh_version is None:
-    try:
-      _ssh_version = _parse_ssh_version()
-    except subprocess.CalledProcessError:
-      print('fatal: unable to detect ssh version', file=sys.stderr)
-      sys.exit(1)
-  return _ssh_version
-
-
-def ssh_sock(create=True):
-  global _ssh_sock_path
-  if _ssh_sock_path is None:
-    if not create:
-      return None
-    tmp_dir = '/tmp'
-    if not os.path.exists(tmp_dir):
-      tmp_dir = tempfile.gettempdir()
-    if ssh_version() < (6, 7):
-      tokens = '%r@%h:%p'
-    else:
-      tokens = '%C'  # hash of %l%h%p%r
-    _ssh_sock_path = os.path.join(
-        tempfile.mkdtemp('', 'ssh-', tmp_dir),
-        'master-' + tokens)
-  return _ssh_sock_path
-
-
-def _ssh_proxy():
-  global _ssh_proxy_path
-  if _ssh_proxy_path is None:
-    _ssh_proxy_path = os.path.join(
-        os.path.dirname(__file__),
-        '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 = []
-
-
-_git_version = None
-
 
 class _GitCall(object):
+  @functools.lru_cache(maxsize=None)
   def version_tuple(self):
-    global _git_version
+    ret = Wrapper().ParseGitVersion()
-    if _git_version is None:
+    if ret is None:
-      _git_version = Wrapper().ParseGitVersion()
+      print('fatal: unable to detect git version', file=sys.stderr)
-      if _git_version is None:
+      sys.exit(1)
-        print('fatal: unable to detect git version', file=sys.stderr)
+    return ret
-        sys.exit(1)
-    return _git_version
 
   def __getattr__(self, name):
     name = name.replace('_', '-')
@@ -254,7 +166,7 @@ class GitCommand(object):
                capture_stderr=False,
                merge_output=False,
                disable_editor=False,
-               ssh_proxy=False,
+               ssh_proxy=None,
                cwd=None,
                gitdir=None):
     env = self._GetBasicEnv()
@@ -262,8 +174,8 @@ class GitCommand(object):
     if disable_editor:
       env['GIT_EDITOR'] = ':'
     if ssh_proxy:
-      env['REPO_SSH_SOCK'] = ssh_sock()
+      env['REPO_SSH_SOCK'] = ssh_proxy.sock()
-      env['GIT_SSH'] = _ssh_proxy()
+      env['GIT_SSH'] = ssh_proxy.proxy
       env['GIT_SSH_VARIANT'] = 'ssh'
     if 'http_proxy' in env and 'darwin' == sys.platform:
       s = "'http.proxy=%s'" % (env['http_proxy'],)
@@ -346,7 +258,7 @@ class GitCommand(object):
       raise GitError('%s: %s' % (command[1], e))
 
     if ssh_proxy:
-      _add_ssh_client(p)
+      ssh_proxy.add_client(p)
 
     self.process = p
     if input:
@@ -358,7 +270,8 @@ class GitCommand(object):
     try:
       self.stdout, self.stderr = p.communicate()
     finally:
-      _remove_ssh_client(p)
+      if ssh_proxy:
+        ssh_proxy.remove_client(p)
     self.rc = p.wait()
 
   @staticmethod

git_config.py

@@ -18,25 +18,16 @@ from http.client import HTTPException
 import json
 import os
 import re
-import signal
 import ssl
 import subprocess
 import sys
-try:
-  import threading as _threading
-except ImportError:
-  import dummy_threading as _threading
-import time
 import urllib.error
 import urllib.request
 
 from error import GitError, UploadError
 import platform_utils
 from repo_trace import Trace
-
 from git_command import GitCommand
-from git_command import ssh_sock
-from git_command import terminate_ssh_clients
 from git_refs import R_CHANGES, R_HEADS, R_TAGS
 
 ID_RE = re.compile(r'^[0-9a-f]{40}$')
@@ -440,127 +431,6 @@ class RefSpec(object):
     return s
 
 
-_master_processes = []
-_master_keys = set()
-_ssh_master = True
-_master_keys_lock = None
-
-
-def init_ssh():
-  """Should be called once at the start of repo to init ssh master handling.
-
-  At the moment, all we do is to create our lock.
-  """
-  global _master_keys_lock
-  assert _master_keys_lock is None, "Should only call init_ssh once"
-  _master_keys_lock = _threading.Lock()
-
-
-def _open_ssh(host, port=None):
-  global _ssh_master
-
-  # Acquire the lock.  This is needed to prevent opening multiple masters for
-  # the same host when we're running "repo sync -jN" (for N > 1) _and_ the
-  # manifest <remote fetch="ssh://xyz"> specifies a different host from the
-  # one that was passed to repo init.
-  _master_keys_lock.acquire()
-  try:
-
-    # Check to see whether we already think that the master is running; if we
-    # think it's already running, return right away.
-    if port is not None:
-      key = '%s:%s' % (host, port)
-    else:
-      key = host
-
-    if key in _master_keys:
-      return True
-
-    if (not _ssh_master
-            or 'GIT_SSH' in os.environ
-            or sys.platform in ('win32', 'cygwin')):
-      # failed earlier, or cygwin ssh can't do this
-      #
-      return False
-
-    # We will make two calls to ssh; this is the common part of both calls.
-    command_base = ['ssh',
-                    '-o', 'ControlPath %s' % ssh_sock(),
-                    host]
-    if port is not None:
-      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
-    # be important because we can't tell that that 'git@myhost.com' is the same
-    # as 'myhost.com' where "User git" is setup in the user's ~/.ssh/config file.
-    check_command = command_base + ['-O', 'check']
-    try:
-      Trace(': %s', ' '.join(check_command))
-      check_process = subprocess.Popen(check_command,
-                                       stdout=subprocess.PIPE,
-                                       stderr=subprocess.PIPE)
-      check_process.communicate()  # read output, but ignore it...
-      isnt_running = check_process.wait()
-
-      if not isnt_running:
-        # Our double-check found that the master _was_ infact running.  Add to
-        # the list of keys.
-        _master_keys.add(key)
-        return True
-    except Exception:
-      # Ignore excpetions.  We we will fall back to the normal command and print
-      # to the log there.
-      pass
-
-    command = command_base[:1] + ['-M', '-N'] + command_base[1:]
-    try:
-      Trace(': %s', ' '.join(command))
-      p = subprocess.Popen(command)
-    except Exception as e:
-      _ssh_master = False
-      print('\nwarn: cannot enable ssh control master for %s:%s\n%s'
-            % (host, port, str(e)), file=sys.stderr)
-      return False
-
-    time.sleep(1)
-    ssh_died = (p.poll() is not None)
-    if ssh_died:
-      return False
-
-    _master_processes.append(p)
-    _master_keys.add(key)
-    return True
-  finally:
-    _master_keys_lock.release()
-
-
-def close_ssh():
-  global _master_keys_lock
-
-  terminate_ssh_clients()
-
-  for p in _master_processes:
-    try:
-      os.kill(p.pid, signal.SIGTERM)
-      p.wait()
-    except OSError:
-      pass
-  del _master_processes[:]
-  _master_keys.clear()
-
-  d = ssh_sock(create=False)
-  if d:
-    try:
-      platform_utils.rmdir(os.path.dirname(d))
-    except OSError:
-      pass
-
-  # We're done with the lock, so we can delete it.
-  _master_keys_lock = None
-
-
-URI_SCP = re.compile(r'^([^@:]*@?[^:/]{1,}):')
 URI_ALL = re.compile(r'^([a-z][a-z+-]*)://([^@/]*@?[^/]*)/')
 
 
@@ -612,27 +482,6 @@ def GetUrlCookieFile(url, quiet):
   yield cookiefile, None
 
 
-def _preconnect(url):
-  m = URI_ALL.match(url)
-  if m:
-    scheme = m.group(1)
-    host = m.group(2)
-    if ':' in host:
-      host, port = host.split(':')
-    else:
-      port = None
-    if scheme in ('ssh', 'git+ssh', 'ssh+git'):
-      return _open_ssh(host, port)
-    return False
-
-  m = URI_SCP.match(url)
-  if m:
-    host = m.group(1)
-    return _open_ssh(host)
-
-  return False
-
-
 class Remote(object):
   """Configuration options related to a remote.
   """
@@ -669,9 +518,23 @@ class Remote(object):
 
     return self.url.replace(longest, longestUrl, 1)
 
-  def PreConnectFetch(self):
+  def PreConnectFetch(self, ssh_proxy):
+    """Run any setup for this remote before we connect to it.
+
+    In practice, if the remote is using SSH, we'll attempt to create a new
+    SSH master session to it for reuse across projects.
+
+    Args:
+      ssh_proxy: The SSH settings for managing master sessions.
+
+    Returns:
+      Whether the preconnect phase for this remote was successful.
+    """
+    if not ssh_proxy:
+      return True
+
     connectionUrl = self._InsteadOf()
-    return _preconnect(connectionUrl)
+    return ssh_proxy.preconnect(connectionUrl)
 
   def ReviewUrl(self, userEmail, validate_certs):
     if self._review_url is None:

git_superproject.py

@@ -26,7 +26,6 @@ import hashlib
 import os
 import sys
 
-from error import BUG_REPORT_URL
 from git_command import GitCommand
 from git_refs import R_HEADS
 
@@ -262,10 +261,20 @@ class Superproject(object):
       return None
 
     projects_missing_commit_ids = []
+    superproject_fetchUrl = self._manifest.superproject['remote'].fetchUrl
     for project in projects:
       path = project.relpath
       if not path:
         continue
+      # Some manifests that pull projects from the "chromium" GoB
+      # (remote="chromium"), and have a private manifest that pulls projects
+      # from both the chromium GoB and "chrome-internal" GoB (remote="chrome").
+      # For such projects, one of the remotes will be different from
+      # superproject's remote. Until superproject, supports multiple remotes,
+      # don't update the commit ids of remotes that don't match superproject's
+      # remote.
+      if project.remote.fetchUrl != superproject_fetchUrl:
+        continue
       commit_id = commit_ids.get(path)
       if commit_id:
         project.SetRevisionId(commit_id)
@@ -273,7 +282,7 @@ class Superproject(object):
         projects_missing_commit_ids.append(path)
     if projects_missing_commit_ids:
       print('error: please file a bug using %s to report missing commit_ids for: %s' %
-            (BUG_REPORT_URL, projects_missing_commit_ids), file=sys.stderr)
+            (self._manifest.contactinfo.bugurl, projects_missing_commit_ids), file=sys.stderr)
       return None
 
     manifest_path = self._WriteManfiestFile()

main.py

@@ -39,7 +39,7 @@ from color import SetDefaultColoring
 import event_log
 from repo_trace import SetTrace
 from git_command import user_agent
-from git_config import init_ssh, close_ssh, RepoConfig
+from git_config import RepoConfig
 from git_trace2_event_log import EventLog
 from command import InteractiveCommand
 from command import MirrorSafeCommand
@@ -591,20 +591,16 @@ def _Main(argv):
 
   repo = _Repo(opt.repodir)
   try:
-    try:
+    init_http()
-      init_ssh()
+    name, gopts, argv = repo._ParseArgs(argv)
-      init_http()
+    run = lambda: repo._Run(name, gopts, argv) or 0
-      name, gopts, argv = repo._ParseArgs(argv)
+    if gopts.trace_python:
-      run = lambda: repo._Run(name, gopts, argv) or 0
+      import trace
-      if gopts.trace_python:
+      tracer = trace.Trace(count=False, trace=True, timing=True,
-        import trace
+                           ignoredirs=set(sys.path[1:]))
-        tracer = trace.Trace(count=False, trace=True, timing=True,
+      result = tracer.runfunc(run)
-                             ignoredirs=set(sys.path[1:]))
+    else:
-        result = tracer.runfunc(run)
+      result = run()
-      else:
-        result = run()
-    finally:
-      close_ssh()
   except KeyboardInterrupt:
     print('aborted by user', file=sys.stderr)
     result = 1

manifest_xml.py

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import collections
 import itertools
 import os
 import platform
@@ -27,11 +28,15 @@ import platform_utils
 from project import RemoteSpec, Project, MetaProject
 from error import (ManifestParseError, ManifestInvalidPathError,
                    ManifestInvalidRevisionError)
+from wrapper import Wrapper
 
 MANIFEST_FILE_NAME = 'manifest.xml'
 LOCAL_MANIFEST_NAME = 'local_manifest.xml'
 LOCAL_MANIFESTS_DIR_NAME = 'local_manifests'
 
+# ContactInfo has the self-registered bug url, supplied by the manifest authors.
+ContactInfo = collections.namedtuple('ContactInfo', 'bugurl')
+
 # urljoin gets confused if the scheme is not known.
 urllib.parse.uses_relative.extend([
     'ssh',
@@ -479,6 +484,12 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
         e.setAttribute('remote', remoteName)
       root.appendChild(e)
 
+    if self._contactinfo.bugurl != Wrapper().BUG_URL:
+      root.appendChild(doc.createTextNode(''))
+      e = doc.createElement('contactinfo')
+      e.setAttribute('bugurl', self._contactinfo.bugurl)
+      root.appendChild(e)
+
     return doc
 
   def ToDict(self, **kwargs):
@@ -490,6 +501,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
         'manifest-server',
         'repo-hooks',
         'superproject',
+        'contactinfo',
     }
     # Elements that may be repeated.
     MULTI_ELEMENTS = {
@@ -565,6 +577,11 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
     self._Load()
     return self._superproject
 
+  @property
+  def contactinfo(self):
+    self._Load()
+    return self._contactinfo
+
   @property
   def notice(self):
     self._Load()
@@ -595,6 +612,10 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
         'repo.partialcloneexclude') or ''
     return set(x.strip() for x in exclude.split(','))
 
+  @property
+  def HasLocalManifests(self):
+    return self._load_local_manifests and self.local_manifests
+
   @property
   def IsMirror(self):
     return self.manifestProject.config.GetBoolean('repo.mirror')
@@ -630,6 +651,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
     self._default = None
     self._repo_hooks_project = None
     self._superproject = {}
+    self._contactinfo = ContactInfo(Wrapper().BUG_URL)
     self._notice = None
     self.branch = None
     self._manifest_server = None
@@ -872,6 +894,11 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
           raise ManifestParseError("no remote for superproject %s within %s" %
                                    (name, self.manifestFile))
         self._superproject['remote'] = remote.ToRemoteSpec(name)
+      if node.nodeName == 'contactinfo':
+        bugurl = self._reqatt(node, 'bugurl')
+        # This element can be repeated, later entries will clobber earlier ones.
+        self._contactinfo = ContactInfo(bugurl)
+
       if node.nodeName == 'remove-project':
         name = self._reqatt(node, 'name')
 
@@ -1199,6 +1226,8 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
     if '~' in path:
       return '~ not allowed (due to 8.3 filenames on Windows filesystems)'
 
+    path_codepoints = set(path)
+
     # Some filesystems (like Apple's HFS+) try to normalize Unicode codepoints
     # which means there are alternative names for ".git".  Reject paths with
     # these in it as there shouldn't be any reasonable need for them here.
@@ -1222,10 +1251,17 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
         u'\u206F',  # NOMINAL DIGIT SHAPES
         u'\uFEFF',  # ZERO WIDTH NO-BREAK SPACE
     }
-    if BAD_CODEPOINTS & set(path):
+    if BAD_CODEPOINTS & path_codepoints:
       # This message is more expansive than reality, but should be fine.
       return 'Unicode combining characters not allowed'
 
+    # Reject newlines as there shouldn't be any legitmate use for them, they'll
+    # be confusing to users, and they can easily break tools that expect to be
+    # able to iterate over newline delimited lists.  This even applies to our
+    # own code like .repo/project.list.
+    if {'\r', '\n'} & path_codepoints:
+      return 'Newlines not allowed'
+
     # Assume paths might be used on case-insensitive filesystems.
     path = path.lower()
 

project.py

@@ -1041,17 +1041,18 @@ class Project(object):
                        verbose=False,
                        output_redir=None,
                        is_new=None,
-                       current_branch_only=False,
+                       current_branch_only=None,
                        force_sync=False,
                        clone_bundle=True,
-                       tags=True,
+                       tags=None,
                        archive=False,
                        optimized_fetch=False,
                        retry_fetches=0,
                        prune=False,
                        submodules=False,
+                       ssh_proxy=None,
                        clone_filter=None,
-                       partial_clone_exclude=None):
+                       partial_clone_exclude=set()):
     """Perform only the network IO portion of the sync process.
        Local working directory/branch state is not affected.
     """
@@ -1116,7 +1117,7 @@ class Project(object):
             and self._ApplyCloneBundle(initial=is_new, quiet=quiet, verbose=verbose)):
       is_new = False
 
-    if not current_branch_only:
+    if current_branch_only is None:
       if self.sync_c:
         current_branch_only = True
       elif not self.manifest._loaded:
@@ -1125,8 +1126,8 @@ class Project(object):
       elif self.manifest.default.sync_c:
         current_branch_only = True
 
-    if not self.sync_tags:
+    if tags is None:
-      tags = False
+      tags = self.sync_tags
 
     if self.clone_depth:
       depth = self.clone_depth
@@ -1143,6 +1144,7 @@ class Project(object):
               alt_dir=alt_dir, current_branch_only=current_branch_only,
               tags=tags, prune=prune, depth=depth,
               submodules=submodules, force_sync=force_sync,
+              ssh_proxy=ssh_proxy,
               clone_filter=clone_filter, retry_fetches=retry_fetches):
         return False
 
@@ -1214,6 +1216,9 @@ class Project(object):
                                          (self.revisionExpr, self.name))
 
   def SetRevisionId(self, revisionId):
+    if self.clone_depth or self.manifest.manifestProject.config.GetString('repo.depth'):
+      self.upstream = self.revisionExpr
+
     self.revisionId = revisionId
 
   def Sync_LocalHalf(self, syncbuf, force_sync=False, submodules=False):
@@ -1991,6 +1996,7 @@ class Project(object):
                    prune=False,
                    depth=None,
                    submodules=False,
+                   ssh_proxy=None,
                    force_sync=False,
                    clone_filter=None,
                    retry_fetches=2,
@@ -2038,16 +2044,14 @@ class Project(object):
     if not name:
       name = self.remote.name
 
-    ssh_proxy = False
     remote = self.GetRemote(name)
-    if remote.PreConnectFetch():
+    if not remote.PreConnectFetch(ssh_proxy):
-      ssh_proxy = True
+      ssh_proxy = None
 
     if initial:
       if alt_dir and 'objects' == os.path.basename(alt_dir):
         ref_dir = os.path.dirname(alt_dir)
         packed_refs = os.path.join(self.gitdir, 'packed-refs')
-        remote = self.GetRemote(name)
 
         all_refs = self.bare_ref.all
         ids = set(all_refs.values())
@@ -2134,6 +2138,8 @@ class Project(object):
       # Shallow checkout of a specific commit, fetch from that commit and not
       # the heads only as the commit might be deeper in the history.
       spec.append(branch)
+      if self.upstream:
+        spec.append(self.upstream)
     else:
       if is_sha1:
         branch = self.upstream
@@ -2191,7 +2197,7 @@ class Project(object):
         ret = prunecmd.Wait()
         if ret:
           break
-        output_redir.write('retrying fetch after pruning remote branches')
+        print('retrying fetch after pruning remote branches', file=output_redir)
         # Continue right away so we don't sleep as we shouldn't need to.
         continue
       elif current_branch_only and is_sha1 and ret == 128:
@@ -2204,10 +2210,11 @@ class Project(object):
         break
 
       # Figure out how long to sleep before the next attempt, if there is one.
-      if not verbose:
+      if not verbose and gitcmd.stdout:
-        output_redir.write('\n%s:\n%s' % (self.name, gitcmd.stdout), file=sys.stderr)
+        print('\n%s:\n%s' % (self.name, gitcmd.stdout), end='', file=output_redir)
       if try_n < retry_fetches - 1:
-        output_redir.write('sleeping %s seconds before retrying' % retry_cur_sleep)
+        print('%s: sleeping %s seconds before retrying' % (self.name, retry_cur_sleep),
+              file=output_redir)
         time.sleep(retry_cur_sleep)
         retry_cur_sleep = min(retry_exp_factor * retry_cur_sleep,
                               MAXIMUM_RETRY_SLEEP_SEC)
@@ -2233,7 +2240,7 @@ class Project(object):
             name=name, quiet=quiet, verbose=verbose, output_redir=output_redir,
             current_branch_only=current_branch_only and depth,
             initial=False, alt_dir=alt_dir,
-            depth=None, clone_filter=clone_filter)
+            depth=None, ssh_proxy=ssh_proxy, clone_filter=clone_filter)
 
     return ok
 
@@ -2438,14 +2445,6 @@ class Project(object):
         self.bare_objdir.init()
 
         if self.use_git_worktrees:
-          # Set up the m/ space to point to the worktree-specific ref space.
-          # We'll update the worktree-specific ref space on each checkout.
-          if self.manifest.branch:
-            self.bare_git.symbolic_ref(
-                '-m', 'redirecting to worktree scope',
-                R_M + self.manifest.branch,
-                R_WORKTREE_M + self.manifest.branch)
-
           # Enable per-worktree config file support if possible.  This is more a
           # nice-to-have feature for users rather than a hard requirement.
           if git_require((2, 20, 0)):
@@ -2582,6 +2581,14 @@ class Project(object):
   def _InitMRef(self):
     if self.manifest.branch:
       if self.use_git_worktrees:
+        # Set up the m/ space to point to the worktree-specific ref space.
+        # We'll update the worktree-specific ref space on each checkout.
+        ref = R_M + self.manifest.branch
+        if not self.bare_ref.symref(ref):
+          self.bare_git.symbolic_ref(
+              '-m', 'redirecting to worktree scope',
+              ref, R_WORKTREE_M + self.manifest.branch)
+
         # We can't update this ref with git worktrees until it exists.
         # We'll wait until the initial checkout to set it.
         if not os.path.exists(self.worktree):

repo

@@ -117,7 +117,7 @@ def check_python_version():
 
     # If the python3 version looks like it's new enough, give it a try.
     if (python3_ver and python3_ver >= MIN_PYTHON_VERSION_HARD
-        and python3_ver != (major, minor)):
+            and python3_ver != (major, minor)):
       reexec('python3')
 
     # We're still here, so diagnose things for the user.
@@ -145,9 +145,11 @@ if not REPO_URL:
 REPO_REV = os.environ.get('REPO_REV')
 if not REPO_REV:
   REPO_REV = 'stable'
+# URL to file bug reports for repo tool issues.
+BUG_URL = 'https://bugs.chromium.org/p/gerrit/issues/entry?template=Repo+tool+issue'
 
 # increment this whenever we make important changes to this script
-VERSION = (2, 14)
+VERSION = (2, 15)
 
 # increment this if the MAINTAINER_KEYS block is modified
 KEYRING_VERSION = (2, 3)
@@ -298,7 +300,7 @@ def InitParser(parser, gitc_init=False):
                    help='manifest repository location', metavar='URL')
   group.add_option('-b', '--manifest-branch', metavar='REVISION',
                    help='manifest branch or revision (use HEAD for default)')
-  group.add_option('-m', '--manifest-name',
+  group.add_option('-m', '--manifest-name', default='default.xml',
                    help='initial manifest file', metavar='NAME.xml')
   group.add_option('-g', '--groups', default='default',
                    help='restrict manifest projects to ones with specified '
@@ -322,8 +324,14 @@ def InitParser(parser, gitc_init=False):
   group.add_option(*cbr_opts,
                    dest='current_branch_only', action='store_true',
                    help='fetch only current manifest branch from server')
+  group.add_option('--no-current-branch',
+                   dest='current_branch_only', action='store_false',
+                   help='fetch all manifest branches from server')
+  group.add_option('--tags',
+                   action='store_true',
+                   help='fetch tags in the manifest')
   group.add_option('--no-tags',
-                   dest='tags', default=True, action='store_false',
+                   dest='tags', action='store_false',
                    help="don't fetch tags in the manifest")
 
   # These are fundamentally different ways of structuring the checkout.
@@ -851,11 +859,10 @@ def _DownloadBundle(url, cwd, quiet, verbose):
     try:
       r = urllib.request.urlopen(url)
     except urllib.error.HTTPError as e:
-      if e.code in [401, 403, 404, 501]:
+      if e.code not in [400, 401, 403, 404, 501]:
-        return False
+        print('warning: Cannot get %s' % url, file=sys.stderr)
-      print('fatal: Cannot get %s' % url, file=sys.stderr)
+        print('warning: HTTP error %s' % e.code, file=sys.stderr)
-      print('fatal: HTTP error %s' % e.code, file=sys.stderr)
+      return False
-      raise CloneFailure()
     except urllib.error.URLError as e:
       print('fatal: Cannot get %s' % url, file=sys.stderr)
       print('fatal: error %s' % e.reason, file=sys.stderr)
@@ -1171,6 +1178,7 @@ The most commonly used repo commands are:
 
 For access to the full online help, install repo ("repo init").
 """)
+  print('Bug reports:', BUG_URL)
   sys.exit(0)
 
 
@@ -1204,6 +1212,7 @@ def _Version():
     print('OS %s %s (%s)' % (uname.system, uname.release, uname.version))
     print('CPU %s (%s)' %
           (uname.machine, uname.processor if uname.processor else 'unknown'))
+  print('Bug reports:', BUG_URL)
   sys.exit(0)
 
 

ssh.py

@@ -0,0 +1,277 @@
+# Copyright (C) 2008 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.
+
+"""Common SSH management logic."""
+
+import functools
+import multiprocessing
+import os
+import re
+import signal
+import subprocess
+import sys
+import tempfile
+import time
+
+import platform_utils
+from repo_trace import Trace
+
+
+PROXY_PATH = os.path.join(os.path.dirname(__file__), 'git_ssh')
+
+
+def _run_ssh_version():
+  """run ssh -V to display the version number"""
+  return subprocess.check_output(['ssh', '-V'], stderr=subprocess.STDOUT).decode()
+
+
+def _parse_ssh_version(ver_str=None):
+  """parse a ssh version string into a tuple"""
+  if ver_str is None:
+    ver_str = _run_ssh_version()
+  m = re.match(r'^OpenSSH_([0-9.]+)(p[0-9]+)?\s', ver_str)
+  if m:
+    return tuple(int(x) for x in m.group(1).split('.'))
+  else:
+    return ()
+
+
+@functools.lru_cache(maxsize=None)
+def version():
+  """return ssh version as a tuple"""
+  try:
+    return _parse_ssh_version()
+  except subprocess.CalledProcessError:
+    print('fatal: unable to detect ssh version', file=sys.stderr)
+    sys.exit(1)
+
+
+URI_SCP = re.compile(r'^([^@:]*@?[^:/]{1,}):')
+URI_ALL = re.compile(r'^([a-z][a-z+-]*)://([^@/]*@?[^/]*)/')
+
+
+class ProxyManager:
+  """Manage various ssh clients & masters that we spawn.
+
+  This will take care of sharing state between multiprocessing children, and
+  make sure that if we crash, we don't leak any of the ssh sessions.
+
+  The code should work with a single-process scenario too, and not add too much
+  overhead due to the manager.
+  """
+
+  # Path to the ssh program to run which will pass our master settings along.
+  # Set here more as a convenience API.
+  proxy = PROXY_PATH
+
+  def __init__(self, manager):
+    # Protect access to the list of active masters.
+    self._lock = multiprocessing.Lock()
+    # List of active masters (pid).  These will be spawned on demand, and we are
+    # responsible for shutting them all down at the end.
+    self._masters = manager.list()
+    # Set of active masters indexed by "host:port" information.
+    # The value isn't used, but multiprocessing doesn't provide a set class.
+    self._master_keys = manager.dict()
+    # Whether ssh masters are known to be broken, so we give up entirely.
+    self._master_broken = manager.Value('b', False)
+    # List of active ssh sesssions.  Clients will be added & removed as
+    # connections finish, so this list is just for safety & cleanup if we crash.
+    self._clients = manager.list()
+    # Path to directory for holding master sockets.
+    self._sock_path = None
+
+  def __enter__(self):
+    """Enter a new context."""
+    return self
+
+  def __exit__(self, exc_type, exc_value, traceback):
+    """Exit a context & clean up all resources."""
+    self.close()
+
+  def add_client(self, proc):
+    """Track a new ssh session."""
+    self._clients.append(proc.pid)
+
+  def remove_client(self, proc):
+    """Remove a completed ssh session."""
+    try:
+      self._clients.remove(proc.pid)
+    except ValueError:
+      pass
+
+  def add_master(self, proc):
+    """Track a new master connection."""
+    self._masters.append(proc.pid)
+
+  def _terminate(self, procs):
+    """Kill all |procs|."""
+    for pid in procs:
+      try:
+        os.kill(pid, signal.SIGTERM)
+        os.waitpid(pid, 0)
+      except OSError:
+        pass
+
+    # The multiprocessing.list() API doesn't provide many standard list()
+    # methods, so we have to manually clear the list.
+    while True:
+      try:
+        procs.pop(0)
+      except:
+        break
+
+  def close(self):
+    """Close this active ssh session.
+
+    Kill all ssh clients & masters we created, and nuke the socket dir.
+    """
+    self._terminate(self._clients)
+    self._terminate(self._masters)
+
+    d = self.sock(create=False)
+    if d:
+      try:
+        platform_utils.rmdir(os.path.dirname(d))
+      except OSError:
+        pass
+
+  def _open_unlocked(self, host, port=None):
+    """Make sure a ssh master session exists for |host| & |port|.
+
+    If one doesn't exist already, we'll create it.
+
+    We won't grab any locks, so the caller has to do that.  This helps keep the
+    business logic of actually creating the master separate from grabbing locks.
+    """
+    # Check to see whether we already think that the master is running; if we
+    # think it's already running, return right away.
+    if port is not None:
+      key = '%s:%s' % (host, port)
+    else:
+      key = host
+
+    if key in self._master_keys:
+      return True
+
+    if self._master_broken.value or 'GIT_SSH' in os.environ:
+      # Failed earlier, so don't retry.
+      return False
+
+    # We will make two calls to ssh; this is the common part of both calls.
+    command_base = ['ssh', '-o', 'ControlPath %s' % self.sock(), host]
+    if port is not None:
+      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
+    # be important because we can't tell that that 'git@myhost.com' is the same
+    # as 'myhost.com' where "User git" is setup in the user's ~/.ssh/config file.
+    check_command = command_base + ['-O', 'check']
+    try:
+      Trace(': %s', ' '.join(check_command))
+      check_process = subprocess.Popen(check_command,
+                                       stdout=subprocess.PIPE,
+                                       stderr=subprocess.PIPE)
+      check_process.communicate()  # read output, but ignore it...
+      isnt_running = check_process.wait()
+
+      if not isnt_running:
+        # Our double-check found that the master _was_ infact running.  Add to
+        # the list of keys.
+        self._master_keys[key] = True
+        return True
+    except Exception:
+      # Ignore excpetions.  We we will fall back to the normal command and print
+      # to the log there.
+      pass
+
+    command = command_base[:1] + ['-M', '-N'] + command_base[1:]
+    try:
+      Trace(': %s', ' '.join(command))
+      p = subprocess.Popen(command)
+    except Exception as e:
+      self._master_broken.value = True
+      print('\nwarn: cannot enable ssh control master for %s:%s\n%s'
+            % (host, port, str(e)), file=sys.stderr)
+      return False
+
+    time.sleep(1)
+    ssh_died = (p.poll() is not None)
+    if ssh_died:
+      return False
+
+    self.add_master(p)
+    self._master_keys[key] = True
+    return True
+
+  def _open(self, host, port=None):
+    """Make sure a ssh master session exists for |host| & |port|.
+
+    If one doesn't exist already, we'll create it.
+
+    This will obtain any necessary locks to avoid inter-process races.
+    """
+    # Bail before grabbing the lock if we already know that we aren't going to
+    # try creating new masters below.
+    if sys.platform in ('win32', 'cygwin'):
+      return False
+
+    # Acquire the lock.  This is needed to prevent opening multiple masters for
+    # the same host when we're running "repo sync -jN" (for N > 1) _and_ the
+    # manifest <remote fetch="ssh://xyz"> specifies a different host from the
+    # one that was passed to repo init.
+    with self._lock:
+      return self._open_unlocked(host, port)
+
+  def preconnect(self, url):
+    """If |uri| will create a ssh connection, setup the ssh master for it."""
+    m = URI_ALL.match(url)
+    if m:
+      scheme = m.group(1)
+      host = m.group(2)
+      if ':' in host:
+        host, port = host.split(':')
+      else:
+        port = None
+      if scheme in ('ssh', 'git+ssh', 'ssh+git'):
+        return self._open(host, port)
+      return False
+
+    m = URI_SCP.match(url)
+    if m:
+      host = m.group(1)
+      return self._open(host)
+
+    return False
+
+  def sock(self, create=True):
+    """Return the path to the ssh socket dir.
+
+    This has all the master sockets so clients can talk to them.
+    """
+    if self._sock_path is None:
+      if not create:
+        return None
+      tmp_dir = '/tmp'
+      if not os.path.exists(tmp_dir):
+        tmp_dir = tempfile.gettempdir()
+      if version() < (6, 7):
+        tokens = '%r@%h:%p'
+      else:
+        tokens = '%C'  # hash of %l%h%p%r
+      self._sock_path = os.path.join(
+          tempfile.mkdtemp('', 'ssh-', tmp_dir),
+          'master-' + tokens)
+    return self._sock_path

subcmds/diff.py

@@ -33,7 +33,7 @@ to the Unix 'patch' command.
   def _Options(self, p):
     p.add_option('-u', '--absolute',
                  dest='absolute', action='store_true',
-                 help='Paths are relative to the repository root')
+                 help='paths are relative to the repository root')
 
   def _ExecuteOne(self, absolute, project):
     """Obtains the diff for a specific project.

subcmds/diffmanifests.py

@@ -68,10 +68,10 @@ synced and their revisions won't be found.
   def _Options(self, p):
     p.add_option('--raw',
                  dest='raw', action='store_true',
-                 help='Display raw diff.')
+                 help='display raw diff')
     p.add_option('--no-color',
                  dest='color', action='store_false', default=True,
-                 help='does not display the diff in color.')
+                 help='does not display the diff in color')
     p.add_option('--pretty-format',
                  dest='pretty_format', action='store',
                  metavar='<FORMAT>',

subcmds/forall.py

@@ -131,30 +131,30 @@ without iterating through the remaining projects.
   def _Options(self, p):
     p.add_option('-r', '--regex',
                  dest='regex', action='store_true',
-                 help="Execute the command only on projects matching regex or wildcard expression")
+                 help='execute the command only on projects matching regex or wildcard expression')
     p.add_option('-i', '--inverse-regex',
                  dest='inverse_regex', action='store_true',
-                 help="Execute the command only on projects not matching regex or "
+                 help='execute the command only on projects not matching regex or '
-                      "wildcard expression")
+                      'wildcard expression')
     p.add_option('-g', '--groups',
                  dest='groups',
-                 help="Execute the command only on projects matching the specified groups")
+                 help='execute the command only on projects matching the specified groups')
     p.add_option('-c', '--command',
-                 help='Command (and arguments) to execute',
+                 help='command (and arguments) to execute',
                  dest='command',
                  action='callback',
                  callback=self._cmd_option)
     p.add_option('-e', '--abort-on-errors',
                  dest='abort_on_errors', action='store_true',
-                 help='Abort if a command exits unsuccessfully')
+                 help='abort if a command exits unsuccessfully')
     p.add_option('--ignore-missing', action='store_true',
-                 help='Silently skip & do not exit non-zero due missing '
+                 help='silently skip & do not exit non-zero due missing '
                       'checkouts')
 
     g = p.get_option_group('--quiet')
     g.add_option('-p',
                  dest='project_header', action='store_true',
-                 help='Show project headers before output')
+                 help='show project headers before output')
     p.add_option('--interactive',
                  action='store_true',
                  help='force interactive usage')

subcmds/gitc_delete.py

@@ -33,7 +33,7 @@ and all locally downloaded sources.
   def _Options(self, p):
     p.add_option('-f', '--force',
                  dest='force', action='store_true',
-                 help='Force the deletion (no prompt).')
+                 help='force the deletion (no prompt)')
 
   def Execute(self, opt, args):
     if not opt.force:

subcmds/help.py

@@ -20,6 +20,7 @@ from subcmds import all_commands
 from color import Coloring
 from command import PagedCommand, MirrorSafeCommand, GitcAvailableCommand, GitcClientCommand
 import gitc_utils
+from wrapper import Wrapper
 
 
 class Help(PagedCommand, MirrorSafeCommand):
@@ -78,6 +79,7 @@ Displays detailed usage information about a command.
     print(
         "See 'repo help <command>' for more information on a specific command.\n"
         "See 'repo help --all' for a complete list of recognized commands.")
+    print('Bug reports:', Wrapper().BUG_URL)
 
   def _PrintCommandHelp(self, cmd, header_prefix=''):
     class _Out(Coloring):

subcmds/info.py

@@ -12,6 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import optparse
+
 from command import PagedCommand
 from color import Coloring
 from git_refs import R_M, R_HEADS
@@ -25,7 +27,7 @@ class _Coloring(Coloring):
 class Info(PagedCommand):
   common = True
   helpSummary = "Get info on the manifest branch, current branch or unmerged branches"
-  helpUsage = "%prog [-dl] [-o [-b]] [<project>...]"
+  helpUsage = "%prog [-dl] [-o [-c]] [<project>...]"
 
   def _Options(self, p):
     p.add_option('-d', '--diff',
@@ -34,12 +36,19 @@ class Info(PagedCommand):
     p.add_option('-o', '--overview',
                  dest='overview', action='store_true',
                  help='show overview of all local commits')
-    p.add_option('-b', '--current-branch',
+    p.add_option('-c', '--current-branch',
                  dest="current_branch", action="store_true",
                  help="consider only checked out branches")
+    p.add_option('--no-current-branch',
+                 dest='current_branch', action='store_false',
+                 help='consider all local branches')
+    # Turn this into a warning & remove this someday.
+    p.add_option('-b',
+                 dest='current_branch', action='store_true',
+                 help=optparse.SUPPRESS_HELP)
     p.add_option('-l', '--local-only',
                  dest="local", action="store_true",
-                 help="Disable all remote operations")
+                 help="disable all remote operations")
 
   def Execute(self, opt, args):
     self.out = _Coloring(self.client.globalConfig)

subcmds/list.py

@@ -36,22 +36,22 @@ This is similar to running: repo forall -c 'echo "$REPO_PATH : $REPO_PROJECT"'.
   def _Options(self, p):
     p.add_option('-r', '--regex',
                  dest='regex', action='store_true',
-                 help="Filter the project list based on regex or wildcard matching of strings")
+                 help='filter the project list based on regex or wildcard matching of strings')
     p.add_option('-g', '--groups',
                  dest='groups',
-                 help="Filter the project list based on the groups the project is in")
+                 help='filter the project list based on the groups the project is in')
     p.add_option('-a', '--all',
                  action='store_true',
-                 help='Show projects regardless of checkout state')
+                 help='show projects regardless of checkout state')
     p.add_option('-f', '--fullpath',
                  dest='fullpath', action='store_true',
-                 help="Display the full work tree path instead of the relative path")
+                 help='display the full work tree path instead of the relative path')
     p.add_option('-n', '--name-only',
                  dest='name_only', action='store_true',
-                 help="Display only the name of the repository")
+                 help='display only the name of the repository')
     p.add_option('-p', '--path-only',
                  dest='path_only', action='store_true',
-                 help="Display only the path of the repository")
+                 help='display only the path of the repository')
 
   def ValidateOptions(self, opt, args):
     if opt.fullpath and opt.name_only:

subcmds/manifest.py

@@ -53,27 +53,27 @@ to indicate the remote ref to push changes to via 'repo upload'.
   def _Options(self, p):
     p.add_option('-r', '--revision-as-HEAD',
                  dest='peg_rev', action='store_true',
-                 help='Save revisions as current HEAD')
+                 help='save revisions as current HEAD')
     p.add_option('-m', '--manifest-name',
                  help='temporary manifest to use for this sync', metavar='NAME.xml')
     p.add_option('--suppress-upstream-revision', dest='peg_rev_upstream',
                  default=True, action='store_false',
-                 help='If in -r mode, do not write the upstream field.  '
+                 help='if in -r mode, do not write the upstream field '
-                 'Only of use if the branch names for a sha1 manifest are '
+                 '(only of use if the branch names for a sha1 manifest are '
-                 'sensitive.')
+                 'sensitive)')
     p.add_option('--suppress-dest-branch', dest='peg_rev_dest_branch',
                  default=True, action='store_false',
-                 help='If in -r mode, do not write the dest-branch field.  '
+                 help='if in -r mode, do not write the dest-branch field '
-                 'Only of use if the branch names for a sha1 manifest are '
+                 '(only of use if the branch names for a sha1 manifest are '
-                 'sensitive.')
+                 'sensitive)')
     p.add_option('--json', default=False, action='store_true',
-                 help='Output manifest in JSON format (experimental).')
+                 help='output manifest in JSON format (experimental)')
     p.add_option('--pretty', default=False, action='store_true',
-                 help='Format output for humans to read.')
+                 help='format output for humans to read')
     p.add_option('-o', '--output-file',
                  dest='output_file',
                  default='-',
-                 help='File to save the manifest to',
+                 help='file to save the manifest to',
                  metavar='-|NAME.xml')
 
   def _Output(self, opt):

subcmds/overview.py

@@ -12,6 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import optparse
+
 from color import Coloring
 from command import PagedCommand
 
@@ -26,15 +28,22 @@ class Overview(PagedCommand):
 The '%prog' command is used to display an overview of the projects branches,
 and list any local commits that have not yet been merged into the project.
 
-The -b/--current-branch option can be used to restrict the output to only
+The -c/--current-branch option can be used to restrict the output to only
 branches currently checked out in each project.  By default, all branches
 are displayed.
 """
 
   def _Options(self, p):
-    p.add_option('-b', '--current-branch',
+    p.add_option('-c', '--current-branch',
                  dest="current_branch", action="store_true",
-                 help="Consider only checked out branches")
+                 help="consider only checked out branches")
+    p.add_option('--no-current-branch',
+                 dest='current_branch', action='store_false',
+                 help='consider all local branches')
+    # Turn this into a warning & remove this someday.
+    p.add_option('-b',
+                 dest='current_branch', action='store_true',
+                 help=optparse.SUPPRESS_HELP)
 
   def Execute(self, opt, args):
     all_branches = []

subcmds/rebase.py

@@ -46,27 +46,27 @@ branch but need to incorporate new upstream changes "underneath" them.
 
     p.add_option('--fail-fast',
                  dest='fail_fast', action='store_true',
-                 help='Stop rebasing after first error is hit')
+                 help='stop rebasing after first error is hit')
     p.add_option('-f', '--force-rebase',
                  dest='force_rebase', action='store_true',
-                 help='Pass --force-rebase to git rebase')
+                 help='pass --force-rebase to git rebase')
     p.add_option('--no-ff',
                  dest='ff', default=True, action='store_false',
-                 help='Pass --no-ff to git rebase')
+                 help='pass --no-ff to git rebase')
     p.add_option('--autosquash',
                  dest='autosquash', action='store_true',
-                 help='Pass --autosquash to git rebase')
+                 help='pass --autosquash to git rebase')
     p.add_option('--whitespace',
                  dest='whitespace', action='store', metavar='WS',
-                 help='Pass --whitespace to git rebase')
+                 help='pass --whitespace to git rebase')
     p.add_option('--auto-stash',
                  dest='auto_stash', action='store_true',
-                 help='Stash local modifications before starting')
+                 help='stash local modifications before starting')
     p.add_option('-m', '--onto-manifest',
                  dest='onto_manifest', action='store_true',
-                 help='Rebase onto the manifest version instead of upstream '
+                 help='rebase onto the manifest version instead of upstream '
-                      'HEAD.  This helps to make sure the local tree stays '
+                      'HEAD (this helps to make sure the local tree stays '
-                      'consistent if you previously synced to a manifest.')
+                      'consistent if you previously synced to a manifest)')
 
   def Execute(self, opt, args):
     all_projects = self.GetProjects(args)

subcmds/sync.py

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import errno
 import functools
 import http.cookiejar as cookielib
 import io
@@ -51,11 +52,12 @@ import git_superproject
 import gitc_utils
 from project import Project
 from project import RemoteSpec
-from command import Command, MirrorSafeCommand
+from command import Command, MirrorSafeCommand, WORKER_BATCH_SIZE
 from error import RepoChangedException, GitError, ManifestParseError
 import platform_utils
 from project import SyncBuffer
 from progress import Progress
+import ssh
 from wrapper import Wrapper
 from manifest_xml import GitcManifest
 
@@ -168,10 +170,11 @@ later is required to fix a server side protocol bug.
   PARALLEL_JOBS = 1
 
   def _CommonOptions(self, p):
-    try:
+    if self.manifest:
-      self.PARALLEL_JOBS = self.manifest.default.sync_j
+      try:
-    except ManifestParseError:
+        self.PARALLEL_JOBS = self.manifest.default.sync_j
-      pass
+      except ManifestParseError:
+        pass
     super()._CommonOptions(p)
 
   def _Options(self, p, show_smart=True):
@@ -212,6 +215,9 @@ later is required to fix a server side protocol bug.
     p.add_option('-c', '--current-branch',
                  dest='current_branch_only', action='store_true',
                  help='fetch only current branch from server')
+    p.add_option('--no-current-branch',
+                 dest='current_branch_only', action='store_false',
+                 help='fetch all branches from server')
     p.add_option('-m', '--manifest-name',
                  dest='manifest_name',
                  help='temporary manifest to use for this sync', metavar='NAME.xml')
@@ -230,8 +236,14 @@ later is required to fix a server side protocol bug.
                  help='fetch submodules from server')
     p.add_option('--use-superproject', action='store_true',
                  help='use the manifest superproject to sync projects')
+    p.add_option('--no-use-superproject', action='store_false',
+                 dest='use_superproject',
+                 help='disable use of manifest superprojects')
+    p.add_option('--tags',
+                 action='store_false',
+                 help='fetch tags')
     p.add_option('--no-tags',
-                 dest='tags', default=True, action='store_false',
+                 dest='tags', action='store_false',
                  help="don't fetch tags")
     p.add_option('--optimized-fetch',
                  dest='optimized_fetch', action='store_true',
@@ -268,15 +280,16 @@ later is required to fix a server side protocol bug.
 
   def _UseSuperproject(self, opt):
     """Returns True if use-superproject option is enabled"""
-    return (opt.use_superproject or
+    if opt.use_superproject is not None:
-            self.manifest.manifestProject.config.GetBoolean(
+      return opt.use_superproject
-                'repo.superproject'))
+    else:
+      return self.manifest.manifestProject.config.GetBoolean('repo.superproject')
 
   def _GetCurrentBranchOnly(self, opt):
     """Returns True if current-branch or use-superproject options are enabled."""
     return opt.current_branch_only or self._UseSuperproject(opt)
 
-  def _UpdateProjectsRevisionId(self, opt, args):
+  def _UpdateProjectsRevisionId(self, opt, args, load_local_manifests):
     """Update revisionId of every project with the SHA from superproject.
 
     This function updates each project's revisionId with SHA from superproject.
@@ -286,6 +299,7 @@ later is required to fix a server side protocol bug.
       opt: Program options returned from optparse.  See _Options().
       args: Arguments to pass to GetProjects. See the GetProjects
           docstring for details.
+      load_local_manifests: Whether to load local manifests.
 
     Returns:
       Returns path to the overriding manifest file.
@@ -298,10 +312,11 @@ later is required to fix a server side protocol bug.
                                     submodules_ok=opt.fetch_submodules)
     manifest_path = superproject.UpdateProjectsRevisionId(all_projects)
     if not manifest_path:
-      print('error: Update of revsionId from superproject has failed',
+      print('error: Update of revsionId from superproject has failed. '
+            'Please resync with --no-use-superproject option',
             file=sys.stderr)
       sys.exit(1)
-    self._ReloadManifest(manifest_path)
+    self._ReloadManifest(manifest_path, load_local_manifests)
     return manifest_path
 
   def _FetchProjectList(self, opt, projects):
@@ -343,6 +358,7 @@ later is required to fix a server side protocol bug.
           optimized_fetch=opt.optimized_fetch,
           retry_fetches=opt.retry_fetches,
           prune=opt.prune,
+          ssh_proxy=self.ssh_proxy,
           clone_filter=self.manifest.CloneFilter,
           partial_clone_exclude=self.manifest.PartialCloneExclude)
 
@@ -364,7 +380,11 @@ later is required to fix a server side protocol bug.
     finish = time.time()
     return (success, project, start, finish)
 
-  def _Fetch(self, projects, opt, err_event):
+  @classmethod
+  def _FetchInitChild(cls, ssh_proxy):
+    cls.ssh_proxy = ssh_proxy
+
+  def _Fetch(self, projects, opt, err_event, ssh_proxy):
     ret = True
 
     jobs = opt.jobs_network if opt.jobs_network else self.jobs
@@ -394,8 +414,14 @@ later is required to fix a server side protocol bug.
           break
       return ret
 
+    # We pass the ssh proxy settings via the class.  This allows multiprocessing
+    # to pickle it up when spawning children.  We can't pass it as an argument
+    # to _FetchProjectList below as multiprocessing is unable to pickle those.
+    Sync.ssh_proxy = None
+
     # NB: Multiprocessing is heavy, so don't spin it up for one job.
     if len(projects_list) == 1 or jobs == 1:
+      self._FetchInitChild(ssh_proxy)
       if not _ProcessResults(self._FetchProjectList(opt, x) for x in projects_list):
         ret = False
     else:
@@ -413,7 +439,8 @@ later is required to fix a server side protocol bug.
       else:
         pm.update(inc=0, msg='warming up')
         chunksize = 4
-      with multiprocessing.Pool(jobs) as pool:
+      with multiprocessing.Pool(
+          jobs, initializer=self._FetchInitChild, initargs=(ssh_proxy,)) as pool:
         results = pool.imap_unordered(
             functools.partial(self._FetchProjectList, opt),
             projects_list,
@@ -422,6 +449,11 @@ later is required to fix a server side protocol bug.
           ret = False
           pool.close()
 
+    # Cleanup the reference now that we're done with it, and we're going to
+    # release any resources it points to.  If we don't, later multiprocessing
+    # usage (e.g. checkouts) will try to pickle and then crash.
+    del Sync.ssh_proxy
+
     pm.end()
     self._fetch_times.Save()
 
@@ -430,6 +462,64 @@ later is required to fix a server side protocol bug.
 
     return (ret, fetched)
 
+  def _FetchMain(self, opt, args, all_projects, err_event, manifest_name,
+                 load_local_manifests, ssh_proxy):
+    """The main network fetch loop.
+
+    Args:
+      opt: Program options returned from optparse.  See _Options().
+      args: Command line args used to filter out projects.
+      all_projects: List of all projects that should be checked out.
+      err_event: Whether an error was hit while processing.
+      manifest_name: Manifest file to be reloaded.
+      load_local_manifests: Whether to load local manifests.
+      ssh_proxy: SSH manager for clients & masters.
+    """
+    rp = self.manifest.repoProject
+
+    to_fetch = []
+    now = time.time()
+    if _ONE_DAY_S <= (now - rp.LastFetch):
+      to_fetch.append(rp)
+    to_fetch.extend(all_projects)
+    to_fetch.sort(key=self._fetch_times.Get, reverse=True)
+
+    success, fetched = self._Fetch(to_fetch, opt, err_event, ssh_proxy)
+    if not success:
+      err_event.set()
+
+    _PostRepoFetch(rp, opt.repo_verify)
+    if opt.network_only:
+      # bail out now; the rest touches the working tree
+      if err_event.is_set():
+        print('\nerror: Exited sync due to fetch errors.\n', file=sys.stderr)
+        sys.exit(1)
+      return
+
+    # Iteratively fetch missing and/or nested unregistered submodules
+    previously_missing_set = set()
+    while True:
+      self._ReloadManifest(manifest_name, load_local_manifests)
+      all_projects = self.GetProjects(args,
+                                      missing_ok=True,
+                                      submodules_ok=opt.fetch_submodules)
+      missing = []
+      for project in all_projects:
+        if project.gitdir not in fetched:
+          missing.append(project)
+      if not missing:
+        break
+      # Stop us from non-stopped fetching actually-missing repos: If set of
+      # missing repos has not been changed from last fetch, we break.
+      missing_set = set(p.name for p in missing)
+      if previously_missing_set == missing_set:
+        break
+      previously_missing_set = missing_set
+      success, new_fetched = self._Fetch(missing, opt, err_event, ssh_proxy)
+      if not success:
+        err_event.set()
+      fetched.update(new_fetched)
+
   def _CheckoutOne(self, detach_head, force_sync, project):
     """Checkout work tree for one project
 
@@ -564,10 +654,18 @@ later is required to fix a server side protocol bug.
       t.join()
     pm.end()
 
-  def _ReloadManifest(self, manifest_name=None):
+  def _ReloadManifest(self, manifest_name=None, load_local_manifests=True):
+    """Reload the manfiest from the file specified by the |manifest_name|.
+
+    It unloads the manifest if |manifest_name| is None.
+
+    Args:
+      manifest_name: Manifest file to be reloaded.
+      load_local_manifests: Whether to load local manifests.
+    """
     if manifest_name:
       # Override calls _Unload already
-      self.manifest.Override(manifest_name)
+      self.manifest.Override(manifest_name, load_local_manifests=load_local_manifests)
     else:
       self.manifest._Unload()
 
@@ -614,6 +712,60 @@ later is required to fix a server side protocol bug.
       fd.write('\n')
     return 0
 
+  def UpdateCopyLinkfileList(self):
+    """Save all dests of copyfile and linkfile, and update them if needed.
+
+    Returns:
+      Whether update was successful.
+    """
+    new_paths = {}
+    new_linkfile_paths = []
+    new_copyfile_paths = []
+    for project in self.GetProjects(None, missing_ok=True):
+      new_linkfile_paths.extend(x.dest for x in project.linkfiles)
+      new_copyfile_paths.extend(x.dest for x in project.copyfiles)
+
+    new_paths = {
+        'linkfile': new_linkfile_paths,
+        'copyfile': new_copyfile_paths,
+    }
+
+    copylinkfile_name = 'copy-link-files.json'
+    copylinkfile_path = os.path.join(self.manifest.repodir, copylinkfile_name)
+    old_copylinkfile_paths = {}
+
+    if os.path.exists(copylinkfile_path):
+      with open(copylinkfile_path, 'rb') as fp:
+        try:
+          old_copylinkfile_paths = json.load(fp)
+        except:
+          print('error: %s is not a json formatted file.' %
+                copylinkfile_path, file=sys.stderr)
+          platform_utils.remove(copylinkfile_path)
+          return False
+
+      need_remove_files = []
+      need_remove_files.extend(
+          set(old_copylinkfile_paths.get('linkfile', [])) -
+          set(new_linkfile_paths))
+      need_remove_files.extend(
+          set(old_copylinkfile_paths.get('copyfile', [])) -
+          set(new_copyfile_paths))
+
+      for need_remove_file in need_remove_files:
+        try:
+          platform_utils.remove(need_remove_file)
+        except OSError as e:
+          if e.errno == errno.ENOENT:
+            # Try to remove the updated copyfile or linkfile.
+            # So, if the file is not exist, nothing need to do.
+            pass
+
+    # Create copy-link-files.json, save dest path of "copyfile" and "linkfile".
+    with open(copylinkfile_path, 'w', encoding='utf-8') as fp:
+      json.dump(new_paths, fp)
+    return True
+
   def _SmartSyncSetup(self, opt, smart_sync_manifest_path):
     if not self.manifest.manifest_server:
       print('error: cannot smart sync: no manifest server defined in '
@@ -730,7 +882,7 @@ later is required to fix a server side protocol bug.
                              start, time.time(), clean)
       if not clean:
         sys.exit(1)
-      self._ReloadManifest(opt.manifest_name)
+      self._ReloadManifest(manifest_name)
       if opt.jobs is None:
         self.jobs = self.manifest.default.sync_j
 
@@ -779,7 +931,7 @@ later is required to fix a server side protocol bug.
           print('error: failed to remove existing smart sync override manifest: %s' %
                 e, file=sys.stderr)
 
-    err_event = _threading.Event()
+    err_event = multiprocessing.Event()
 
     rp = self.manifest.repoProject
     rp.PreSync()
@@ -802,8 +954,9 @@ later is required to fix a server side protocol bug.
     else:
       self._UpdateManifestProject(opt, mp, manifest_name)
 
+    load_local_manifests = not self.manifest.HasLocalManifests
     if self._UseSuperproject(opt):
-      manifest_name = self._UpdateProjectsRevisionId(opt, args)
+      manifest_name = self._UpdateProjectsRevisionId(opt, args, load_local_manifests)
 
     if self.gitc_manifest:
       gitc_manifest_projects = self.GetProjects(args,
@@ -849,49 +1002,16 @@ later is required to fix a server side protocol bug.
 
     self._fetch_times = _FetchTimes(self.manifest)
     if not opt.local_only:
-      to_fetch = []
+      with multiprocessing.Manager() as manager:
-      now = time.time()
+        with ssh.ProxyManager(manager) as ssh_proxy:
-      if _ONE_DAY_S <= (now - rp.LastFetch):
+          # Initialize the socket dir once in the parent.
-        to_fetch.append(rp)
+          ssh_proxy.sock()
-      to_fetch.extend(all_projects)
+          self._FetchMain(opt, args, all_projects, err_event, manifest_name,
-      to_fetch.sort(key=self._fetch_times.Get, reverse=True)
+                          load_local_manifests, ssh_proxy)
 
-      success, fetched = self._Fetch(to_fetch, opt, err_event)
-      if not success:
-        err_event.set()
-
-      _PostRepoFetch(rp, opt.repo_verify)
       if opt.network_only:
-        # bail out now; the rest touches the working tree
-        if err_event.is_set():
-          print('\nerror: Exited sync due to fetch errors.\n', file=sys.stderr)
-          sys.exit(1)
         return
 
-      # Iteratively fetch missing and/or nested unregistered submodules
-      previously_missing_set = set()
-      while True:
-        self._ReloadManifest(manifest_name)
-        all_projects = self.GetProjects(args,
-                                        missing_ok=True,
-                                        submodules_ok=opt.fetch_submodules)
-        missing = []
-        for project in all_projects:
-          if project.gitdir not in fetched:
-            missing.append(project)
-        if not missing:
-          break
-        # Stop us from non-stopped fetching actually-missing repos: If set of
-        # missing repos has not been changed from last fetch, we break.
-        missing_set = set(p.name for p in missing)
-        if previously_missing_set == missing_set:
-          break
-        previously_missing_set = missing_set
-        success, new_fetched = self._Fetch(to_fetch, opt, err_event)
-        if not success:
-          err_event.set()
-        fetched.update(new_fetched)
-
       # If we saw an error, exit with code 1 so that other scripts can check.
       if err_event.is_set():
         err_network_sync = True
@@ -914,6 +1034,13 @@ later is required to fix a server side protocol bug.
         print('\nerror: Local checkouts *not* updated.', file=sys.stderr)
         sys.exit(1)
 
+    err_update_linkfiles = not self.UpdateCopyLinkfileList()
+    if err_update_linkfiles:
+      err_event.set()
+      if opt.fail_fast:
+        print('\nerror: Local update copyfile or linkfile failed.', file=sys.stderr)
+        sys.exit(1)
+
     err_results = []
     # NB: We don't exit here because this is the last step.
     err_checkout = not self._Checkout(all_projects, opt, err_results)
@@ -932,6 +1059,8 @@ later is required to fix a server side protocol bug.
         print('error: Downloading network changes failed.', file=sys.stderr)
       if err_update_projects:
         print('error: Updating local project lists failed.', file=sys.stderr)
+      if err_update_linkfiles:
+        print('error: Updating copyfiles or linkfiles failed.', file=sys.stderr)
       if err_checkout:
         print('error: Checking out local projects failed.', file=sys.stderr)
         if err_results:

subcmds/upload.py

@@ -13,10 +13,12 @@
 # limitations under the License.
 
 import copy
+import functools
+import optparse
 import re
 import sys
 
-from command import InteractiveCommand
+from command import DEFAULT_LOCAL_JOBS, InteractiveCommand
 from editor import Editor
 from error import UploadError
 from git_command import GitCommand
@@ -145,58 +147,66 @@ https://gerrit-review.googlesource.com/Documentation/user-upload.html#notify
 Gerrit Code Review:  https://www.gerritcodereview.com/
 
 """
+  PARALLEL_JOBS = DEFAULT_LOCAL_JOBS
 
   def _Options(self, p):
     p.add_option('-t',
                  dest='auto_topic', action='store_true',
-                 help='Send local branch name to Gerrit Code Review')
+                 help='send local branch name to Gerrit Code Review')
     p.add_option('--hashtag', '--ht',
                  dest='hashtags', action='append', default=[],
-                 help='Add hashtags (comma delimited) to the review.')
+                 help='add hashtags (comma delimited) to the review')
     p.add_option('--hashtag-branch', '--htb',
                  action='store_true',
-                 help='Add local branch name as a hashtag.')
+                 help='add local branch name as a hashtag')
     p.add_option('-l', '--label',
                  dest='labels', action='append', default=[],
-                 help='Add a label when uploading.')
+                 help='add a label when uploading')
     p.add_option('--re', '--reviewers',
                  type='string', action='append', dest='reviewers',
-                 help='Request reviews from these people.')
+                 help='request reviews from these people')
     p.add_option('--cc',
                  type='string', action='append', dest='cc',
-                 help='Also send email to these email addresses.')
+                 help='also send email to these email addresses')
-    p.add_option('--br',
+    p.add_option('--br', '--branch',
                  type='string', action='store', dest='branch',
-                 help='Branch to upload.')
+                 help='(local) branch to upload')
-    p.add_option('--cbr', '--current-branch',
+    p.add_option('-c', '--current-branch',
                  dest='current_branch', action='store_true',
-                 help='Upload current git branch.')
+                 help='upload current git branch')
+    p.add_option('--no-current-branch',
+                 dest='current_branch', action='store_false',
+                 help='upload all git branches')
+    # Turn this into a warning & remove this someday.
+    p.add_option('--cbr',
+                 dest='current_branch', action='store_true',
+                 help=optparse.SUPPRESS_HELP)
     p.add_option('--ne', '--no-emails',
                  action='store_false', dest='notify', default=True,
-                 help='If specified, do not send emails on upload.')
+                 help='do not send e-mails on upload')
     p.add_option('-p', '--private',
                  action='store_true', dest='private', default=False,
-                 help='If specified, upload as a private change.')
+                 help='upload as a private change (deprecated; use --wip)')
     p.add_option('-w', '--wip',
                  action='store_true', dest='wip', default=False,
-                 help='If specified, upload as a work-in-progress change.')
+                 help='upload as a work-in-progress change')
     p.add_option('-o', '--push-option',
                  type='string', action='append', dest='push_options',
                  default=[],
-                 help='Additional push options to transmit')
+                 help='additional push options to transmit')
     p.add_option('-D', '--destination', '--dest',
                  type='string', action='store', dest='dest_branch',
                  metavar='BRANCH',
-                 help='Submit for review on this target branch.')
+                 help='submit for review on this target branch')
     p.add_option('-n', '--dry-run',
                  dest='dryrun', default=False, action='store_true',
-                 help='Do everything except actually upload the CL.')
+                 help='do everything except actually upload the CL')
     p.add_option('-y', '--yes',
                  default=False, action='store_true',
-                 help='Answer yes to all safe prompts.')
+                 help='answer yes to all safe prompts')
     p.add_option('--no-cert-checks',
                  dest='validate_certs', action='store_false', default=True,
-                 help='Disable verifying ssl certs (unsafe).')
+                 help='disable verifying ssl certs (unsafe)')
     RepoHook.AddOptionGroup(p, 'pre-upload')
 
   def _SingleBranch(self, opt, branch, people):
@@ -502,40 +512,46 @@ Gerrit Code Review:  https://www.gerritcodereview.com/
     merge_branch = p.stdout.strip()
     return merge_branch
 
+  @staticmethod
+  def _GatherOne(opt, project):
+    """Figure out the upload status for |project|."""
+    if opt.current_branch:
+      cbr = project.CurrentBranch
+      up_branch = project.GetUploadableBranch(cbr)
+      avail = [up_branch] if up_branch else None
+    else:
+      avail = project.GetUploadableBranches(opt.branch)
+    return (project, avail)
+
   def Execute(self, opt, args):
-    project_list = self.GetProjects(args)
+    projects = self.GetProjects(args)
-    pending = []
-    reviewers = []
-    cc = []
-    branch = None
 
-    if opt.branch:
+    def _ProcessResults(_pool, _out, results):
-      branch = opt.branch
+      pending = []
-
+      for result in results:
-    for project in project_list:
+        project, avail = result
-      if opt.current_branch:
+        if avail is None:
-        cbr = project.CurrentBranch
+          print('repo: error: %s: Unable to upload branch "%s". '
-        up_branch = project.GetUploadableBranch(cbr)
-        if up_branch:
-          avail = [up_branch]
-        else:
-          avail = None
-          print('repo: error: Unable to upload branch "%s". '
                 'You might be able to fix the branch by running:\n'
                 '  git branch --set-upstream-to m/%s' %
-                (str(cbr), self.manifest.branch),
+                (project.relpath, project.CurrentBranch, self.manifest.branch),
                 file=sys.stderr)
-      else:
+        elif avail:
-        avail = project.GetUploadableBranches(branch)
+          pending.append(result)
-      if avail:
+      return pending
-        pending.append((project, avail))
+
+    pending = self.ExecuteInParallel(
+        opt.jobs,
+        functools.partial(self._GatherOne, opt),
+        projects,
+        callback=_ProcessResults)
 
     if not pending:
-      if branch is None:
+      if opt.branch is None:
         print('repo: error: no branches ready for upload', file=sys.stderr)
       else:
         print('repo: error: no branches named "%s" ready for upload' %
-              (branch,), file=sys.stderr)
+              (opt.branch,), file=sys.stderr)
       return 1
 
     pending_proj_names = [project.name for (project, available) in pending]
@@ -548,10 +564,8 @@ Gerrit Code Review:  https://www.gerritcodereview.com/
         worktree_list=pending_worktrees):
       return 1
 
-    if opt.reviewers:
+    reviewers = _SplitEmails(opt.reviewers) if opt.reviewers else []
-      reviewers = _SplitEmails(opt.reviewers)
+    cc = _SplitEmails(opt.cc) if opt.cc else []
-    if opt.cc:
-      cc = _SplitEmails(opt.cc)
     people = (reviewers, cc)
 
     if len(pending) == 1 and len(pending[0][1]) == 1:

subcmds/version.py

@@ -18,6 +18,7 @@ import sys
 from command import Command, MirrorSafeCommand
 from git_command import git, RepoSourceVersion, user_agent
 from git_refs import HEAD
+from wrapper import Wrapper
 
 
 class Version(Command, MirrorSafeCommand):
@@ -62,3 +63,4 @@ class Version(Command, MirrorSafeCommand):
       print('OS %s %s (%s)' % (uname.system, uname.release, uname.version))
       print('CPU %s (%s)' %
             (uname.machine, uname.processor if uname.processor else 'unknown'))
+    print('Bug reports:', Wrapper().BUG_URL)

tests/test_git_command.py

@@ -26,33 +26,6 @@ import git_command
 import wrapper
 
 
-class SSHUnitTest(unittest.TestCase):
-  """Tests the ssh functions."""
-
-  def test_ssh_version(self):
-    """Check ssh_version() handling."""
-    ver = git_command._parse_ssh_version('Unknown\n')
-    self.assertEqual(ver, ())
-    ver = git_command._parse_ssh_version('OpenSSH_1.0\n')
-    self.assertEqual(ver, (1, 0))
-    ver = git_command._parse_ssh_version('OpenSSH_6.6.1p1 Ubuntu-2ubuntu2.13, OpenSSL 1.0.1f 6 Jan 2014\n')
-    self.assertEqual(ver, (6, 6, 1))
-    ver = git_command._parse_ssh_version('OpenSSH_7.6p1 Ubuntu-4ubuntu0.3, OpenSSL 1.0.2n  7 Dec 2017\n')
-    self.assertEqual(ver, (7, 6))
-
-  def test_ssh_sock(self):
-    """Check ssh_sock() function."""
-    with mock.patch('tempfile.mkdtemp', return_value='/tmp/foo'):
-      # old ssh version uses port
-      with mock.patch('git_command.ssh_version', return_value=(6, 6)):
-        self.assertTrue(git_command.ssh_sock().endswith('%p'))
-      git_command._ssh_sock_path = None
-      # new ssh version uses hash
-      with mock.patch('git_command.ssh_version', return_value=(6, 7)):
-        self.assertTrue(git_command.ssh_sock().endswith('%C'))
-      git_command._ssh_sock_path = None
-
-
 class GitCallUnitTest(unittest.TestCase):
   """Tests the _GitCall class (via git_command.git)."""
 

tests/test_git_superproject.py

@@ -141,12 +141,12 @@ class SuperprojectTestCase(unittest.TestCase):
       manifest_xml = fp.read()
     self.assertEqual(
         manifest_xml,
-        '<?xml version="1.0" ?><manifest>' +
+        '<?xml version="1.0" ?><manifest>'
-        '<remote name="default-remote" fetch="http://localhost"/>' +
+        '<remote name="default-remote" fetch="http://localhost"/>'
-        '<default remote="default-remote" revision="refs/heads/main"/>' +
+        '<default remote="default-remote" revision="refs/heads/main"/>'
-        '<project name="platform/art" path="art" revision="ABCDEF" ' +
+        '<project name="platform/art" path="art" revision="ABCDEF" '
-        'groups="notdefault,platform-' + self.platform + '"/>' +
+        'groups="notdefault,platform-' + self.platform + '"/>'
-        '<superproject name="superproject"/>' +
+        '<superproject name="superproject"/>'
         '</manifest>')
 
   def test_superproject_update_project_revision_id(self):
@@ -168,13 +168,57 @@ class SuperprojectTestCase(unittest.TestCase):
             manifest_xml = fp.read()
           self.assertEqual(
               manifest_xml,
-              '<?xml version="1.0" ?><manifest>' +
+              '<?xml version="1.0" ?><manifest>'
-              '<remote name="default-remote" fetch="http://localhost"/>' +
+              '<remote name="default-remote" fetch="http://localhost"/>'
-              '<default remote="default-remote" revision="refs/heads/main"/>' +
+              '<default remote="default-remote" revision="refs/heads/main"/>'
-              '<project name="platform/art" path="art" ' +
+              '<project name="platform/art" path="art" '
-              'revision="2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea" ' +
+              'revision="2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea" '
-              'groups="notdefault,platform-' + self.platform + '"/>' +
+              'groups="notdefault,platform-' + self.platform + '"/>'
-              '<superproject name="superproject"/>' +
+              '<superproject name="superproject"/>'
+              '</manifest>')
+
+  def test_superproject_update_project_revision_id_with_different_remotes(self):
+    """Test update of commit ids of a manifest with mutiple remotes."""
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="default-remote" fetch="http://localhost" />
+  <remote name="goog" fetch="http://localhost2" />
+  <default remote="default-remote" revision="refs/heads/main" />
+  <superproject name="superproject"/>
+  <project path="vendor/x" name="platform/vendor/x" remote="goog" groups="vendor"
+           revision="master-with-vendor" clone-depth="1" />
+  <project path="art" name="platform/art" groups="notdefault,platform-""" + self.platform + """
+  " /></manifest>
+""")
+    self.maxDiff = None
+    self._superproject = git_superproject.Superproject(manifest, self.repodir)
+    self.assertEqual(len(self._superproject._manifest.projects), 2)
+    projects = self._superproject._manifest.projects
+    data = ('160000 commit 2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea\tart\x00'
+            '160000 commit e9d25da64d8d365dbba7c8ee00fe8c4473fe9a06\tbootable/recovery\x00')
+    with mock.patch.object(self._superproject, '_Init', return_value=True):
+      with mock.patch.object(self._superproject, '_Fetch', return_value=True):
+        with mock.patch.object(self._superproject,
+                               '_LsTree',
+                               return_value=data):
+          # Create temporary directory so that it can write the file.
+          os.mkdir(self._superproject._superproject_path)
+          manifest_path = self._superproject.UpdateProjectsRevisionId(projects)
+          self.assertIsNotNone(manifest_path)
+          with open(manifest_path, 'r') as fp:
+            manifest_xml = fp.read()
+          self.assertEqual(
+              manifest_xml,
+              '<?xml version="1.0" ?><manifest>'
+              '<remote name="default-remote" fetch="http://localhost"/>'
+              '<remote name="goog" fetch="http://localhost2"/>'
+              '<default remote="default-remote" revision="refs/heads/main"/>'
+              '<project name="platform/art" path="art" '
+              'revision="2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea" '
+              'groups="notdefault,platform-' + self.platform + '"/>'
+              '<project name="platform/vendor/x" path="vendor/x" remote="goog" '
+              'revision="master-with-vendor" groups="vendor" clone-depth="1"/>'
+              '<superproject name="superproject"/>'
               '</manifest>')
 
 

tests/test_manifest_xml.py

@@ -52,6 +52,9 @@ INVALID_FS_PATHS = (
     'blah/foo~',
     # Block Unicode characters that get normalized out by filesystems.
     u'foo\u200Cbar',
+    # Block newlines.
+    'f\n/bar',
+    'f\r/bar',
 )
 
 # Make sure platforms that use path separators (e.g. Windows) are also
@@ -91,6 +94,11 @@ class ManifestParseTestCase(unittest.TestCase):
       fp.write(data)
     return manifest_xml.XmlManifest(self.repodir, self.manifest_file)
 
+  @staticmethod
+  def encodeXmlAttr(attr):
+    """Encode |attr| using XML escape rules."""
+    return attr.replace('\r', '
').replace('\n', '
')
+
 
 class ManifestValidateFilePaths(unittest.TestCase):
   """Check _ValidateFilePaths helper.
@@ -247,10 +255,10 @@ class XmlManifestTests(ManifestParseTestCase):
     self.assertEqual(manifest.superproject['remote'].name, 'test-remote')
     self.assertEqual(
         manifest.ToXml().toxml(),
-        '<?xml version="1.0" ?><manifest>' +
+        '<?xml version="1.0" ?><manifest>'
-        '<remote name="test-remote" fetch="http://localhost"/>' +
+        '<remote name="test-remote" fetch="http://localhost"/>'
-        '<default remote="test-remote" revision="refs/heads/main"/>' +
+        '<default remote="test-remote" revision="refs/heads/main"/>'
-        '<superproject name="superproject"/>' +
+        '<superproject name="superproject"/>'
         '</manifest>')
 
 
@@ -303,6 +311,7 @@ class IncludeElementTests(ManifestParseTestCase):
   def test_allow_bad_name_from_user(self):
     """Check handling of bad name attribute from the user's input."""
     def parse(name):
+      name = self.encodeXmlAttr(name)
       manifest = self.getXmlManifest(f"""
 <manifest>
   <remote name="default-remote" fetch="http://localhost" />
@@ -327,6 +336,7 @@ class IncludeElementTests(ManifestParseTestCase):
   def test_bad_name_checks(self):
     """Check handling of bad name attribute."""
     def parse(name):
+      name = self.encodeXmlAttr(name)
       # Setup target of the include.
       with open(os.path.join(self.manifest_dir, 'target.xml'), 'w') as fp:
         fp.write(f'<manifest><include name="{name}"/></manifest>')
@@ -399,15 +409,17 @@ class ProjectElementTests(ManifestParseTestCase):
     project.SetRevisionId('ABCDEF')
     self.assertEqual(
         manifest.ToXml().toxml(),
-        '<?xml version="1.0" ?><manifest>' +
+        '<?xml version="1.0" ?><manifest>'
-        '<remote name="default-remote" fetch="http://localhost"/>' +
+        '<remote name="default-remote" fetch="http://localhost"/>'
-        '<default remote="default-remote" revision="refs/heads/main"/>' +
+        '<default remote="default-remote" revision="refs/heads/main"/>'
-        '<project name="test-name" revision="ABCDEF"/>' +
+        '<project name="test-name" revision="ABCDEF"/>'
         '</manifest>')
 
   def test_trailing_slash(self):
     """Check handling of trailing slashes in attributes."""
     def parse(name, path):
+      name = self.encodeXmlAttr(name)
+      path = self.encodeXmlAttr(path)
       return self.getXmlManifest(f"""
 <manifest>
   <remote name="default-remote" fetch="http://localhost" />
@@ -437,6 +449,8 @@ class ProjectElementTests(ManifestParseTestCase):
   def test_toplevel_path(self):
     """Check handling of path=. specially."""
     def parse(name, path):
+      name = self.encodeXmlAttr(name)
+      path = self.encodeXmlAttr(path)
       return self.getXmlManifest(f"""
 <manifest>
   <remote name="default-remote" fetch="http://localhost" />
@@ -453,6 +467,8 @@ class ProjectElementTests(ManifestParseTestCase):
   def test_bad_path_name_checks(self):
     """Check handling of bad path & name attributes."""
     def parse(name, path):
+      name = self.encodeXmlAttr(name)
+      path = self.encodeXmlAttr(path)
       manifest = self.getXmlManifest(f"""
 <manifest>
   <remote name="default-remote" fetch="http://localhost" />
@@ -501,10 +517,10 @@ class SuperProjectElementTests(ManifestParseTestCase):
     self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject')
     self.assertEqual(
         manifest.ToXml().toxml(),
-        '<?xml version="1.0" ?><manifest>' +
+        '<?xml version="1.0" ?><manifest>'
-        '<remote name="test-remote" fetch="http://localhost"/>' +
+        '<remote name="test-remote" fetch="http://localhost"/>'
-        '<default remote="test-remote" revision="refs/heads/main"/>' +
+        '<default remote="test-remote" revision="refs/heads/main"/>'
-        '<superproject name="superproject"/>' +
+        '<superproject name="superproject"/>'
         '</manifest>')
 
   def test_remote(self):
@@ -522,11 +538,11 @@ class SuperProjectElementTests(ManifestParseTestCase):
     self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/platform/superproject')
     self.assertEqual(
         manifest.ToXml().toxml(),
-        '<?xml version="1.0" ?><manifest>' +
+        '<?xml version="1.0" ?><manifest>'
-        '<remote name="default-remote" fetch="http://localhost"/>' +
+        '<remote name="default-remote" fetch="http://localhost"/>'
-        '<remote name="superproject-remote" fetch="http://localhost"/>' +
+        '<remote name="superproject-remote" fetch="http://localhost"/>'
-        '<default remote="default-remote" revision="refs/heads/main"/>' +
+        '<default remote="default-remote" revision="refs/heads/main"/>'
-        '<superproject name="platform/superproject" remote="superproject-remote"/>' +
+        '<superproject name="platform/superproject" remote="superproject-remote"/>'
         '</manifest>')
 
   def test_defalut_remote(self):
@@ -542,8 +558,27 @@ class SuperProjectElementTests(ManifestParseTestCase):
     self.assertEqual(manifest.superproject['remote'].name, 'default-remote')
     self.assertEqual(
         manifest.ToXml().toxml(),
-        '<?xml version="1.0" ?><manifest>' +
+        '<?xml version="1.0" ?><manifest>'
-        '<remote name="default-remote" fetch="http://localhost"/>' +
+        '<remote name="default-remote" fetch="http://localhost"/>'
-        '<default remote="default-remote" revision="refs/heads/main"/>' +
+        '<default remote="default-remote" revision="refs/heads/main"/>'
-        '<superproject name="superproject"/>' +
+        '<superproject name="superproject"/>'
+        '</manifest>')
+
+
+class ContactinfoElementTests(ManifestParseTestCase):
+  """Tests for <contactinfo>."""
+
+  def test_contactinfo(self):
+    """Check contactinfo settings."""
+    bugurl = 'http://localhost/contactinfo'
+    manifest = self.getXmlManifest(f"""
+<manifest>
+  <contactinfo bugurl="{bugurl}"/>
+</manifest>
+""")
+    self.assertEqual(manifest.contactinfo.bugurl, bugurl)
+    self.assertEqual(
+        manifest.ToXml().toxml(),
+        '<?xml version="1.0" ?><manifest>'
+        f'<contactinfo bugurl="{bugurl}"/>'
         '</manifest>')

tests/test_ssh.py

@@ -0,0 +1,74 @@
+# Copyright 2019 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.
+
+"""Unittests for the ssh.py module."""
+
+import multiprocessing
+import subprocess
+import unittest
+from unittest import mock
+
+import ssh
+
+
+class SshTests(unittest.TestCase):
+  """Tests the ssh functions."""
+
+  def test_parse_ssh_version(self):
+    """Check _parse_ssh_version() handling."""
+    ver = ssh._parse_ssh_version('Unknown\n')
+    self.assertEqual(ver, ())
+    ver = ssh._parse_ssh_version('OpenSSH_1.0\n')
+    self.assertEqual(ver, (1, 0))
+    ver = ssh._parse_ssh_version('OpenSSH_6.6.1p1 Ubuntu-2ubuntu2.13, OpenSSL 1.0.1f 6 Jan 2014\n')
+    self.assertEqual(ver, (6, 6, 1))
+    ver = ssh._parse_ssh_version('OpenSSH_7.6p1 Ubuntu-4ubuntu0.3, OpenSSL 1.0.2n  7 Dec 2017\n')
+    self.assertEqual(ver, (7, 6))
+
+  def test_version(self):
+    """Check version() handling."""
+    with mock.patch('ssh._run_ssh_version', return_value='OpenSSH_1.2\n'):
+      self.assertEqual(ssh.version(), (1, 2))
+
+  def test_context_manager_empty(self):
+    """Verify context manager with no clients works correctly."""
+    with multiprocessing.Manager() as manager:
+      with ssh.ProxyManager(manager):
+        pass
+
+  def test_context_manager_child_cleanup(self):
+    """Verify orphaned clients & masters get cleaned up."""
+    with multiprocessing.Manager() as manager:
+      with ssh.ProxyManager(manager) as ssh_proxy:
+        client = subprocess.Popen(['sleep', '964853320'])
+        ssh_proxy.add_client(client)
+        master = subprocess.Popen(['sleep', '964853321'])
+        ssh_proxy.add_master(master)
+    # If the process still exists, these will throw timeout errors.
+    client.wait(0)
+    master.wait(0)
+
+  def test_ssh_sock(self):
+    """Check sock() function."""
+    manager = multiprocessing.Manager()
+    proxy = ssh.ProxyManager(manager)
+    with mock.patch('tempfile.mkdtemp', return_value='/tmp/foo'):
+      # old ssh version uses port
+      with mock.patch('ssh.version', return_value=(6, 6)):
+        self.assertTrue(proxy.sock().endswith('%p'))
+
+      proxy._sock_path = None
+      # new ssh version uses hash
+      with mock.patch('ssh.version', return_value=(6, 7)):
+        self.assertTrue(proxy.sock().endswith('%C'))

tests/test_subcmds.py

@@ -14,6 +14,7 @@
 
 """Unittests for the subcmds module (mostly __init__.py than subcommands)."""
 
+import optparse
 import unittest
 
 import subcmds
@@ -41,3 +42,32 @@ class AllCommands(unittest.TestCase):
 
       # Reject internal python paths like "__init__".
       self.assertFalse(cmd.startswith('__'))
+
+  def test_help_desc_style(self):
+    """Force some consistency in option descriptions.
+
+    Python's optparse & argparse has a few default options like --help.  Their
+    option description text uses lowercase sentence fragments, so enforce our
+    options follow the same style so UI is consistent.
+
+    We enforce:
+    * Text starts with lowercase.
+    * Text doesn't end with period.
+    """
+    for name, cls in subcmds.all_commands.items():
+      cmd = cls()
+      parser = cmd.OptionParser
+      for option in parser.option_list:
+        if option.help == optparse.SUPPRESS_HELP:
+          continue
+
+        c = option.help[0]
+        self.assertEqual(
+            c.lower(), c,
+            msg=f'subcmds/{name}.py: {option.get_opt_string()}: help text '
+                f'should start with lowercase: "{option.help}"')
+
+        self.assertNotEqual(
+            option.help[-1], '.',
+            msg=f'subcmds/{name}.py: {option.get_opt_string()}: help text '
+                f'should not end in a period: "{option.help}"')