Merge branch 'main' of https://gerrit.googlesource.com/git-repo

Change-Id: Icf78aaf646ef269b59bf953dda71c92806d6d643

docs/manifest-format.md

@@ -36,7 +36,7 @@ following DTD:
 
   <!ELEMENT notice (#PCDATA)>
 
-  <!ELEMENT remote EMPTY>
+  <!ELEMENT remote (annotation*)>
   <!ATTLIST remote name         ID    #REQUIRED>
   <!ATTLIST remote alias        CDATA #IMPLIED>
   <!ATTLIST remote fetch        CDATA #REQUIRED>
@@ -348,12 +348,12 @@ project.  Same syntax as the corresponding element of `project`.
 ### Element annotation
 
 Zero or more annotation elements may be specified as children of a
-project element. Each element describes a name-value pair that will be
+project or remote element. Each element describes a name-value pair.
-exported into each project's environment during a 'forall' command,
+For projects, this name-value pair will be exported into each project's
-prefixed with REPO__.  In addition, there is an optional attribute
+environment during a 'forall' command, prefixed with `REPO__`.  In addition,
-"keep" which accepts the case insensitive values "true" (default) or
+there is an optional attribute "keep" which accepts the case insensitive values
-"false".  This attribute determines whether or not the annotation will
+"true" (default) or "false".  This attribute determines whether or not the
-be kept when exported with the manifest subcommand.
+annotation will be kept when exported with the manifest subcommand.
 
 ### Element copyfile
 

git_config.py

@@ -13,7 +13,7 @@
 # limitations under the License.
 
 import contextlib
-from datetime import datetime
+import datetime
 import errno
 from http.client import HTTPException
 import json
@@ -31,6 +31,8 @@ from repo_trace import Trace
 from git_command import GitCommand
 from git_refs import R_CHANGES, R_HEADS, R_TAGS
 
+_SYNC_STATE_PREFIX = 'syncstate.'
+
 ID_RE = re.compile(r'^[0-9a-f]{40}$')
 
 REVIEW_CACHE = dict()
@@ -263,17 +265,21 @@ class GitConfig(object):
       self._branches[b.name] = b
     return b
 
-  def GetSyncState(self):
+  def GetSyncAnalysisStateData(self):
-    """Get the state sync object."""
+    """Returns data to be logged for the analysis of sync performance."""
-    return self._syncState
+    return {k: v for k, v in self.DumpConfigDict().items() if k.startswith(_SYNC_STATE_PREFIX)}
 
-  def SetSyncState(self, sync_state):
+  def UpdateSyncAnalysisState(self, options, superproject_logging_data):
-    """Update Config's SyncState object with the new |sync_state| object.
+    """Update Config's SyncAnalysisState with the latest sync data.
+
+    Creates SyncAnalysisState object with |options| and |superproject_logging_data|
+    which in turn persists the data into the |self| object.
 
     Args:
-      sync_state: Current SyncState object.
+      options: Options passed to sync returned from optparse. See _Options().
+      superproject_logging_data: A dictionary of superproject data that is to be logged.
     """
-    self._syncState = sync_state
+    self._syncState = SyncAnalysisState(self, options, superproject_logging_data)
 
   def GetSubSections(self, section):
     """List all subsection names matching $section.*.*
@@ -732,12 +738,13 @@ class Branch(object):
     return self._config.GetString(key, all_keys=all_keys)
 
 
-class SyncState(object):
+class SyncAnalysisState():
-  """Configuration options related Sync object.
+  """Configuration options related to logging of Sync state for analysis.
-  """
 
-  def __init__(self, config, options, superproject):
+  This object is versioned.
-    """Initializes SyncState.
+  """
+  def __init__(self, config, options, superproject_logging_data):
+    """Initializes SyncAnalysisState.
 
     Saves argv, |options|, superproject and repo.*, branch.* and remote.*
     parameters from |config| object. It also saves current time as synctime.
@@ -747,36 +754,45 @@ class SyncState(object):
     Args:
       config: GitConfig object to store all options.
       options: Options passed to sync returned from optparse. See _Options().
-      superproject: A dictionary of superproject configuration parameters.
+      superproject_logging_data: A dictionary of superproject data that is to be logged.
     """
     self._config = config
-    now = datetime.utcnow()
+    now = datetime.datetime.utcnow()
-    self._Set('synctime', now.strftime('%d/%m/%Y %H:%M:%S'))
+    self._Set('main.synctime', now.isoformat() + 'Z')
-    self._Set('version', '1.0')
+    self._Set('main.version', '1')
-    self._Set('argv', sys.argv)
+    self._Set('sys.argv', sys.argv)
-    self._SetDictionary(superproject)
+    for key, value in superproject_logging_data.items():
+      self._Set(f'superproject.{key}', value)
     for key, value in options.__dict__.items():
-      self._Set(key, value)
+      self._Set(f'options.{key}', value)
     config_items = config.DumpConfigDict().items()
     self._SetDictionary({k: v for k, v in config_items if k.startswith('repo.')})
     self._SetDictionary({k: v for k, v in config_items if k.startswith('branch.')})
     self._SetDictionary({k: v for k, v in config_items if k.startswith('remote.')})
 
-  def _SetDictionary(self, config_dict):
+  def _SetDictionary(self, data):
-    for key, value in config_dict.items():
+    """Save all key/value pairs of |data| dictionary.
+
+    Args:
+      data: A dictionary whose key/value are to be saved,
+    """
+    for key, value in data.items():
       self._Set(key, value)
 
   def _Set(self, key, value):
-    if value is None:
+    """Set the |value| for a |key| in the |_config| member.
-      return None
-    key = 'syncstate.%s' % (key)
-    if isinstance(value, str):
-      return self._config.SetString(key, value)
-    elif isinstance(value, bool):
-      return self._config.SetBoolean(key, value)
-    else:
-      return self._config.SetString(key, str(value))
 
-  def _Get(self, key, all_keys=False):
+    Args:
-    key = 'syncstate.%s' % (key)
+      key: Name of the key.
-    return self._config.GetString(key, all_keys=all_keys)
+      value: |value| could be of any type. If it is 'bool', it will be saved
+             as a Boolean and for all other types, it will be saved as a String.
+    """
+    if value is None:
+      return
+    key = f'{_SYNC_STATE_PREFIX}.{key}'
+    if isinstance(value, str):
+      self._config.SetString(key, value)
+    elif isinstance(value, bool):
+      self._config.SetBoolean(key, value)
+    else:
+      self._config.SetString(key, str(value))

git_trace2_event_log.py

@@ -145,10 +145,10 @@ class EventLog(object):
     self._log.append(command_event)
 
   def _LogConfigEvents(self, config, event_dict_name):
-    """Append a |event_dict_name| event for each config key in |config| to the current log.
+    """Append a |event_dict_name| event for each config key in |config|.
 
     Args:
-      config: Configuration dictionary
+      config: Configuration dictionary.
       event_dict_name: Name of the event dictionary for items to be logged under.
     """
     for param, value in config.items():
@@ -167,16 +167,14 @@ class EventLog(object):
     repo_config = {k: v for k, v in config.items() if k.startswith('repo.')}
     self._LogConfigEvents(repo_config, 'def_param')
 
-  def AddSyncStateEvents(self, config, event_dict_name):
+  def AddSyncAnalysisStateEvents(self, config, event_dict_name):
-    """Append a log event for each syncstate.* config key to the current log.
+    """Append log events for all the data in |config|'s SyncAnalysisState object.
 
     Args:
-      config: SyncState configuration dictionary
+      config: GitConfig object which has SyncAnalysisState data.
       event_dict_name: Name of the event dictionary for items to be logged under.
     """
-    # Only output syncstate.* config parameters.
+    self._LogConfigEvents(config.GetSyncAnalysisStateData(), event_dict_name)
-    sync_config = {k: v for k, v in config.items() if k.startswith('syncstate.')}
-    self._LogConfigEvents(sync_config, event_dict_name)
 
   def ErrorEvent(self, msg, fmt):
     """Append a 'error' event to the current log."""

manifest_xml.py

@@ -25,7 +25,7 @@ import gitc_utils
 from git_config import GitConfig, IsId
 from git_refs import R_HEADS, HEAD
 import platform_utils
-from project import RemoteSpec, Project, MetaProject
+from project import Annotation, RemoteSpec, Project, MetaProject
 from error import (ManifestParseError, ManifestInvalidPathError,
                    ManifestInvalidRevisionError)
 from wrapper import Wrapper
@@ -149,16 +149,18 @@ class _XmlRemote(object):
     self.reviewUrl = review
     self.revision = revision
     self.resolvedFetchUrl = self._resolveFetchUrl()
+    self.annotations = []
 
   def __eq__(self, other):
     if not isinstance(other, _XmlRemote):
       return False
-    return self.__dict__ == other.__dict__
+    return (sorted(self.annotations) == sorted(other.annotations) and
+      self.name == other.name and self.fetchUrl == other.fetchUrl and
+      self.pushUrl == other.pushUrl and self.remoteAlias == other.remoteAlias
+      and self.reviewUrl == other.reviewUrl and self.revision == other.revision)
 
   def __ne__(self, other):
-    if not isinstance(other, _XmlRemote):
+    return not self.__eq__(other)
-      return True
-    return self.__dict__ != other.__dict__
 
   def _resolveFetchUrl(self):
     if self.fetchUrl is None:
@@ -191,6 +193,9 @@ class _XmlRemote(object):
                       orig_name=self.name,
                       fetchUrl=self.fetchUrl)
 
+  def AddAnnotation(self, name, value, keep):
+    self.annotations.append(Annotation(name, value, keep))
+
 
 class XmlManifest(object):
   """manages the repo configuration file"""
@@ -300,6 +305,13 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
     if r.revision is not None:
       e.setAttribute('revision', r.revision)
 
+    for a in r.annotations:
+      if a.keep == 'true':
+        ae = doc.createElement('annotation')
+        ae.setAttribute('name', a.name)
+        ae.setAttribute('value', a.value)
+        e.appendChild(ae)
+
   def _ParseList(self, field):
     """Parse fields that contain flattened lists.
 
@@ -625,6 +637,13 @@ 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 UseLocalManifests(self):
+    return self._load_local_manifests
+
+  def SetUseLocalManifests(self, value):
+    self._load_local_manifests = value
+
   @property
   def HasLocalManifests(self):
     return self._load_local_manifests and self.local_manifests
@@ -988,7 +1007,14 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
     if revision == '':
       revision = None
     manifestUrl = self.manifestProject.config.GetString('remote.origin.url')
-    return _XmlRemote(name, alias, fetch, pushUrl, manifestUrl, review, revision)
+
+    remote = _XmlRemote(name, alias, fetch, pushUrl, manifestUrl, review, revision)
+
+    for n in node.childNodes:
+      if n.nodeName == 'annotation':
+        self._ParseAnnotation(remote, n)
+
+    return remote
 
   def _ParseDefault(self, node):
     """
@@ -1355,7 +1381,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
       self._ValidateFilePaths('linkfile', src, dest)
       project.AddLinkFile(src, dest, self.topdir)
 
-  def _ParseAnnotation(self, project, node):
+  def _ParseAnnotation(self, element, node):
     name = self._reqatt(node, 'name')
     value = self._reqatt(node, 'value')
     try:
@@ -1365,7 +1391,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
     if keep != "true" and keep != "false":
       raise ManifestParseError('optional "keep" attribute must be '
                                '"true" or "false"')
-    project.AddAnnotation(name, value, keep)
+    element.AddAnnotation(name, value, keep)
 
   def _get_remote(self, node):
     name = node.getAttribute('remote')

project.py

@@ -251,13 +251,29 @@ class DiffColoring(Coloring):
     self.fail = self.printer('fail', fg='red')
 
 
-class _Annotation(object):
+class Annotation(object):
 
   def __init__(self, name, value, keep):
     self.name = name
     self.value = value
     self.keep = keep
 
+  def __eq__(self, other):
+    if not isinstance(other, Annotation):
+      return False
+    return self.__dict__ == other.__dict__
+
+  def __lt__(self, other):
+    # This exists just so that lists of Annotation objects can be sorted, for
+    # use in comparisons.
+    if not isinstance(other, Annotation):
+      raise ValueError('comparison is not between two Annotation objects')
+    if self.name == other.name:
+      if self.value == other.value:
+        return self.keep < other.keep
+      return self.value < other.value
+    return self.name < other.name
+
 
 def _SafeExpandPath(base, subpath, skipfinal=False):
   """Make sure |subpath| is completely safe under |base|.
@@ -1448,7 +1464,7 @@ class Project(object):
     self.linkfiles.append(_LinkFile(self.worktree, src, topdir, dest))
 
   def AddAnnotation(self, name, value, keep):
-    self.annotations.append(_Annotation(name, value, keep))
+    self.annotations.append(Annotation(name, value, keep))
 
   def DownloadPatchSet(self, change_id, patch_id):
     """Download a single patch set of a single change to FETCH_HEAD.
@@ -1971,6 +1987,7 @@ class Project(object):
         rev = self.GetRemote(self.remote.name).ToLocal(self.upstream)
         self.bare_git.rev_list('-1', '--missing=allow-any',
                                '%s^0' % rev, '--')
+        self.bare_git.merge_base('--is-ancestor', self.revisionExpr, rev)
       return True
     except GitError:
       # There is no such persistent revision. We have to fetch it.

subcmds/manifest.py

@@ -70,6 +70,8 @@ to indicate the remote ref to push changes to via 'repo upload'.
                  help='output manifest in JSON format (experimental)')
     p.add_option('--pretty', default=False, action='store_true',
                  help='format output for humans to read')
+    p.add_option('--no-local-manifests', default=False, action='store_true',
+                 dest='ignore_local_manifests', help='ignore local manifests')
     p.add_option('-o', '--output-file',
                  dest='output_file',
                  default='-',
@@ -85,6 +87,9 @@ to indicate the remote ref to push changes to via 'repo upload'.
       fd = sys.stdout
     else:
       fd = open(opt.output_file, 'w')
+
+    self.manifest.SetUseLocalManifests(not opt.ignore_local_manifests)
+
     if opt.json:
       print('warning: --json is experimental!', file=sys.stderr)
       doc = self.manifest.ToDict(peg_rev=opt.peg_rev,

subcmds/sync.py

@@ -46,7 +46,7 @@ except ImportError:
 
 import event_log
 from git_command import git_require
-from git_config import GetUrlCookieFile, SyncState
+from git_config import GetUrlCookieFile
 from git_refs import R_HEADS, HEAD
 import git_superproject
 import gitc_utils
@@ -282,7 +282,7 @@ later is required to fix a server side protocol bug.
     """Returns True if current-branch or use-superproject options are enabled."""
     return opt.current_branch_only or git_superproject.UseSuperproject(opt, self.manifest)
 
-  def _UpdateProjectsRevisionId(self, opt, args, load_local_manifests):
+  def _UpdateProjectsRevisionId(self, opt, args, load_local_manifests, superproject_logging_data):
     """Update revisionId of every project with the SHA from superproject.
 
     This function updates each project's revisionId with SHA from superproject.
@@ -293,6 +293,7 @@ later is required to fix a server side protocol bug.
       args: Arguments to pass to GetProjects. See the GetProjects
           docstring for details.
       load_local_manifests: Whether to load local manifests.
+      superproject_logging_data: A dictionary of superproject data that is to be logged.
 
     Returns:
       Returns path to the overriding manifest file instead of None.
@@ -306,7 +307,7 @@ later is required to fix a server side protocol bug.
                                     submodules_ok=opt.fetch_submodules)
     update_result = superproject.UpdateProjectsRevisionId(all_projects)
     manifest_path = update_result.manifest_path
-    self.superproject['superprojectSyncSuccessful'] = True if manifest_path else False
+    superproject_logging_data['updatedrevisionid'] = bool(manifest_path)
     if manifest_path:
       self._ReloadManifest(manifest_path, load_local_manifests)
     else:
@@ -959,12 +960,13 @@ later is required to fix a server side protocol bug.
       self._UpdateManifestProject(opt, mp, manifest_name)
 
     load_local_manifests = not self.manifest.HasLocalManifests
-    self.superproject = {}
+    superproject_logging_data = {}
     use_superproject = git_superproject.UseSuperproject(opt, self.manifest)
-    self.superproject['superproject'] = use_superproject
+    superproject_logging_data['superproject'] = use_superproject
-    self.superproject['hasLocalManifests'] = True if self.manifest.HasLocalManifests else False
+    superproject_logging_data['haslocalmanifests'] = bool(self.manifest.HasLocalManifests)
     if use_superproject:
-      manifest_name = self._UpdateProjectsRevisionId(opt, args, load_local_manifests) or opt.manifest_name
+      manifest_name = self._UpdateProjectsRevisionId(
+          opt, args, load_local_manifests, superproject_logging_data) or opt.manifest_name
 
     if self.gitc_manifest:
       gitc_manifest_projects = self.GetProjects(args,
@@ -1079,12 +1081,11 @@ later is required to fix a server side protocol bug.
       sys.exit(1)
 
     # Log the previous sync state from the config.
-    self.git_event_log.AddSyncStateEvents(mp.config.DumpConfigDict(), 'previous_sync_state')
+    self.git_event_log.AddSyncAnalysisStateEvents(mp.config, 'previous_sync_state')
 
     # Update and log with the new sync state.
-    sync_state = SyncState(config=mp.config, options=opt, superproject=self.superproject)
+    mp.config.UpdateSyncAnalysisState(opt, superproject_logging_data)
-    mp.config.SetSyncState(sync_state)
+    self.git_event_log.AddSyncAnalysisStateEvents(mp.config, 'current_sync_state')
-    self.git_event_log.AddSyncStateEvents(mp.config.DumpConfigDict(), 'current_sync_state')
 
     if not opt.quiet:
       print('repo sync has finished successfully.')

tests/test_manifest_xml.py

@@ -286,6 +286,25 @@ class XmlManifestTests(ManifestParseTestCase):
         '<superproject name="superproject"/>'
         '</manifest>')
 
+  def test_remote_annotations(self):
+    """Check remote settings."""
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="test-remote" fetch="http://localhost">
+    <annotation name="foo" value="bar"/>
+  </remote>
+</manifest>
+""")
+    self.assertEqual(manifest.remotes['test-remote'].annotations[0].name, 'foo')
+    self.assertEqual(manifest.remotes['test-remote'].annotations[0].value, 'bar')
+    self.assertEqual(
+        sort_attributes(manifest.ToXml().toxml()),
+        '<?xml version="1.0" ?><manifest>'
+        '<remote fetch="http://localhost" name="test-remote">'
+        '<annotation name="foo" value="bar"/>'
+        '</remote>'
+        '</manifest>')
+
 
 class IncludeElementTests(ManifestParseTestCase):
   """Tests for <include>."""
@@ -632,9 +651,17 @@ class RemoteElementTests(ManifestParseTestCase):
   def test_remote(self):
     """Check remote settings."""
     a = manifest_xml._XmlRemote(name='foo')
-    b = manifest_xml._XmlRemote(name='bar')
+    a.AddAnnotation('key1', 'value1', 'true')
+    b = manifest_xml._XmlRemote(name='foo')
+    b.AddAnnotation('key2', 'value1', 'true')
+    c = manifest_xml._XmlRemote(name='foo')
+    c.AddAnnotation('key1', 'value2', 'true')
+    d = manifest_xml._XmlRemote(name='foo')
+    d.AddAnnotation('key1', 'value1', 'false')
     self.assertEqual(a, a)
     self.assertNotEqual(a, b)
+    self.assertNotEqual(a, c)
+    self.assertNotEqual(a, d)
     self.assertNotEqual(a, manifest_xml._Default())
     self.assertNotEqual(a, 123)
     self.assertNotEqual(a, None)