command: filter projects by active manifest groups

`repo forall <proj>` will look up all <proj> in the manifest for all
manifest groups regardless of which are active.  If <proj> is checked
out to different locations depending on the group, this ultimately
fails as we're unable to locate all of them.

Simple fix is to only include projects that match the manifest groups
that we already passed down & initialized to the active set, and that
we already use when getting the default project list.

Bug: https://crbug.com/gerrit/11677
Bug: https://crbug.com/1011226
Change-Id: I975f10f9a9e5a1cad7d87344123f8003732dab27
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/239652
Tested-by: Mike Frysinger <vapier@google.com>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Reviewed-by: David Pursehouse <dpursehouse@collab.net>

command.py

@@ -175,7 +175,10 @@ class Command(object):
       self._ResetPathToProjectMap(all_projects_list)
 
       for arg in args:
-        projects = manifest.GetProjectsWithName(arg)
+        # We have to filter by manifest groups in case the requested project is
+        # checked out multiple times or differently based on them.
+        projects = [project for project in manifest.GetProjectsWithName(arg)
+                    if project.MatchesGroups(groups)]
 
         if not projects:
           path = os.path.abspath(arg).replace('\\', '/')
@@ -200,7 +203,7 @@ class Command(object):
 
         for project in projects:
           if not missing_ok and not project.Exists:
-            raise NoSuchProjectError(arg)
+            raise NoSuchProjectError('%s (%s)' % (arg, project.relpath))
           if not project.MatchesGroups(groups):
             raise InvalidProjectGroupsError(arg)
 

git_command.py

@@ -22,6 +22,7 @@ import tempfile
 from signal import SIGTERM
 
 from error import GitError
+from git_refs import HEAD
 import platform_utils
 from repo_trace import REPO_TRACE, IsTrace, Trace
 from wrapper import Wrapper
@@ -98,6 +99,86 @@ class _GitCall(object):
     return fun
 git = _GitCall()
 
+
+def RepoSourceVersion():
+  """Return the version of the repo.git tree."""
+  ver = getattr(RepoSourceVersion, 'version', None)
+
+  # We avoid GitCommand so we don't run into circular deps -- GitCommand needs
+  # to initialize version info we provide.
+  if ver is None:
+    env = GitCommand._GetBasicEnv()
+
+    proj = os.path.dirname(os.path.abspath(__file__))
+    env[GIT_DIR] = os.path.join(proj, '.git')
+
+    p = subprocess.Popen([GIT, 'describe', HEAD], stdout=subprocess.PIPE,
+                         env=env)
+    if p.wait() == 0:
+      ver = p.stdout.read().strip().decode('utf-8')
+      if ver.startswith('v'):
+        ver = ver[1:]
+    else:
+      ver = 'unknown'
+    setattr(RepoSourceVersion, 'version', ver)
+
+  return ver
+
+
+class UserAgent(object):
+  """Mange User-Agent settings when talking to external services
+
+  We follow the style as documented here:
+  https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/User-Agent
+  """
+
+  _os = None
+  _repo_ua = None
+  _git_ua = None
+
+  @property
+  def os(self):
+    """The operating system name."""
+    if self._os is None:
+      os_name = sys.platform
+      if os_name.lower().startswith('linux'):
+        os_name = 'Linux'
+      elif os_name == 'win32':
+        os_name = 'Win32'
+      elif os_name == 'cygwin':
+        os_name = 'Cygwin'
+      elif os_name == 'darwin':
+        os_name = 'Darwin'
+      self._os = os_name
+
+    return self._os
+
+  @property
+  def repo(self):
+    """The UA when connecting directly from repo."""
+    if self._repo_ua is None:
+      py_version = sys.version_info
+      self._repo_ua = 'git-repo/%s (%s) git/%s Python/%d.%d.%d' % (
+          RepoSourceVersion(),
+          self.os,
+          git.version_tuple().full,
+          py_version.major, py_version.minor, py_version.micro)
+
+    return self._repo_ua
+
+  @property
+  def git(self):
+    """The UA when running git."""
+    if self._git_ua is None:
+      self._git_ua = 'git/%s (%s) git-repo/%s' % (
+          git.version_tuple().full,
+          self.os,
+          RepoSourceVersion())
+
+    return self._git_ua
+
+user_agent = UserAgent()
+
 def git_require(min_version, fail=False, msg=''):
   git_version = git.version_tuple()
   if min_version <= git_version:
@@ -125,17 +206,7 @@ class GitCommand(object):
                ssh_proxy = False,
                cwd = None,
                gitdir = None):
-    env = os.environ.copy()
+    env = self._GetBasicEnv()
-
-    for key in [REPO_TRACE,
-              GIT_DIR,
-              'GIT_ALTERNATE_OBJECT_DIRECTORIES',
-              'GIT_OBJECT_DIRECTORY',
-              'GIT_WORK_TREE',
-              'GIT_GRAFT_FILE',
-              'GIT_INDEX_FILE']:
-      if key in env:
-        del env[key]
 
     # If we are not capturing std* then need to print it.
     self.tee = {'stdout': not capture_stdout, 'stderr': not capture_stderr}
@@ -155,6 +226,7 @@ class GitCommand(object):
     if 'GIT_ALLOW_PROTOCOL' not in env:
       _setenv(env, 'GIT_ALLOW_PROTOCOL',
               'file:git:http:https:ssh:persistent-http:persistent-https:sso:rpc')
+    _setenv(env, 'GIT_HTTP_USER_AGENT', user_agent.git)
 
     if project:
       if not cwd:
@@ -227,6 +299,23 @@ class GitCommand(object):
     self.process = p
     self.stdin = p.stdin
 
+  @staticmethod
+  def _GetBasicEnv():
+    """Return a basic env for running git under.
+
+    This is guaranteed to be side-effect free.
+    """
+    env = os.environ.copy()
+    for key in (REPO_TRACE,
+                GIT_DIR,
+                'GIT_ALTERNATE_OBJECT_DIRECTORIES',
+                'GIT_OBJECT_DIRECTORY',
+                'GIT_WORK_TREE',
+                'GIT_GRAFT_FILE',
+                'GIT_INDEX_FILE'):
+      env.pop(key, None)
+    return env
+
   def Wait(self):
     try:
       p = self.process

main.py

@@ -23,7 +23,6 @@ which takes care of execing this entry point.
 
 from __future__ import print_function
 import getpass
-import imp
 import netrc
 import optparse
 import os
@@ -34,6 +33,7 @@ from pyversion import is_python3
 if is_python3():
   import urllib.request
 else:
+  import imp
   import urllib2
   urllib = imp.new_module('urllib')
   urllib.request = urllib2
@@ -46,7 +46,7 @@ except ImportError:
 from color import SetDefaultColoring
 import event_log
 from repo_trace import SetTrace
-from git_command import git, GitCommand
+from git_command import git, GitCommand, user_agent
 from git_config import init_ssh, close_ssh
 from command import InteractiveCommand
 from command import MirrorSafeCommand
@@ -244,10 +244,6 @@ class _Repo(object):
     return result
 
 
-def _MyRepoPath():
-  return os.path.dirname(__file__)
-
-
 def _CheckWrapperVersion(ver, repo_path):
   if not repo_path:
     repo_path = '~/bin/repo'
@@ -299,51 +295,13 @@ def _PruneOptions(argv, opt):
       continue
     i += 1
 
-_user_agent = None
-
-def _UserAgent():
-  global _user_agent
-
-  if _user_agent is None:
-    py_version = sys.version_info
-
-    os_name = sys.platform
-    if os_name == 'linux2':
-      os_name = 'Linux'
-    elif os_name == 'win32':
-      os_name = 'Win32'
-    elif os_name == 'cygwin':
-      os_name = 'Cygwin'
-    elif os_name == 'darwin':
-      os_name = 'Darwin'
-
-    p = GitCommand(
-      None, ['describe', 'HEAD'],
-      cwd = _MyRepoPath(),
-      capture_stdout = True)
-    if p.Wait() == 0:
-      repo_version = p.stdout
-      if len(repo_version) > 0 and repo_version[-1] == '\n':
-        repo_version = repo_version[0:-1]
-      if len(repo_version) > 0 and repo_version[0] == 'v':
-        repo_version = repo_version[1:]
-    else:
-      repo_version = 'unknown'
-
-    _user_agent = 'git-repo/%s (%s) git/%s Python/%d.%d.%d' % (
-      repo_version,
-      os_name,
-      git.version_tuple().full,
-      py_version[0], py_version[1], py_version[2])
-  return _user_agent
-
 class _UserAgentHandler(urllib.request.BaseHandler):
   def http_request(self, req):
-    req.add_header('User-Agent', _UserAgent())
+    req.add_header('User-Agent', user_agent.repo)
     return req
 
   def https_request(self, req):
-    req.add_header('User-Agent', _UserAgent())
+    req.add_header('User-Agent', user_agent.repo)
     return req
 
 def _AddPasswordFromUserInput(handler, msg, req):

progress.py

@@ -21,6 +21,11 @@ from repo_trace import IsTrace
 
 _NOT_TTY = not os.isatty(2)
 
+# This will erase all content in the current line (wherever the cursor is).
+# It does not move the cursor, so this is usually followed by \r to move to
+# column 0.
+CSI_ERASE_LINE = '\x1b[2K'
+
 class Progress(object):
   def __init__(self, title, total=0, units='', print_newline=False,
                always_print_percentage=False):
@@ -47,7 +52,8 @@ class Progress(object):
         return
 
     if self._total <= 0:
-      sys.stderr.write('\r%s: %d, ' % (
+      sys.stderr.write('%s\r%s: %d,' % (
+        CSI_ERASE_LINE,
         self._title,
         self._done))
       sys.stderr.flush()
@@ -56,7 +62,8 @@ class Progress(object):
 
       if self._lastp != p or self._always_print_percentage:
         self._lastp = p
-        sys.stderr.write('\r%s: %3d%% (%d%s/%d%s)%s' % (
+        sys.stderr.write('%s\r%s: %3d%% (%d%s/%d%s)%s' % (
+          CSI_ERASE_LINE,
           self._title,
           p,
           self._done, self._units,
@@ -69,13 +76,15 @@ class Progress(object):
       return
 
     if self._total <= 0:
-      sys.stderr.write('\r%s: %d, done.  \n' % (
+      sys.stderr.write('%s\r%s: %d, done.\n' % (
+        CSI_ERASE_LINE,
         self._title,
         self._done))
       sys.stderr.flush()
     else:
       p = (100 * self._done) / self._total
-      sys.stderr.write('\r%s: %3d%% (%d%s/%d%s), done.  \n' % (
+      sys.stderr.write('%s\r%s: %3d%% (%d%s/%d%s), done.\n' % (
+        CSI_ERASE_LINE,
         self._title,
         p,
         self._done, self._units,

project.py

@@ -39,6 +39,7 @@ from error import GitError, HookError, UploadError, DownloadError
 from error import ManifestInvalidRevisionError
 from error import NoManifestException
 import platform_utils
+import progress
 from repo_trace import IsTrace, Trace
 
 from git_refs import GitRefs, HEAD, R_HEADS, R_TAGS, R_PUB, R_M
@@ -229,6 +230,7 @@ class DiffColoring(Coloring):
   def __init__(self, config):
     Coloring.__init__(self, config, 'diff')
     self.project = self.printer('header', attr='bold')
+    self.fail = self.printer('fail', fg='red')
 
 
 class _Annotation(object):
@@ -864,10 +866,17 @@ class Project(object):
   @property
   def CurrentBranch(self):
     """Obtain the name of the currently checked out branch.
+
     The branch name omits the 'refs/heads/' prefix.
-       None is returned if the project is on a detached HEAD.
+    None is returned if the project is on a detached HEAD, or if the work_git is
+    otheriwse inaccessible (e.g. an incomplete sync).
     """
+    try:
       b = self.work_git.GetHead()
+    except NoManifestException:
+      # If the local checkout is in a bad state, don't barf.  Let the callers
+      # process this like the head is unreadable.
+      return None
     if b.startswith(R_HEADS):
       return b[len(R_HEADS):]
     return None
@@ -1135,10 +1144,18 @@ class Project(object):
       cmd.append('--src-prefix=a/%s/' % self.relpath)
       cmd.append('--dst-prefix=b/%s/' % self.relpath)
     cmd.append('--')
+    try:
       p = GitCommand(self,
                      cmd,
                      capture_stdout=True,
                      capture_stderr=True)
+    except GitError as e:
+      out.nl()
+      out.project('project %s/' % self.relpath)
+      out.nl()
+      out.fail('%s', str(e))
+      out.nl()
+      return False
     has_diff = False
     for line in p.process.stdout:
       if not hasattr(line, 'encode'):
@@ -1149,7 +1166,7 @@ class Project(object):
         out.nl()
         has_diff = True
       print(line[:-1])
-    p.Wait()
+    return p.Wait() == 0
 
 
 # Publish / Upload ##
@@ -1697,7 +1714,7 @@ class Project(object):
 
 # Branch Management ##
 
-  def StartBranch(self, name, branch_merge=''):
+  def StartBranch(self, name, branch_merge='', revision=None):
     """Create a new branch off the manifest's revision.
     """
     if not branch_merge:
@@ -1718,7 +1735,11 @@ class Project(object):
     branch.merge = branch_merge
     if not branch.merge.startswith('refs/') and not ID_RE.match(branch_merge):
       branch.merge = R_HEADS + branch_merge
+
+    if revision is None:
       revid = self.GetRevisionId(all_refs)
+    else:
+      revid = self.work_git.rev_parse(revision)
 
     if head.startswith(R_HEADS):
       try:
@@ -2181,12 +2202,15 @@ class Project(object):
       cmd.append('--update-head-ok')
     cmd.append(name)
 
+    spec = []
+
     # If using depth then we should not get all the tags since they may
     # be outside of the depth.
     if no_tags or depth:
       cmd.append('--no-tags')
     else:
       cmd.append('--tags')
+      spec.append(str((u'+refs/tags/*:') + remote.ToLocal('refs/tags/*')))
 
     if force_sync:
       cmd.append('--force')
@@ -2197,12 +2221,9 @@ class Project(object):
     if submodules:
       cmd.append('--recurse-submodules=on-demand')
 
-    spec = []
     if not current_branch_only:
       # Fetch whole repo
       spec.append(str((u'+refs/heads/*:') + remote.ToLocal('refs/heads/*')))
-      if not (no_tags or depth):
-        spec.append(str((u'+refs/tags/*:') + remote.ToLocal('refs/tags/*')))
     elif tag_name is not None:
       spec.append('tag')
       spec.append(tag_name)
@@ -3109,6 +3130,11 @@ class SyncBuffer(object):
     return True
 
   def _PrintMessages(self):
+    if self._messages or self._failures:
+      if os.isatty(2):
+        self.out.write(progress.CSI_ERASE_LINE)
+      self.out.write('\r')
+
     for m in self._messages:
       m.Print(self)
     for m in self._failures:

repo

@@ -33,7 +33,7 @@ REPO_REV = 'stable'
 # limitations under the License.
 
 # increment this whenever we make important changes to this script
-VERSION = (1, 25)
+VERSION = (1, 26)
 
 # increment this if the MAINTAINER_KEYS block is modified
 KEYRING_VERSION = (1, 2)
@@ -443,7 +443,7 @@ def _CheckGitVersion():
     raise CloneFailure()
 
   if ver_act is None:
-    print('error: "%s" unsupported' % ver_str, file=sys.stderr)
+    print('fatal: unable to detect git version', file=sys.stderr)
     raise CloneFailure()
 
   if ver_act < MIN_GIT_VERSION:
@@ -505,7 +505,7 @@ def SetupGnuPG(quiet):
       print(file=sys.stderr)
     return False
 
-  proc.stdin.write(MAINTAINER_KEYS)
+  proc.stdin.write(MAINTAINER_KEYS.encode('utf-8'))
   proc.stdin.close()
 
   if proc.wait() != 0:
@@ -584,6 +584,7 @@ def _DownloadBundle(url, local, quiet):
       cwd=local,
       stdout=subprocess.PIPE)
   for line in proc.stdout:
+    line = line.decode('utf-8')
     m = re.compile(r'^url\.(.*)\.insteadof (.*)$').match(line)
     if m:
       new_url = m.group(1)
@@ -676,7 +677,7 @@ def _Verify(cwd, branch, quiet):
                           stdout=subprocess.PIPE,
                           stderr=subprocess.PIPE,
                           cwd=cwd)
-  cur = proc.stdout.read().strip()
+  cur = proc.stdout.read().strip().decode('utf-8')
   proc.stdout.close()
 
   proc.stderr.read()
@@ -708,10 +709,10 @@ def _Verify(cwd, branch, quiet):
                           stderr=subprocess.PIPE,
                           cwd=cwd,
                           env=env)
-  out = proc.stdout.read()
+  out = proc.stdout.read().decode('utf-8')
   proc.stdout.close()
 
-  err = proc.stderr.read()
+  err = proc.stderr.read().decode('utf-8')
   proc.stderr.close()
 
   if proc.wait() != 0:
@@ -861,7 +862,7 @@ def _SetDefaultsTo(gitdir):
                            'HEAD'],
                           stdout=subprocess.PIPE,
                           stderr=subprocess.PIPE)
-  REPO_REV = proc.stdout.read().strip()
+  REPO_REV = proc.stdout.read().strip().decode('utf-8')
   proc.stdout.close()
 
   proc.stderr.read()

subcmds/diff.py

@@ -37,5 +37,8 @@ to the Unix 'patch' command.
                  help='Paths are relative to the repository root')
 
   def Execute(self, opt, args):
+    ret = 0
     for project in self.GetProjects(args):
-      project.PrintWorkTreeDiff(opt.absolute)
+      if not project.PrintWorkTreeDiff(opt.absolute):
+        ret = 1
+    return ret

subcmds/forall.py

@@ -323,10 +323,10 @@ def DoWork(project, mirror, opt, cmd, shell, cnt, config):
     cwd = project['worktree']
 
   if not os.path.exists(cwd):
-    if (opt.project_header and opt.verbose) \
+    if ((opt.project_header and opt.verbose)
-    or not opt.project_header:
+        or not opt.project_header):
       print('skipping %s/' % project['relpath'], file=sys.stderr)
-    return
+    return 1
 
   if opt.project_header:
     stdin = subprocess.PIPE

subcmds/grep.py

@@ -15,15 +15,19 @@
 # limitations under the License.
 
 from __future__ import print_function
+
 import sys
+
 from color import Coloring
 from command import PagedCommand
+from error import GitError
 from git_command import git_require, GitCommand
 
 class GrepColoring(Coloring):
   def __init__(self, config):
     Coloring.__init__(self, config, 'grep')
     self.project = self.printer('project', attr='bold')
+    self.fail = self.printer('fail', fg='red')
 
 class Grep(PagedCommand):
   common = True
@@ -184,15 +188,25 @@ contain a line that matches both expressions:
       cmd_argv.extend(opt.revision)
     cmd_argv.append('--')
 
+    git_failed = False
     bad_rev = False
     have_match = False
 
     for project in projects:
+      try:
         p = GitCommand(project,
                        cmd_argv,
-                     bare = False,
+                       bare=False,
-                     capture_stdout = True,
+                       capture_stdout=True,
-                     capture_stderr = True)
+                       capture_stderr=True)
+      except GitError as e:
+        git_failed = True
+        out.project('--- project %s ---' % project.relpath)
+        out.nl()
+        out.fail('%s', str(e))
+        out.nl()
+        continue
+
       if p.Wait() != 0:
         # no results
         #
@@ -202,7 +216,7 @@ contain a line that matches both expressions:
           else:
             out.project('--- project %s ---' % project.relpath)
             out.nl()
-            out.write("%s", p.stderr)
+            out.fail('%s', p.stderr.strip())
             out.nl()
         continue
       have_match = True
@@ -231,7 +245,9 @@ contain a line that matches both expressions:
         for line in r:
           print(line)
 
-    if have_match:
+    if git_failed:
+      sys.exit(1)
+    elif have_match:
       sys.exit(0)
     elif have_rev and bad_rev:
       for r in opt.revision:

subcmds/info.py

@@ -16,7 +16,6 @@
 
 from command import PagedCommand
 from color import Coloring
-from error import NoSuchProjectError
 from git_refs import R_M
 
 class _Coloring(Coloring):
@@ -82,10 +81,8 @@ class Info(PagedCommand):
     self.out.nl()
 
   def printDiffInfo(self, args):
-    try:
+    # We let exceptions bubble up to main as they'll be well structured.
     projs = self.GetProjects(args)
-    except NoSuchProjectError:
-      return
 
     for p in projs:
       self.heading("Project: ")
@@ -97,13 +94,19 @@ class Info(PagedCommand):
       self.out.nl()
 
       self.heading("Current revision: ")
-      self.headtext(p.revisionExpr)
+      self.headtext(p.GetRevisionId())
+      self.out.nl()
+
+      currentBranch = p.CurrentBranch
+      if currentBranch:
+        self.heading('Current branch: ')
+        self.headtext(currentBranch)
         self.out.nl()
 
       localBranches = list(p.GetBranches().keys())
       self.heading("Local Branches: ")
       self.redtext(str(len(localBranches)))
-      if len(localBranches) > 0:
+      if localBranches:
         self.text(" [")
         self.text(", ".join(localBranches))
         self.text("]")

subcmds/rebase.py

@@ -17,9 +17,18 @@
 from __future__ import print_function
 import sys
 
+from color import Coloring
 from command import Command
 from git_command import GitCommand
 
+
+class RebaseColoring(Coloring):
+  def __init__(self, config):
+    Coloring.__init__(self, config, 'rebase')
+    self.project = self.printer('project', attr='bold')
+    self.fail = self.printer('fail', fg='red')
+
+
 class Rebase(Command):
   common = True
   helpSummary = "Rebase local branches on upstream branch"
@@ -37,6 +46,9 @@ branch but need to incorporate new upstream changes "underneath" them.
                 dest="interactive", action="store_true",
                 help="interactive rebase (single project only)")
 
+    p.add_option('--fail-fast',
+                 dest='fail_fast', action='store_true',
+                 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')
@@ -88,7 +100,15 @@ branch but need to incorporate new upstream changes "underneath" them.
     if opt.interactive:
       common_args.append('-i')
 
+    config = self.manifest.manifestProject.config
+    out = RebaseColoring(config)
+    out.redirect(sys.stdout)
+
+    ret = 0
     for project in all_projects:
+      if ret and opt.fail_fast:
+        break
+
       cb = project.CurrentBranch
       if not cb:
         if one_project:
@@ -114,8 +134,10 @@ branch but need to incorporate new upstream changes "underneath" them.
 
       args.append(upbranch.LocalMerge)
 
-      print('# %s: rebasing %s -> %s'
+      out.project('project %s: rebasing %s -> %s',
-            % (project.relpath, cb, upbranch.LocalMerge), file=sys.stderr)
+                  project.relpath, cb, upbranch.LocalMerge)
+      out.nl()
+      out.flush()
 
       needs_stash = False
       if opt.auto_stash:
@@ -127,13 +149,21 @@ branch but need to incorporate new upstream changes "underneath" them.
           stash_args = ["stash"]
 
           if GitCommand(project, stash_args).Wait() != 0:
-            return 1
+            ret += 1
+            continue
 
       if GitCommand(project, args).Wait() != 0:
-        return 1
+        ret += 1
+        continue
 
       if needs_stash:
         stash_args.append('pop')
         stash_args.append('--quiet')
         if GitCommand(project, stash_args).Wait() != 0:
-          return 1
+          ret += 1
+
+    if ret:
+      out.fail('%i projects had errors', ret)
+      out.nl()
+
+    return ret

subcmds/start.py

@@ -40,6 +40,10 @@ revision specified in the manifest.
     p.add_option('--all',
                  dest='all', action='store_true',
                  help='begin branch in all projects')
+    p.add_option('-r', '--rev', '--revision', dest='revision',
+                 help='point branch at this revision instead of upstream')
+    p.add_option('--head', dest='revision', action='store_const', const='HEAD',
+                 help='abbreviation for --rev HEAD')
 
   def ValidateOptions(self, opt, args):
     if not args:
@@ -108,7 +112,8 @@ revision specified in the manifest.
         else:
           branch_merge = self.manifest.default.revisionExpr
 
-      if not project.StartBranch(nb, branch_merge=branch_merge):
+      if not project.StartBranch(
+          nb, branch_merge=branch_merge, revision=opt.revision):
         err.append(project)
     pm.end()
 

subcmds/sync.py

@@ -199,6 +199,7 @@ later is required to fix a server side protocol bug.
       self.jobs = 1
 
     p.add_option('-f', '--force-broken',
+                 dest='force_broken', action='store_true',
                  help='obsolete option (to be deleted in the future)')
     p.add_option('--fail-fast',
                  dest='fail_fast', action='store_true',
@@ -738,40 +739,7 @@ later is required to fix a server side protocol bug.
       fd.close()
     return 0
 
-  def ValidateOptions(self, opt, args):
+  def _SmartSyncSetup(self, opt, smart_sync_manifest_path):
-    if opt.force_broken:
-      print('warning: -f/--force-broken is now the default behavior, and the '
-            'options are deprecated', file=sys.stderr)
-    if opt.network_only and opt.detach_head:
-      self.OptionParser.error('cannot combine -n and -d')
-    if opt.network_only and opt.local_only:
-      self.OptionParser.error('cannot combine -n and -l')
-    if opt.manifest_name and opt.smart_sync:
-      self.OptionParser.error('cannot combine -m and -s')
-    if opt.manifest_name and opt.smart_tag:
-      self.OptionParser.error('cannot combine -m and -t')
-    if opt.manifest_server_username or opt.manifest_server_password:
-      if not (opt.smart_sync or opt.smart_tag):
-        self.OptionParser.error('-u and -p may only be combined with -s or -t')
-      if None in [opt.manifest_server_username, opt.manifest_server_password]:
-        self.OptionParser.error('both -u and -p must be given')
-
-  def Execute(self, opt, args):
-    if opt.jobs:
-      self.jobs = opt.jobs
-    if self.jobs > 1:
-      soft_limit, _ = _rlimit_nofile()
-      self.jobs = min(self.jobs, (soft_limit - 5) // 3)
-
-    if opt.manifest_name:
-      self.manifest.Override(opt.manifest_name)
-
-    manifest_name = opt.manifest_name
-    smart_sync_manifest_name = "smart_sync_override.xml"
-    smart_sync_manifest_path = os.path.join(
-      self.manifest.manifestProject.worktree, smart_sync_manifest_name)
-
-    if opt.smart_sync or opt.smart_tag:
     if not self.manifest.manifest_server:
       print('error: cannot smart sync: no manifest server defined in '
             'manifest', file=sys.stderr)
@@ -839,7 +807,7 @@ later is required to fix a server side protocol bug.
         [success, manifest_str] = server.GetManifest(opt.smart_tag)
 
       if success:
-          manifest_name = smart_sync_manifest_name
+        manifest_name = os.path.basename(smart_sync_manifest_path)
         try:
           f = open(smart_sync_manifest_path, 'w')
           try:
@@ -865,23 +833,11 @@ later is required to fix a server side protocol bug.
             % (self.manifest.manifest_server, e.errcode, e.errmsg),
             file=sys.stderr)
       sys.exit(1)
-    else:  # Not smart sync or smart tag mode
-      if os.path.isfile(smart_sync_manifest_path):
-        try:
-          platform_utils.remove(smart_sync_manifest_path)
-        except OSError as e:
-          print('error: failed to remove existing smart sync override manifest: %s' %
-                e, file=sys.stderr)
 
-    rp = self.manifest.repoProject
+    return manifest_name
-    rp.PreSync()
-
-    mp = self.manifest.manifestProject
-    mp.PreSync()
-
-    if opt.repo_upgraded:
-      _PostRepoUpgrade(self.manifest, quiet=opt.quiet)
 
+  def _UpdateManifestProject(self, opt, mp, manifest_name):
+    """Fetch & update the local manifest project."""
     if not opt.local_only:
       start = time.time()
       success = mp.Sync_NetworkHalf(quiet=opt.quiet,
@@ -903,10 +859,63 @@ later is required to fix a server side protocol bug.
                              start, time.time(), clean)
       if not clean:
         sys.exit(1)
-      self._ReloadManifest(manifest_name)
+      self._ReloadManifest(opt.manifest_name)
       if opt.jobs is None:
         self.jobs = self.manifest.default.sync_j
 
+  def ValidateOptions(self, opt, args):
+    if opt.force_broken:
+      print('warning: -f/--force-broken is now the default behavior, and the '
+            'options are deprecated', file=sys.stderr)
+    if opt.network_only and opt.detach_head:
+      self.OptionParser.error('cannot combine -n and -d')
+    if opt.network_only and opt.local_only:
+      self.OptionParser.error('cannot combine -n and -l')
+    if opt.manifest_name and opt.smart_sync:
+      self.OptionParser.error('cannot combine -m and -s')
+    if opt.manifest_name and opt.smart_tag:
+      self.OptionParser.error('cannot combine -m and -t')
+    if opt.manifest_server_username or opt.manifest_server_password:
+      if not (opt.smart_sync or opt.smart_tag):
+        self.OptionParser.error('-u and -p may only be combined with -s or -t')
+      if None in [opt.manifest_server_username, opt.manifest_server_password]:
+        self.OptionParser.error('both -u and -p must be given')
+
+  def Execute(self, opt, args):
+    if opt.jobs:
+      self.jobs = opt.jobs
+    if self.jobs > 1:
+      soft_limit, _ = _rlimit_nofile()
+      self.jobs = min(self.jobs, (soft_limit - 5) // 3)
+
+    if opt.manifest_name:
+      self.manifest.Override(opt.manifest_name)
+
+    manifest_name = opt.manifest_name
+    smart_sync_manifest_path = os.path.join(
+      self.manifest.manifestProject.worktree, 'smart_sync_override.xml')
+
+    if opt.smart_sync or opt.smart_tag:
+      manifest_name = self._SmartSyncSetup(opt, smart_sync_manifest_path)
+    else:
+      if os.path.isfile(smart_sync_manifest_path):
+        try:
+          platform_utils.remove(smart_sync_manifest_path)
+        except OSError as e:
+          print('error: failed to remove existing smart sync override manifest: %s' %
+                e, file=sys.stderr)
+
+    rp = self.manifest.repoProject
+    rp.PreSync()
+
+    mp = self.manifest.manifestProject
+    mp.PreSync()
+
+    if opt.repo_upgraded:
+      _PostRepoUpgrade(self.manifest, quiet=opt.quiet)
+
+    self._UpdateManifestProject(opt, mp, manifest_name)
+
     if self.gitc_manifest:
       gitc_manifest_projects = self.GetProjects(args,
                                                 missing_ok=True)

subcmds/version.py

@@ -17,7 +17,7 @@
 from __future__ import print_function
 import sys
 from command import Command, MirrorSafeCommand
-from git_command import git
+from git_command import git, RepoSourceVersion, user_agent
 from git_refs import HEAD
 
 class Version(Command, MirrorSafeCommand):
@@ -34,12 +34,20 @@ class Version(Command, MirrorSafeCommand):
     rp = self.manifest.repoProject
     rem = rp.GetRemote(rp.remote.name)
 
-    print('repo version %s' % rp.work_git.describe(HEAD))
+    # These might not be the same.  Report them both.
+    src_ver = RepoSourceVersion()
+    rp_ver = rp.bare_git.describe(HEAD)
+    print('repo version %s' % rp_ver)
     print('       (from %s)' % rem.url)
 
     if Version.wrapper_path is not None:
       print('repo launcher version %s' % Version.wrapper_version)
       print('       (from %s)' % Version.wrapper_path)
 
+      if src_ver != rp_ver:
+        print('       (currently at %s)' % src_ver)
+
+    print('repo User-Agent %s' % user_agent.repo)
     print('git %s' % git.version_tuple().full)
+    print('git User-Agent %s' % user_agent.git)
     print('Python %s' % sys.version)

tests/test_git_command.py

@@ -18,6 +18,7 @@
 
 from __future__ import print_function
 
+import re
 import unittest
 
 import git_command
@@ -47,3 +48,31 @@ class GitCallUnitTest(unittest.TestCase):
     self.assertLess(ver, (9999, 9999, 9999))
 
     self.assertNotEqual('', ver.full)
+
+
+class UserAgentUnitTest(unittest.TestCase):
+  """Tests the UserAgent function."""
+
+  def test_smoke_os(self):
+    """Make sure UA OS setting returns something useful."""
+    os_name = git_command.user_agent.os
+    # We can't dive too deep because of OS/tool differences, but we can check
+    # the general form.
+    m = re.match(r'^[^ ]+$', os_name)
+    self.assertIsNotNone(m)
+
+  def test_smoke_repo(self):
+    """Make sure repo UA returns something useful."""
+    ua = git_command.user_agent.repo
+    # We can't dive too deep because of OS/tool differences, but we can check
+    # the general form.
+    m = re.match(r'^git-repo/[^ ]+ ([^ ]+) git/[^ ]+ Python/[0-9.]+', ua)
+    self.assertIsNotNone(m)
+
+  def test_smoke_git(self):
+    """Make sure git UA returns something useful."""
+    ua = git_command.user_agent.git
+    # We can't dive too deep because of OS/tool differences, but we can check
+    # the general form.
+    m = re.match(r'^git/[^ ]+ ([^ ]+) git-repo/[^ ]+', ua)
+    self.assertIsNotNone(m)