[edk2-staging/EdkRepo] [PATCH v1 0/3] Manifest parser cleanup

[edk2-staging/EdkRepo] [PATCH v1 0/3] Manifest parser cleanup

[Bjorge, Erik C]

General cleanup of the manifest parser and minor improvements to make
the code more flexible.

Cc: Ashley E Desimone <ashley.e.desimone@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Puja Pandya <puja.pandya@intel.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Prince Agyeman <prince.agyeman@intel.com>

Erik Bjorge (3):
  EdkRepo: Flake8 cleanup
  EdkRepo: Improve error message with invalid XML
  EdkRepo: Refactoring private class members

 edkrepo_manifest_parser/edk_manifest.py | 249 +++++++++++++-----------
 1 file changed, 139 insertions(+), 110 deletions(-)

-- 
2.21.0.windows.1

[edk2-staging/EdkRepo] [PATCH v1 1/3] EdkRepo: Flake8 cleanup

[Bjorge, Erik C]

Just cleaning up the file to meet Flake8 expectations.

Signed-off-by: Erik Bjorge <erik.c.bjorge@intel.com>
Cc: Ashley E Desimone <ashley.e.desimone@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Puja Pandya <puja.pandya@intel.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Prince Agyeman <prince.agyeman@intel.com>
---
 edkrepo_manifest_parser/edk_manifest.py | 159 ++++++++++++++----------
 1 file changed, 94 insertions(+), 65 deletions(-)

diff --git a/edkrepo_manifest_parser/edk_manifest.py b/edkrepo_manifest_parser/edk_manifest.py
index 080448f..7e5f0fb 100644
--- a/edkrepo_manifest_parser/edk_manifest.py
+++ b/edkrepo_manifest_parser/edk_manifest.py
@@ -25,19 +25,22 @@ GeneralConfig = namedtuple('GeneralConfig', ['default_combo', 'current_combo', '
 RemoteRepo = namedtuple('RemoteRepo', ['name', 'url'])
 RepoHook = namedtuple('RepoHook', ['source', 'dest_path', 'dest_file', 'remote_url'])
 Combination = namedtuple('Combination', ['name', 'description'])
-RepoSource = namedtuple('RepoSource', ['root', 'remote_name', 'remote_url', 'branch', 'commit', 'sparse', 'enable_submodule', 'tag'])
+RepoSource = namedtuple('RepoSource', ['root', 'remote_name', 'remote_url', 'branch', 'commit', 'sparse',
+                                       'enable_submodule', 'tag'])
 
 SparseSettings = namedtuple('SparseSettings', ['sparse_by_default'])
 SparseData = namedtuple('SparseData', ['combination', 'remote_name', 'always_include', 'always_exclude'])
 
 FolderToFolderMapping = namedtuple('FolderToFolderMapping', ['project1', 'project2', 'remote_name', 'folders'])
-FolderToFolderMappingFolder = namedtuple('FolderToFolderMappingFolder', ['project1_folder', 'project2_folder', 'excludes'])
+FolderToFolderMappingFolder = namedtuple('FolderToFolderMappingFolder', ['project1_folder', 'project2_folder',
+                                                                         'excludes'])
 FolderToFolderMappingFolderExclude = namedtuple('FolderToFolderMappingFolderExclude', ['path'])
 
 SubmoduleAlternateRemote = namedtuple('SubmoduleAlternateRemote', ['remote_name', 'original_url', 'alternate_url'])
 
 REQUIRED_ATTRIB_ERROR_MSG = "Required attribute malformed in <{}>: {}"
-NO_ASSOCIATED_REMOTE = 'There are no remotes associated with the ClientGitHook entry: \nsource:{} destination:{} \nThis hook will not be installed, updated or deleted.\n'
+NO_ASSOCIATED_REMOTE = 'There are no remotes associated with the ClientGitHook entry:\nsource:{} destination:{}' \
+                       '\nThis hook will not be installed, updated or deleted.\n'
 NO_REMOTE_EXISTS_WITH_NAME = 'There are no remotes with the name: {} listed in the manifest file.'
 PIN_COMB_ERROR = "Pin \"{}\" Pin did not have a single <Combination> tag."
 DUPLICATE_TAG_ERROR = "Duplicate <{}> tag not allowed: '{}' (Note: check <include>'s"
@@ -50,12 +53,13 @@ INVALID_PROJECTNAME_ERROR = "Invalid input: {} not found in CiIndexXml"
 UNSUPPORTED_TYPE_ERROR = "{} is not a supported xml type: {}"
 INVALID_XML_ERROR = "{} is not a valid xml file"
 
+
 class BaseXmlHelper():
     def __init__(self, fileref, xml_types):
         self._fileref = fileref
         try:
-            self._tree = ET.ElementTree(file=fileref)  #fileref can be a filename or filestream
-        except:
+            self._tree = ET.ElementTree(file=fileref)  # fileref can be a filename or filestream
+        except Exception:
             raise TypeError(INVALID_XML_ERROR.format(fileref))
 
         self._xml_type = self._tree.getroot().tag
@@ -97,6 +101,7 @@ class CiIndexXml(BaseXmlHelper):
         else:
             raise ValueError(INVALID_PROJECTNAME_ERROR.format(project_name))
 
+
 class _Project():
     def __init__(self, element):
         try:
@@ -105,33 +110,33 @@ class _Project():
         except KeyError as k:
             raise KeyError(REQUIRED_ATTRIB_ERROR_MSG.format(k, element.tag))
         try:
-            #if the archived attrib is not explicitly set to true, then assume false
+            # if the archived attrib is not explicitly set to true, then assume false
             self.archived = (element.attrib['archived'].lower() == 'true')
-        except:
+        except Exception:
             self.archived = False
 
+
 #
 #  This class will parse and the manifest XML file and populate the named
 #  tuples defined above to provide abstracted access to the manifest data
 #
 class ManifestXml(BaseXmlHelper):
     def __init__(self, fileref):
-
         # Most of the attributes of this class are intended to be private as they are used for
         # internally gathering and storing the manifest data. As such, all access to them should be
         # done through the provided methods to ensure future compatibility if the xml schema changes
         super().__init__(fileref, ['Pin', 'Manifest'])
         self.__project_info = None
         self.__general_config = None
-        self.__remotes = {}                 #dict of _Remote objs, with Remote.name as key
+        self.__remotes = {}                    # dict of _Remote objs, with Remote.name as key
         self.__client_hook_list = []
-        self.__combinations = {}            #dict of _Combination objs, with Combination.name as key
-        self.__combo_sources = {}           #dict of _RepoSource obj lists, with Combination.name as key
+        self.__combinations = {}               # dict of _Combination objs, with Combination.name as key
+        self.__combo_sources = {}              # dict of _RepoSource obj lists, with Combination.name as key
         self.__dsc_list = []
-        self.__sparse_settings = None       # A single instance of platform sparse checkout settings
-        self.__sparse_data = []             # List of SparseData objects
-        self.__commit_templates = {}        #dict of commit message templates with the remote name as the key
-        self.__folder_to_folder_mappings = [] # List of FolderToFolderMapping objects
+        self.__sparse_settings = None          # A single instance of platform sparse checkout settings
+        self.__sparse_data = []                # List of SparseData objects
+        self.__commit_templates = {}           # dict of commit message templates with the remote name as the key
+        self.__folder_to_folder_mappings = []  # List of FolderToFolderMapping objects
         self.__submodule_alternate_remotes = []
 
         #
@@ -143,12 +148,12 @@ class ManifestXml(BaseXmlHelper):
             incl_file = os.path.join(incl_path, include_elem.attrib['xml'])
             try:
                 include_tree = ET.ElementTree(file=incl_file)
-            except:
+            except Exception:
                 raise TypeError("{} is not a valid xml file".format(incl_file))
             for elem in include_tree.iterfind('*'):
                 if elem.tag != 'ProjectInfo' and elem.tag != 'GeneralConfig':
                     tree_root.append(elem)
-            #remove include tags after added to etree to prevent feedback issues
+            # remove include tags after added to etree to prevent feedback issues
             tree_root.remove(include_elem)
 
         #
@@ -388,7 +393,7 @@ class ManifestXml(BaseXmlHelper):
         # Note: It will also strip all the comments from the file
         #
         if self._xml_type == 'Pin':
-            #raise Warning("This method is not supported for Pin xmls")
+            # raise Warning("This method is not supported for Pin xmls")
             return
         if filename is None:
             filename = self._fileref
@@ -444,12 +449,12 @@ class ManifestXml(BaseXmlHelper):
         # Only one of Branch or SHA is required to write PIN and checkout code
         for src_tuple in repo_source_list:
             if (src_tuple.root is None or src_tuple.remote_name is None or src_tuple.remote_url is
-                None or (src_tuple.commit is None and src_tuple.branch is None and src_tuple.tag is None)):
+                    None or (src_tuple.commit is None and src_tuple.branch is None and src_tuple.tag is None)):
                 raise ValueError("Invalid input: empty values in source list")
 
             # the data to create the remote elements could also be retrieved
             # from __remotes, but this is easier
-            elem = ET.SubElement(remote_root, 'Remote', {'name' : src_tuple.remote_name})
+            elem = ET.SubElement(remote_root, 'Remote', {'name': src_tuple.remote_name})
             elem.text = src_tuple.remote_url
             elem.tail = '\n    '
 
@@ -469,22 +474,33 @@ class ManifestXml(BaseXmlHelper):
             if src_tuple.commit:
                 if src_tuple.branch:
                     if src_tuple.tag:
-                        elem = ET.SubElement(source_root, 'Source', {'localRoot':src_tuple.root, 'remote':src_tuple.remote_name,
-                                                                     'branch':src_tuple.branch, 'commit':src_tuple.commit,
-                                                                     'sparseCheckout':sparse, 'enable_submodule':sub,
-                                                                     'tag':src_tuple.tag})
+                        elem = ET.SubElement(source_root, 'Source', {'localRoot': src_tuple.root,
+                                                                     'remote': src_tuple.remote_name,
+                                                                     'branch': src_tuple.branch,
+                                                                     'commit': src_tuple.commit,
+                                                                     'sparseCheckout': sparse,
+                                                                     'enable_submodule': sub,
+                                                                     'tag': src_tuple.tag})
                     else:
-                        elem = ET.SubElement(source_root, 'Source', {'localRoot':src_tuple.root, 'remote':src_tuple.remote_name,
-                                                                     'branch':src_tuple.branch, 'commit':src_tuple.commit,
-                                                                     'sparseCheckout':sparse, 'enable_submodule':sub})
+                        elem = ET.SubElement(source_root, 'Source', {'localRoot': src_tuple.root,
+                                                                     'remote': src_tuple.remote_name,
+                                                                     'branch': src_tuple.branch,
+                                                                     'commit': src_tuple.commit,
+                                                                     'sparseCheckout': sparse,
+                                                                     'enable_submodule': sub})
                 elif src_tuple.branch is None and src_tuple.tag:
-                    elem = ET.SubElement(source_root, 'Source', {'localRoot':src_tuple.root, 'remote':src_tuple.remote_name,
-                                                                 'commit':src_tuple.commit,'sparseCheckout':sparse,
-                                                                 'enable_submodule':sub, 'tag':src_tuple.tag})
+                    elem = ET.SubElement(source_root, 'Source', {'localRoot': src_tuple.root,
+                                                                 'remote': src_tuple.remote_name,
+                                                                 'commit': src_tuple.commit,
+                                                                 'sparseCheckout': sparse,
+                                                                 'enable_submodule': sub,
+                                                                 'tag': src_tuple.tag})
                 elif src_tuple.branch is None and src_tuple.tag is None:
-                    elem = ET.SubElement(source_root, 'Source', {'localRoot':src_tuple.root, 'remote':src_tuple.remote_name,
-                                                                 'commit':src_tuple.commit,'sparseCheckout':sparse,
-                                                                 'enable_submodule':sub})
+                    elem = ET.SubElement(source_root, 'Source', {'localRoot': src_tuple.root,
+                                                                 'remote': src_tuple.remote_name,
+                                                                 'commit': src_tuple.commit,
+                                                                 'sparseCheckout': sparse,
+                                                                 'enable_submodule': sub})
             else:
                 raise ValueError('Pin.xml cannot be generated with an empty commit value')
 
@@ -552,9 +568,9 @@ class ManifestXml(BaseXmlHelper):
     def __ne__(self, other):
         return not self.__eq__(other)
 
+
 class _ProjectInfo():
     def __init__(self, element):
-
         try:
             self.codename = element.find('CodeName').text
             self.descript = element.find('Description').text
@@ -565,17 +581,17 @@ class _ProjectInfo():
             self.lead_list = []
             for lead in element.findall('DevLead'):
                 self.lead_list.append(lead.text)
-        except:
+        except Exception:
             self.lead_list = None
 
         try:
             self.org = element.find('Org').text
-        except:
+        except Exception:
             self.org = None
 
         try:
             self.short_name = element.find('ShortName').text
-        except:
+        except Exception:
             self.short_name = None
 
         try:
@@ -583,33 +599,34 @@ class _ProjectInfo():
             subroot = element.find('LeadReviewers')
             for reviewer in subroot.iter(tag='Reviewer'):
                 self.reviewer_list.append(reviewer.text)
-        except:
+        except Exception:
             self.reviewer_list = None
 
-
     @property
     def tuple(self):
         return ProjectInfo(self.codename, self.descript, self.lead_list, self.reviewer_list, self.org, self.short_name)
 
+
 class _GeneralConfig():
     def __init__(self, element):
         try:
             self.pin_path = element.find('PinPath').text
-        except:
+        except Exception:
             self.pin_path = None
         try:
             self.default_combo = element.find('DefaultCombo').attrib['combination']
-        except:
+        except Exception:
             self.default_combo = None
         try:
             self.curr_combo = element.find('CurrentClonedCombo').attrib['combination']
-        except:
+        except Exception:
             self.curr_combo = None
 
     @property
     def tuple(self):
         return GeneralConfig(self.default_combo, self.curr_combo, self.pin_path)
 
+
 class _RemoteRepo():
     def __init__(self, element):
         try:
@@ -622,6 +639,7 @@ class _RemoteRepo():
     def tuple(self):
         return RemoteRepo(self.name, self.url)
 
+
 class _RepoHook():
     def __init__(self, element, remotes):
         try:
@@ -631,18 +649,19 @@ class _RepoHook():
             raise KeyError(REQUIRED_ATTRIB_ERROR_MSG.format(k, element.tag))
         try:
             self.remote_url = remotes[element.attrib['remote']].url
-        except:
+        except Exception:
             self.remote_url = None
             print(NO_ASSOCIATED_REMOTE.format(self.source, self.dest_path))
         try:
             self.dest_file = element.attrib['destination_file']
-        except:
+        except Exception:
             self.dest_file = None
 
     @property
     def tuple(self):
         return RepoHook(self.source, self.dest_path, self.dest_file, self.remote_url)
 
+
 class _Combination():
     def __init__(self, element):
         try:
@@ -651,17 +670,18 @@ class _Combination():
             raise KeyError(REQUIRED_ATTRIB_ERROR_MSG.format(k, element.tag))
         try:
             self.description = element.attrib['description']
-        except:
-            self.description = None   #description is optional attribute
+        except Exception:
+            self.description = None   # description is optional attribute
         try:
             self.archived = (element.attrib['archived'].lower() == 'true')
-        except:
+        except Exception:
             self.archived = False
 
     @property
     def tuple(self):
         return Combination(self.name, self.description)
 
+
 class _RepoSource():
     def __init__(self, element, remotes):
         try:
@@ -672,25 +692,25 @@ class _RepoSource():
             raise KeyError(REQUIRED_ATTRIB_ERROR_MSG.format(k, element.tag))
         try:
             self.branch = element.attrib['branch']
-        except:
+        except Exception:
             self.branch = None
         try:
             self.commit = element.attrib['commit']
-        except:
+        except Exception:
             self.commit = None
         try:
             self.tag = element.attrib['tag']
-        except:
+        except Exception:
             self.tag = None
         try:
-            #if the sparse attrib is not explicitly set to true, then assume false
+            # if the sparse attrib is not explicitly set to true, then assume false
             self.sparse = (element.attrib['sparseCheckout'].lower() == 'true')
-        except:
+        except Exception:
             self.sparse = False
         try:
-            #If enableSubmodule is not set to True then default to False
+            # If enableSubmodule is not set to True then default to False
             self.enableSub = (element.attrib['enableSubmodule'].lower() == 'true')
-        except:
+        except Exception:
             self.enableSub = False
 
         if self.branch is None and self.commit is None and self.tag is None:
@@ -698,20 +718,23 @@ class _RepoSource():
 
     @property
     def tuple(self):
-        return RepoSource(self.root, self.remote_name, self.remote_url, self.branch, self.commit, self.sparse, self.enableSub, self.tag)
+        return RepoSource(self.root, self.remote_name, self.remote_url, self.branch,
+                          self.commit, self.sparse, self.enableSub, self.tag)
+
 
 class _SparseSettings():
     def __init__(self, element):
         self.sparse_by_default = False
         try:
             self.sparse_by_default = (element.attrib['sparseByDefault'].lower() == 'true')
-        except:
+        except Exception:
             pass
 
     @property
     def tuple(self):
         return SparseSettings(self.sparse_by_default)
 
+
 class _SparseData():
     def __init__(self, element):
         self.combination = None
@@ -720,11 +743,11 @@ class _SparseData():
         self.always_exclude = []
         try:
             self.combination = element.attrib['combination']
-        except:
+        except Exception:
             pass
         try:
             self.remote_name = element.attrib['remote']
-        except:
+        except Exception:
             pass
         for includes in element.iter(tag='AlwaysInclude'):
             self.always_include.extend(includes.text.split('|'))
@@ -735,18 +758,20 @@ class _SparseData():
     def tuple(self):
         return SparseData(self.combination, self.remote_name, self.always_include, self.always_exclude)
 
+
 class _FolderToFolderMappingFolderExclude():
     def __init__(self, element):
         self.path = None
         try:
             self.path = element.attrib['path']
-        except:
+        except Exception:
             pass
 
     @property
     def tuple(self):
         return FolderToFolderMappingFolderExclude(self.path)
 
+
 class _FolderToFolderMappingFolder():
     def __init__(self, element):
         self.project1_folder = None
@@ -754,11 +779,11 @@ class _FolderToFolderMappingFolder():
         self.excludes = []
         try:
             self.project1_folder = element.attrib['project1']
-        except:
+        except Exception:
             pass
         try:
             self.project2_folder = element.attrib['project2']
-        except:
+        except Exception:
             pass
         for exclude in element.iter(tag='Exclude'):
             self.excludes.append(_FolderToFolderMappingFolderExclude(exclude))
@@ -767,6 +792,7 @@ class _FolderToFolderMappingFolder():
     def tuple(self):
         return FolderToFolderMappingFolder(self.project1_folder, self.project2_folder, self.excludes)
 
+
 class _FolderToFolderMapping():
     def __init__(self, element):
         self.project1 = None
@@ -775,15 +801,15 @@ class _FolderToFolderMapping():
         self.folders = []
         try:
             self.project1 = element.attrib['project1']
-        except:
+        except Exception:
             pass
         try:
             self.project2 = element.attrib['project2']
-        except:
+        except Exception:
             pass
         try:
             self.remote_name = element.attrib['remote']
-        except:
+        except Exception:
             pass
         for folder in element.iter(tag='Folder'):
             self.folders.append(_FolderToFolderMappingFolder(folder))
@@ -811,6 +837,7 @@ class _SubmoduleAlternateRemote():
     def tuple(self):
         return SubmoduleAlternateRemote(self.remote_name, self.originalUrl, self.altUrl)
 
+
 #
 # Optional entry point for debug and validation of the CiIndexXml & ManifestXml classes
 #
@@ -824,7 +851,8 @@ def main():
 
     parser = argparse.ArgumentParser()
     parser.add_argument("InputFile", help="Xml file to parse", nargs='?', default="manifest.xml")
-    parser.add_argument('-v', '--verbose', action='store_true', help='Increased verbosity including exception tracebacks')
+    parser.add_argument('-v', '--verbose', action='store_true',
+                        help='Increased verbosity including exception tracebacks')
 
     args = parser.parse_args()
 
@@ -934,5 +962,6 @@ def main():
         if args.verbose:
             traceback.print_exc()
 
+
 if __name__ == "__main__":
     main()
-- 
2.21.0.windows.1

[edk2-staging/EdkRepo] [PATCH v1 2/3] EdkRepo: Improve error message with invalid XML

[Bjorge, Erik C]

The ETree exception contains additional information about the location
of XML tag errors.  Adding this information to the error message for
better debugging support.

Signed-off-by: Erik Bjorge <erik.c.bjorge@intel.com>
Cc: Ashley E Desimone <ashley.e.desimone@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Puja Pandya <puja.pandya@intel.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Prince Agyeman <prince.agyeman@intel.com>
---
 edkrepo_manifest_parser/edk_manifest.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/edkrepo_manifest_parser/edk_manifest.py b/edkrepo_manifest_parser/edk_manifest.py
index 7e5f0fb..2ec6cd1 100644
--- a/edkrepo_manifest_parser/edk_manifest.py
+++ b/edkrepo_manifest_parser/edk_manifest.py
@@ -51,7 +51,7 @@ GENERAL_CONFIG_MISSING_ERROR = "Unable to locate <GeneralConfig>"
 SOURCELIST_EMPTY_ERROR = "Invalid input: empty values in source list"
 INVALID_PROJECTNAME_ERROR = "Invalid input: {} not found in CiIndexXml"
 UNSUPPORTED_TYPE_ERROR = "{} is not a supported xml type: {}"
-INVALID_XML_ERROR = "{} is not a valid xml file"
+INVALID_XML_ERROR = "{} is not a valid xml file ({})"
 
 
 class BaseXmlHelper():
@@ -59,8 +59,8 @@ class BaseXmlHelper():
         self._fileref = fileref
         try:
             self._tree = ET.ElementTree(file=fileref)  # fileref can be a filename or filestream
-        except Exception:
-            raise TypeError(INVALID_XML_ERROR.format(fileref))
+        except Exception as et_error:
+            raise TypeError(INVALID_XML_ERROR.format(fileref, et_error))
 
         self._xml_type = self._tree.getroot().tag
         if self._xml_type not in xml_types:
-- 
2.21.0.windows.1

[edk2-staging/EdkRepo] [PATCH v1 3/3] EdkRepo: Refactoring private class members

[Bjorge, Erik C]

Refactoring private variables to remove name mangling from double
underscores to using single underscores.  This allows for better
creation of derived classes.

Signed-off-by: Erik Bjorge <erik.c.bjorge@intel.com>
Cc: Ashley E Desimone <ashley.e.desimone@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Puja Pandya <puja.pandya@intel.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Prince Agyeman <prince.agyeman@intel.com>
---
 edkrepo_manifest_parser/edk_manifest.py | 100 ++++++++++++------------
 1 file changed, 50 insertions(+), 50 deletions(-)

diff --git a/edkrepo_manifest_parser/edk_manifest.py b/edkrepo_manifest_parser/edk_manifest.py
index 2ec6cd1..d65f5f6 100644
--- a/edkrepo_manifest_parser/edk_manifest.py
+++ b/edkrepo_manifest_parser/edk_manifest.py
@@ -73,16 +73,16 @@ class BaseXmlHelper():
 class CiIndexXml(BaseXmlHelper):
     def __init__(self, fileref):
         super().__init__(fileref, 'ProjectList')
-        self.__projects = {}
+        self._projects = {}
         for element in self._tree.iter(tag='Project'):
             proj = _Project(element)
             # Todo: add check for unique
-            self.__projects[proj.name] = proj
+            self._projects[proj.name] = proj
 
     @property
     def project_list(self):
         proj_names = []
-        for proj in self.__projects.values():
+        for proj in self._projects.values():
             if proj.archived is False:
                 proj_names.append(proj.name)
         return proj_names
@@ -90,14 +90,14 @@ class CiIndexXml(BaseXmlHelper):
     @property
     def archived_project_list(self):
         proj_names = []
-        for proj in self.__projects.values():
+        for proj in self._projects.values():
             if proj.archived is True:
                 proj_names.append(proj.name)
         return proj_names
 
     def get_project_xml(self, project_name):
-        if project_name in self.__projects:
-            return self.__projects[project_name].xmlPath
+        if project_name in self._projects:
+            return self._projects[project_name].xmlPath
         else:
             raise ValueError(INVALID_PROJECTNAME_ERROR.format(project_name))
 
@@ -126,18 +126,18 @@ class ManifestXml(BaseXmlHelper):
         # internally gathering and storing the manifest data. As such, all access to them should be
         # done through the provided methods to ensure future compatibility if the xml schema changes
         super().__init__(fileref, ['Pin', 'Manifest'])
-        self.__project_info = None
-        self.__general_config = None
-        self.__remotes = {}                    # dict of _Remote objs, with Remote.name as key
-        self.__client_hook_list = []
-        self.__combinations = {}               # dict of _Combination objs, with Combination.name as key
-        self.__combo_sources = {}              # dict of _RepoSource obj lists, with Combination.name as key
-        self.__dsc_list = []
-        self.__sparse_settings = None          # A single instance of platform sparse checkout settings
-        self.__sparse_data = []                # List of SparseData objects
-        self.__commit_templates = {}           # dict of commit message templates with the remote name as the key
-        self.__folder_to_folder_mappings = []  # List of FolderToFolderMapping objects
-        self.__submodule_alternate_remotes = []
+        self._project_info = None
+        self._general_config = None
+        self._remotes = {}                    # dict of _Remote objs, with Remote.name as key
+        self._client_hook_list = []
+        self._combinations = {}               # dict of _Combination objs, with Combination.name as key
+        self._combo_sources = {}              # dict of _RepoSource obj lists, with Combination.name as key
+        self._dsc_list = []
+        self._sparse_settings = None          # A single instance of platform sparse checkout settings
+        self._sparse_data = []                # List of SparseData objects
+        self._commit_templates = {}           # dict of commit message templates with the remote name as the key
+        self._folder_to_folder_mappings = []  # List of FolderToFolderMapping objects
+        self._submodule_alternate_remotes = []
 
         #
         # Append include XML's to the Manifest etree before parsing
@@ -161,19 +161,19 @@ class ManifestXml(BaseXmlHelper):
         #
         for subroot in self._tree.iter(tag='RemoteList'):
             for element in subroot.iter(tag='Remote'):
-                self._add_unique_item(_RemoteRepo(element), self.__remotes, element.tag)
+                self._add_unique_item(_RemoteRepo(element), self._remotes, element.tag)
 
         #
         # parse <ProjectInfo> tags
         #
         subroot = self._tree.find('ProjectInfo')
-        self.__project_info = _ProjectInfo(subroot)
+        self._project_info = _ProjectInfo(subroot)
 
         #
         # parse <GeneralConfig> tags
         #
         subroot = self._tree.find('GeneralConfig')
-        self.__general_config = _GeneralConfig(subroot)
+        self._general_config = _GeneralConfig(subroot)
 
         #
         # parse <ClientGitHookList> tags
@@ -181,7 +181,7 @@ class ManifestXml(BaseXmlHelper):
         #
         for subroot in self._tree.iter(tag='ClientGitHookList'):
             for element in subroot.iter(tag='ClientGitHook'):
-                self.__client_hook_list.append(_RepoHook(element, self.__remotes))
+                self._client_hook_list.append(_RepoHook(element, self._remotes))
 
         #
         # Parse <SubmoduleAlternateRemotes>
@@ -189,7 +189,7 @@ class ManifestXml(BaseXmlHelper):
         #
         for subroot in self._tree.iter(tag='SubmoduleAlternateRemotes'):
             for element in subroot.iter(tag='SubmoduleAlternateRemote'):
-                self.__submodule_alternate_remotes.append(_SubmoduleAlternateRemote(element, self.__remotes))
+                self._submodule_alternate_remotes.append(_SubmoduleAlternateRemote(element, self._remotes))
 
         #
         # parse <CombinationList> tags
@@ -223,7 +223,7 @@ class ManifestXml(BaseXmlHelper):
         #
         for subroot in self._tree.iter(tag='DscList'):
             for element in subroot.iter(tag='Dsc'):
-                self.__dsc_list.append(element.text)
+                self._dsc_list.append(element.text)
 
         #
         # Process <SparseCheckout> tag
@@ -231,11 +231,11 @@ class ManifestXml(BaseXmlHelper):
         subroot = self._tree.find('SparseCheckout')
         if subroot is not None:
             try:
-                self.__sparse_settings = _SparseSettings(subroot.find('SparseSettings'))
+                self._sparse_settings = _SparseSettings(subroot.find('SparseSettings'))
             except KeyError as k:
                 raise KeyError(REQUIRED_ATTRIB_ERROR_MSG.format(k, subroot.tag))
             for sparse_data in subroot.iter(tag='SparseData'):
-                self.__sparse_data.append(_SparseData(sparse_data))
+                self._sparse_data.append(_SparseData(sparse_data))
 
         #
         # Process any commit log templates that may exist (optional)
@@ -248,7 +248,7 @@ class ManifestXml(BaseXmlHelper):
                     template_text = template_element.text
                 except KeyError as k:
                     raise KeyError(REQUIRED_ATTRIB_ERROR_MSG.format(k, subroot.tag))
-                self.__commit_templates[remote_name] = template_text
+                self._commit_templates[remote_name] = template_text
 
         #
         # Process <FolderToFolderMappingList> tag
@@ -256,7 +256,7 @@ class ManifestXml(BaseXmlHelper):
         subroot = self._tree.find('FolderToFolderMappingList')
         if subroot is not None:
             for f2f_mapping in subroot.iter(tag='FolderToFolderMapping'):
-                self.__folder_to_folder_mappings.append(_FolderToFolderMapping(f2f_mapping))
+                self._folder_to_folder_mappings.append(_FolderToFolderMapping(f2f_mapping))
 
         return
 
@@ -274,11 +274,11 @@ class ManifestXml(BaseXmlHelper):
     def _add_combo_source(self, subroot, combo):
         # create a list of _RepoSource objs from the <Source> tags in subroot
         # and add it to the __combo_sources dictionary
-        self._add_unique_item(combo, self.__combinations, subroot.tag)
+        self._add_unique_item(combo, self._combinations, subroot.tag)
         temp_sources = []
         for element in subroot.iter(tag='Source'):
-            temp_sources.append(_RepoSource(element, self.__remotes))
-        self.__combo_sources[combo.name] = temp_sources
+            temp_sources.append(_RepoSource(element, self._remotes))
+        self._combo_sources[combo.name] = temp_sources
 
     def _add_unique_item(self, obj, item_dict, tag):
         # add the 'obj' to 'dict', or raise error if it already exists
@@ -299,56 +299,56 @@ class ManifestXml(BaseXmlHelper):
     #
     @property
     def project_info(self):
-        return self.__project_info.tuple
+        return self._project_info.tuple
 
     @property
     def general_config(self):
-        return self.__general_config.tuple
+        return self._general_config.tuple
 
     @property
     def remotes(self):
-        return self._tuple_list(self.__remotes.values())
+        return self._tuple_list(self._remotes.values())
 
     @property
     def combinations(self):
-        return self._tuple_list([x for x in self.__combinations.values() if not x.archived])
+        return self._tuple_list([x for x in self._combinations.values() if not x.archived])
 
     @property
     def archived_combinations(self):
-        return self._tuple_list([x for x in self.__combinations.values() if x.archived])
+        return self._tuple_list([x for x in self._combinations.values() if x.archived])
 
     def get_repo_sources(self, combo_name):
-        if combo_name in self.__combo_sources:
-            return self._tuple_list(self.__combo_sources[combo_name])
+        if combo_name in self._combo_sources:
+            return self._tuple_list(self._combo_sources[combo_name])
         elif combo_name.startswith('Pin:'):
             # If currently checked out onto a pin file reture the sources in the
             # default combo
-            return self._tuple_list(self.__combo_sources[self.general_config.default_combo])
+            return self._tuple_list(self._combo_sources[self.general_config.default_combo])
         else:
             raise ValueError(COMB_INVALIDINPUT_ERROR.format(combo_name))
 
     @property
     def repo_hooks(self):
-        return self._tuple_list(self.__client_hook_list)
+        return self._tuple_list(self._client_hook_list)
 
     @property
     def dsc_list(self):
-        return self.__dsc_list
+        return self._dsc_list
 
     @property
     def sparse_settings(self):
-        if self.__sparse_settings:
-            return self.__sparse_settings.tuple
+        if self._sparse_settings:
+            return self._sparse_settings.tuple
         return None
 
     @property
     def sparse_data(self):
-        return self._tuple_list(self.__sparse_data)
+        return self._tuple_list(self._sparse_data)
 
     @property
     def folder_to_folder_mappings(self):
         f2f_tuples = []
-        for f2f_mapping in self.__folder_to_folder_mappings:
+        for f2f_mapping in self._folder_to_folder_mappings:
             folders = f2f_mapping.folders
             folder_tuples = []
             for folder in folders:
@@ -373,15 +373,15 @@ class ManifestXml(BaseXmlHelper):
 
     @property
     def commit_templates(self):
-        return self.__commit_templates
+        return self._commit_templates
 
     @property
     def submodule_alternate_remotes(self):
-        return self._tuple_list(self.__submodule_alternate_remotes)
+        return self._tuple_list(self._submodule_alternate_remotes)
 
     def get_submodule_alternates_for_remote(self, remote_name):
         alternates = []
-        for alternate in self.__submodule_alternate_remotes:
+        for alternate in self._submodule_alternate_remotes:
             if alternate.remote_name == remote_name:
                 alternates.append(alternate.tuple)
         return alternates
@@ -409,7 +409,7 @@ class ManifestXml(BaseXmlHelper):
 
         element.attrib['combination'] = combo_name
         self._tree.write(filename)
-        self.__general_config.current_combo = combo_name
+        self._general_config.current_combo = combo_name
 
     def generate_pin_xml(self, description, combo_name, repo_source_list, filename=None):
 
@@ -443,7 +443,7 @@ class ManifestXml(BaseXmlHelper):
 
         remote_root = ET.SubElement(pin_root, 'RemoteList')
         source_root = ET.SubElement(pin_root, 'Combination')
-        source_root.attrib['name'] = self.__combinations[combo_name].name
+        source_root.attrib['name'] = self._combinations[combo_name].name
 
         # Add tags for each RepoSource tuple in the list provided
         # Only one of Branch or SHA is required to write PIN and checkout code
-- 
2.21.0.windows.1

Re: [edk2-staging/EdkRepo] [PATCH v1 0/3] Manifest parser cleanup

[Nate DeSimone]

For the series...

Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>

-----Original Message-----
From: Bjorge, Erik C <erik.c.bjorge@intel.com> 
Sent: Thursday, April 30, 2020 3:53 PM
To: devel@edk2.groups.io
Cc: Desimone, Ashley E <ashley.e.desimone@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Pandya, Puja <puja.pandya@intel.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>; Agyeman, Prince <prince.agyeman@intel.com>
Subject: [edk2-staging/EdkRepo] [PATCH v1 0/3] Manifest parser cleanup

General cleanup of the manifest parser and minor improvements to make the code more flexible.

Cc: Ashley E Desimone <ashley.e.desimone@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Puja Pandya <puja.pandya@intel.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Prince Agyeman <prince.agyeman@intel.com>

Erik Bjorge (3):
  EdkRepo: Flake8 cleanup
  EdkRepo: Improve error message with invalid XML
  EdkRepo: Refactoring private class members

 edkrepo_manifest_parser/edk_manifest.py | 249 +++++++++++++-----------
 1 file changed, 139 insertions(+), 110 deletions(-)

--
2.21.0.windows.1

Re: [edk2-staging/EdkRepo] [PATCH v1 0/3] Manifest parser cleanup

[Nate DeSimone]

After a some rebasing pain... Pushed as f2f6f5a2~.. 6153e4a2

-----Original Message-----
From: Bjorge, Erik C <erik.c.bjorge@intel.com> 
Sent: Thursday, April 30, 2020 3:53 PM
To: devel@edk2.groups.io
Cc: Desimone, Ashley E <ashley.e.desimone@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Pandya, Puja <puja.pandya@intel.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>; Agyeman, Prince <prince.agyeman@intel.com>
Subject: [edk2-staging/EdkRepo] [PATCH v1 0/3] Manifest parser cleanup

General cleanup of the manifest parser and minor improvements to make the code more flexible.

Cc: Ashley E Desimone <ashley.e.desimone@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Puja Pandya <puja.pandya@intel.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Prince Agyeman <prince.agyeman@intel.com>

Erik Bjorge (3):
  EdkRepo: Flake8 cleanup
  EdkRepo: Improve error message with invalid XML
  EdkRepo: Refactoring private class members

 edkrepo_manifest_parser/edk_manifest.py | 249 +++++++++++++-----------
 1 file changed, 139 insertions(+), 110 deletions(-)

--
2.21.0.windows.1

Re: [edk2-staging/EdkRepo] [PATCH v1 0/3] Manifest parser cleanup

[Bjorge, Erik C]

Nate, if you run into this issue again please let me know and I can send out a new version.

Thanks,
-Erik

-----Original Message-----
From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com> 
Sent: Monday, May 4, 2020 8:59 PM
To: Bjorge, Erik C <erik.c.bjorge@intel.com>; devel@edk2.groups.io
Cc: Desimone, Ashley E <ashley.e.desimone@intel.com>; Pandya, Puja <puja.pandya@intel.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>; Agyeman, Prince <prince.agyeman@intel.com>
Subject: RE: [edk2-staging/EdkRepo] [PATCH v1 0/3] Manifest parser cleanup

After a some rebasing pain... Pushed as f2f6f5a2~.. 6153e4a2

-----Original Message-----
From: Bjorge, Erik C <erik.c.bjorge@intel.com> 
Sent: Thursday, April 30, 2020 3:53 PM
To: devel@edk2.groups.io
Cc: Desimone, Ashley E <ashley.e.desimone@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Pandya, Puja <puja.pandya@intel.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>; Agyeman, Prince <prince.agyeman@intel.com>
Subject: [edk2-staging/EdkRepo] [PATCH v1 0/3] Manifest parser cleanup

General cleanup of the manifest parser and minor improvements to make the code more flexible.

Cc: Ashley E Desimone <ashley.e.desimone@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Puja Pandya <puja.pandya@intel.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Prince Agyeman <prince.agyeman@intel.com>

Erik Bjorge (3):
  EdkRepo: Flake8 cleanup
  EdkRepo: Improve error message with invalid XML
  EdkRepo: Refactoring private class members

 edkrepo_manifest_parser/edk_manifest.py | 249 +++++++++++++-----------
 1 file changed, 139 insertions(+), 110 deletions(-)

--
2.21.0.windows.1