public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-staging/EdkRepo] [PATCH 0/7] Support for consuming multiple manifest repositories
@ 2020-04-28 21:57 Ashley E Desimone
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 1/7] EdkRepo: Add check for conflicting/duplicated manifest repo definitions Ashley E Desimone
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Ashley E Desimone @ 2020-04-28 21:57 UTC (permalink / raw)
  To: devel
  Cc: Nate DeSimone, Puja Pandya, Erik Bjorge, Bret Barkelew,
	Prince Agyeman

Add the required support functionaility for consuming multiple
manifest repositories defined in both the edkrepo.cfg and
edkrepo_user.cfg files.

Includes support for determining the source manifest repository,
pulling only the manifest repository relevant to the workspace,
and finding projects within all manifest repositories.

Signed-off-by: Ashley E Desimone <ashley.e.desimone@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Puja Pandya <puja.pandya@intel.com>
Cc: Erik Bjorge <erik.c.bjorge@intel.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Prince Agyeman <prince.agyeman@intel.com>

Ashley E Desimone (7):
  EdkRepo: Add check for conflicting/duplicated manifest repo
    definitions
  EdkRepo: Add downloading all available manifest repositories
  EdkRepo: Add optional field to edkrepo_manifst to track the source
    manifest repo
  EdkRepo: Add list_available_manifest_repos()
  EdkRepo: Add ability to find projects across all manifest repositories
  EdkRepo: Add ability to determine the source manifest of a workspace
  EdkRepo: Add the ability to pull only the global manifest repository
    for a given workspace.

 .../humble/manifest_repos_maintenance_humble.py    |   6 +
 .../manifest_repos_maintenance.py                  | 182 ++++++++++++++++++++-
 .../workspace_maintenance/workspace_maintenance.py |  17 +-
 edkrepo_manifest_parser/edk_manifest.py            |  26 ++-
 4 files changed, 226 insertions(+), 5 deletions(-)

-- 
2.16.2.windows.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [edk2-staging/EdkRepo] [PATCH 1/7] EdkRepo: Add check for conflicting/duplicated manifest repo definitions
  2020-04-28 21:57 [edk2-staging/EdkRepo] [PATCH 0/7] Support for consuming multiple manifest repositories Ashley E Desimone
@ 2020-04-28 21:57 ` Ashley E Desimone
  2020-04-30 21:28   ` [edk2-devel] " Nate DeSimone
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 2/7] EdkRepo: Add downloading all available manifest repositories Ashley E Desimone
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Ashley E Desimone @ 2020-04-28 21:57 UTC (permalink / raw)
  To: devel
  Cc: Nate DeSimone, Puja Pandya, Erik Bjorge, Bret Barkelew,
	Prince Agyeman

Add a functions to check for conflicting or duplicated manifest
repository definitions in the edkrepo.cfg and the edkrepo_user.cfg
files. Two manifest repositories definitions are
considered conflicting if they have the same name and at least one
of URL, branch or local path differ. Two manifest repository
definitions are considered duplicates if the name, URL, branch
and local path are the same.

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

diff --git a/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py b/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py
index 6e26d4f..ad6ddbc 100644
--- a/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py
+++ b/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py
@@ -57,3 +57,28 @@ def pull_single_manifest_repo(url, branch, local_path, reset_hard=False):
             print (humble.CLONE_SINGLE_MAN_REPO.format(local_path, url))
             repo = Repo.clone_from(url, local_path, progress=GitProgressHandler(), branch=branch)
 
+def detect_man_repo_conflicts_duplicates(edkrepo_cfg, edkrepo_user_cfg):
+    '''
+    Determines whether there is are conflicting or duplicated manifest
+    repositories listed in the edkrepo.cfg and the edkrepo_user.cfg.
+    '''
+    conflicts = []
+    duplicates = []
+    if not edkrepo_user_cfg.manifest_repo_list:
+        return conflicts, duplicates
+    else:
+        config_repos = set(edkrepo_cfg.manifest_repo_list)
+        user_cfg_repos = set(edkrepo_user_cfg.manifest_repo_list)
+    if config_repos.isdisjoint(user_cfg_repos):
+        return conflicts, duplicates
+    else:
+        for repo in config_repos.intersection(user_cfg_repos):
+            if edkrepo_cfg.get_manifest_repo_url(repo) != edkrepo_user_cfg.get_manifest_repo_url(repo):
+                conflicts.append(repo)
+            elif edkrepo_cfg.get_manifest_repo_branch(repo) != edkrepo_user_cfg.get_manifest_repo_branch(repo):
+                conflicts.append(repo)
+            elif edkrepo_cfg.get_manifest_repo_local_path(repo) != edkrepo_user_cfg.get_manifest_repo_local_path(repo):
+                conflicts.append(repo)
+            else:
+                duplicates.append(repo)
+    return conflicts, duplicates
-- 
2.16.2.windows.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [edk2-staging/EdkRepo] [PATCH 2/7] EdkRepo: Add downloading all available manifest repositories
  2020-04-28 21:57 [edk2-staging/EdkRepo] [PATCH 0/7] Support for consuming multiple manifest repositories Ashley E Desimone
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 1/7] EdkRepo: Add check for conflicting/duplicated manifest repo definitions Ashley E Desimone
@ 2020-04-28 21:57 ` Ashley E Desimone
  2020-04-30 21:28   ` [edk2-devel] " Nate DeSimone
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 3/7] EdkRepo: Add optional field to edkrepo_manifst to track the source manifest repo Ashley E Desimone
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Ashley E Desimone @ 2020-04-28 21:57 UTC (permalink / raw)
  To: devel
  Cc: Nate DeSimone, Puja Pandya, Erik Bjorge, Bret Barkelew,
	Prince Agyeman

Add a function that will download all available manifest
repositories defined in either the edkrepo.cfg or the
edkrepo_user.cfg

Signed-off-by: Ashley E Desimone <ashley.e.desimone@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Puja Pandya <puja.pandya@intel.com>
Cc: Erik Bjorge <erik.c.bjorge@intel.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Prince Agyeman <prince.agyeman@intel.com>
---
 .../humble/manifest_repos_maintenance_humble.py    |  4 +++
 .../manifest_repos_maintenance.py                  | 38 ++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/edkrepo/common/workspace_maintenance/humble/manifest_repos_maintenance_humble.py b/edkrepo/common/workspace_maintenance/humble/manifest_repos_maintenance_humble.py
index 440fd8a..e592f19 100644
--- a/edkrepo/common/workspace_maintenance/humble/manifest_repos_maintenance_humble.py
+++ b/edkrepo/common/workspace_maintenance/humble/manifest_repos_maintenance_humble.py
@@ -21,3 +21,7 @@ SINGLE_MAN_REPO_NOT_CFG_BRANCH = ('The current active branch, {}, is not the '
                                   'specified branch for global manifst repository: {}')
 SINGLE_MAN_REPO_CHECKOUT_CFG_BRANCH = 'Checking out the specified branch: {} prior to syncing'
 SINGLE_MAN_REPO_MOVED = '{}{}WARNING:{}{} The global manifest repository has moved. Backing up previous global manifest repository to: {{}}{}\n'.format(Style.BRIGHT, Fore.RED, Style.RESET_ALL, Fore.RED, Style.RESET_ALL)
+CONFLICT_NO_CLONE = ('The definition of global manifest repository, {}, '
+                     'in the edkrepo_user.cfg does not match the definition in the edkrepo.cfg. '
+                     'This global manifest repository will not be downloaded or updated. '
+                     'Resolve the conflict and then re-run the failed operation')
diff --git a/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py b/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py
index ad6ddbc..24ad76a 100644
--- a/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py
+++ b/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py
@@ -57,6 +57,44 @@ def pull_single_manifest_repo(url, branch, local_path, reset_hard=False):
             print (humble.CLONE_SINGLE_MAN_REPO.format(local_path, url))
             repo = Repo.clone_from(url, local_path, progress=GitProgressHandler(), branch=branch)
 
+def pull_all_manifest_repos(edkrepo_cfg, edkrepo_user_cfg, reset_hard=False):
+    '''
+    Clones or syncs all global manifest repositories defined in both the
+    edkrepo_cfg and the edkrepo_user.cfg)
+    '''
+    cfg_man_repos = []
+    user_cfg_man_repos = []
+    conflicts, duplicates = detect_man_repo_conflicts_duplicates(edkrepo_cfg, edkrepo_user_cfg)
+    if not conflicts and not duplicates:
+        cfg_man_repos.extend(edkrepo_cfg.manifest_repo_list)
+        user_cfg_man_repos.extend(edkrepo_user_cfg.manifest_repo_list)
+    elif conflicts:
+        for conflict in conflicts:
+            # In the case of a conflict do not pull conflicting repo
+            print(humble.CONFLICT_NO_CLONE.format(conflict))
+            cfg_man_repos.extend(edkrepo_cfg.manifest_repo_list)
+            cfg_man_repos.remove(conflict)
+            user_cfg_man_repos.extend(edkrepo_user_cfg.manifest_repo_list)
+            user_cfg_man_repos.remove(conflict)
+    elif duplicates:
+        for duplicate in duplicates:
+            # the duplicate needs to be ignored in on of the repo lists so it is
+            # not cloned/pulled twice
+            cfg_man_repos.extend(edkrepo_cfg.manifest_repo_list)
+            user_cfg_man_repos.extend(edkrepo_user_cfg.manifest_repo_list)
+            user_cfg_man_repos.remove(conflict)
+    for repo in cfg_man_repos:
+        pull_single_manifest_repo(edkrepo_cfg.get_manifest_repo_url(repo),
+                                  edkrepo_cfg.get_manifest_repo_branch(repo),
+                                  edkrepo_cfg.get_manifest_repo_local_path(repo),
+                                  reset_hard)
+    for repo in user_cfg_man_repos:
+        pull_single_manifest_repo(edkrepo_user_cfg.get_manifest_repo_url(repo),
+                                  edkrepo_user_cfg.get_manifest_repo_branch(repo),
+                                  edkrepo_user_cfg.get_manifest_repo_local_path(repo),
+                                  reset_hard)
+
+
 def detect_man_repo_conflicts_duplicates(edkrepo_cfg, edkrepo_user_cfg):
     '''
     Determines whether there is are conflicting or duplicated manifest
-- 
2.16.2.windows.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [edk2-staging/EdkRepo] [PATCH 3/7] EdkRepo: Add optional field to edkrepo_manifst to track the source manifest repo
  2020-04-28 21:57 [edk2-staging/EdkRepo] [PATCH 0/7] Support for consuming multiple manifest repositories Ashley E Desimone
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 1/7] EdkRepo: Add check for conflicting/duplicated manifest repo definitions Ashley E Desimone
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 2/7] EdkRepo: Add downloading all available manifest repositories Ashley E Desimone
@ 2020-04-28 21:57 ` Ashley E Desimone
  2020-04-30 21:28   ` Nate DeSimone
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 4/7] EdkRepo: Add list_available_manifest_repos() Ashley E Desimone
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Ashley E Desimone @ 2020-04-28 21:57 UTC (permalink / raw)
  To: devel
  Cc: Nate DeSimone, Puja Pandya, Erik Bjorge, Bret Barkelew,
	Prince Agyeman

Add the SourceManifestRepository to the edkrepo manfiest general config
for use by edkrepo to track the source manifest repository for the workspace.

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

diff --git a/edkrepo_manifest_parser/edk_manifest.py b/edkrepo_manifest_parser/edk_manifest.py
index 080448f..dcf9c29 100644
--- a/edkrepo_manifest_parser/edk_manifest.py
+++ b/edkrepo_manifest_parser/edk_manifest.py
@@ -21,7 +21,7 @@ import copy
 # All the namedtuple data structures that consumers of this module will need.
 #
 ProjectInfo = namedtuple('ProjectInfo', ['codename', 'description', 'dev_leads', 'reviewers', 'org', 'short_name'])
-GeneralConfig = namedtuple('GeneralConfig', ['default_combo', 'current_combo', 'pin_path'])
+GeneralConfig = namedtuple('GeneralConfig', ['default_combo', 'current_combo', 'pin_path', 'source_man_repo'])
 RemoteRepo = namedtuple('RemoteRepo', ['name', 'url'])
 RepoHook = namedtuple('RepoHook', ['source', 'dest_path', 'dest_file', 'remote_url'])
 Combination = namedtuple('Combination', ['name', 'description'])
@@ -406,6 +406,24 @@ class ManifestXml(BaseXmlHelper):
         self._tree.write(filename)
         self.__general_config.current_combo = combo_name
 
+    def write_source_manifest_repo(self, manifest_repo, filename=None):
+        '''
+        Writes the name of the source manifest repository to the
+        general config sections of the manifest file.
+        '''
+        if filename is None:
+            filename = self._fileref
+        subroot = self._tree.find('GeneralConfig')
+        if subroot is None:
+            raise KeyError(GENERAL_CONFIG_MISSING_ERROR)
+
+        element = subroot.find('SourceManifestRepository')
+        if element is None:
+            element = ET.SubElement(subroot, 'SourceManifestRepository')
+        element.attrib['manifest_repo'] = manifest_repo
+        self._tree.write(filename)
+        self.__general_config.source_man_repo = manifest_repo
+
     def generate_pin_xml(self, description, combo_name, repo_source_list, filename=None):
 
         pin_tree = ET.ElementTree(ET.Element('Pin'))
@@ -605,10 +623,14 @@ class _GeneralConfig():
             self.curr_combo = element.find('CurrentClonedCombo').attrib['combination']
         except:
             self.curr_combo = None
+        try:
+            self.source_man_repo = element.find('SourceManifestRepository').attrib['manifest_repo']
+        except:
+            self.source_man_repo = None
 
     @property
     def tuple(self):
-        return GeneralConfig(self.default_combo, self.curr_combo, self.pin_path)
+        return GeneralConfig(self.default_combo, self.curr_combo, self.pin_path, self.source_man_repo)
 
 class _RemoteRepo():
     def __init__(self, element):
-- 
2.16.2.windows.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [edk2-staging/EdkRepo] [PATCH 4/7] EdkRepo: Add list_available_manifest_repos()
  2020-04-28 21:57 [edk2-staging/EdkRepo] [PATCH 0/7] Support for consuming multiple manifest repositories Ashley E Desimone
                   ` (2 preceding siblings ...)
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 3/7] EdkRepo: Add optional field to edkrepo_manifst to track the source manifest repo Ashley E Desimone
@ 2020-04-28 21:57 ` Ashley E Desimone
  2020-04-30 21:28   ` Nate DeSimone
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 5/7] EdkRepo: Add ability to find projects across all manifest repositories Ashley E Desimone
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Ashley E Desimone @ 2020-04-28 21:57 UTC (permalink / raw)
  To: devel
  Cc: Nate DeSimone, Puja Pandya, Erik Bjorge, Bret Barkelew,
	Prince Agyeman

Add the ability to calculate a list of available
manifest repositories from the contents of the
edkrepo.cfg and the edkrepo_user.cfg files.

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

diff --git a/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py b/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py
index 24ad76a..4bded46 100644
--- a/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py
+++ b/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py
@@ -64,25 +64,10 @@ def pull_all_manifest_repos(edkrepo_cfg, edkrepo_user_cfg, reset_hard=False):
     '''
     cfg_man_repos = []
     user_cfg_man_repos = []
-    conflicts, duplicates = detect_man_repo_conflicts_duplicates(edkrepo_cfg, edkrepo_user_cfg)
-    if not conflicts and not duplicates:
-        cfg_man_repos.extend(edkrepo_cfg.manifest_repo_list)
-        user_cfg_man_repos.extend(edkrepo_user_cfg.manifest_repo_list)
-    elif conflicts:
-        for conflict in conflicts:
-            # In the case of a conflict do not pull conflicting repo
-            print(humble.CONFLICT_NO_CLONE.format(conflict))
-            cfg_man_repos.extend(edkrepo_cfg.manifest_repo_list)
-            cfg_man_repos.remove(conflict)
-            user_cfg_man_repos.extend(edkrepo_user_cfg.manifest_repo_list)
-            user_cfg_man_repos.remove(conflict)
-    elif duplicates:
-        for duplicate in duplicates:
-            # the duplicate needs to be ignored in on of the repo lists so it is
-            # not cloned/pulled twice
-            cfg_man_repos.extend(edkrepo_cfg.manifest_repo_list)
-            user_cfg_man_repos.extend(edkrepo_user_cfg.manifest_repo_list)
-            user_cfg_man_repos.remove(conflict)
+    conflicts = []
+    cfg_man_repos, user_cfg_man_repos, conflicts = list_available_man_repos(edkrepo_cfg, edkrepo_user_cfg)
+    for conflict in conflicts:
+        print(humble.CONFLICT_NO_CLONE.format(conflict))
     for repo in cfg_man_repos:
         pull_single_manifest_repo(edkrepo_cfg.get_manifest_repo_url(repo),
                                   edkrepo_cfg.get_manifest_repo_branch(repo),
@@ -120,3 +105,33 @@ def detect_man_repo_conflicts_duplicates(edkrepo_cfg, edkrepo_user_cfg):
             else:
                 duplicates.append(repo)
     return conflicts, duplicates
+
+def list_available_man_repos(edkrepo_cfg, edkrepo_user_cfg):
+    '''
+    Checks for conflicts/duplicates within all manifest repositories defined in
+    both the edkrepo.cfg and the edkrepo_user.cfg and resturns a list of available
+    manifest_repos for each and a list of conflicting manifest repository entries.
+    '''
+    cfg_man_repos = []
+    user_cfg_man_repos = []
+    conflicts, duplicates = detect_man_repo_conflicts_duplicates(edkrepo_cfg, edkrepo_user_cfg)
+    if not conflicts and not duplicates:
+        cfg_man_repos.extend(edkrepo_cfg.manifest_repo_list)
+        user_cfg_man_repos.extend(edkrepo_user_cfg.manifest_repo_list)
+    elif conflicts:
+        for conflict in conflicts:
+            # In the case of a conflict do not pull conflicting repo
+            cfg_man_repos.extend(edkrepo_cfg.manifest_repo_list)
+            cfg_man_repos.remove(conflict)
+            user_cfg_man_repos.extend(edkrepo_user_cfg.manifest_repo_list)
+            user_cfg_man_repos.remove(conflict)
+    elif duplicates:
+        for duplicate in duplicates:
+            # the duplicate needs to be ignored in on of the repo lists so it is
+            # not cloned/pulled twice
+            cfg_man_repos.extend(edkrepo_cfg.manifest_repo_list)
+            user_cfg_man_repos.extend(edkrepo_user_cfg.manifest_repo_list)
+            user_cfg_man_repos.remove(duplicate)
+    return cfg_man_repos, user_cfg_man_repos, conflicts
+
+
-- 
2.16.2.windows.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [edk2-staging/EdkRepo] [PATCH 5/7] EdkRepo: Add ability to find projects across all manifest repositories
  2020-04-28 21:57 [edk2-staging/EdkRepo] [PATCH 0/7] Support for consuming multiple manifest repositories Ashley E Desimone
                   ` (3 preceding siblings ...)
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 4/7] EdkRepo: Add list_available_manifest_repos() Ashley E Desimone
@ 2020-04-28 21:57 ` Ashley E Desimone
  2020-04-30 21:28   ` Nate DeSimone
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 6/7] EdkRepo: Add ability to determine the source manifest of a workspace Ashley E Desimone
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 7/7] EdkRepo: Add the ability to pull only the global manifest repository for a given workspace Ashley E Desimone
  6 siblings, 1 reply; 17+ messages in thread
From: Ashley E Desimone @ 2020-04-28 21:57 UTC (permalink / raw)
  To: devel
  Cc: Nate DeSimone, Puja Pandya, Erik Bjorge, Bret Barkelew,
	Prince Agyeman

Add find_project_in_all_indicies() to search for and return
a tuple (source repo, source config, path to manifest) if a
matching project is found.

Add find_project_in_single_index() to find the path to a project
within a single manifest repositories index file and return a whether
the specified project was found and if so its path within the manifest
repository.

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

diff --git a/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py b/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py
index 4bded46..9b441ac 100644
--- a/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py
+++ b/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py
@@ -15,11 +15,12 @@ import git
 from git import Repo
 
 import edkrepo.config.config_factory as cfg
-from edkrepo.common.edkrepo_exception import EdkrepoUncommitedChangesException
+from edkrepo.common.edkrepo_exception import EdkrepoUncommitedChangesException, EdkrepoInvalidParametersException
 from edkrepo.common.progress_handler import GitProgressHandler
 import edkrepo.common.workspace_maintenance.humble.manifest_repos_maintenance_humble as humble
 from edkrepo.common.workspace_maintenance.workspace_maintenance import generate_name_for_obsolete_backup
-
+from edkrepo.common.workspace_maintenance.workspace_maintenance import case_insensitive_single_match
+from edkrepo_manifest_parser.edk_manifest import CiIndexXml, ManifestXml
 
 def pull_single_manifest_repo(url, branch, local_path, reset_hard=False):
     '''
@@ -135,3 +136,68 @@ def list_available_man_repos(edkrepo_cfg, edkrepo_user_cfg):
     return cfg_man_repos, user_cfg_man_repos, conflicts
 
 
+def find_project_in_single_index (project, index_file, manifest_dir):
+    '''
+    Finds a project in a single global manifest repositories index file. If found
+    returns (True, path to file) if not returns (False, None)
+    '''
+    global_manifest_path = None
+    try:
+        proj_name = case_insensitive_single_match(project, index_file.project_list)
+    except:
+        proj_name = None
+    if proj_name:
+        ci_index_xml_rel_path = os.path.normpath(index_file.get_project_xml(proj_name))
+        global_manifest_path = os.path.join(manifest_dir, ci_index_xml_rel_path)
+        return True, global_manifest_path
+    else:
+        return False, global_manifest_path
+
+
+def find_project_in_all_indices (project, edkrepo_cfg, edkrepo_user_cfg, except_msg_man_repo, except_msg_not_found, man_repo=None):
+    '''
+    Finds the project in all manifest repositories listed in the edkrepo.efg and
+    edkrepo_user.cfg. If a project with the same name is found uses man_repo to select
+    the correct entry
+    '''
+    cfg_man_repos, user_cfg_man_repos, conflicts = list_available_man_repos(edkrepo_cfg, edkrepo_user_cfg)
+    projects = {}
+    for repo in cfg_man_repos:
+        manifest_dir = edkrepo_cfg.manifest_repo_abs_path(repo)
+        index_file = CiIndexXml(os.path.join(manifest_dir, 'CiIndex.xml'))
+        found, man_path = find_project_in_single_index(project, index_file, manifest_dir)
+        if found:
+            projects[repo] = ('edkrepo_cfg', man_path)
+    for repo in user_cfg_man_repos:
+        manifest_dir = edkrepo_user_cfg.manifest_repo_abs_path(repo)
+        index_file = CiIndexXml(os.path.join(manifest_dir, 'CiIndex.xml'))
+        found, man_path = find_project_in_single_index(project, index_file, manifest_dir)
+        if found:
+            projects[repo] = ('edkrepo_user_cfg', man_path)
+    if len(projects.keys()) == 1:
+        repo = list(projects.keys())[0]
+        return repo, projects[repo][0], projects[repo][1]
+    elif len(projects.keys()) > 1 and man_repo:
+        try:
+            return man_repo, projects[man_repo][0], projects[man_repo][1]
+        except KeyError:
+            raise EdkrepoInvalidParametersException(except_msg_man_repo)
+    elif os.path.isabs(project):
+        return None, None, project
+    elif os.path.isfile(os.path.join(os.getcwd(), project)):
+        return None, None, os.path.join(os.getcwd(), project)
+    elif not os.path.dirname(project):
+        for repo in cfg_man_repos:
+            if (man_repo and (repo == man_repo)) or not man_repo:
+                for dirpath, dirname, filenames in os.walk(edkrepo_cfg.manifest_repo_abs_path(repo)):
+                    if project in filenames:
+                        return repo, 'edkrepo_cfg', os.path.join(dirpath, project)
+        for repo in user_cfg_man_repos:
+            if (man_repo and (repo == man_repo)) or not man_repo:
+                for dirpath, dirname, filenames in os.walk(edkrepo_user_cfg.manifest_repo_abs_path(repo)):
+                    if project in filenames:
+                        return repo, 'edkrepo_user_cfg', os.path.join(dirpath, project)
+
+
+
+
diff --git a/edkrepo/common/workspace_maintenance/workspace_maintenance.py b/edkrepo/common/workspace_maintenance/workspace_maintenance.py
index 6e20d43..ba62f6d 100644
--- a/edkrepo/common/workspace_maintenance/workspace_maintenance.py
+++ b/edkrepo/common/workspace_maintenance/workspace_maintenance.py
@@ -10,6 +10,10 @@
 ''' Contains shared workspace maintenance functions. '''
 
 import os
+import unicodedata
+
+from edkrepo.common.edkrepo_exception import EdkrepoFoundMultipleException, EdkrepoNotFoundException
+from edkrepo.common.humble import GEN_A_NOT_IN_B, GEN_FOUND_MULT_A_IN_B
 
 def generate_name_for_obsolete_backup(absolute_path):
     if not os.path.exists(absolute_path):
@@ -27,4 +31,15 @@ def generate_name_for_obsolete_backup(absolute_path):
         if not os.path.exists(os.path.join(dir_name, unique_name)):
             unique_name_found = True
         index += 1
-    return unique_name
\ No newline at end of file
+    return unique_name
+
+def case_insensitive_equal(str1, str2):
+    return unicodedata.normalize("NFKD", str1.casefold()) == unicodedata.normalize("NFKD", str2.casefold())
+
+def case_insensitive_single_match(str1, str_list):
+    matches = [x for x in str_list if case_insensitive_equal(str1, x)]
+    if len(matches) == 0:
+        raise EdkrepoNotFoundException(GEN_A_NOT_IN_B.format(str1, str_list))
+    elif len(matches) > 1:
+        raise EdkrepoFoundMultipleException(GEN_FOUND_MULT_A_IN_B.format(str1, str_list))
+    return matches[0]
-- 
2.16.2.windows.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [edk2-staging/EdkRepo] [PATCH 6/7] EdkRepo: Add ability to determine the source manifest of a workspace
  2020-04-28 21:57 [edk2-staging/EdkRepo] [PATCH 0/7] Support for consuming multiple manifest repositories Ashley E Desimone
                   ` (4 preceding siblings ...)
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 5/7] EdkRepo: Add ability to find projects across all manifest repositories Ashley E Desimone
@ 2020-04-28 21:57 ` Ashley E Desimone
  2020-04-30 21:28   ` Nate DeSimone
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 7/7] EdkRepo: Add the ability to pull only the global manifest repository for a given workspace Ashley E Desimone
  6 siblings, 1 reply; 17+ messages in thread
From: Ashley E Desimone @ 2020-04-28 21:57 UTC (permalink / raw)
  To: devel
  Cc: Nate DeSimone, Puja Pandya, Erik Bjorge, Bret Barkelew,
	Prince Agyeman

Add find_source_man_repo() to check if for the source manifest
repo is contained in the workspaces project manifest file.
If it is not determine the value and write it to the manifest.

Signed-off-by: Ashley E Desimone <ashley.e.desimone@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Puja Pandya <puja.pandya@intel.com>
Cc: Erik Bjorge <erik.c.bjorge@intel.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Prince Agyeman <prince.agyeman@intel.com>
---
 .../humble/manifest_repos_maintenance_humble.py           |  2 ++
 .../workspace_maintenance/manifest_repos_maintenance.py   | 15 +++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/edkrepo/common/workspace_maintenance/humble/manifest_repos_maintenance_humble.py b/edkrepo/common/workspace_maintenance/humble/manifest_repos_maintenance_humble.py
index e592f19..05e76b1 100644
--- a/edkrepo/common/workspace_maintenance/humble/manifest_repos_maintenance_humble.py
+++ b/edkrepo/common/workspace_maintenance/humble/manifest_repos_maintenance_humble.py
@@ -25,3 +25,5 @@ CONFLICT_NO_CLONE = ('The definition of global manifest repository, {}, '
                      'in the edkrepo_user.cfg does not match the definition in the edkrepo.cfg. '
                      'This global manifest repository will not be downloaded or updated. '
                      'Resolve the conflict and then re-run the failed operation')
+SOURCE_MAN_REPO_NOT_FOUND = 'Could not determine the source global manifest repository for project: {}'
+PROJ_NOT_IN_REPO = 'Project: {} does not exist in any global manifest repository'
\ No newline at end of file
diff --git a/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py b/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py
index 9b441ac..7b3f866 100644
--- a/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py
+++ b/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py
@@ -199,5 +199,20 @@ def find_project_in_all_indices (project, edkrepo_cfg, edkrepo_user_cfg, except_
                         return repo, 'edkrepo_user_cfg', os.path.join(dirpath, project)
 
 
+def find_source_man_repo (project_manifest, edkrepo_cfg, edkrepo_user_cfg):
+    '''
+    Finds the source manifest repo for a given project.
+    '''
+    if project_manifest.general_config.source_man_repo:
+       return project_manifest.general_config.source_man_repo
+    else:
+        src_man_repo, src_config, src_man_path = find_project_in_all_indices(project_manifest.project_info.codename,
+                                                                             edkrepo_cfg,
+                                                                             edkrepo_user_cfg,
+                                                                             humble.PROJ_NOT_IN_REPO.format(project_manifest.project_info.codename),
+                                                                             humble.SOURCE_MAN_REPO_NOT_FOUND.format(project_manifest.project_info.codename),
+                                                                             man_repo=None)
+        project_manifest.write_source_manifest_repo(src_man_repo)
+        return src_man_repo    
 
 
-- 
2.16.2.windows.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [edk2-staging/EdkRepo] [PATCH 7/7] EdkRepo: Add the ability to pull only the global manifest repository for a given workspace.
  2020-04-28 21:57 [edk2-staging/EdkRepo] [PATCH 0/7] Support for consuming multiple manifest repositories Ashley E Desimone
                   ` (5 preceding siblings ...)
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 6/7] EdkRepo: Add ability to determine the source manifest of a workspace Ashley E Desimone
@ 2020-04-28 21:57 ` Ashley E Desimone
  2020-04-30 21:28   ` Nate DeSimone
  6 siblings, 1 reply; 17+ messages in thread
From: Ashley E Desimone @ 2020-04-28 21:57 UTC (permalink / raw)
  To: devel
  Cc: Nate DeSimone, Puja Pandya, Erik Bjorge, Bret Barkelew,
	Prince Agyeman

Add pull_workspace_man_repo() to pull only the global manifest
repository affiliated with a single workspace.

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

diff --git a/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py b/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py
index 7b3f866..7a7f946 100644
--- a/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py
+++ b/edkrepo/common/workspace_maintenance/manifest_repos_maintenance.py
@@ -199,7 +199,7 @@ def find_project_in_all_indices (project, edkrepo_cfg, edkrepo_user_cfg, except_
                         return repo, 'edkrepo_user_cfg', os.path.join(dirpath, project)
 
 
-def find_source_man_repo (project_manifest, edkrepo_cfg, edkrepo_user_cfg):
+def find_source_man_repo(project_manifest, edkrepo_cfg, edkrepo_user_cfg, man_repo=None):
     '''
     Finds the source manifest repo for a given project.
     '''
@@ -213,6 +213,25 @@ def find_source_man_repo (project_manifest, edkrepo_cfg, edkrepo_user_cfg):
                                                                              humble.SOURCE_MAN_REPO_NOT_FOUND.format(project_manifest.project_info.codename),
                                                                              man_repo=None)
         project_manifest.write_source_manifest_repo(src_man_repo)
-        return src_man_repo    
+        return src_man_repo
+
+def pull_workspace_man_repo(project_manifest, edkrepo_cfg, edkrepo_user_cfg, man_repo=None, reset_hard=False):
+    '''
+    Pulls only the global manifest repo for the current workspace.
+    '''
+    src_man_repo = find_source_man_repo(project_manifest, edkrepo_cfg, edkrepo_user_cfg, man_repo)
+    config_repos, user_config_repos, conflicts = list_available_man_repos(edkrepo_cfg, edkrepo_user_cfg)
+    if src_man_repo in config_repos:
+        pull_single_manifest_repo(edkrepo_cfg.get_manifest_repo_url(src_man_repo),
+                                  edkrepo_cfg.get_manifest_repo_branch(src_man_repo),
+                                  edkrepo_cfg.get_manifest_repo_local_path(src_man_repo),
+                                  reset_hard)
+    elif src_man_repo in user_config_repos:
+        pull_single_manifest_repo(edkrepo_user_cfg.get_manifest_repo_url(src_man_repo),
+                                  edkrepo_user_cfg.get_manifest_repo_branch(src_man_repo),
+                                  edkrepo_user_cfg.get_manifest_repo_local_path(src_man_repo),
+                                  reset_hard)
+    elif src_man_repo in conflicts:
+        raise EdkrepoInvalidParametersException(humble.CONFLICT_NO_CLONE.format(src_man_repo))
 
 
-- 
2.16.2.windows.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] [edk2-staging/EdkRepo] [PATCH 1/7] EdkRepo: Add check for conflicting/duplicated manifest repo definitions
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 1/7] EdkRepo: Add check for conflicting/duplicated manifest repo definitions Ashley E Desimone
@ 2020-04-30 21:28   ` Nate DeSimone
  0 siblings, 0 replies; 17+ messages in thread
From: Nate DeSimone @ 2020-04-30 21:28 UTC (permalink / raw)
  To: devel@edk2.groups.io, Desimone, Ashley E
  Cc: Pandya, Puja, Bjorge, Erik C, Bret Barkelew, Agyeman, Prince

Hi Ashley,

Please see comments inline.

Thanks,
Nate

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ashley E
> Desimone
> Sent: Tuesday, April 28, 2020 2:57 PM
> To: devel@edk2.groups.io
> Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Pandya, Puja
> <puja.pandya@intel.com>; Bjorge, Erik C <erik.c.bjorge@intel.com>; Bret
> Barkelew <Bret.Barkelew@microsoft.com>; Agyeman, Prince
> <prince.agyeman@intel.com>
> Subject: [edk2-devel] [edk2-staging/EdkRepo] [PATCH 1/7] EdkRepo: Add
> check for conflicting/duplicated manifest repo definitions
> 
> Add a functions to check for conflicting or duplicated manifest repository
> definitions in the edkrepo.cfg and the edkrepo_user.cfg files. Two manifest
> repositories definitions are considered conflicting if they have the same
> name and at least one of URL, branch or local path differ. Two manifest
> repository definitions are considered duplicates if the name, URL, branch and
> local path are the same.
> 
> Signed-off-by: Ashley E Desimone <ashley.e.desimone@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Puja Pandya <puja.pandya@intel.com>
> Cc: Erik Bjorge <erik.c.bjorge@intel.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Cc: Prince Agyeman <prince.agyeman@intel.com>
> ---
>  .../manifest_repos_maintenance.py                  | 25
> ++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git
> a/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> b/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> index 6e26d4f..ad6ddbc 100644
> ---
> a/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> +++
> b/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> @@ -57,3 +57,28 @@ def pull_single_manifest_repo(url, branch, local_path,
> reset_hard=False):
>              print (humble.CLONE_SINGLE_MAN_REPO.format(local_path, url))
>              repo = Repo.clone_from(url, local_path,
> progress=GitProgressHandler(), branch=branch)
> 
> +def detect_man_repo_conflicts_duplicates(edkrepo_cfg,
> edkrepo_user_cfg):

All the pre-existing EdkRepo code tends to spell out "manifest" please rename this function to "detect_manifest_repo_conflicts_duplicates()"

> +    '''
> +    Determines whether there is are conflicting or duplicated manifest
> +    repositories listed in the edkrepo.cfg and the edkrepo_user.cfg.
> +    '''
> +    conflicts = []
> +    duplicates = []
> +    if not edkrepo_user_cfg.manifest_repo_list:
> +        return conflicts, duplicates
> +    else:
> +        config_repos = set(edkrepo_cfg.manifest_repo_list)
> +        user_cfg_repos = set(edkrepo_user_cfg.manifest_repo_list)
> +    if config_repos.isdisjoint(user_cfg_repos):
> +        return conflicts, duplicates
> +    else:
> +        for repo in config_repos.intersection(user_cfg_repos):
> +            if edkrepo_cfg.get_manifest_repo_url(repo) !=
> edkrepo_user_cfg.get_manifest_repo_url(repo):
> +                conflicts.append(repo)
> +            elif edkrepo_cfg.get_manifest_repo_branch(repo) !=
> edkrepo_user_cfg.get_manifest_repo_branch(repo):
> +                conflicts.append(repo)
> +            elif edkrepo_cfg.get_manifest_repo_local_path(repo) !=
> edkrepo_user_cfg.get_manifest_repo_local_path(repo):
> +                conflicts.append(repo)
> +            else:
> +                duplicates.append(repo)
> +    return conflicts, duplicates
> --
> 2.16.2.windows.1
> 
> 
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] [edk2-staging/EdkRepo] [PATCH 2/7] EdkRepo: Add downloading all available manifest repositories
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 2/7] EdkRepo: Add downloading all available manifest repositories Ashley E Desimone
@ 2020-04-30 21:28   ` Nate DeSimone
  0 siblings, 0 replies; 17+ messages in thread
From: Nate DeSimone @ 2020-04-30 21:28 UTC (permalink / raw)
  To: devel@edk2.groups.io, Desimone, Ashley E
  Cc: Pandya, Puja, Bjorge, Erik C, Bret Barkelew, Agyeman, Prince

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

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ashley E
> Desimone
> Sent: Tuesday, April 28, 2020 2:57 PM
> To: devel@edk2.groups.io
> Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Pandya, Puja
> <puja.pandya@intel.com>; Bjorge, Erik C <erik.c.bjorge@intel.com>; Bret
> Barkelew <Bret.Barkelew@microsoft.com>; Agyeman, Prince
> <prince.agyeman@intel.com>
> Subject: [edk2-devel] [edk2-staging/EdkRepo] [PATCH 2/7] EdkRepo: Add
> downloading all available manifest repositories
> 
> Add a function that will download all available manifest repositories defined
> in either the edkrepo.cfg or the edkrepo_user.cfg
> 
> Signed-off-by: Ashley E Desimone <ashley.e.desimone@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Puja Pandya <puja.pandya@intel.com>
> Cc: Erik Bjorge <erik.c.bjorge@intel.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Cc: Prince Agyeman <prince.agyeman@intel.com>
> ---
>  .../humble/manifest_repos_maintenance_humble.py    |  4 +++
>  .../manifest_repos_maintenance.py                  | 38
> ++++++++++++++++++++++
>  2 files changed, 42 insertions(+)
> 
> diff --git
> a/edkrepo/common/workspace_maintenance/humble/manifest_repos_ma
> intenance_humble.py
> b/edkrepo/common/workspace_maintenance/humble/manifest_repos_ma
> intenance_humble.py
> index 440fd8a..e592f19 100644
> ---
> a/edkrepo/common/workspace_maintenance/humble/manifest_repos_ma
> intenance_humble.py
> +++
> b/edkrepo/common/workspace_maintenance/humble/manifest_repos_ma
> inten
> +++ ance_humble.py
> @@ -21,3 +21,7 @@ SINGLE_MAN_REPO_NOT_CFG_BRANCH = ('The
> current active branch, {}, is not the '
>                                    'specified branch for global manifst repository: {}')
> SINGLE_MAN_REPO_CHECKOUT_CFG_BRANCH = 'Checking out the
> specified branch: {} prior to syncing'
>  SINGLE_MAN_REPO_MOVED = '{}{}WARNING:{}{} The global manifest
> repository has moved. Backing up previous global manifest repository to:
> {{}}{}\n'.format(Style.BRIGHT, Fore.RED, Style.RESET_ALL, Fore.RED,
> Style.RESET_ALL)
> +CONFLICT_NO_CLONE = ('The definition of global manifest repository, {}, '
> +                     'in the edkrepo_user.cfg does not match the definition in the
> edkrepo.cfg. '
> +                     'This global manifest repository will not be downloaded or
> updated. '
> +                     'Resolve the conflict and then re-run the failed
> +operation')
> diff --git
> a/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> b/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> index ad6ddbc..24ad76a 100644
> ---
> a/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> +++
> b/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> @@ -57,6 +57,44 @@ def pull_single_manifest_repo(url, branch, local_path,
> reset_hard=False):
>              print (humble.CLONE_SINGLE_MAN_REPO.format(local_path, url))
>              repo = Repo.clone_from(url, local_path,
> progress=GitProgressHandler(), branch=branch)
> 
> +def pull_all_manifest_repos(edkrepo_cfg, edkrepo_user_cfg,
> reset_hard=False):
> +    '''
> +    Clones or syncs all global manifest repositories defined in both the
> +    edkrepo_cfg and the edkrepo_user.cfg)
> +    '''
> +    cfg_man_repos = []
> +    user_cfg_man_repos = []
> +    conflicts, duplicates =
> detect_man_repo_conflicts_duplicates(edkrepo_cfg, edkrepo_user_cfg)
> +    if not conflicts and not duplicates:
> +        cfg_man_repos.extend(edkrepo_cfg.manifest_repo_list)
> +        user_cfg_man_repos.extend(edkrepo_user_cfg.manifest_repo_list)
> +    elif conflicts:
> +        for conflict in conflicts:
> +            # In the case of a conflict do not pull conflicting repo
> +            print(humble.CONFLICT_NO_CLONE.format(conflict))
> +            cfg_man_repos.extend(edkrepo_cfg.manifest_repo_list)
> +            cfg_man_repos.remove(conflict)
> +            user_cfg_man_repos.extend(edkrepo_user_cfg.manifest_repo_list)
> +            user_cfg_man_repos.remove(conflict)
> +    elif duplicates:
> +        for duplicate in duplicates:
> +            # the duplicate needs to be ignored in on of the repo lists so it is
> +            # not cloned/pulled twice
> +            cfg_man_repos.extend(edkrepo_cfg.manifest_repo_list)
> +            user_cfg_man_repos.extend(edkrepo_user_cfg.manifest_repo_list)
> +            user_cfg_man_repos.remove(conflict)
> +    for repo in cfg_man_repos:
> +        pull_single_manifest_repo(edkrepo_cfg.get_manifest_repo_url(repo),
> +                                  edkrepo_cfg.get_manifest_repo_branch(repo),
> +                                  edkrepo_cfg.get_manifest_repo_local_path(repo),
> +                                  reset_hard)
> +    for repo in user_cfg_man_repos:
> +
> pull_single_manifest_repo(edkrepo_user_cfg.get_manifest_repo_url(repo)
> ,
> +                                  edkrepo_user_cfg.get_manifest_repo_branch(repo),
> +                                  edkrepo_user_cfg.get_manifest_repo_local_path(repo),
> +                                  reset_hard)
> +
> +
>  def detect_man_repo_conflicts_duplicates(edkrepo_cfg,
> edkrepo_user_cfg):
>      '''
>      Determines whether there is are conflicting or duplicated manifest
> --
> 2.16.2.windows.1
> 
> 
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-staging/EdkRepo] [PATCH 3/7] EdkRepo: Add optional field to edkrepo_manifst to track the source manifest repo
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 3/7] EdkRepo: Add optional field to edkrepo_manifst to track the source manifest repo Ashley E Desimone
@ 2020-04-30 21:28   ` Nate DeSimone
  0 siblings, 0 replies; 17+ messages in thread
From: Nate DeSimone @ 2020-04-30 21:28 UTC (permalink / raw)
  To: Desimone, Ashley E, devel@edk2.groups.io
  Cc: Pandya, Puja, Bjorge, Erik C, Bret Barkelew, Agyeman, Prince

Hi Ashley,

Please see comments inline.

Thanks,
Nate

> -----Original Message-----
> From: Desimone, Ashley E <ashley.e.desimone@intel.com>
> Sent: Tuesday, April 28, 2020 2:57 PM
> To: devel@edk2.groups.io
> Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Pandya, Puja
> <puja.pandya@intel.com>; Bjorge, Erik C <erik.c.bjorge@intel.com>; Bret
> Barkelew <Bret.Barkelew@microsoft.com>; Agyeman, Prince
> <prince.agyeman@intel.com>
> Subject: [edk2-staging/EdkRepo] [PATCH 3/7] EdkRepo: Add optional field to
> edkrepo_manifst to track the source manifest repo
> 
> Add the SourceManifestRepository to the edkrepo manfiest general config
> for use by edkrepo to track the source manifest repository for the
> workspace.
> 
> Signed-off-by: Ashley E Desimone <ashley.e.desimone@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Puja Pandya <puja.pandya@intel.com>
> Cc: Erik Bjorge <erik.c.bjorge@intel.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Cc: Prince Agyeman <prince.agyeman@intel.com>
> ---
>  edkrepo_manifest_parser/edk_manifest.py | 26
> ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/edkrepo_manifest_parser/edk_manifest.py
> b/edkrepo_manifest_parser/edk_manifest.py
> index 080448f..dcf9c29 100644
> --- a/edkrepo_manifest_parser/edk_manifest.py
> +++ b/edkrepo_manifest_parser/edk_manifest.py
> @@ -21,7 +21,7 @@ import copy
>  # All the namedtuple data structures that consumers of this module will
> need.
>  #
>  ProjectInfo = namedtuple('ProjectInfo', ['codename', 'description',
> 'dev_leads', 'reviewers', 'org', 'short_name']) -GeneralConfig =
> namedtuple('GeneralConfig', ['default_combo', 'current_combo',
> 'pin_path'])
> +GeneralConfig = namedtuple('GeneralConfig', ['default_combo',
> +'current_combo', 'pin_path', 'source_man_repo'])

The variable name "source_man_repo" is a bit confusing. My initial thought when I saw that name was that it was something about documentation. Could you please rename to "source_manifest_repo"?

>  RemoteRepo = namedtuple('RemoteRepo', ['name', 'url'])  RepoHook =
> namedtuple('RepoHook', ['source', 'dest_path', 'dest_file', 'remote_url'])
> Combination = namedtuple('Combination', ['name', 'description']) @@ -406,6
> +406,24 @@ class ManifestXml(BaseXmlHelper):
>          self._tree.write(filename)
>          self.__general_config.current_combo = combo_name
> 
> +    def write_source_manifest_repo(self, manifest_repo, filename=None):
> +        '''
> +        Writes the name of the source manifest repository to the
> +        general config sections of the manifest file.
> +        '''
> +        if filename is None:
> +            filename = self._fileref
> +        subroot = self._tree.find('GeneralConfig')
> +        if subroot is None:
> +            raise KeyError(GENERAL_CONFIG_MISSING_ERROR)
> +
> +        element = subroot.find('SourceManifestRepository')
> +        if element is None:
> +            element = ET.SubElement(subroot, 'SourceManifestRepository')
> +        element.attrib['manifest_repo'] = manifest_repo
> +        self._tree.write(filename)
> +        self.__general_config.source_man_repo = manifest_repo
> +
>      def generate_pin_xml(self, description, combo_name, repo_source_list,
> filename=None):
> 
>          pin_tree = ET.ElementTree(ET.Element('Pin')) @@ -605,10 +623,14 @@
> class _GeneralConfig():
>              self.curr_combo =
> element.find('CurrentClonedCombo').attrib['combination']
>          except:
>              self.curr_combo = None
> +        try:
> +            self.source_man_repo =
> element.find('SourceManifestRepository').attrib['manifest_repo']
> +        except:
> +            self.source_man_repo = None
> 
>      @property
>      def tuple(self):
> -        return GeneralConfig(self.default_combo, self.curr_combo,
> self.pin_path)
> +        return GeneralConfig(self.default_combo, self.curr_combo,
> + self.pin_path, self.source_man_repo)
> 
>  class _RemoteRepo():
>      def __init__(self, element):
> --
> 2.16.2.windows.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-staging/EdkRepo] [PATCH 4/7] EdkRepo: Add list_available_manifest_repos()
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 4/7] EdkRepo: Add list_available_manifest_repos() Ashley E Desimone
@ 2020-04-30 21:28   ` Nate DeSimone
  0 siblings, 0 replies; 17+ messages in thread
From: Nate DeSimone @ 2020-04-30 21:28 UTC (permalink / raw)
  To: Desimone, Ashley E, devel@edk2.groups.io
  Cc: Pandya, Puja, Bjorge, Erik C, Bret Barkelew, Agyeman, Prince

Hi Ashley,

Please see comments inline.

Thanks,
Nate

> -----Original Message-----
> From: Desimone, Ashley E <ashley.e.desimone@intel.com>
> Sent: Tuesday, April 28, 2020 2:57 PM
> To: devel@edk2.groups.io
> Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Pandya, Puja
> <puja.pandya@intel.com>; Bjorge, Erik C <erik.c.bjorge@intel.com>; Bret
> Barkelew <Bret.Barkelew@microsoft.com>; Agyeman, Prince
> <prince.agyeman@intel.com>
> Subject: [edk2-staging/EdkRepo] [PATCH 4/7] EdkRepo: Add
> list_available_manifest_repos()
> 
> Add the ability to calculate a list of available manifest repositories from the
> contents of the edkrepo.cfg and the edkrepo_user.cfg files.
> 
> Signed-off-by: Ashley E Desimone <ashley.e.desimone@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Puja Pandya <puja.pandya@intel.com>
> Cc: Erik Bjorge <erik.c.bjorge@intel.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Cc: Prince Agyeman <prince.agyeman@intel.com>
> ---
>  .../manifest_repos_maintenance.py                  | 53 ++++++++++++++--------
>  1 file changed, 34 insertions(+), 19 deletions(-)
> 
> diff --git
> a/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> b/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> index 24ad76a..4bded46 100644
> ---
> a/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> +++
> b/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> @@ -64,25 +64,10 @@ def pull_all_manifest_repos(edkrepo_cfg,
> edkrepo_user_cfg, reset_hard=False):
>      '''
>      cfg_man_repos = []
>      user_cfg_man_repos = []
> -    conflicts, duplicates =
> detect_man_repo_conflicts_duplicates(edkrepo_cfg, edkrepo_user_cfg)
> -    if not conflicts and not duplicates:
> -        cfg_man_repos.extend(edkrepo_cfg.manifest_repo_list)
> -        user_cfg_man_repos.extend(edkrepo_user_cfg.manifest_repo_list)
> -    elif conflicts:
> -        for conflict in conflicts:
> -            # In the case of a conflict do not pull conflicting repo
> -            print(humble.CONFLICT_NO_CLONE.format(conflict))
> -            cfg_man_repos.extend(edkrepo_cfg.manifest_repo_list)
> -            cfg_man_repos.remove(conflict)
> -            user_cfg_man_repos.extend(edkrepo_user_cfg.manifest_repo_list)
> -            user_cfg_man_repos.remove(conflict)
> -    elif duplicates:
> -        for duplicate in duplicates:
> -            # the duplicate needs to be ignored in on of the repo lists so it is
> -            # not cloned/pulled twice
> -            cfg_man_repos.extend(edkrepo_cfg.manifest_repo_list)
> -            user_cfg_man_repos.extend(edkrepo_user_cfg.manifest_repo_list)
> -            user_cfg_man_repos.remove(conflict)
> +    conflicts = []
> +    cfg_man_repos, user_cfg_man_repos, conflicts =
> list_available_man_repos(edkrepo_cfg, edkrepo_user_cfg)
> +    for conflict in conflicts:
> +        print(humble.CONFLICT_NO_CLONE.format(conflict))
>      for repo in cfg_man_repos:
>          pull_single_manifest_repo(edkrepo_cfg.get_manifest_repo_url(repo),
>                                    edkrepo_cfg.get_manifest_repo_branch(repo),
> @@ -120,3 +105,33 @@ def
> detect_man_repo_conflicts_duplicates(edkrepo_cfg, edkrepo_user_cfg):
>              else:
>                  duplicates.append(repo)
>      return conflicts, duplicates
> +
> +def list_available_man_repos(edkrepo_cfg, edkrepo_user_cfg):

Your commit message says that you are adding "list_available_manifest_repos()" but in actuality you are not. You are adding "list_available_man_repos()" Given that all the pre-existing EdkRepo code tends to spell out "manifest" please rename this function to "list_available_manifest_repos()"

> +    '''
> +    Checks for conflicts/duplicates within all manifest repositories defined in
> +    both the edkrepo.cfg and the edkrepo_user.cfg and resturns a list of
> available
> +    manifest_repos for each and a list of conflicting manifest repository
> entries.
> +    '''
> +    cfg_man_repos = []
> +    user_cfg_man_repos = []
> +    conflicts, duplicates =
> detect_man_repo_conflicts_duplicates(edkrepo_cfg, edkrepo_user_cfg)
> +    if not conflicts and not duplicates:
> +        cfg_man_repos.extend(edkrepo_cfg.manifest_repo_list)
> +        user_cfg_man_repos.extend(edkrepo_user_cfg.manifest_repo_list)
> +    elif conflicts:
> +        for conflict in conflicts:
> +            # In the case of a conflict do not pull conflicting repo
> +            cfg_man_repos.extend(edkrepo_cfg.manifest_repo_list)
> +            cfg_man_repos.remove(conflict)
> +            user_cfg_man_repos.extend(edkrepo_user_cfg.manifest_repo_list)
> +            user_cfg_man_repos.remove(conflict)
> +    elif duplicates:
> +        for duplicate in duplicates:
> +            # the duplicate needs to be ignored in on of the repo lists so it is
> +            # not cloned/pulled twice
> +            cfg_man_repos.extend(edkrepo_cfg.manifest_repo_list)
> +            user_cfg_man_repos.extend(edkrepo_user_cfg.manifest_repo_list)
> +            user_cfg_man_repos.remove(duplicate)
> +    return cfg_man_repos, user_cfg_man_repos, conflicts
> +
> +
> --
> 2.16.2.windows.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-staging/EdkRepo] [PATCH 5/7] EdkRepo: Add ability to find projects across all manifest repositories
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 5/7] EdkRepo: Add ability to find projects across all manifest repositories Ashley E Desimone
@ 2020-04-30 21:28   ` Nate DeSimone
  2020-04-30 21:40     ` Bjorge, Erik C
  0 siblings, 1 reply; 17+ messages in thread
From: Nate DeSimone @ 2020-04-30 21:28 UTC (permalink / raw)
  To: Desimone, Ashley E, devel@edk2.groups.io
  Cc: Pandya, Puja, Bjorge, Erik C, Bret Barkelew, Agyeman, Prince

Hi Ashley,

Please see comments inline.

Thanks,
Nate

> -----Original Message-----
> From: Desimone, Ashley E <ashley.e.desimone@intel.com>
> Sent: Tuesday, April 28, 2020 2:57 PM
> To: devel@edk2.groups.io
> Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Pandya, Puja
> <puja.pandya@intel.com>; Bjorge, Erik C <erik.c.bjorge@intel.com>; Bret
> Barkelew <Bret.Barkelew@microsoft.com>; Agyeman, Prince
> <prince.agyeman@intel.com>
> Subject: [edk2-staging/EdkRepo] [PATCH 5/7] EdkRepo: Add ability to find
> projects across all manifest repositories
> 
> Add find_project_in_all_indicies() to search for and return a tuple (source
> repo, source config, path to manifest) if a matching project is found.
> 
> Add find_project_in_single_index() to find the path to a project within a
> single manifest repositories index file and return a whether the specified
> project was found and if so its path within the manifest repository.
> 
> Signed-off-by: Ashley E Desimone <ashley.e.desimone@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Puja Pandya <puja.pandya@intel.com>
> Cc: Erik Bjorge <erik.c.bjorge@intel.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Cc: Prince Agyeman <prince.agyeman@intel.com>
> ---
>  .../manifest_repos_maintenance.py                  | 70
> +++++++++++++++++++++-
>  .../workspace_maintenance/workspace_maintenance.py | 17 +++++-
>  2 files changed, 84 insertions(+), 3 deletions(-)
> 
> diff --git
> a/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> b/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> index 4bded46..9b441ac 100644
> ---
> a/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> +++
> b/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> @@ -15,11 +15,12 @@ import git
>  from git import Repo
> 
>  import edkrepo.config.config_factory as cfg -from
> edkrepo.common.edkrepo_exception import
> EdkrepoUncommitedChangesException
> +from edkrepo.common.edkrepo_exception import
> +EdkrepoUncommitedChangesException,
> EdkrepoInvalidParametersException
>  from edkrepo.common.progress_handler import GitProgressHandler
> import
> edkrepo.common.workspace_maintenance.humble.manifest_repos_maint
> enance_humble as humble  from
> edkrepo.common.workspace_maintenance.workspace_maintenance import
> generate_name_for_obsolete_backup
> -
> +from edkrepo.common.workspace_maintenance.workspace_maintenance
> import
> +case_insensitive_single_match from
> edkrepo_manifest_parser.edk_manifest
> +import CiIndexXml, ManifestXml
> 
>  def pull_single_manifest_repo(url, branch, local_path, reset_hard=False):
>      '''
> @@ -135,3 +136,68 @@ def list_available_man_repos(edkrepo_cfg,
> edkrepo_user_cfg):
>      return cfg_man_repos, user_cfg_man_repos, conflicts
> 
> 
> +def find_project_in_single_index (project, index_file, manifest_dir):
> +    '''
> +    Finds a project in a single global manifest repositories index file. If found
> +    returns (True, path to file) if not returns (False, None)
> +    '''
> +    global_manifest_path = None
> +    try:
> +        proj_name = case_insensitive_single_match(project,
> index_file.project_list)
> +    except:
> +        proj_name = None
> +    if proj_name:
> +        ci_index_xml_rel_path =
> os.path.normpath(index_file.get_project_xml(proj_name))
> +        global_manifest_path = os.path.join(manifest_dir,
> ci_index_xml_rel_path)
> +        return True, global_manifest_path
> +    else:
> +        return False, global_manifest_path
> +
> +
> +def find_project_in_all_indices (project, edkrepo_cfg, edkrepo_user_cfg,
> except_msg_man_repo, except_msg_not_found, man_repo=None):
> +    '''
> +    Finds the project in all manifest repositories listed in the edkrepo.efg and
> +    edkrepo_user.cfg. If a project with the same name is found uses
> man_repo to select
> +    the correct entry
> +    '''
> +    cfg_man_repos, user_cfg_man_repos, conflicts =
> list_available_man_repos(edkrepo_cfg, edkrepo_user_cfg)
> +    projects = {}
> +    for repo in cfg_man_repos:
> +        manifest_dir = edkrepo_cfg.manifest_repo_abs_path(repo)
> +        index_file = CiIndexXml(os.path.join(manifest_dir, 'CiIndex.xml'))

The magic string 'CiIndex.xml' is repeated several times. It would be good to have something like humble.py that stores the magic string, so that if we ever change it that can be done quickly.

> +        found, man_path = find_project_in_single_index(project, index_file,
> manifest_dir)
> +        if found:
> +            projects[repo] = ('edkrepo_cfg', man_path)
> +    for repo in user_cfg_man_repos:
> +        manifest_dir = edkrepo_user_cfg.manifest_repo_abs_path(repo)
> +        index_file = CiIndexXml(os.path.join(manifest_dir, 'CiIndex.xml'))
> +        found, man_path = find_project_in_single_index(project, index_file,
> manifest_dir)
> +        if found:
> +            projects[repo] = ('edkrepo_user_cfg', man_path)
> +    if len(projects.keys()) == 1:
> +        repo = list(projects.keys())[0]
> +        return repo, projects[repo][0], projects[repo][1]
> +    elif len(projects.keys()) > 1 and man_repo:
> +        try:
> +            return man_repo, projects[man_repo][0], projects[man_repo][1]
> +        except KeyError:
> +            raise EdkrepoInvalidParametersException(except_msg_man_repo)
> +    elif os.path.isabs(project):
> +        return None, None, project
> +    elif os.path.isfile(os.path.join(os.getcwd(), project)):
> +        return None, None, os.path.join(os.getcwd(), project)
> +    elif not os.path.dirname(project):
> +        for repo in cfg_man_repos:
> +            if (man_repo and (repo == man_repo)) or not man_repo:
> +                for dirpath, dirname, filenames in
> os.walk(edkrepo_cfg.manifest_repo_abs_path(repo)):
> +                    if project in filenames:
> +                        return repo, 'edkrepo_cfg', os.path.join(dirpath, project)
> +        for repo in user_cfg_man_repos:
> +            if (man_repo and (repo == man_repo)) or not man_repo:
> +                for dirpath, dirname, filenames in
> os.walk(edkrepo_user_cfg.manifest_repo_abs_path(repo)):
> +                    if project in filenames:
> +                        return repo, 'edkrepo_user_cfg',
> +os.path.join(dirpath, project)

If man_repo is None, and the project name exists in more than 1 manifest repository, then you will reach the end of this function and return None. It seems like that should be a raised exception instead, telling the user that they need to specific a specific manifest repository to disambiguate.

> +
> +
> +
> +
> diff --git
> a/edkrepo/common/workspace_maintenance/workspace_maintenance.py
> b/edkrepo/common/workspace_maintenance/workspace_maintenance.py
> index 6e20d43..ba62f6d 100644
> ---
> a/edkrepo/common/workspace_maintenance/workspace_maintenance.py
> +++
> b/edkrepo/common/workspace_maintenance/workspace_maintenance.py
> @@ -10,6 +10,10 @@
>  ''' Contains shared workspace maintenance functions. '''
> 
>  import os
> +import unicodedata
> +
> +from edkrepo.common.edkrepo_exception import
> +EdkrepoFoundMultipleException, EdkrepoNotFoundException from
> +edkrepo.common.humble import GEN_A_NOT_IN_B,
> GEN_FOUND_MULT_A_IN_B
> 
>  def generate_name_for_obsolete_backup(absolute_path):
>      if not os.path.exists(absolute_path):
> @@ -27,4 +31,15 @@ def
> generate_name_for_obsolete_backup(absolute_path):
>          if not os.path.exists(os.path.join(dir_name, unique_name)):
>              unique_name_found = True
>          index += 1
> -    return unique_name
> \ No newline at end of file
> +    return unique_name
> +
> +def case_insensitive_equal(str1, str2):
> +    return unicodedata.normalize("NFKD", str1.casefold()) ==
> +unicodedata.normalize("NFKD", str2.casefold())
> +
> +def case_insensitive_single_match(str1, str_list):
> +    matches = [x for x in str_list if case_insensitive_equal(str1, x)]
> +    if len(matches) == 0:
> +        raise EdkrepoNotFoundException(GEN_A_NOT_IN_B.format(str1,
> str_list))
> +    elif len(matches) > 1:
> +        raise
> EdkrepoFoundMultipleException(GEN_FOUND_MULT_A_IN_B.format(str1,
> str_list))
> +    return matches[0]
> --
> 2.16.2.windows.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-staging/EdkRepo] [PATCH 6/7] EdkRepo: Add ability to determine the source manifest of a workspace
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 6/7] EdkRepo: Add ability to determine the source manifest of a workspace Ashley E Desimone
@ 2020-04-30 21:28   ` Nate DeSimone
  0 siblings, 0 replies; 17+ messages in thread
From: Nate DeSimone @ 2020-04-30 21:28 UTC (permalink / raw)
  To: Desimone, Ashley E, devel@edk2.groups.io
  Cc: Pandya, Puja, Bjorge, Erik C, Bret Barkelew, Agyeman, Prince

Hi Ashley,

Please see comments inline.

Thanks,
Nate

> -----Original Message-----
> From: Desimone, Ashley E <ashley.e.desimone@intel.com>
> Sent: Tuesday, April 28, 2020 2:57 PM
> To: devel@edk2.groups.io
> Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Pandya, Puja
> <puja.pandya@intel.com>; Bjorge, Erik C <erik.c.bjorge@intel.com>; Bret
> Barkelew <Bret.Barkelew@microsoft.com>; Agyeman, Prince
> <prince.agyeman@intel.com>
> Subject: [edk2-staging/EdkRepo] [PATCH 6/7] EdkRepo: Add ability to
> determine the source manifest of a workspace
> 
> Add find_source_man_repo() to check if for the source manifest repo is
> contained in the workspaces project manifest file.
> If it is not determine the value and write it to the manifest.
> 
> Signed-off-by: Ashley E Desimone <ashley.e.desimone@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Puja Pandya <puja.pandya@intel.com>
> Cc: Erik Bjorge <erik.c.bjorge@intel.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Cc: Prince Agyeman <prince.agyeman@intel.com>
> ---
>  .../humble/manifest_repos_maintenance_humble.py           |  2 ++
>  .../workspace_maintenance/manifest_repos_maintenance.py   | 15
> +++++++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git
> a/edkrepo/common/workspace_maintenance/humble/manifest_repos_ma
> intenance_humble.py
> b/edkrepo/common/workspace_maintenance/humble/manifest_repos_ma
> intenance_humble.py
> index e592f19..05e76b1 100644
> ---
> a/edkrepo/common/workspace_maintenance/humble/manifest_repos_ma
> intenance_humble.py
> +++
> b/edkrepo/common/workspace_maintenance/humble/manifest_repos_ma
> inten
> +++ ance_humble.py
> @@ -25,3 +25,5 @@ CONFLICT_NO_CLONE = ('The definition of global
> manifest repository, {}, '
>                       'in the edkrepo_user.cfg does not match the definition in the
> edkrepo.cfg. '
>                       'This global manifest repository will not be downloaded or
> updated. '
>                       'Resolve the conflict and then re-run the failed operation')
> +SOURCE_MAN_REPO_NOT_FOUND = 'Could not determine the source
> global manifest repository for project: {}'
> +PROJ_NOT_IN_REPO = 'Project: {} does not exist in any global manifest
> repository'
> \ No newline at end of file
> diff --git
> a/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> b/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> index 9b441ac..7b3f866 100644
> ---
> a/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> +++
> b/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> @@ -199,5 +199,20 @@ def find_project_in_all_indices (project,
> edkrepo_cfg, edkrepo_user_cfg, except_
>                          return repo, 'edkrepo_user_cfg', os.path.join(dirpath, project)
> 
> 
> +def find_source_man_repo (project_manifest, edkrepo_cfg,
> edkrepo_user_cfg):

All the pre-existing EdkRepo code tends to spell out "manifest" please rename this function to "find_source_manifest_repo()"

> +    '''
> +    Finds the source manifest repo for a given project.
> +    '''
> +    if project_manifest.general_config.source_man_repo:
> +       return project_manifest.general_config.source_man_repo
> +    else:
> +        src_man_repo, src_config, src_man_path =
> find_project_in_all_indices(project_manifest.project_info.codename,
> +                                                                             edkrepo_cfg,
> +                                                                             edkrepo_user_cfg,
> +
> humble.PROJ_NOT_IN_REPO.format(project_manifest.project_info.codena
> me),
> +
> humble.SOURCE_MAN_REPO_NOT_FOUND.format(project_manifest.proje
> ct_info.codename),
> +                                                                             man_repo=None)
> +        project_manifest.write_source_manifest_repo(src_man_repo)
> +        return src_man_repo
> 
> 
> --
> 2.16.2.windows.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-staging/EdkRepo] [PATCH 7/7] EdkRepo: Add the ability to pull only the global manifest repository for a given workspace.
  2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 7/7] EdkRepo: Add the ability to pull only the global manifest repository for a given workspace Ashley E Desimone
@ 2020-04-30 21:28   ` Nate DeSimone
  0 siblings, 0 replies; 17+ messages in thread
From: Nate DeSimone @ 2020-04-30 21:28 UTC (permalink / raw)
  To: Desimone, Ashley E, devel@edk2.groups.io
  Cc: Pandya, Puja, Bjorge, Erik C, Bret Barkelew, Agyeman, Prince

Hi Ashley,

Please see comments inline.

Thanks,
Nate

> -----Original Message-----
> From: Desimone, Ashley E <ashley.e.desimone@intel.com>
> Sent: Tuesday, April 28, 2020 2:57 PM
> To: devel@edk2.groups.io
> Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Pandya, Puja
> <puja.pandya@intel.com>; Bjorge, Erik C <erik.c.bjorge@intel.com>; Bret
> Barkelew <Bret.Barkelew@microsoft.com>; Agyeman, Prince
> <prince.agyeman@intel.com>
> Subject: [edk2-staging/EdkRepo] [PATCH 7/7] EdkRepo: Add the ability to
> pull only the global manifest repository for a given workspace.
> 
> Add pull_workspace_man_repo() to pull only the global manifest repository
> affiliated with a single workspace.
> 
> Signed-off-by: Ashley E Desimone <ashley.e.desimone@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Puja Pandya <puja.pandya@intel.com>
> Cc: Erik Bjorge <erik.c.bjorge@intel.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Cc: Prince Agyeman <prince.agyeman@intel.com>
> ---
>  .../manifest_repos_maintenance.py                  | 23
> ++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git
> a/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> b/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> index 7b3f866..7a7f946 100644
> ---
> a/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> +++
> b/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> @@ -199,7 +199,7 @@ def find_project_in_all_indices (project,
> edkrepo_cfg, edkrepo_user_cfg, except_
>                          return repo, 'edkrepo_user_cfg', os.path.join(dirpath, project)
> 
> 
> -def find_source_man_repo (project_manifest, edkrepo_cfg,
> edkrepo_user_cfg):
> +def find_source_man_repo(project_manifest, edkrepo_cfg,
> edkrepo_user_cfg, man_repo=None):
>      '''
>      Finds the source manifest repo for a given project.
>      '''
> @@ -213,6 +213,25 @@ def find_source_man_repo (project_manifest,
> edkrepo_cfg, edkrepo_user_cfg):
> 
> humble.SOURCE_MAN_REPO_NOT_FOUND.format(project_manifest.proje
> ct_info.codename),
>                                                                               man_repo=None)
>          project_manifest.write_source_manifest_repo(src_man_repo)
> -        return src_man_repo
> +        return src_man_repo
> +
> +def pull_workspace_man_repo(project_manifest, edkrepo_cfg,
> edkrepo_user_cfg, man_repo=None, reset_hard=False):

All the pre-existing EdkRepo code tends to spell out "manifest" please rename this function to "pull_workspace_man_repo()"

> +    '''
> +    Pulls only the global manifest repo for the current workspace.
> +    '''
> +    src_man_repo = find_source_man_repo(project_manifest, edkrepo_cfg,
> edkrepo_user_cfg, man_repo)
> +    config_repos, user_config_repos, conflicts =
> list_available_man_repos(edkrepo_cfg, edkrepo_user_cfg)
> +    if src_man_repo in config_repos:
> +
> pull_single_manifest_repo(edkrepo_cfg.get_manifest_repo_url(src_man_r
> epo),
> +                                  edkrepo_cfg.get_manifest_repo_branch(src_man_repo),
> +
> edkrepo_cfg.get_manifest_repo_local_path(src_man_repo),
> +                                  reset_hard)
> +    elif src_man_repo in user_config_repos:
> +
> pull_single_manifest_repo(edkrepo_user_cfg.get_manifest_repo_url(src_
> man_repo),
> +
> edkrepo_user_cfg.get_manifest_repo_branch(src_man_repo),
> +
> edkrepo_user_cfg.get_manifest_repo_local_path(src_man_repo),
> +                                  reset_hard)
> +    elif src_man_repo in conflicts:
> +        raise
> +EdkrepoInvalidParametersException(humble.CONFLICT_NO_CLONE.format
> (src_m
> +an_repo))
> 
> 
> --
> 2.16.2.windows.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-staging/EdkRepo] [PATCH 5/7] EdkRepo: Add ability to find projects across all manifest repositories
  2020-04-30 21:28   ` Nate DeSimone
@ 2020-04-30 21:40     ` Bjorge, Erik C
  2020-04-30 22:07       ` Nate DeSimone
  0 siblings, 1 reply; 17+ messages in thread
From: Bjorge, Erik C @ 2020-04-30 21:40 UTC (permalink / raw)
  To: Desimone, Nathaniel L, Desimone, Ashley E, devel@edk2.groups.io
  Cc: Pandya, Puja, Bret Barkelew, Agyeman, Prince

I agree with your comment about moving magic strings to a common location.  I would suggest that we put it someplace other than humble.py.  I think we should add a new file for configuration settings like this.  I think something like tool_config.py would be good.

Thanks,
-Erik

-----Original Message-----
From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com> 
Sent: Thursday, April 30, 2020 2:29 PM
To: Desimone, Ashley E <ashley.e.desimone@intel.com>; devel@edk2.groups.io
Cc: Pandya, Puja <puja.pandya@intel.com>; Bjorge, Erik C <erik.c.bjorge@intel.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>; Agyeman, Prince <prince.agyeman@intel.com>
Subject: RE: [edk2-staging/EdkRepo] [PATCH 5/7] EdkRepo: Add ability to find projects across all manifest repositories

Hi Ashley,

Please see comments inline.

Thanks,
Nate

> -----Original Message-----
> From: Desimone, Ashley E <ashley.e.desimone@intel.com>
> Sent: Tuesday, April 28, 2020 2:57 PM
> To: devel@edk2.groups.io
> Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Pandya, 
> Puja <puja.pandya@intel.com>; Bjorge, Erik C 
> <erik.c.bjorge@intel.com>; Bret Barkelew 
> <Bret.Barkelew@microsoft.com>; Agyeman, Prince 
> <prince.agyeman@intel.com>
> Subject: [edk2-staging/EdkRepo] [PATCH 5/7] EdkRepo: Add ability to 
> find projects across all manifest repositories
> 
> Add find_project_in_all_indicies() to search for and return a tuple 
> (source repo, source config, path to manifest) if a matching project is found.
> 
> Add find_project_in_single_index() to find the path to a project 
> within a single manifest repositories index file and return a whether 
> the specified project was found and if so its path within the manifest repository.
> 
> Signed-off-by: Ashley E Desimone <ashley.e.desimone@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Puja Pandya <puja.pandya@intel.com>
> Cc: Erik Bjorge <erik.c.bjorge@intel.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Cc: Prince Agyeman <prince.agyeman@intel.com>
> ---
>  .../manifest_repos_maintenance.py                  | 70
> +++++++++++++++++++++-
>  .../workspace_maintenance/workspace_maintenance.py | 17 +++++-
>  2 files changed, 84 insertions(+), 3 deletions(-)
> 
> diff --git
> a/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> b/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> index 4bded46..9b441ac 100644
> ---
> a/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> +++
> b/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
> e.py
> @@ -15,11 +15,12 @@ import git
>  from git import Repo
> 
>  import edkrepo.config.config_factory as cfg -from 
> edkrepo.common.edkrepo_exception import 
> EdkrepoUncommitedChangesException
> +from edkrepo.common.edkrepo_exception import 
> +EdkrepoUncommitedChangesException,
> EdkrepoInvalidParametersException
>  from edkrepo.common.progress_handler import GitProgressHandler import 
> edkrepo.common.workspace_maintenance.humble.manifest_repos_maint
> enance_humble as humble  from
> edkrepo.common.workspace_maintenance.workspace_maintenance import 
> generate_name_for_obsolete_backup
> -
> +from edkrepo.common.workspace_maintenance.workspace_maintenance
> import
> +case_insensitive_single_match from
> edkrepo_manifest_parser.edk_manifest
> +import CiIndexXml, ManifestXml
> 
>  def pull_single_manifest_repo(url, branch, local_path, reset_hard=False):
>      '''
> @@ -135,3 +136,68 @@ def list_available_man_repos(edkrepo_cfg,
> edkrepo_user_cfg):
>      return cfg_man_repos, user_cfg_man_repos, conflicts
> 
> 
> +def find_project_in_single_index (project, index_file, manifest_dir):
> +    '''
> +    Finds a project in a single global manifest repositories index file. If found
> +    returns (True, path to file) if not returns (False, None)
> +    '''
> +    global_manifest_path = None
> +    try:
> +        proj_name = case_insensitive_single_match(project,
> index_file.project_list)
> +    except:
> +        proj_name = None
> +    if proj_name:
> +        ci_index_xml_rel_path =
> os.path.normpath(index_file.get_project_xml(proj_name))
> +        global_manifest_path = os.path.join(manifest_dir,
> ci_index_xml_rel_path)
> +        return True, global_manifest_path
> +    else:
> +        return False, global_manifest_path
> +
> +
> +def find_project_in_all_indices (project, edkrepo_cfg, 
> +edkrepo_user_cfg,
> except_msg_man_repo, except_msg_not_found, man_repo=None):
> +    '''
> +    Finds the project in all manifest repositories listed in the edkrepo.efg and
> +    edkrepo_user.cfg. If a project with the same name is found uses
> man_repo to select
> +    the correct entry
> +    '''
> +    cfg_man_repos, user_cfg_man_repos, conflicts =
> list_available_man_repos(edkrepo_cfg, edkrepo_user_cfg)
> +    projects = {}
> +    for repo in cfg_man_repos:
> +        manifest_dir = edkrepo_cfg.manifest_repo_abs_path(repo)
> +        index_file = CiIndexXml(os.path.join(manifest_dir, 
> + 'CiIndex.xml'))

The magic string 'CiIndex.xml' is repeated several times. It would be good to have something like humble.py that stores the magic string, so that if we ever change it that can be done quickly.

> +        found, man_path = find_project_in_single_index(project, 
> + index_file,
> manifest_dir)
> +        if found:
> +            projects[repo] = ('edkrepo_cfg', man_path)
> +    for repo in user_cfg_man_repos:
> +        manifest_dir = edkrepo_user_cfg.manifest_repo_abs_path(repo)
> +        index_file = CiIndexXml(os.path.join(manifest_dir, 'CiIndex.xml'))
> +        found, man_path = find_project_in_single_index(project, 
> + index_file,
> manifest_dir)
> +        if found:
> +            projects[repo] = ('edkrepo_user_cfg', man_path)
> +    if len(projects.keys()) == 1:
> +        repo = list(projects.keys())[0]
> +        return repo, projects[repo][0], projects[repo][1]
> +    elif len(projects.keys()) > 1 and man_repo:
> +        try:
> +            return man_repo, projects[man_repo][0], projects[man_repo][1]
> +        except KeyError:
> +            raise EdkrepoInvalidParametersException(except_msg_man_repo)
> +    elif os.path.isabs(project):
> +        return None, None, project
> +    elif os.path.isfile(os.path.join(os.getcwd(), project)):
> +        return None, None, os.path.join(os.getcwd(), project)
> +    elif not os.path.dirname(project):
> +        for repo in cfg_man_repos:
> +            if (man_repo and (repo == man_repo)) or not man_repo:
> +                for dirpath, dirname, filenames in
> os.walk(edkrepo_cfg.manifest_repo_abs_path(repo)):
> +                    if project in filenames:
> +                        return repo, 'edkrepo_cfg', os.path.join(dirpath, project)
> +        for repo in user_cfg_man_repos:
> +            if (man_repo and (repo == man_repo)) or not man_repo:
> +                for dirpath, dirname, filenames in
> os.walk(edkrepo_user_cfg.manifest_repo_abs_path(repo)):
> +                    if project in filenames:
> +                        return repo, 'edkrepo_user_cfg', 
> +os.path.join(dirpath, project)

If man_repo is None, and the project name exists in more than 1 manifest repository, then you will reach the end of this function and return None. It seems like that should be a raised exception instead, telling the user that they need to specific a specific manifest repository to disambiguate.

> +
> +
> +
> +
> diff --git
> a/edkrepo/common/workspace_maintenance/workspace_maintenance.py
> b/edkrepo/common/workspace_maintenance/workspace_maintenance.py
> index 6e20d43..ba62f6d 100644
> ---
> a/edkrepo/common/workspace_maintenance/workspace_maintenance.py
> +++
> b/edkrepo/common/workspace_maintenance/workspace_maintenance.py
> @@ -10,6 +10,10 @@
>  ''' Contains shared workspace maintenance functions. '''
> 
>  import os
> +import unicodedata
> +
> +from edkrepo.common.edkrepo_exception import 
> +EdkrepoFoundMultipleException, EdkrepoNotFoundException from 
> +edkrepo.common.humble import GEN_A_NOT_IN_B,
> GEN_FOUND_MULT_A_IN_B
> 
>  def generate_name_for_obsolete_backup(absolute_path):
>      if not os.path.exists(absolute_path):
> @@ -27,4 +31,15 @@ def
> generate_name_for_obsolete_backup(absolute_path):
>          if not os.path.exists(os.path.join(dir_name, unique_name)):
>              unique_name_found = True
>          index += 1
> -    return unique_name
> \ No newline at end of file
> +    return unique_name
> +
> +def case_insensitive_equal(str1, str2):
> +    return unicodedata.normalize("NFKD", str1.casefold()) == 
> +unicodedata.normalize("NFKD", str2.casefold())
> +
> +def case_insensitive_single_match(str1, str_list):
> +    matches = [x for x in str_list if case_insensitive_equal(str1, x)]
> +    if len(matches) == 0:
> +        raise EdkrepoNotFoundException(GEN_A_NOT_IN_B.format(str1,
> str_list))
> +    elif len(matches) > 1:
> +        raise
> EdkrepoFoundMultipleException(GEN_FOUND_MULT_A_IN_B.format(str1,
> str_list))
> +    return matches[0]
> --
> 2.16.2.windows.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-staging/EdkRepo] [PATCH 5/7] EdkRepo: Add ability to find projects across all manifest repositories
  2020-04-30 21:40     ` Bjorge, Erik C
@ 2020-04-30 22:07       ` Nate DeSimone
  0 siblings, 0 replies; 17+ messages in thread
From: Nate DeSimone @ 2020-04-30 22:07 UTC (permalink / raw)
  To: Bjorge, Erik C, Desimone, Ashley E, devel@edk2.groups.io
  Cc: Pandya, Puja, Bret Barkelew, Agyeman, Prince

Hi Erik,

Sure, sounds good.

Thanks,
Nate

On 4/30/20, 2:40 PM, "Bjorge, Erik C" <erik.c.bjorge@intel.com> wrote:

    I agree with your comment about moving magic strings to a common location.  I would suggest that we put it someplace other than humble.py.  I think we should add a new file for configuration settings like this.  I think something like tool_config.py would be good.

    Thanks,
    -Erik

    -----Original Message-----
    From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com> 
    Sent: Thursday, April 30, 2020 2:29 PM
    To: Desimone, Ashley E <ashley.e.desimone@intel.com>; devel@edk2.groups.io
    Cc: Pandya, Puja <puja.pandya@intel.com>; Bjorge, Erik C <erik.c.bjorge@intel.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>; Agyeman, Prince <prince.agyeman@intel.com>
    Subject: RE: [edk2-staging/EdkRepo] [PATCH 5/7] EdkRepo: Add ability to find projects across all manifest repositories

    Hi Ashley,

    Please see comments inline.

    Thanks,
    Nate

    > -----Original Message-----
    > From: Desimone, Ashley E <ashley.e.desimone@intel.com>
    > Sent: Tuesday, April 28, 2020 2:57 PM
    > To: devel@edk2.groups.io
    > Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Pandya, 
    > Puja <puja.pandya@intel.com>; Bjorge, Erik C 
    > <erik.c.bjorge@intel.com>; Bret Barkelew 
    > <Bret.Barkelew@microsoft.com>; Agyeman, Prince 
    > <prince.agyeman@intel.com>
    > Subject: [edk2-staging/EdkRepo] [PATCH 5/7] EdkRepo: Add ability to 
    > find projects across all manifest repositories
    > 
    > Add find_project_in_all_indicies() to search for and return a tuple 
    > (source repo, source config, path to manifest) if a matching project is found.
    > 
    > Add find_project_in_single_index() to find the path to a project 
    > within a single manifest repositories index file and return a whether 
    > the specified project was found and if so its path within the manifest repository.
    > 
    > Signed-off-by: Ashley E Desimone <ashley.e.desimone@intel.com>
    > Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
    > Cc: Puja Pandya <puja.pandya@intel.com>
    > Cc: Erik Bjorge <erik.c.bjorge@intel.com>
    > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
    > Cc: Prince Agyeman <prince.agyeman@intel.com>
    > ---
    >  .../manifest_repos_maintenance.py                  | 70
    > +++++++++++++++++++++-
    >  .../workspace_maintenance/workspace_maintenance.py | 17 +++++-
    >  2 files changed, 84 insertions(+), 3 deletions(-)
    > 
    > diff --git
    > a/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
    > e.py
    > b/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
    > e.py
    > index 4bded46..9b441ac 100644
    > ---
    > a/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
    > e.py
    > +++
    > b/edkrepo/common/workspace_maintenance/manifest_repos_maintenanc
    > e.py
    > @@ -15,11 +15,12 @@ import git
    >  from git import Repo
    > 
    >  import edkrepo.config.config_factory as cfg -from 
    > edkrepo.common.edkrepo_exception import 
    > EdkrepoUncommitedChangesException
    > +from edkrepo.common.edkrepo_exception import 
    > +EdkrepoUncommitedChangesException,
    > EdkrepoInvalidParametersException
    >  from edkrepo.common.progress_handler import GitProgressHandler import 
    > edkrepo.common.workspace_maintenance.humble.manifest_repos_maint
    > enance_humble as humble  from
    > edkrepo.common.workspace_maintenance.workspace_maintenance import 
    > generate_name_for_obsolete_backup
    > -
    > +from edkrepo.common.workspace_maintenance.workspace_maintenance
    > import
    > +case_insensitive_single_match from
    > edkrepo_manifest_parser.edk_manifest
    > +import CiIndexXml, ManifestXml
    > 
    >  def pull_single_manifest_repo(url, branch, local_path, reset_hard=False):
    >      '''
    > @@ -135,3 +136,68 @@ def list_available_man_repos(edkrepo_cfg,
    > edkrepo_user_cfg):
    >      return cfg_man_repos, user_cfg_man_repos, conflicts
    > 
    > 
    > +def find_project_in_single_index (project, index_file, manifest_dir):
    > +    '''
    > +    Finds a project in a single global manifest repositories index file. If found
    > +    returns (True, path to file) if not returns (False, None)
    > +    '''
    > +    global_manifest_path = None
    > +    try:
    > +        proj_name = case_insensitive_single_match(project,
    > index_file.project_list)
    > +    except:
    > +        proj_name = None
    > +    if proj_name:
    > +        ci_index_xml_rel_path =
    > os.path.normpath(index_file.get_project_xml(proj_name))
    > +        global_manifest_path = os.path.join(manifest_dir,
    > ci_index_xml_rel_path)
    > +        return True, global_manifest_path
    > +    else:
    > +        return False, global_manifest_path
    > +
    > +
    > +def find_project_in_all_indices (project, edkrepo_cfg, 
    > +edkrepo_user_cfg,
    > except_msg_man_repo, except_msg_not_found, man_repo=None):
    > +    '''
    > +    Finds the project in all manifest repositories listed in the edkrepo.efg and
    > +    edkrepo_user.cfg. If a project with the same name is found uses
    > man_repo to select
    > +    the correct entry
    > +    '''
    > +    cfg_man_repos, user_cfg_man_repos, conflicts =
    > list_available_man_repos(edkrepo_cfg, edkrepo_user_cfg)
    > +    projects = {}
    > +    for repo in cfg_man_repos:
    > +        manifest_dir = edkrepo_cfg.manifest_repo_abs_path(repo)
    > +        index_file = CiIndexXml(os.path.join(manifest_dir, 
    > + 'CiIndex.xml'))

    The magic string 'CiIndex.xml' is repeated several times. It would be good to have something like humble.py that stores the magic string, so that if we ever change it that can be done quickly.

    > +        found, man_path = find_project_in_single_index(project, 
    > + index_file,
    > manifest_dir)
    > +        if found:
    > +            projects[repo] = ('edkrepo_cfg', man_path)
    > +    for repo in user_cfg_man_repos:
    > +        manifest_dir = edkrepo_user_cfg.manifest_repo_abs_path(repo)
    > +        index_file = CiIndexXml(os.path.join(manifest_dir, 'CiIndex.xml'))
    > +        found, man_path = find_project_in_single_index(project, 
    > + index_file,
    > manifest_dir)
    > +        if found:
    > +            projects[repo] = ('edkrepo_user_cfg', man_path)
    > +    if len(projects.keys()) == 1:
    > +        repo = list(projects.keys())[0]
    > +        return repo, projects[repo][0], projects[repo][1]
    > +    elif len(projects.keys()) > 1 and man_repo:
    > +        try:
    > +            return man_repo, projects[man_repo][0], projects[man_repo][1]
    > +        except KeyError:
    > +            raise EdkrepoInvalidParametersException(except_msg_man_repo)
    > +    elif os.path.isabs(project):
    > +        return None, None, project
    > +    elif os.path.isfile(os.path.join(os.getcwd(), project)):
    > +        return None, None, os.path.join(os.getcwd(), project)
    > +    elif not os.path.dirname(project):
    > +        for repo in cfg_man_repos:
    > +            if (man_repo and (repo == man_repo)) or not man_repo:
    > +                for dirpath, dirname, filenames in
    > os.walk(edkrepo_cfg.manifest_repo_abs_path(repo)):
    > +                    if project in filenames:
    > +                        return repo, 'edkrepo_cfg', os.path.join(dirpath, project)
    > +        for repo in user_cfg_man_repos:
    > +            if (man_repo and (repo == man_repo)) or not man_repo:
    > +                for dirpath, dirname, filenames in
    > os.walk(edkrepo_user_cfg.manifest_repo_abs_path(repo)):
    > +                    if project in filenames:
    > +                        return repo, 'edkrepo_user_cfg', 
    > +os.path.join(dirpath, project)

    If man_repo is None, and the project name exists in more than 1 manifest repository, then you will reach the end of this function and return None. It seems like that should be a raised exception instead, telling the user that they need to specific a specific manifest repository to disambiguate.

    > +
    > +
    > +
    > +
    > diff --git
    > a/edkrepo/common/workspace_maintenance/workspace_maintenance.py
    > b/edkrepo/common/workspace_maintenance/workspace_maintenance.py
    > index 6e20d43..ba62f6d 100644
    > ---
    > a/edkrepo/common/workspace_maintenance/workspace_maintenance.py
    > +++
    > b/edkrepo/common/workspace_maintenance/workspace_maintenance.py
    > @@ -10,6 +10,10 @@
    >  ''' Contains shared workspace maintenance functions. '''
    > 
    >  import os
    > +import unicodedata
    > +
    > +from edkrepo.common.edkrepo_exception import 
    > +EdkrepoFoundMultipleException, EdkrepoNotFoundException from 
    > +edkrepo.common.humble import GEN_A_NOT_IN_B,
    > GEN_FOUND_MULT_A_IN_B
    > 
    >  def generate_name_for_obsolete_backup(absolute_path):
    >      if not os.path.exists(absolute_path):
    > @@ -27,4 +31,15 @@ def
    > generate_name_for_obsolete_backup(absolute_path):
    >          if not os.path.exists(os.path.join(dir_name, unique_name)):
    >              unique_name_found = True
    >          index += 1
    > -    return unique_name
    > \ No newline at end of file
    > +    return unique_name
    > +
    > +def case_insensitive_equal(str1, str2):
    > +    return unicodedata.normalize("NFKD", str1.casefold()) == 
    > +unicodedata.normalize("NFKD", str2.casefold())
    > +
    > +def case_insensitive_single_match(str1, str_list):
    > +    matches = [x for x in str_list if case_insensitive_equal(str1, x)]
    > +    if len(matches) == 0:
    > +        raise EdkrepoNotFoundException(GEN_A_NOT_IN_B.format(str1,
    > str_list))
    > +    elif len(matches) > 1:
    > +        raise
    > EdkrepoFoundMultipleException(GEN_FOUND_MULT_A_IN_B.format(str1,
    > str_list))
    > +    return matches[0]
    > --
    > 2.16.2.windows.1



^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2020-04-30 22:39 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-28 21:57 [edk2-staging/EdkRepo] [PATCH 0/7] Support for consuming multiple manifest repositories Ashley E Desimone
2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 1/7] EdkRepo: Add check for conflicting/duplicated manifest repo definitions Ashley E Desimone
2020-04-30 21:28   ` [edk2-devel] " Nate DeSimone
2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 2/7] EdkRepo: Add downloading all available manifest repositories Ashley E Desimone
2020-04-30 21:28   ` [edk2-devel] " Nate DeSimone
2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 3/7] EdkRepo: Add optional field to edkrepo_manifst to track the source manifest repo Ashley E Desimone
2020-04-30 21:28   ` Nate DeSimone
2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 4/7] EdkRepo: Add list_available_manifest_repos() Ashley E Desimone
2020-04-30 21:28   ` Nate DeSimone
2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 5/7] EdkRepo: Add ability to find projects across all manifest repositories Ashley E Desimone
2020-04-30 21:28   ` Nate DeSimone
2020-04-30 21:40     ` Bjorge, Erik C
2020-04-30 22:07       ` Nate DeSimone
2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 6/7] EdkRepo: Add ability to determine the source manifest of a workspace Ashley E Desimone
2020-04-30 21:28   ` Nate DeSimone
2020-04-28 21:57 ` [edk2-staging/EdkRepo] [PATCH 7/7] EdkRepo: Add the ability to pull only the global manifest repository for a given workspace Ashley E Desimone
2020-04-30 21:28   ` Nate DeSimone

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox