project: fix diff printing with embedded %

The recent commit 84230009ee ("project:
make diff tools synchronous") broke repo diff if it includes % formats.
Add an explicit format string to fix.

Bug: https://crbug.com/gerrit/14208
Change-Id: Ie255a43c5b767488616b2b3dd15abc18f93bfab2
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/299402
Reviewed-by: Michael Mortensen <mmortensen@google.com>
Tested-by: Mike Frysinger <vapier@google.com>

git_config.py

@@ -145,6 +145,21 @@ class GitConfig(object):
     except ValueError:
       return None
 
+  def DumpConfigDict(self):
+    """Returns the current configuration dict.
+
+    Configuration data is information only (e.g. logging) and
+    should not be considered a stable data-source.
+
+    Returns:
+      dict of {<key>, <value>} for git configuration cache.
+      <value> are strings converted by GetString.
+    """
+    config_dict = {}
+    for key in self._cache:
+      config_dict[key] = self.GetString(key)
+    return config_dict
+
   def GetBoolean(self, name):
     """Returns a boolean from the configuration file.
        None : The value was not defined, or is not a boolean.

git_superproject.py

@@ -41,7 +41,8 @@ class Superproject(object):
   lookup of commit ids for all projects. It contains _project_commit_ids which
   is a dictionary with project/commit id entries.
   """
-  def __init__(self, manifest, repodir, superproject_dir='exp-superproject'):
+  def __init__(self, manifest, repodir, superproject_dir='exp-superproject',
+               quiet=False):
     """Initializes superproject.
 
     Args:
@@ -49,9 +50,11 @@ class Superproject(object):
       repodir: Path to the .repo/ dir for holding all internal checkout state.
           It must be in the top directory of the repo client checkout.
       superproject_dir: Relative path under |repodir| to checkout superproject.
+      quiet:  If True then only print the progress messages.
     """
     self._project_commit_ids = None
     self._manifest = manifest
+    self._quiet = quiet
     self._branch = self._GetBranch()
     self._repodir = os.path.abspath(repodir)
     self._superproject_dir = superproject_dir
@@ -89,6 +92,9 @@ class Superproject(object):
     """
     if not os.path.exists(self._superproject_path):
       os.mkdir(self._superproject_path)
+    if not self._quiet and not os.path.exists(self._work_git):
+      print('%s: Performing initial setup for superproject; this might take '
+            'several minutes.' % self._work_git)
     cmd = ['init', '--bare', self._work_git_name]
     p = GitCommand(None,
                    cmd,
@@ -183,6 +189,8 @@ class Superproject(object):
       return False
     if not self._Fetch(url):
       return False
+    if not self._quiet:
+      print('%s: Initial setup for superproject completed.' % self._work_git)
     return True
 
   def _GetAllProjectsCommitIds(self):

git_trace2_event_log.py

@@ -132,6 +132,21 @@ class EventLog(object):
     exit_event['code'] = result
     self._log.append(exit_event)
 
+  def DefParamRepoEvents(self, config):
+    """Append a 'def_param' event for each repo.* config key to the current log.
+
+    Args:
+      config: Repo configuration dictionary
+    """
+    # Only output the repo.* config parameters.
+    repo_config = {k: v for k, v in config.items() if k.startswith('repo.')}
+
+    for param, value in repo_config.items():
+      def_param_event = self._CreateEventDict('def_param')
+      def_param_event['param'] = param
+      def_param_event['value'] = value
+      self._log.append(def_param_event)
+
   def _GetEventTargetPath(self):
     """Get the 'trace2.eventtarget' path from git configuration.
 

main.py

@@ -297,6 +297,8 @@ class _Repo(object):
 
       cmd.event_log.FinishEvent(cmd_event, finish,
                                 result is None or result == 0)
+      git_trace2_event_log.DefParamRepoEvents(
+          cmd.manifest.manifestProject.config.DumpConfigDict())
       git_trace2_event_log.ExitEvent(result)
 
       if gopts.event_log:

project.py

@@ -863,7 +863,7 @@ class Project(object):
       out.nl()
       out.project('project %s/' % self.relpath)
       out.nl()
-      out.write(p.stdout)
+      out.write('%s', p.stdout)
     return p.Wait() == 0
 
 # Publish / Upload ##

repo

@@ -318,7 +318,7 @@ def GetParser(gitc_init=False):
                    help='filter for use with --partial-clone '
                         '[default: %default]')
   group.add_option('--worktree', action='store_true',
-                   help=optparse.SUPPRESS_HELP)
+                   help='use git-worktree to manage projects')
   group.add_option('--archive', action='store_true',
                    help='checkout an archive instead of a git repository for '
                         'each project. See git archive.')

subcmds/init.py

@@ -127,10 +127,8 @@ to update the working directory files.
     g.add_option('--clone-filter', action='store', default='blob:none',
                  dest='clone_filter',
                  help='filter for use with --partial-clone [default: %default]')
-    # TODO(vapier): Expose option with real help text once this has been in the
-    # wild for a while w/out significant bug reports.  Goal is by ~Sep 2020.
     g.add_option('--worktree', action='store_true',
-                 help=optparse.SUPPRESS_HELP)
+                 help='use git-worktree to manage projects')
     g.add_option('--archive',
                  dest='archive', action='store_true',
                  help='checkout an archive instead of a git repository for '
@@ -185,10 +183,15 @@ to update the working directory files.
     return {'REPO_MANIFEST_URL': 'manifest_url',
             'REPO_MIRROR_LOCATION': 'reference'}
 
-  def _CloneSuperproject(self):
+  def _CloneSuperproject(self, opt):
-    """Clone the superproject based on the superproject's url and branch."""
+    """Clone the superproject based on the superproject's url and branch.
+
+    Args:
+      opt: Program options returned from optparse.  See _Options().
+    """
     superproject = git_superproject.Superproject(self.manifest,
-                                                 self.repodir)
+                                                 self.repodir,
+                                                 quiet=opt.quiet)
     if not superproject.Sync():
       print('error: git update of superproject failed', file=sys.stderr)
       sys.exit(1)
@@ -553,7 +556,7 @@ to update the working directory files.
     self._LinkManifest(opt.manifest_name)
 
     if self.manifest.manifestProject.config.GetBoolean('repo.superproject'):
-      self._CloneSuperproject()
+      self._CloneSuperproject(opt)
 
     if os.isatty(0) and os.isatty(1) and not self.manifest.IsMirror:
       if opt.config_name or self._ShouldConfigureUser(opt):

subcmds/sync.py

@@ -292,7 +292,8 @@ later is required to fix a server side protocol bug.
       Returns path to the overriding manifest file.
     """
     superproject = git_superproject.Superproject(self.manifest,
-                                                 self.repodir)
+                                                 self.repodir,
+                                                 quiet=opt.quiet)
     all_projects = self.GetProjects(args,
                                     missing_ok=True,
                                     submodules_ok=opt.fetch_submodules)

tests/test_git_trace2_event_log.py

@@ -161,6 +161,55 @@ class EventLogTestCase(unittest.TestCase):
     self.assertIn('code', exit_event)
     self.assertEqual(exit_event['code'], 2)
 
+  def test_def_params_event_repo_config(self):
+    """Test 'def_params' event data outputs only repo config keys.
+
+    Expected event log:
+    <version event>
+    <def_param event>
+    <def_param event>
+    """
+    config = {
+        'git.foo': 'bar',
+        'repo.partialclone': 'true',
+        'repo.partialclonefilter': 'blob:none',
+    }
+    self._event_log_module.DefParamRepoEvents(config)
+
+    with tempfile.TemporaryDirectory(prefix='event_log_tests') as tempdir:
+      log_path = self._event_log_module.Write(path=tempdir)
+      self._log_data = self.readLog(log_path)
+
+    self.assertEqual(len(self._log_data), 3)
+    def_param_events = self._log_data[1:]
+    self.verifyCommonKeys(self._log_data[0], expected_event_name='version')
+
+    for event in def_param_events:
+      self.verifyCommonKeys(event, expected_event_name='def_param')
+      # Check for 'def_param' event specific fields.
+      self.assertIn('param', event)
+      self.assertIn('value', event)
+      self.assertTrue(event['param'].startswith('repo.'))
+
+  def test_def_params_event_no_repo_config(self):
+    """Test 'def_params' event data won't output non-repo config keys.
+
+    Expected event log:
+    <version event>
+    """
+    config = {
+        'git.foo': 'bar',
+        'git.core.foo2': 'baz',
+    }
+    self._event_log_module.DefParamRepoEvents(config)
+
+    with tempfile.TemporaryDirectory(prefix='event_log_tests') as tempdir:
+      log_path = self._event_log_module.Write(path=tempdir)
+      self._log_data = self.readLog(log_path)
+
+    self.assertEqual(len(self._log_data), 1)
+    self.verifyCommonKeys(self._log_data[0], expected_event_name='version')
+
   def test_write_with_filename(self):
     """Test Write() with a path to a file exits with None."""
     self.assertIsNone(self._event_log_module.Write(path='path/to/file'))

tests/test_wrapper.py

@@ -305,8 +305,8 @@ class Requirements(RepoWrapperTestCase):
     reqs = self.wrapper.Requirements({'python': {'hard': sys.version_info}})
     reqs.assert_all()
 
-  def test_assert_all_old_repo(self):
+  def test_assert_all_old_python(self):
-    """Check assert_all rejects old repo."""
+    """Check assert_all rejects old python."""
     reqs = self.wrapper.Requirements({'python': {'hard': [99999, 0]}})
     with self.assertRaises(SystemExit):
       reqs.assert_all()