diff --git a/git_superproject.py b/git_superproject.py index f4dbb27b..9f585e5d 100644 --- a/git_superproject.py +++ b/git_superproject.py @@ -71,42 +71,50 @@ 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, git_event_log, - superproject_dir='exp-superproject', quiet=False, print_messages=False): + def __init__(self, manifest, name, remote, revision, + superproject_dir='exp-superproject'): """Initializes superproject. Args: manifest: A Manifest object that is to be written to a file. - repodir: Path to the .repo/ dir for holding all internal checkout state. - It must be in the top directory of the repo client checkout. - git_event_log: A git trace2 event log to log events. - superproject_dir: Relative path under |repodir| to checkout superproject. - quiet: If True then only print the progress messages. - print_messages: if True then print error/warning messages. + name: The unique name of the superproject + remote: The RemoteSpec for the remote. + revision: The name of the git branch to track. + superproject_dir: Relative path under |manifest.subdir| to checkout + superproject. """ self._project_commit_ids = None self._manifest = manifest - self._git_event_log = git_event_log - self._quiet = quiet - self._print_messages = print_messages - self._branch = manifest.branch - self._repodir = os.path.abspath(repodir) + self.name = name + self.remote = remote + self.revision = self._branch = revision + self._repodir = manifest.repodir self._superproject_dir = superproject_dir self._superproject_path = manifest.SubmanifestInfoDir(manifest.path_prefix, superproject_dir) self._manifest_path = os.path.join(self._superproject_path, _SUPERPROJECT_MANIFEST_NAME) - git_name = '' - if self._manifest.superproject: - remote = self._manifest.superproject['remote'] - git_name = hashlib.md5(remote.name.encode('utf8')).hexdigest() + '-' - self._branch = self._manifest.superproject['revision'] - self._remote_url = remote.url - else: - self._remote_url = None + git_name = hashlib.md5(remote.name.encode('utf8')).hexdigest() + '-' + self._remote_url = remote.url self._work_git_name = git_name + _SUPERPROJECT_GIT_NAME self._work_git = os.path.join(self._superproject_path, self._work_git_name) + # The following are command arguemnts, rather then superproject attributes, + # and where included here originally. They should eventually become + # arguments that are passed down from the public methods, instead of being + # treated as attributes. + self._git_event_log = None + self._quiet = False + self._print_messages = False + + def SetQuiet(self, value): + """Set the _quiet attribute.""" + self._quiet = value + + def SetPrintMessages(self, value): + """Set the _print_messages attribute.""" + self._print_messages = value + @property def project_commit_ids(self): """Returns a dictionary of projects and their commit ids.""" @@ -215,12 +223,16 @@ class Superproject(object): f'return code: {retval}, stderr: {p.stderr}') return data - def Sync(self): + def Sync(self, git_event_log): """Gets a local copy of a superproject for the manifest. + Args: + git_event_log: an EventLog, for git tracing. + Returns: SyncResult """ + self._git_event_log = git_event_log if not self._manifest.superproject: self._LogWarning(f'superproject tag is not defined in manifest: ' f'{self._manifest.manifestFile}') @@ -248,7 +260,7 @@ class Superproject(object): Returns: CommitIdsResult """ - sync_result = self.Sync() + sync_result = self.Sync(self._git_event_log) if not sync_result.success: return CommitIdsResult(None, sync_result.fatal) @@ -313,7 +325,7 @@ class Superproject(object): # Skip the project if it comes from the local manifest. return project.manifest.IsFromLocalManifest(project) - def UpdateProjectsRevisionId(self, projects): + def UpdateProjectsRevisionId(self, projects, git_event_log): """Update revisionId of every project in projects with the commit id. Args: @@ -322,6 +334,7 @@ class Superproject(object): Returns: UpdateProjectsResult """ + self._git_event_log = git_event_log commit_ids_result = self._GetAllProjectsCommitIds() commit_ids = commit_ids_result.commit_ids if not commit_ids: @@ -397,7 +410,7 @@ def _UseSuperprojectFromConfiguration(): def PrintMessages(opt, manifest): """Returns a boolean if error/warning messages are to be printed.""" - return opt.use_superproject is not None or manifest.superproject + return opt.use_superproject is not None or bool(manifest.superproject) def UseSuperproject(opt, manifest): @@ -409,7 +422,7 @@ def UseSuperproject(opt, manifest): client_value = manifest.manifestProject.use_superproject if client_value is not None: return client_value - else: - if not manifest.superproject: - return False + elif manifest.superproject: return _UseSuperprojectFromConfiguration() + else: + return False diff --git a/manifest_xml.py b/manifest_xml.py index 8718dc54..7d19d63e 100644 --- a/manifest_xml.py +++ b/manifest_xml.py @@ -24,6 +24,7 @@ import urllib.parse import gitc_utils from git_config import GitConfig, IsId from git_refs import R_HEADS, HEAD +from git_superproject import Superproject import platform_utils from project import (Annotation, RemoteSpec, Project, RepoProject, ManifestProject) @@ -670,17 +671,17 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md if self._superproject: root.appendChild(doc.createTextNode('')) e = doc.createElement('superproject') - e.setAttribute('name', self._superproject['name']) + e.setAttribute('name', self._superproject.name) remoteName = None if d.remote: remoteName = d.remote.name - remote = self._superproject.get('remote') + remote = self._superproject.remote if not d.remote or remote.orig_name != remoteName: remoteName = remote.orig_name e.setAttribute('remote', remoteName) revision = remote.revision or d.revisionExpr - if not revision or revision != self._superproject['revision']: - e.setAttribute('revision', self._superproject['revision']) + if not revision or revision != self._superproject.revision: + e.setAttribute('revision', self._superproject.revision) root.appendChild(e) if self._contactinfo.bugurl != Wrapper().BUG_URL: @@ -984,7 +985,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md self._default = None self._submanifests = {} self._repo_hooks_project = None - self._superproject = {} + self._superproject = None self._contactinfo = ContactInfo(Wrapper().BUG_URL) self._notice = None self.branch = None @@ -1052,20 +1053,19 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md # Now that we have loaded this manifest, load any submanifest manifests # as well. We need to do this after self._loaded is set to avoid looping. - if self._outer_client: - for name in self._submanifests: - tree = self._submanifests[name] - spec = tree.ToSubmanifestSpec(self) - present = os.path.exists(os.path.join(self.subdir, MANIFEST_FILE_NAME)) - if present and tree.present and not tree.repo_client: - if initial_client and initial_client.topdir == self.topdir: - tree.repo_client = self - tree.present = present - elif not os.path.exists(self.subdir): - tree.present = False - if present and tree.present: - tree.repo_client._Load(initial_client=initial_client, - submanifest_depth=submanifest_depth + 1) + for name in self._submanifests: + tree = self._submanifests[name] + spec = tree.ToSubmanifestSpec(self) + present = os.path.exists(os.path.join(self.subdir, MANIFEST_FILE_NAME)) + if present and tree.present and not tree.repo_client: + if initial_client and initial_client.topdir == self.topdir: + tree.repo_client = self + tree.present = present + elif not os.path.exists(self.subdir): + tree.present = False + if present and tree.present: + tree.repo_client._Load(initial_client=initial_client, + submanifest_depth=submanifest_depth + 1) def _ParseManifestXml(self, path, include_root, parent_groups='', restrict_includes=True): @@ -1267,11 +1267,10 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md if node.nodeName == 'superproject': name = self._reqatt(node, 'name') # There can only be one superproject. - if self._superproject.get('name'): + if self._superproject: raise ManifestParseError( 'duplicate superproject in %s' % (self.manifestFile)) - self._superproject['name'] = name remote_name = node.getAttribute('remote') if not remote_name: remote = self._default.remote @@ -1280,14 +1279,16 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md if remote is None: raise ManifestParseError("no remote for superproject %s within %s" % (name, self.manifestFile)) - self._superproject['remote'] = remote.ToRemoteSpec(name) revision = node.getAttribute('revision') or remote.revision if not revision: revision = self._default.revisionExpr if not revision: raise ManifestParseError('no revision for superproject %s within %s' % (name, self.manifestFile)) - self._superproject['revision'] = revision + self._superproject = Superproject(self, + name=name, + remote=remote.ToRemoteSpec(name), + revision=revision) if node.nodeName == 'contactinfo': bugurl = self._reqatt(node, 'bugurl') # This element can be repeated, later entries will clobber earlier ones. diff --git a/project.py b/project.py index b8d834aa..5ed103b9 100644 --- a/project.py +++ b/project.py @@ -36,7 +36,6 @@ from git_trace2_event_log import EventLog from error import GitError, UploadError, DownloadError from error import ManifestInvalidRevisionError, ManifestInvalidPathError from error import NoManifestException, ManifestParseError -from git_superproject import Superproject import platform_utils import progress from repo_trace import IsTrace, Trace diff --git a/subcmds/init.py b/subcmds/init.py index 65b63efd..99f30dce 100644 --- a/subcmds/init.py +++ b/subcmds/init.py @@ -25,7 +25,6 @@ from project import SyncBuffer from git_config import GitConfig from git_command import git_require, MIN_GIT_VERSION_SOFT, MIN_GIT_VERSION_HARD import fetch -import git_superproject import platform_utils from wrapper import Wrapper diff --git a/subcmds/sync.py b/subcmds/sync.py index 9e783205..4d0a5ec6 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py @@ -304,12 +304,10 @@ later is required to fix a server side protocol bug. Returns: Returns path to the overriding manifest file instead of None. """ + superproject = self.manifest.superproject + superproject.SetQuiet(opt.quiet) print_messages = git_superproject.PrintMessages(opt, self.manifest) - superproject = git_superproject.Superproject(self.manifest, - self.repodir, - self.git_event_log, - quiet=opt.quiet, - print_messages=print_messages) + superproject.SetPrintMessages(print_messages) if opt.local_only: manifest_path = superproject.manifest_path if manifest_path: @@ -319,7 +317,8 @@ later is required to fix a server side protocol bug. all_projects = self.GetProjects(args, missing_ok=True, submodules_ok=opt.fetch_submodules) - update_result = superproject.UpdateProjectsRevisionId(all_projects) + update_result = superproject.UpdateProjectsRevisionId( + all_projects, git_event_log=self.git_event_log) manifest_path = update_result.manifest_path superproject_logging_data['updatedrevisionid'] = bool(manifest_path) if manifest_path: diff --git a/tests/test_git_superproject.py b/tests/test_git_superproject.py index a24fc7f0..1e7b1201 100644 --- a/tests/test_git_superproject.py +++ b/tests/test_git_superproject.py @@ -68,8 +68,10 @@ class SuperprojectTestCase(unittest.TestCase): """) - self._superproject = git_superproject.Superproject(manifest, self.repodir, - self.git_event_log) + self._superproject = git_superproject.Superproject( + manifest, name='superproject', + remote=manifest.remotes.get('default-remote').ToRemoteSpec('superproject'), + revision='refs/heads/main') def tearDown(self): """Tear down superproject every time.""" @@ -125,12 +127,7 @@ class SuperprojectTestCase(unittest.TestCase): """) - superproject = git_superproject.Superproject(manifest, self.repodir, self.git_event_log) - # Test that exit condition is false when there is no superproject tag. - sync_result = superproject.Sync() - self.assertFalse(sync_result.success) - self.assertFalse(sync_result.fatal) - self.verifyErrorEvent() + self.assertIsNone(manifest.superproject) def test_superproject_get_superproject_invalid_url(self): """Test with an invalid url.""" @@ -141,8 +138,11 @@ class SuperprojectTestCase(unittest.TestCase): """) - superproject = git_superproject.Superproject(manifest, self.repodir, self.git_event_log) - sync_result = superproject.Sync() + superproject = git_superproject.Superproject( + manifest, name='superproject', + remote=manifest.remotes.get('test-remote').ToRemoteSpec('superproject'), + revision='refs/heads/main') + sync_result = superproject.Sync(self.git_event_log) self.assertFalse(sync_result.success) self.assertTrue(sync_result.fatal) @@ -155,17 +155,19 @@ class SuperprojectTestCase(unittest.TestCase): """) - self._superproject = git_superproject.Superproject(manifest, self.repodir, - self.git_event_log) + self._superproject = git_superproject.Superproject( + manifest, name='superproject', + remote=manifest.remotes.get('test-remote').ToRemoteSpec('superproject'), + revision='refs/heads/main') with mock.patch.object(self._superproject, '_branch', 'junk'): - sync_result = self._superproject.Sync() + sync_result = self._superproject.Sync(self.git_event_log) self.assertFalse(sync_result.success) self.assertTrue(sync_result.fatal) def test_superproject_get_superproject_mock_init(self): """Test with _Init failing.""" with mock.patch.object(self._superproject, '_Init', return_value=False): - sync_result = self._superproject.Sync() + sync_result = self._superproject.Sync(self.git_event_log) self.assertFalse(sync_result.success) self.assertTrue(sync_result.fatal) @@ -174,7 +176,7 @@ class SuperprojectTestCase(unittest.TestCase): with mock.patch.object(self._superproject, '_Init', return_value=True): os.mkdir(self._superproject._superproject_path) with mock.patch.object(self._superproject, '_Fetch', return_value=False): - sync_result = self._superproject.Sync() + sync_result = self._superproject.Sync(self.git_event_log) self.assertFalse(sync_result.success) self.assertTrue(sync_result.fatal) @@ -230,7 +232,7 @@ class SuperprojectTestCase(unittest.TestCase): return_value=data): # Create temporary directory so that it can write the file. os.mkdir(self._superproject._superproject_path) - update_result = self._superproject.UpdateProjectsRevisionId(projects) + update_result = self._superproject.UpdateProjectsRevisionId(projects, self.git_event_log) self.assertIsNotNone(update_result.manifest_path) self.assertFalse(update_result.fatal) with open(update_result.manifest_path, 'r') as fp: @@ -256,22 +258,13 @@ class SuperprojectTestCase(unittest.TestCase): """) self.maxDiff = None - self._superproject = git_superproject.Superproject(manifest, self.repodir, - self.git_event_log) - self.assertEqual(len(self._superproject._manifest.projects), 1) - projects = self._superproject._manifest.projects - project = projects[0] - project.SetRevisionId('ABCDEF') - update_result = self._superproject.UpdateProjectsRevisionId(projects) - self.assertIsNone(update_result.manifest_path) - self.assertFalse(update_result.fatal) - self.verifyErrorEvent() + self.assertIsNone(manifest.superproject) self.assertEqual( sort_attributes(manifest.ToXml().toxml()), '' '' '' - '' + '' '') def test_superproject_update_project_revision_id_from_local_manifest_group(self): @@ -290,8 +283,10 @@ class SuperprojectTestCase(unittest.TestCase): " /> """) self.maxDiff = None - self._superproject = git_superproject.Superproject(manifest, self.repodir, - self.git_event_log) + self._superproject = git_superproject.Superproject( + manifest, name='superproject', + remote=manifest.remotes.get('default-remote').ToRemoteSpec('superproject'), + revision='refs/heads/main') self.assertEqual(len(self._superproject._manifest.projects), 2) projects = self._superproject._manifest.projects data = ('160000 commit 2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea\tart\x00') @@ -302,7 +297,7 @@ class SuperprojectTestCase(unittest.TestCase): return_value=data): # Create temporary directory so that it can write the file. os.mkdir(self._superproject._superproject_path) - update_result = self._superproject.UpdateProjectsRevisionId(projects) + update_result = self._superproject.UpdateProjectsRevisionId(projects, self.git_event_log) self.assertIsNotNone(update_result.manifest_path) self.assertFalse(update_result.fatal) with open(update_result.manifest_path, 'r') as fp: @@ -337,8 +332,10 @@ class SuperprojectTestCase(unittest.TestCase): " /> """) self.maxDiff = None - self._superproject = git_superproject.Superproject(manifest, self.repodir, - self.git_event_log) + self._superproject = git_superproject.Superproject( + manifest, name='superproject', + remote=manifest.remotes.get('default-remote').ToRemoteSpec('superproject'), + revision='refs/heads/main') self.assertEqual(len(self._superproject._manifest.projects), 3) projects = self._superproject._manifest.projects data = ('160000 commit 2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea\tart\x00' @@ -350,7 +347,7 @@ class SuperprojectTestCase(unittest.TestCase): return_value=data): # Create temporary directory so that it can write the file. os.mkdir(self._superproject._superproject_path) - update_result = self._superproject.UpdateProjectsRevisionId(projects) + update_result = self._superproject.UpdateProjectsRevisionId(projects, self.git_event_log) self.assertIsNotNone(update_result.manifest_path) self.assertFalse(update_result.fatal) with open(update_result.manifest_path, 'r') as fp: diff --git a/tests/test_manifest_xml.py b/tests/test_manifest_xml.py index cb3eb855..ede41547 100644 --- a/tests/test_manifest_xml.py +++ b/tests/test_manifest_xml.py @@ -289,8 +289,8 @@ class XmlManifestTests(ManifestParseTestCase): X tags are always ignored """) - self.assertEqual(manifest.superproject['name'], 'superproject') - self.assertEqual(manifest.superproject['remote'].name, 'test-remote') + self.assertEqual(manifest.superproject.name, 'superproject') + self.assertEqual(manifest.superproject.remote.name, 'test-remote') self.assertEqual( sort_attributes(manifest.ToXml().toxml()), '' @@ -569,10 +569,10 @@ class SuperProjectElementTests(ManifestParseTestCase): """) - self.assertEqual(manifest.superproject['name'], 'superproject') - self.assertEqual(manifest.superproject['remote'].name, 'test-remote') - self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject') - self.assertEqual(manifest.superproject['revision'], 'refs/heads/main') + self.assertEqual(manifest.superproject.name, 'superproject') + self.assertEqual(manifest.superproject.remote.name, 'test-remote') + self.assertEqual(manifest.superproject.remote.url, 'http://localhost/superproject') + self.assertEqual(manifest.superproject.revision, 'refs/heads/main') self.assertEqual( sort_attributes(manifest.ToXml().toxml()), '' @@ -591,10 +591,10 @@ class SuperProjectElementTests(ManifestParseTestCase): """) - self.assertEqual(manifest.superproject['name'], 'superproject') - self.assertEqual(manifest.superproject['remote'].name, 'test-remote') - self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject') - self.assertEqual(manifest.superproject['revision'], 'refs/heads/stable') + self.assertEqual(manifest.superproject.name, 'superproject') + self.assertEqual(manifest.superproject.remote.name, 'test-remote') + self.assertEqual(manifest.superproject.remote.url, 'http://localhost/superproject') + self.assertEqual(manifest.superproject.revision, 'refs/heads/stable') self.assertEqual( sort_attributes(manifest.ToXml().toxml()), '' @@ -613,10 +613,10 @@ class SuperProjectElementTests(ManifestParseTestCase): """) - self.assertEqual(manifest.superproject['name'], 'superproject') - self.assertEqual(manifest.superproject['remote'].name, 'test-remote') - self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject') - self.assertEqual(manifest.superproject['revision'], 'refs/heads/stable') + self.assertEqual(manifest.superproject.name, 'superproject') + self.assertEqual(manifest.superproject.remote.name, 'test-remote') + self.assertEqual(manifest.superproject.remote.url, 'http://localhost/superproject') + self.assertEqual(manifest.superproject.revision, 'refs/heads/stable') self.assertEqual( sort_attributes(manifest.ToXml().toxml()), '' @@ -635,10 +635,10 @@ class SuperProjectElementTests(ManifestParseTestCase): """) - self.assertEqual(manifest.superproject['name'], 'superproject') - self.assertEqual(manifest.superproject['remote'].name, 'test-remote') - self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject') - self.assertEqual(manifest.superproject['revision'], 'refs/heads/stable') + self.assertEqual(manifest.superproject.name, 'superproject') + self.assertEqual(manifest.superproject.remote.name, 'test-remote') + self.assertEqual(manifest.superproject.remote.url, 'http://localhost/superproject') + self.assertEqual(manifest.superproject.revision, 'refs/heads/stable') self.assertEqual( sort_attributes(manifest.ToXml().toxml()), '' @@ -657,10 +657,10 @@ class SuperProjectElementTests(ManifestParseTestCase): """) - self.assertEqual(manifest.superproject['name'], 'platform/superproject') - self.assertEqual(manifest.superproject['remote'].name, 'superproject-remote') - self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/platform/superproject') - self.assertEqual(manifest.superproject['revision'], 'refs/heads/main') + self.assertEqual(manifest.superproject.name, 'platform/superproject') + self.assertEqual(manifest.superproject.remote.name, 'superproject-remote') + self.assertEqual(manifest.superproject.remote.url, 'http://localhost/platform/superproject') + self.assertEqual(manifest.superproject.revision, 'refs/heads/main') self.assertEqual( sort_attributes(manifest.ToXml().toxml()), '' @@ -679,9 +679,9 @@ class SuperProjectElementTests(ManifestParseTestCase): """) - self.assertEqual(manifest.superproject['name'], 'superproject') - self.assertEqual(manifest.superproject['remote'].name, 'default-remote') - self.assertEqual(manifest.superproject['revision'], 'refs/heads/main') + self.assertEqual(manifest.superproject.name, 'superproject') + self.assertEqual(manifest.superproject.remote.name, 'default-remote') + self.assertEqual(manifest.superproject.revision, 'refs/heads/main') self.assertEqual( sort_attributes(manifest.ToXml().toxml()), ''