15 files changed, 149 insertions, 379 deletions

diff --git a/git_command.py b/git_command.py
index 56e18e02..19100fa9 100644
--- a/git_command.py
+++ b/git_command.py
@@ -230,11 +230,12 @@ class GitCommand(object):
     stderr = (subprocess.STDOUT if merge_output else
               (subprocess.PIPE if capture_stderr else None))
 
-    dbg = ''
     if IsTrace():
       global LAST_CWD
       global LAST_GITDIR
 
+      dbg = ''
+
       if cwd and LAST_CWD != cwd:
         if LAST_GITDIR or LAST_CWD:
           dbg += '\n'
@@ -262,31 +263,31 @@ class GitCommand(object):
         dbg += ' 2>|'
       elif stderr == subprocess.STDOUT:
         dbg += ' 2>&1'
+      Trace('%s', dbg)
+
+    try:
+      p = subprocess.Popen(command,
+                           cwd=cwd,
+                           env=env,
+                           encoding='utf-8',
+                           errors='backslashreplace',
+                           stdin=stdin,
+                           stdout=stdout,
+                           stderr=stderr)
+    except Exception as e:
+      raise GitError('%s: %s' % (command[1], e))
 
-    with Trace('git command %s %s with debug: %s', LAST_GITDIR, command, dbg):
-      try:
-        p = subprocess.Popen(command,
-                            cwd=cwd,
-                            env=env,
-                            encoding='utf-8',
-                            errors='backslashreplace',
-                            stdin=stdin,
-                            stdout=stdout,
-                            stderr=stderr)
-      except Exception as e:
-        raise GitError('%s: %s' % (command[1], e))
-
-      if ssh_proxy:
-        ssh_proxy.add_client(p)
+    if ssh_proxy:
+      ssh_proxy.add_client(p)
 
-      self.process = p
+    self.process = p
 
-      try:
-        self.stdout, self.stderr = p.communicate(input=input)
-      finally:
-        if ssh_proxy:
-          ssh_proxy.remove_client(p)
-      self.rc = p.wait()
+    try:
+      self.stdout, self.stderr = p.communicate(input=input)
+    finally:
+      if ssh_proxy:
+        ssh_proxy.remove_client(p)
+    self.rc = p.wait()
 
   @staticmethod
   def _GetBasicEnv():
diff --git a/git_config.py b/git_config.py
index 94378e9a..6f80ae08 100644
--- a/git_config.py
+++ b/git_config.py
@@ -219,8 +219,8 @@ class GitConfig(object):
     """Set the value(s) for a key.
        Only this configuration file is modified.
 
-       The supplied value should be either a string, or a list of strings (to
-       store multiple values), or None (to delete the key).
+       The supplied value should be either a string,
+       or a list of strings (to store multiple values).
     """
     key = _key(name)
 
@@ -349,9 +349,9 @@ class GitConfig(object):
     except OSError:
       return None
     try:
-      with Trace(': parsing %s', self.file):
-        with open(self._json) as fd:
-          return json.load(fd)
+      Trace(': parsing %s', self.file)
+      with open(self._json) as fd:
+        return json.load(fd)
     except (IOError, ValueError):
       platform_utils.remove(self._json, missing_ok=True)
       return None
diff --git a/git_refs.py b/git_refs.py
index 300d2b30..2d4a8090 100644
--- a/git_refs.py
+++ b/git_refs.py
@@ -67,37 +67,38 @@ class GitRefs(object):
       self._LoadAll()
 
   def _NeedUpdate(self):
-    with Trace(': scan refs %s', self._gitdir):
-      for name, mtime in self._mtime.items():
-        try:
-          if mtime != os.path.getmtime(os.path.join(self._gitdir, name)):
-            return True
-        except OSError:
+    Trace(': scan refs %s', self._gitdir)
+
+    for name, mtime in self._mtime.items():
+      try:
+        if mtime != os.path.getmtime(os.path.join(self._gitdir, name)):
           return True
-      return False
+      except OSError:
+        return True
+    return False
 
   def _LoadAll(self):
-    with Trace(': load refs %s', self._gitdir):
-
-      self._phyref = {}
-      self._symref = {}
-      self._mtime = {}
-
-      self._ReadPackedRefs()
-      self._ReadLoose('refs/')
-      self._ReadLoose1(os.path.join(self._gitdir, HEAD), HEAD)
-
-      scan = self._symref
-      attempts = 0
-      while scan and attempts < 5:
-        scan_next = {}
-        for name, dest in scan.items():
-          if dest in self._phyref:
-            self._phyref[name] = self._phyref[dest]
-          else:
-            scan_next[name] = dest
-        scan = scan_next
-        attempts += 1
+    Trace(': load refs %s', self._gitdir)
+
+    self._phyref = {}
+    self._symref = {}
+    self._mtime = {}
+
+    self._ReadPackedRefs()
+    self._ReadLoose('refs/')
+    self._ReadLoose1(os.path.join(self._gitdir, HEAD), HEAD)
+
+    scan = self._symref
+    attempts = 0
+    while scan and attempts < 5:
+      scan_next = {}
+      for name, dest in scan.items():
+        if dest in self._phyref:
+          self._phyref[name] = self._phyref[dest]
+        else:
+          scan_next[name] = dest
+      scan = scan_next
+      attempts += 1
 
   def _ReadPackedRefs(self):
     path = os.path.join(self._gitdir, 'packed-refs')
diff --git a/main.py b/main.py
index e629b30f..c54f9281 100755
--- a/main.py
+++ b/main.py
@@ -37,7 +37,7 @@ except ImportError:
 
 from color import SetDefaultColoring
 import event_log
-from repo_trace import SetTrace, Trace, SetTraceToStderr
+from repo_trace import SetTrace
 from git_command import user_agent
 from git_config import RepoConfig
 from git_trace2_event_log import EventLog
@@ -109,9 +109,6 @@ global_options.add_option('--color',
 global_options.add_option('--trace',
                           dest='trace', action='store_true',
                           help='trace git command execution (REPO_TRACE=1)')
-global_options.add_option('--trace_to_stderr',
-                          dest='trace_to_stderr', action='store_true',
-                          help='trace outputs go to stderr in addition to .repo/TRACE_FILE')
 global_options.add_option('--trace-python',
                           dest='trace_python', action='store_true',
                           help='trace python command execution')
@@ -201,6 +198,9 @@ class _Repo(object):
     """Execute the requested subcommand."""
     result = 0
 
+    if gopts.trace:
+      SetTrace()
+
     # Handle options that terminate quickly first.
     if gopts.help or gopts.help_all:
       self._PrintHelp(short=False, all_commands=gopts.help_all)
@@ -652,26 +652,17 @@ def _Main(argv):
   Version.wrapper_path = opt.wrapper_path
 
   repo = _Repo(opt.repodir)
-
   try:
     init_http()
     name, gopts, argv = repo._ParseArgs(argv)
-
-    if gopts.trace:
-      SetTrace()
-
-    if gopts.trace_to_stderr:
-      SetTraceToStderr()
-
-    with Trace('starting new command: %s', ', '.join([name] + argv), first_trace=True):
-      run = lambda: repo._Run(name, gopts, argv) or 0
-      if gopts.trace_python:
-        import trace
-        tracer = trace.Trace(count=False, trace=True, timing=True,
-                             ignoredirs=set(sys.path[1:]))
-        result = tracer.runfunc(run)
-      else:
-        result = run()
+    run = lambda: repo._Run(name, gopts, argv) or 0
+    if gopts.trace_python:
+      import trace
+      tracer = trace.Trace(count=False, trace=True, timing=True,
+                           ignoredirs=set(sys.path[1:]))
+      result = tracer.runfunc(run)
+    else:
+      result = run()
   except KeyboardInterrupt:
     print('aborted by user', file=sys.stderr)
     result = 1
diff --git a/project.py b/project.py
index 9d7476b4..1c85b044 100644
--- a/project.py
+++ b/project.py
@@ -41,7 +41,7 @@ from error import ManifestInvalidRevisionError, ManifestInvalidPathError
 from error import NoManifestException, ManifestParseError
 import platform_utils
 import progress
-from repo_trace import Trace
+from repo_trace import IsTrace, Trace
 
 from git_refs import GitRefs, HEAD, R_HEADS, R_TAGS, R_PUB, R_M, R_WORKTREE_M
 
@@ -59,7 +59,7 @@ MAXIMUM_RETRY_SLEEP_SEC = 3600.0
 # +-10% random jitter is added to each Fetches retry sleep duration.
 RETRY_JITTER_PERCENT = 0.1
 
-# Whether to use alternates.  Switching back and forth is *NOT* supported.
+# Whether to use alternates.
 # TODO(vapier): Remove knob once behavior is verified.
 _ALTERNATES = os.environ.get('REPO_USE_ALTERNATES') == '1'
 
@@ -2416,16 +2416,16 @@ class Project(object):
         srcUrl = 'http' + srcUrl[len('persistent-http'):]
       cmd += [srcUrl]
 
-      proc = None
-      with Trace('Fetching bundle: %s', ' '.join(cmd)):
-        if verbose:
-          print('%s: Downloading bundle: %s' % (self.name, srcUrl))
-        stdout = None if verbose else subprocess.PIPE
-        stderr = None if verbose else subprocess.STDOUT
-        try:
-          proc = subprocess.Popen(cmd, stdout=stdout, stderr=stderr)
-        except OSError:
-          return False
+      if IsTrace():
+        Trace('%s', ' '.join(cmd))
+      if verbose:
+        print('%s: Downloading bundle: %s' % (self.name, srcUrl))
+      stdout = None if verbose else subprocess.PIPE
+      stderr = None if verbose else subprocess.STDOUT
+      try:
+        proc = subprocess.Popen(cmd, stdout=stdout, stderr=stderr)
+      except OSError:
+        return False
 
       (output, _) = proc.communicate()
       curlret = proc.returncode
diff --git a/repo_trace.py b/repo_trace.py
index 0ff3b694..7be0c045 100644
--- a/repo_trace.py
+++ b/repo_trace.py
@@ -15,128 +15,26 @@
 """Logic for tracing repo interactions.
 
 Activated via `repo --trace ...` or `REPO_TRACE=1 repo ...`.
-
-Temporary: Tracing is always on. Set `REPO_TRACE=0` to turn off.
-To also include trace outputs in stderr do `repo --trace_to_stderr ...`
 """
 
 import sys
 import os
-import tempfile
-import time
-from contextlib import ContextDecorator
 
 # Env var to implicitly turn on tracing.
 REPO_TRACE = 'REPO_TRACE'
 
-# Temporarily set tracing to always on unless user expicitly sets to 0.
-_TRACE = os.environ.get(REPO_TRACE) != '0'
-
-_TRACE_TO_STDERR = False
-
-_TRACE_FILE = None
-
-_TRACE_FILE_NAME = 'TRACE_FILE'
-
-_MAX_SIZE = 5  # in mb
-
-_NEW_COMMAND_SEP = '+++++++++++++++NEW COMMAND+++++++++++++++++++'
-
-
-def IsStraceToStderr():
-  return _TRACE_TO_STDERR
+_TRACE = os.environ.get(REPO_TRACE) == '1'
 
 
 def IsTrace():
   return _TRACE
 
 
-def SetTraceToStderr():
-  global _TRACE_TO_STDERR
-  _TRACE_TO_STDERR = True
-
-
 def SetTrace():
   global _TRACE
   _TRACE = True
 
 
-def _SetTraceFile():
-  global _TRACE_FILE
-  _TRACE_FILE = _GetTraceFile()
-
-
-class Trace(ContextDecorator):
-
-    def _time(self):
-      """Generate nanoseconds of time in a py3.6 safe way"""
-      return int(time.time()*1e+9)
-
-    def __init__(self, fmt, *args, first_trace=False):
-      if not IsTrace():
-        return
-      self._trace_msg = fmt % args
-
-      if not _TRACE_FILE:
-        _SetTraceFile()
-
-      if first_trace:
-        _ClearOldTraces()
-        self._trace_msg = '%s %s' % (_NEW_COMMAND_SEP, self._trace_msg)
-
-
-    def __enter__(self):
-      if not IsTrace():
-        return self
-
-      print_msg = f"PID: {os.getpid()} START: {self._time()} :" + self._trace_msg + '\n'
-
-      with open(_TRACE_FILE, 'a') as f:
-        print(print_msg, file=f)
-
-      if _TRACE_TO_STDERR:
-        print(print_msg, file=sys.stderr)
-
-      return self
-
-    def __exit__(self, *exc):
-      if not IsTrace():
-        return False
-
-      print_msg = f"PID: {os.getpid()} END: {self._time()} :" + self._trace_msg + '\n'
-
-      with open(_TRACE_FILE, 'a') as f:
-        print(print_msg, file=f)
-
-      if _TRACE_TO_STDERR:
-        print(print_msg, file=sys.stderr)
-
-      return False
-
-
-def _GetTraceFile():
-  """Get the trace file or create one."""
-  # TODO: refactor to pass repodir to Trace.
-  repo_dir = os.path.dirname(os.path.dirname(__file__))
-  trace_file = os.path.join(repo_dir, _TRACE_FILE_NAME)
-  print('Trace outputs in %s' % trace_file)
-  return trace_file
-
-def _ClearOldTraces():
-  """Clear traces from old commands if trace file is too big.
-
-  Note: If the trace file contains output from two `repo`
-        commands that were running at the same time, this
-        will not work precisely.
-  """
-  if os.path.isfile(_TRACE_FILE):
-    while os.path.getsize(_TRACE_FILE)/(1024*1024) > _MAX_SIZE:
-      temp = tempfile.NamedTemporaryFile(mode='w', delete=False)
-      with open(_TRACE_FILE, 'r', errors='ignore') as fin:
-        trace_lines = fin.readlines()
-        for i , l in enumerate(trace_lines):
-          if 'END:' in l and _NEW_COMMAND_SEP in l:
-            temp.writelines(trace_lines[i+1:])
-            break
-      temp.close()
-      os.replace(temp.name, _TRACE_FILE)
+def Trace(fmt, *args):
+  if IsTrace():
+    print(fmt % args, file=sys.stderr)
diff --git a/run_tests b/run_tests
index 7c9ff41d..573dd446 100755
--- a/run_tests
+++ b/run_tests
@@ -20,7 +20,6 @@ import os
 import shutil
 import subprocess
 import sys
-import repo_trace
 
 
 def find_pytest():
diff --git a/ssh.py b/ssh.py
index 004fdbad..450383dc 100644
--- a/ssh.py
+++ b/ssh.py
@@ -182,29 +182,28 @@ class ProxyManager:
     # 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']
-    with Trace('Call to ssh (check call): %s', ' '.join(check_command)):
-      try:
-        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
+    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:]
-    p = None
     try:
-      with Trace('Call to ssh: %s', ' '.join(command)):
-        p = subprocess.Popen(command)
+      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'
diff --git a/subcmds/gitc_init.py b/subcmds/gitc_init.py
index e3a5813d..1d81baf5 100644
--- a/subcmds/gitc_init.py
+++ b/subcmds/gitc_init.py
@@ -68,8 +68,7 @@ use for this GITC client.
         sys.exit(1)
       manifest_file = opt.manifest_file
 
-    manifest = GitcManifest(self.repodir, os.path.join(self.client_dir,
-                                                       '.manifest'))
+    manifest = GitcManifest(self.repodir, gitc_client)
     manifest.Override(manifest_file)
     gitc_utils.generate_gitc_manifest(None, manifest)
     print('Please run `cd %s` to view your GITC client.' %
diff --git a/subcmds/sync.py b/subcmds/sync.py
index 83c9ad36..fe63b484 100644
--- a/subcmds/sync.py
+++ b/subcmds/sync.py
@@ -60,7 +60,7 @@ from error import RepoChangedException, GitError, ManifestParseError
 import platform_utils
 from project import SyncBuffer
 from progress import Progress
-from repo_trace import Trace
+from repo_trace import IsTrace, Trace
 import ssh
 from wrapper import Wrapper
 from manifest_xml import GitcManifest
@@ -739,6 +739,7 @@ later is required to fix a server side protocol bug.
     bak_dir = os.path.join(objdir, '.repo', 'pack.bak')
     if not _BACKUP_OBJECTS or not platform_utils.isdir(pack_dir):
       return
+    saved = []
     files = set(platform_utils.listdir(pack_dir))
     to_backup = []
     for f in files:
@@ -750,83 +751,12 @@ later is required to fix a server side protocol bug.
     for fname in to_backup:
       bak_fname = os.path.join(bak_dir, fname)
       if not os.path.exists(bak_fname):
-        with Trace('%s saved %s', bare_git._project.name, fname):
-          # Use a tmp file so that we are sure of a complete copy.
-          shutil.copy(os.path.join(pack_dir, fname), bak_fname + '.tmp')
-          shutil.move(bak_fname + '.tmp', bak_fname)
-
-  @staticmethod
-  def _GetPreciousObjectsState(project: Project, opt):
-    """Get the preciousObjects state for the project.
-
-    Args:
-      project (Project): the project to examine, and possibly correct.
-      opt (optparse.Values): options given to sync.
-
-    Returns:
-      Expected state of extensions.preciousObjects:
-        False: Should be disabled. (not present)
-        True: Should be enabled.
-    """
-    if project.use_git_worktrees:
-      return False
-    projects = project.manifest.GetProjectsWithName(project.name,
-                                                    all_manifests=True)
-    if len(projects) == 1:
-      return False
-    relpath = project.RelPath(local=opt.this_manifest_only)
-    if len(projects) > 1:
-      # Objects are potentially shared with another project.
-      # See the logic in Project.Sync_NetworkHalf regarding UseAlternates.
-      # - When False, shared projects share (via symlink)
-      #   .repo/project-objects/{PROJECT_NAME}.git as the one-and-only objects
-      #   directory.  All objects are precious, since there is no project with a
-      #   complete set of refs.
-      # - When True, shared projects share (via info/alternates)
-      #   .repo/project-objects/{PROJECT_NAME}.git as an alternate object store,
-      #   which is written only on the first clone of the project, and is not
-      #   written subsequently.  (When Sync_NetworkHalf sees that it exists, it
-      #   makes sure that the alternates file points there, and uses a
-      #   project-local .git/objects directory for all syncs going forward.
-      # We do not support switching between the options.  The environment
-      # variable is present for testing and migration only.
-      return not project.UseAlternates
-    print(f'\r{relpath}: project not found in manifest.', file=sys.stderr)
-    return False
-
-  def _RepairPreciousObjectsState(self, project: Project, opt):
-    """Correct the preciousObjects state for the project.
-
-    Args:
-      project (Project): the project to examine, and possibly correct.
-      opt (optparse.Values): options given to sync.
-    """
-    expected = self._GetPreciousObjectsState(project, opt)
-    actual = project.config.GetBoolean('extensions.preciousObjects') or False
-    relpath = project.RelPath(local = opt.this_manifest_only)
-
-    if (expected != actual and
-        not project.config.GetBoolean('repo.preservePreciousObjects')):
-      # If this is unexpected, log it and repair.
-      Trace(f'{relpath} expected preciousObjects={expected}, got {actual}')
-      if expected:
-        if not opt.quiet:
-          print('\r%s: Shared project %s found, disabling pruning.' %
-                (relpath, project.name))
-        if git_require((2, 7, 0)):
-          project.EnableRepositoryExtension('preciousObjects')
-        else:
-          # This isn't perfect, but it's the best we can do with old git.
-          print('\r%s: WARNING: shared projects are unreliable when using '
-                'old versions of git; please upgrade to git-2.7.0+.'
-                % (relpath,),
-                file=sys.stderr)
-          project.config.SetString('gc.pruneExpire', 'never')
-      else:
-        if not opt.quiet:
-          print(f'\r{relpath}: not shared, disabling pruning.')
-        project.config.SetString('extensions.preciousObjects', None)
-        project.config.SetString('gc.pruneExpire', None)
+        saved.append(fname)
+        # Use a tmp file so that we are sure of a complete copy.
+        shutil.copy(os.path.join(pack_dir, fname), bak_fname + '.tmp')
+        shutil.move(bak_fname + '.tmp', bak_fname)
+    if saved:
+      Trace('%s saved %s', bare_git._project.name, ' '.join(saved))
 
   def _GCProjects(self, projects, opt, err_event):
     pm = Progress('Garbage collecting', len(projects), delay=False, quiet=opt.quiet)
@@ -834,8 +764,27 @@ later is required to fix a server side protocol bug.
 
     tidy_dirs = {}
     for project in projects:
-      self._RepairPreciousObjectsState(project, opt)
-
+      # Make sure pruning never kicks in with shared projects that do not use
+      # alternates to avoid corruption.
+      if (not project.use_git_worktrees and
+              len(project.manifest.GetProjectsWithName(project.name, all_manifests=True)) > 1):
+        if project.UseAlternates:
+          # Undo logic set by previous versions of repo.
+          project.config.SetString('extensions.preciousObjects', None)
+          project.config.SetString('gc.pruneExpire', None)
+        else:
+          if not opt.quiet:
+            print('\r%s: Shared project %s found, disabling pruning.' %
+                  (project.relpath, project.name))
+          if git_require((2, 7, 0)):
+            project.EnableRepositoryExtension('preciousObjects')
+          else:
+            # This isn't perfect, but it's the best we can do with old git.
+            print('\r%s: WARNING: shared projects are unreliable when using old '
+                  'versions of git; please upgrade to git-2.7.0+.'
+                  % (project.relpath,),
+                  file=sys.stderr)
+            project.config.SetString('gc.pruneExpire', 'never')
       project.config.SetString('gc.autoDetach', 'false')
       # Only call git gc once per objdir, but call pack-refs for the remainder.
       if project.objdir not in tidy_dirs:
diff --git a/tests/test_git_config.py b/tests/test_git_config.py
index 0df38430..a4fad9ef 100644
--- a/tests/test_git_config.py
+++ b/tests/test_git_config.py
@@ -19,7 +19,6 @@ import tempfile
 import unittest
 
 import git_config
-import repo_trace
 
 
 def fixture(*paths):
@@ -34,16 +33,9 @@ class GitConfigReadOnlyTests(unittest.TestCase):
   def setUp(self):
     """Create a GitConfig object using the test.gitconfig fixture.
     """
-
-    self.tempdirobj = tempfile.TemporaryDirectory(prefix='repo_tests')
-    repo_trace._TRACE_FILE = os.path.join(self.tempdirobj.name, 'TRACE_FILE_from_test')
-
     config_fixture = fixture('test.gitconfig')
     self.config = git_config.GitConfig(config_fixture)
 
-  def tearDown(self):
-    self.tempdirobj.cleanup()
-
   def test_GetString_with_empty_config_values(self):
     """
     Test config entries with no value.
@@ -117,15 +109,9 @@ class GitConfigReadWriteTests(unittest.TestCase):
   """Read/write tests of the GitConfig class."""
 
   def setUp(self):
-    self.tempdirobj = tempfile.TemporaryDirectory(prefix='repo_tests')
-    repo_trace._TRACE_FILE = os.path.join(self.tempdirobj.name, 'TRACE_FILE_from_test')
-
     self.tmpfile = tempfile.NamedTemporaryFile()
     self.config = self.get_config()
 
-  def tearDown(self):
-    self.tempdirobj.cleanup()
-
   def get_config(self):
     """Get a new GitConfig instance."""
     return git_config.GitConfig(self.tmpfile.name)
diff --git a/tests/test_git_superproject.py b/tests/test_git_superproject.py
index 0bb77185..0ad9b01d 100644
--- a/tests/test_git_superproject.py
+++ b/tests/test_git_superproject.py
@@ -24,7 +24,6 @@ from unittest import mock
 import git_superproject
 import git_trace2_event_log
 import manifest_xml
-import repo_trace
 from test_manifest_xml import sort_attributes
 
 
@@ -40,7 +39,6 @@ class SuperprojectTestCase(unittest.TestCase):
     """Set up superproject every time."""
     self.tempdirobj = tempfile.TemporaryDirectory(prefix='repo_tests')
     self.tempdir = self.tempdirobj.name
-    repo_trace._TRACE_FILE = os.path.join(self.tempdir, 'TRACE_FILE_from_test')
     self.repodir = os.path.join(self.tempdir, '.repo')
     self.manifest_file = os.path.join(
         self.repodir, manifest_xml.MANIFEST_FILE_NAME)
diff --git a/tests/test_manifest_xml.py b/tests/test_manifest_xml.py
index f92108e1..e181b642 100644
--- a/tests/test_manifest_xml.py
+++ b/tests/test_manifest_xml.py
@@ -23,7 +23,6 @@ import xml.dom.minidom
 
 import error
 import manifest_xml
-import repo_trace
 
 
 # Invalid paths that we don't want in the filesystem.
@@ -94,7 +93,6 @@ class ManifestParseTestCase(unittest.TestCase):
   def setUp(self):
     self.tempdirobj = tempfile.TemporaryDirectory(prefix='repo_tests')
     self.tempdir = self.tempdirobj.name
-    repo_trace._TRACE_FILE = os.path.join(self.tempdir, 'TRACE_FILE_from_test')
     self.repodir = os.path.join(self.tempdir, '.repo')
     self.manifest_dir = os.path.join(self.repodir, 'manifests')
     self.manifest_file = os.path.join(
@@ -264,10 +262,10 @@ class XmlManifestTests(ManifestParseTestCase):
         '<project name="r" groups="keep"/>'
         '</manifest>')
     self.assertEqual(
-        sort_attributes(manifest.ToXml(omit_local=True).toxml()),
+        manifest.ToXml(omit_local=True).toxml(),
         '<?xml version="1.0" ?><manifest>'
-        '<remote fetch=".." name="a"/><default remote="a" revision="r"/>'
-        '<project name="q"/><project groups="keep" name="r"/></manifest>')
+        '<remote name="a" fetch=".."/><default remote="a" revision="r"/>'
+        '<project name="q"/><project name="r" groups="keep"/></manifest>')
 
   def test_toxml_with_local(self):
     """Does include local_manifests projects when omit_local=False."""
@@ -279,11 +277,11 @@ class XmlManifestTests(ManifestParseTestCase):
         '<project name="r" groups="keep"/>'
         '</manifest>')
     self.assertEqual(
-        sort_attributes(manifest.ToXml(omit_local=False).toxml()),
+        manifest.ToXml(omit_local=False).toxml(),
         '<?xml version="1.0" ?><manifest>'
-        '<remote fetch=".." name="a"/><default remote="a" revision="r"/>'
-        '<project groups="local::me" name="p"/>'
-        '<project name="q"/><project groups="keep" name="r"/></manifest>')
+        '<remote name="a" fetch=".."/><default remote="a" revision="r"/>'
+        '<project name="p" groups="local::me"/>'
+        '<project name="q"/><project name="r" groups="keep"/></manifest>')
 
   def test_repo_hooks(self):
     """Check repo-hooks settings."""
diff --git a/tests/test_project.py b/tests/test_project.py
index 5c600be7..acd44ccc 100644
--- a/tests/test_project.py
+++ b/tests/test_project.py
@@ -26,7 +26,6 @@ import git_command
 import git_config
 import platform_utils
 import project
-import repo_trace
 
 
 @contextlib.contextmanager
@@ -65,13 +64,6 @@ class FakeProject(object):
 class ReviewableBranchTests(unittest.TestCase):
   """Check ReviewableBranch behavior."""
 
-  def setUp(self):
-    self.tempdirobj = tempfile.TemporaryDirectory(prefix='repo_tests')
-    repo_trace._TRACE_FILE = os.path.join(self.tempdirobj.name, 'TRACE_FILE_from_test')
-
-  def tearDown(self):
-    self.tempdirobj.cleanup()
-
   def test_smoke(self):
     """A quick run through everything."""
     with TempGitTree() as tempdir:
diff --git a/tests/test_subcmds_sync.py b/tests/test_subcmds_sync.py
index 13f3f873..aad713f2 100644
--- a/tests/test_subcmds_sync.py
+++ b/tests/test_subcmds_sync.py
@@ -11,9 +11,9 @@
 # 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 subcmds/sync.py module."""
 
-import unittest
 from unittest import mock
 
 import pytest
@@ -21,14 +21,17 @@ import pytest
 from subcmds import sync
 
 
-@pytest.mark.parametrize('use_superproject, cli_args, result', [
+@pytest.mark.parametrize(
+  'use_superproject, cli_args, result',
+  [
     (True, ['--current-branch'], True),
     (True, ['--no-current-branch'], True),
     (True, [], True),
     (False, ['--current-branch'], True),
     (False, ['--no-current-branch'], False),
     (False, [], None),
-])
+  ]
+)
 def test_get_current_branch_only(use_superproject, cli_args, result):
   """Test Sync._GetCurrentBranchOnly logic.
 
@@ -38,49 +41,5 @@ def test_get_current_branch_only(use_superproject, cli_args, result):
   cmd = sync.Sync()
   opts, _ = cmd.OptionParser.parse_args(cli_args)
 
-  with mock.patch('git_superproject.UseSuperproject',
-                  return_value=use_superproject):
+  with mock.patch('git_superproject.UseSuperproject', return_value=use_superproject):
     assert cmd._GetCurrentBranchOnly(opts, cmd.manifest) == result
-
-
-class GetPreciousObjectsState(unittest.TestCase):
-  """Tests for _GetPreciousObjectsState."""
-
-  def setUp(self):
-    """Common setup."""
-    self.cmd = sync.Sync()
-    self.project = p = mock.MagicMock(use_git_worktrees=False,
-                                      UseAlternates=False)
-    p.manifest.GetProjectsWithName.return_value = [p]
-
-    self.opt = mock.Mock(spec_set=['this_manifest_only'])
-    self.opt.this_manifest_only = False
-
-  def test_worktrees(self):
-    """False for worktrees."""
-    self.project.use_git_worktrees = True
-    self.assertFalse(self.cmd._GetPreciousObjectsState(self.project, self.opt))
-
-  def test_not_shared(self):
-    """Singleton project."""
-    self.assertFalse(self.cmd._GetPreciousObjectsState(self.project, self.opt))
-
-  def test_shared(self):
-    """Shared project."""
-    self.project.manifest.GetProjectsWithName.return_value = [
-        self.project, self.project
-    ]
-    self.assertTrue(self.cmd._GetPreciousObjectsState(self.project, self.opt))
-
-  def test_shared_with_alternates(self):
-    """Shared project, with alternates."""
-    self.project.manifest.GetProjectsWithName.return_value = [
-        self.project, self.project
-    ]
-    self.project.UseAlternates = True
-    self.assertFalse(self.cmd._GetPreciousObjectsState(self.project, self.opt))
-
-  def test_not_found(self):
-    """Project not found in manifest."""
-    self.project.manifest.GetProjectsWithName.return_value = []
-    self.assertFalse(self.cmd._GetPreciousObjectsState(self.project, self.opt))