diff --git a/docs/manifest-format.md b/docs/manifest-format.md index c3bfcff0..854e5e1b 100644 --- a/docs/manifest-format.md +++ b/docs/manifest-format.md @@ -36,7 +36,7 @@ following DTD: - + @@ -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 -exported into each project's environment during a 'forall' command, -prefixed with REPO__. In addition, there is an optional attribute -"keep" which accepts the case insensitive values "true" (default) or -"false". This attribute determines whether or not the annotation will -be kept when exported with the manifest subcommand. +project or remote element. Each element describes a name-value pair. +For projects, this name-value pair will be exported into each project's +environment during a 'forall' command, prefixed with `REPO__`. In addition, +there is an optional attribute "keep" which accepts the case insensitive values +"true" (default) or "false". This attribute determines whether or not the +annotation will be kept when exported with the manifest subcommand. ### Element copyfile diff --git a/git_config.py b/git_config.py index 23557c6e..4416bfa9 100644 --- a/git_config.py +++ b/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,9 @@ from repo_trace import Trace from git_command import GitCommand from git_refs import R_CHANGES, R_HEADS, R_TAGS +# Prefix Add all the data of SyncAnalysisState object. +SYNC_STATE_PREFIX = 'syncstate.' + ID_RE = re.compile(r'^[0-9a-f]{40}$') REVIEW_CACHE = dict() @@ -263,17 +266,24 @@ class GitConfig(object): self._branches[b.name] = b return b - def GetSyncState(self): - """Get the state sync object.""" - return self._syncState + def GetSyncAnalysisStateData(self): + """Returns data to be logged for the analysis of sync performance.""" + return {k: v for k, v in self.DumpConfigDict().items() if k.startswith(SYNC_STATE_PREFIX)} - def SetSyncState(self, sync_state): - """Update Config's SyncState object with the new |sync_state| object. + def UpdateSyncAnalysisState(self, options, superproject_logging_data): + """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. + + Returns: + SyncAnalysisState object. """ - self._syncState = sync_state + return SyncAnalysisState(self, options, superproject_logging_data) def GetSubSections(self, section): """List all subsection names matching $section.*.* @@ -732,12 +742,13 @@ class Branch(object): return self._config.GetString(key, all_keys=all_keys) -class SyncState(object): - """Configuration options related Sync object. - """ +class SyncAnalysisState(): + """Configuration options related to logging of Sync state for analysis. - def __init__(self, config, options, superproject): - """Initializes SyncState. + This object is versioned. + """ + 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 +758,47 @@ 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() - self._Set('synctime', now.strftime('%d/%m/%Y %H:%M:%S')) - self._Set('version', '1.0') - self._Set('argv', sys.argv) - self._SetDictionary(superproject) + now = datetime.datetime.utcnow() + self._Set('main.synctime', now.isoformat() + 'Z') + self._Set('main.version', '1') + self._Set('sys.argv', sys.argv) + 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): - for key, value in config_dict.items(): + def _SetDictionary(self, data): + """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: - 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)) + """Set the |value| for a |key| in the |_config| member. - def _Get(self, key, all_keys=False): - key = 'syncstate.%s' % (key) - return self._config.GetString(key, all_keys=all_keys) + Every key is stored by prepending the value of SYNC_STATE_PREFIX constant to it. + + Args: + key: Name of the key. + 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 + sync_key = f'{SYNC_STATE_PREFIX}{key}' + if isinstance(value, str): + self._config.SetString(sync_key, value) + elif isinstance(value, bool): + self._config.SetBoolean(sync_key, value) + else: + self._config.SetString(sync_key, str(value)) diff --git a/git_trace2_event_log.py b/git_trace2_event_log.py index 60c1ee54..9c9e5a70 100644 --- a/git_trace2_event_log.py +++ b/git_trace2_event_log.py @@ -144,11 +144,11 @@ class EventLog(object): command_event['subcommands'] = subcommands 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. + def LogConfigEvents(self, config, event_dict_name): + """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(): @@ -165,18 +165,7 @@ class EventLog(object): """ # Only output the repo.* config parameters. 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): - """Append a log event for each syncstate.* config key to the current log. - - Args: - config: SyncState configuration dictionary - event_dict_name: Name of the event dictionary for items to be logged under. - """ - # Only output syncstate.* config parameters. - sync_config = {k: v for k, v in config.items() if k.startswith('syncstate.')} - self._LogConfigEvents(sync_config, event_dict_name) + self.LogConfigEvents(repo_config, 'def_param') def ErrorEvent(self, msg, fmt): """Append a 'error' event to the current log.""" diff --git a/manifest_xml.py b/manifest_xml.py index be74bf49..55ad6c08 100644 --- a/manifest_xml.py +++ b/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 True - return self.__dict__ != other.__dict__ + return not self.__eq__(other) 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') diff --git a/project.py b/project.py index e777dbd2..fe88a505 100644 --- a/project.py +++ b/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. diff --git a/subcmds/manifest.py b/subcmds/manifest.py index 00587d8d..0fbdeac0 100644 --- a/subcmds/manifest.py +++ b/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, diff --git a/subcmds/sync.py b/subcmds/sync.py index d659500e..11f71bf1 100644 --- a/subcmds/sync.py +++ b/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 - self.superproject['hasLocalManifests'] = True if self.manifest.HasLocalManifests else False + superproject_logging_data['superproject'] = use_superproject + 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,13 @@ 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.LogConfigEvents(mp.config.GetSyncAnalysisStateData(), + 'previous_sync_state') # Update and log with the new sync state. - sync_state = SyncState(config=mp.config, options=opt, superproject=self.superproject) - mp.config.SetSyncState(sync_state) - self.git_event_log.AddSyncStateEvents(mp.config.DumpConfigDict(), 'current_sync_state') + mp.config.UpdateSyncAnalysisState(opt, superproject_logging_data) + self.git_event_log.LogConfigEvents(mp.config.GetSyncAnalysisStateData(), + 'current_sync_state') if not opt.quiet: print('repo sync has finished successfully.') diff --git a/tests/fixtures/test.gitconfig b/tests/fixtures/test.gitconfig index 9b3f2574..002a4a33 100644 --- a/tests/fixtures/test.gitconfig +++ b/tests/fixtures/test.gitconfig @@ -11,3 +11,12 @@ intk = 10k intm = 10m intg = 10g +[syncstate "main"] + synctime = 2021-07-28T19:42:03.866355Z + version = 1 +[syncstate "sys"] + argv = ['/usr/bin/pytest-3'] +[syncstate "superproject"] + test = false +[syncstate "options"] + verbose = true diff --git a/tests/test_git_config.py b/tests/test_git_config.py index 3300c12f..0282fa98 100644 --- a/tests/test_git_config.py +++ b/tests/test_git_config.py @@ -104,6 +104,24 @@ class GitConfigReadOnlyTests(unittest.TestCase): for key, value in TESTS: self.assertEqual(value, self.config.GetInt('section.%s' % (key,))) + def test_GetSyncAnalysisStateData(self): + """ + Test config entries with a sync state analysis data. + """ + superproject_logging_data = {} + superproject_logging_data['test'] = False + options = type('options', (object,), {})() + options.verbose = 'true' + TESTS = ( + ('superproject.test', 'false'), + ('options.verbose', 'true'), + ('main.version', '1'), + ) + self.config.UpdateSyncAnalysisState(options, superproject_logging_data) + sync_data = self.config.GetSyncAnalysisStateData() + for key, value in TESTS: + self.assertEqual(sync_data[f'{git_config.SYNC_STATE_PREFIX}{key}'], value) + self.assertTrue(len(sync_data[f'{git_config.SYNC_STATE_PREFIX}main.synctime'])) class GitConfigReadWriteTests(unittest.TestCase): """Read/write tests of the GitConfig class.""" diff --git a/tests/test_manifest_xml.py b/tests/test_manifest_xml.py index 96ee4c4a..59f2a779 100644 --- a/tests/test_manifest_xml.py +++ b/tests/test_manifest_xml.py @@ -286,6 +286,25 @@ class XmlManifestTests(ManifestParseTestCase): '' '') + def test_remote_annotations(self): + """Check remote settings.""" + manifest = self.getXmlManifest(""" + + + + + +""") + 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()), + '' + '' + '' + '' + '') + class IncludeElementTests(ManifestParseTestCase): """Tests for .""" @@ -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)