manifest_xml: refactor manifest parsing from client management
We conflate the manifest & parsing logic with the management of the
repo client checkout in a single class. This makes testing just one
part (the manifest parsing) hard as it requires a full checkout too.
Start splitting the two apart into separate classes to make it easy
to reason about & test.
Change-Id: Iaf897c93db9c724baba6044bfe7a589c024523b2
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/288682
Reviewed-by: Michael Mortensen <mmortensen@google.com>
Tested-by: Mike Frysinger <vapier@google.com>
diff --git a/manifest_xml.py b/manifest_xml.py
index e1ef330..95c67d7 100644
--- a/manifest_xml.py
+++ b/manifest_xml.py
@@ -187,12 +187,24 @@
class XmlManifest(object):
"""manages the repo configuration file"""
- def __init__(self, repodir):
+ def __init__(self, repodir, manifest_file, local_manifests=None):
+ """Initialize.
+
+ Args:
+ repodir: Path to the .repo/ dir for holding all internal checkout state.
+ It must be in the top directory of the repo client checkout.
+ manifest_file: Full path to the manifest file to parse. This will usually
+ be |repodir|/|MANIFEST_FILE_NAME|.
+ local_manifests: Full path to the directory of local override manifests.
+ This will usually be |repodir|/|LOCAL_MANIFESTS_DIR_NAME|.
+ """
+ # TODO(vapier): Move this out of this class.
+ self.globalConfig = GitConfig.ForUser()
+
self.repodir = os.path.abspath(repodir)
self.topdir = os.path.dirname(self.repodir)
- self.manifestFile = os.path.join(self.repodir, MANIFEST_FILE_NAME)
- self.globalConfig = GitConfig.ForUser()
- self.isGitcClient = False
+ self.manifestFile = manifest_file
+ self.local_manifests = local_manifests
self._load_local_manifests = True
self.repoProject = MetaProject(self, 'repo',
@@ -602,20 +614,11 @@
nodes.append(self._ParseManifestXml(self.manifestFile,
self.manifestProject.worktree))
- if self._load_local_manifests:
- if os.path.exists(os.path.join(self.repodir, LOCAL_MANIFEST_NAME)):
- print('error: %s is not supported; put local manifests in `%s`'
- 'instead' % (LOCAL_MANIFEST_NAME,
- os.path.join(self.repodir, LOCAL_MANIFESTS_DIR_NAME)),
- file=sys.stderr)
- sys.exit(1)
-
- local_dir = os.path.abspath(os.path.join(self.repodir,
- LOCAL_MANIFESTS_DIR_NAME))
+ if self._load_local_manifests and self.local_manifests:
try:
- for local_file in sorted(platform_utils.listdir(local_dir)):
+ for local_file in sorted(platform_utils.listdir(self.local_manifests)):
if local_file.endswith('.xml'):
- local = os.path.join(local_dir, local_file)
+ local = os.path.join(self.local_manifests, local_file)
nodes.append(self._ParseManifestXml(local, self.repodir))
except OSError:
pass
@@ -1253,15 +1256,7 @@
class GitcManifest(XmlManifest):
-
- def __init__(self, repodir, gitc_client_name):
- """Initialize the GitcManifest object."""
- super(GitcManifest, self).__init__(repodir)
- self.isGitcClient = True
- self.gitc_client_name = gitc_client_name
- self.gitc_client_dir = os.path.join(gitc_utils.get_gitc_manifest_dir(),
- gitc_client_name)
- self.manifestFile = os.path.join(self.gitc_client_dir, '.manifest')
+ """Parser for GitC (git-in-the-cloud) manifests."""
def _ParseProject(self, node, parent=None):
"""Override _ParseProject and add support for GITC specific attributes."""
@@ -1272,3 +1267,38 @@
"""Output GITC Specific Project attributes"""
if p.old_revision:
e.setAttribute('old-revision', str(p.old_revision))
+
+
+class RepoClient(XmlManifest):
+ """Manages a repo client checkout."""
+
+ def __init__(self, repodir, manifest_file=None):
+ self.isGitcClient = False
+
+ if os.path.exists(os.path.join(repodir, LOCAL_MANIFEST_NAME)):
+ print('error: %s is not supported; put local manifests in `%s` instead' %
+ (LOCAL_MANIFEST_NAME, os.path.join(repodir, LOCAL_MANIFESTS_DIR_NAME)),
+ file=sys.stderr)
+ sys.exit(1)
+
+ if manifest_file is None:
+ manifest_file = os.path.join(repodir, MANIFEST_FILE_NAME)
+ local_manifests = os.path.abspath(os.path.join(repodir, LOCAL_MANIFESTS_DIR_NAME))
+ super(RepoClient, self).__init__(repodir, manifest_file, local_manifests)
+
+ # TODO: Completely separate manifest logic out of the client.
+ self.manifest = self
+
+
+class GitcClient(RepoClient, GitcManifest):
+ """Manages a GitC client checkout."""
+
+ def __init__(self, repodir, gitc_client_name):
+ """Initialize the GitcManifest object."""
+ self.gitc_client_name = gitc_client_name
+ self.gitc_client_dir = os.path.join(gitc_utils.get_gitc_manifest_dir(),
+ gitc_client_name)
+
+ super(GitcManifest, self).__init__(
+ repodir, os.path.join(self.gitc_client_dir, '.manifest'))
+ self.isGitcClient = True