Daisy-chaining PatchSet fails to create branch

Currently, two PatchSet can be chained together to create a dynamic branch.

If a third or more PatchSet is chained in a row, these PatchSet fail to create a dynamic branch when used, and non-gracefully fails when the PatchSet steps are attempted on the wrong branch or commit.

Locate the correct base PatchSet from the manifest before attempting to create the branch.
If a PatchSet circular dependency is found, raise an exception.
Do not attempt to create the branch if any exception besides KeyError is encountered.

Signed-off-by: Nathaniel Haller <[email protected]>
Reviewed-by: Kevin Sun <[email protected]>
Reviewed-by: Ashley E Desimone <[email protected]>
Reviewed-by: Nate Desimone <[email protected]>

edkrepo/common/common_repo_functions.py

@@ -77,6 +77,7 @@
 PRIMARY_REMOTE_NAME = 'primary'
 PATCH = "Patch"
 REVERT = "Revert"
+PATCHSET_CIRCULAR_DEPENDENCY_ERROR = "The PatchSet {} has a circular dependency with another PatchSet"
 
 def clone_repos(args, workspace_dir, repos_to_clone, project_client_side_hooks, config, manifest, global_manifest_path, cache_obj=None):
     for repo_to_clone in repos_to_clone:
@@ -842,12 +843,26 @@ def create_local_branch(name, patchset, global_manifest_path, manifest_obj, repo
                 repo.remotes.origin.fetch(patchset.fetch_branch, progress=GitProgressHandler())
             except:
                 raise EdkrepoFetchBranchNotFoundException(FETCH_BRANCH_DOES_NOT_EXIST.format(patchset.fetch_branch))
+            parent_patchsets = list()
             try:
-                parent_patchset = manifest_obj.get_patchset(patchset.parent_sha, patchset.remote)
-                repo.git.checkout(parent_patchset[2], b=name)
-            except:
-                if patchset.parent_sha in repo.tags:
-                    repo.git.checkout("tags/{}".format(patchset.parent_sha), b=name)
+                parent_patchsets.append(manifest_obj.get_patchset(patchset.parent_sha, patchset.remote))
+                while True:
+                    # Allow daisy-chaining PatchSet.  Continue until KeyError is caught.
+                    parent_patchsets.append(manifest_obj.get_patchset(parent_patchsets[-1].parent_sha, parent_patchsets[-1].remote))
+                    if parent_patchsets.count(parent_patchsets[-1]) > 1:
+                        # Do not continue to branch creation step if a circular dependency is detected between multiple PatchSet.
+                        raise ValueError(PATCHSET_CIRCULAR_DEPENDENCY_ERROR.format(parent_patchsets[-1]))
+            except KeyError:
+                if parent_patchsets != list():
+                    base_patchset_parent_sha = parent_patchsets[-1].parent_sha
+                    if base_patchset_parent_sha in repo.tags:
+                        # tag names are prioritized over branch names
+                        repo.git.checkout("refs/tags/{}".format(base_patchset_parent_sha), b=name)
+                    else:
+                        repo.git.checkout(base_patchset_parent_sha, b=name)
+                elif patchset.parent_sha in repo.tags:
+                    # tag names are prioritized over branch names
+                    repo.git.checkout("refs/tags/{}".format(patchset.parent_sha), b=name)
                 else:
                     repo.git.checkout(patchset.parent_sha, b=name)
             try:

edkrepo_manifest_parser/edk_manifest.py

@@ -61,6 +61,7 @@
 NO_PATCHSET_IN_COMBO = "The Combination: {} does not have any Patchsets."
 NO_PATCHSET_EXISTS = "The Patchset: {} does not exist"
 INVALID_PATCHSET_NAME_ERROR = "The Patchset cannot be named: {}. Please rename the Patchset"
+PATCHSET_PARENT_CIRCULAR_DEPENDENCY = "The PatchSet parent failed to be determined through recursion.  Check manifest for circular dependencies in daisy-chained PatchSet"
 
 class BaseXmlHelper():
     def __init__(self, fileref, xml_types):
@@ -835,8 +836,11 @@ def get_patchset_operations(self, name, remote):
         '''
         patch_set_operations = []
         if (name, remote) in self._patch_sets:
-            self.get_parent_patchset_operations(name, remote, patch_set_operations)
-            patch_set_operations.append(self._patch_set_operations[(name, remote)])
+            try:
+                self.get_parent_patchset_operations(name, remote, patch_set_operations)
+                patch_set_operations.append(self._patch_set_operations[(name, remote)])
+            except RecursionError:
+                raise ValueError(PATCHSET_PARENT_CIRCULAR_DEPENDENCY)
             return patch_set_operations
         raise ValueError(PATCHSET_UNKNOWN_ERROR.format(name, self._fileref))