Fix the Repo Resolver (#209)

This does more specific cloning of branches and does --single-branch

edk2toolext/edk2_git.py

@@ -186,10 +186,14 @@ def submodule(self, command, *args):
 
         return True
 
-    def fetch(self):
+    def fetch(self, remote="origin", branch=None):
         return_buffer = StringIO()
 
-        params = "fetch"
+        param_list = ["fetch", remote]
+        if branch is not None:
+            param_list.append(f"{branch}:{branch}")
+
+        params = " ".join(param_list)
 
         ret = RunCmd("git", params, workingdir=self._path,
                      outstream=return_buffer)
@@ -233,19 +237,25 @@ def checkout(self, branch=None, commit=None):
         return True
 
     @classmethod
-    def clone_from(self, url, to_path, progress=None, env=None, shallow=False, reference=None, **kwargs):
+    def clone_from(self, url, to_path, branch=None, shallow=False, reference=None, **kwargs):
         _logger = logging.getLogger("git.repo")
         _logger.debug("Cloning {0} into {1}".format(url, to_path))
         # make sure we get the commit if
         # use run command from utilities
         cmd = "git"
         params = ["clone"]
+        if branch:
+            shallow = True
+            params.append(f'--branch {branch}')
+            params.append('--single-branch')
         if shallow:
-            params.append("--shallow-submodules")
+            # params.append("--shallow-submodules")
+            params.append("--depth=5")
         if reference:
             params.append("--reference %s" % reference)
         else:
             params.append("--recurse-submodules")  # if we don't have a reference we can just recurse the submodules
+
         params.append(url)
         params.append(to_path)
 

edk2toolext/environment/repo_resolver.py

@@ -26,20 +26,19 @@ def resolve(file_system_path, dependency, force=False, ignore=False, update_ok=F
     if "Path" in dependency and not git_path.endswith(os.path.relpath(dependency["Path"])):
         # if we don't already the the path from the dependency at the end of the path we've been giving
         git_path = os.path.join(git_path, dependency["Path"])
-
+    logging.info(f"Resolving at {git_path}")
     ##
     # NOTE - this process is defined in the Readme.md including flow chart for this behavior
     ##
     if not os.path.isdir(git_path):
-        clone_repo(git_path, dependency)
-        r = Repo(git_path)
+        logging.info(f"Cloning at {git_path}")
+        _, r = clone_repo(git_path, dependency)
         checkout(git_path, dependency, r, True, False)
         return r
 
     folder_empty = len(os.listdir(git_path)) == 0
     if folder_empty:  # if the folder is empty, we can clone into it
-        clone_repo(git_path, dependency)
-        r = Repo(git_path)
+        _, r = clone_repo(git_path, dependency)
         checkout(git_path, dependency, r, True, False)
         return r
 
@@ -49,7 +48,7 @@ def resolve(file_system_path, dependency, force=False, ignore=False, update_ok=F
             clear_folder(git_path)
             logger.warning(
                 "Folder {0} is not a git repo and is being overwritten!".format(git_path))
-            clone_repo(git_path, dependency)
+            _, r = clone_repo(git_path, dependency)
             checkout(git_path, dependency, repo, True, False)
             return repo
         else:
@@ -69,7 +68,7 @@ def resolve(file_system_path, dependency, force=False, ignore=False, update_ok=F
             clear_folder(git_path)
             logger.warning(
                 "Folder {0} is a git repo but is dirty and is being overwritten as requested!".format(git_path))
-            clone_repo(git_path, dependency)
+            _, r = clone_repo(git_path, dependency)
             checkout(git_path, dependency, repo, True, False)
             return repo
         else:
@@ -103,7 +102,7 @@ def resolve(file_system_path, dependency, force=False, ignore=False, update_ok=F
                     git_path, dependency["Url"], repo.remotes.origin.url))
                 raise Exception("The URL of the git Repo {2} in the folder {0} does not match {1}".format(
                     git_path, dependency["Url"], repo.remotes.origin.url))
-
+    # if we've gotten here, we should just checkout as normal
     checkout(git_path, dependency, repo, update_ok, ignore, force)
     return repo
 
@@ -174,35 +173,46 @@ def clone_repo(abs_file_system_path, DepObj):
     if not os.path.isdir(dest):
         os.makedirs(dest, exist_ok=True)
     shallow = False
+    branch = None
     if "Commit" in DepObj:
         shallow = False
     if "Full" in DepObj and DepObj["Full"] is True:
         shallow = False
+    if "Branch" in DepObj:
+        shallow = True
+        branch = DepObj["Branch"]
+
     reference = None
     if "ReferencePath" in DepObj and os.path.exists(DepObj["ReferencePath"]):
         reference = os.path.abspath(DepObj["ReferencePath"])
-    result = Repo.clone_from(DepObj["Url"], dest, shallow=shallow, reference=reference)
+    result = Repo.clone_from(DepObj["Url"], dest, branch=branch, shallow=shallow, reference=reference)
 
     if result is None:
         if "ReferencePath" in DepObj:
             # attempt a retry without the reference
             logger.warning("Reattempting to clone without a reference. {0}".format(DepObj["Url"]))
-            result = Repo.clone_from(DepObj["Url"], dest, shallow=shallow)
+            result = Repo.clone_from(DepObj["Url"], dest, branch=branch, shallow=shallow)
             if result is None:
-                return None
+                return (dest, None)
 
-    return dest
+    return (dest, result)
 
 
 def checkout(abs_file_system_path, dep, repo, update_ok=False, ignore_dep_state_mismatch=False, force=False):
     logger = logging.getLogger("git")
+    if repo is None:
+        repo = Repo(abs_file_system_path)
     if "Commit" in dep:
+        commit = dep["Commit"]
         if update_ok or force:
             repo.fetch()
-            repo.checkout(commit=dep["Commit"])
+            result = repo.checkout(commit=commit)
+            if result is False:
+                repo.fetch()
+                repo.checkout(commit=commit)
             repo.submodule("update", "--init", "--recursive")
         else:
-            if repo.head.commit == dep["Commit"]:
+            if repo.head.commit == commit:
                 logger.debug(
                     "Dependency {0} state ok without update".format(dep["Path"]))
                 return
@@ -217,9 +227,15 @@ def checkout(abs_file_system_path, dep, repo, update_ok=False, ignore_dep_state_
                     "Dependency {0} is not in sync with requested commit.  Fail.".format(dep["Path"]))
 
     elif "Branch" in dep:
+        branch = dep["Branch"]
         if update_ok or force:
             repo.fetch()
-            repo.checkout(branch=dep["Branch"])
+            result = repo.checkout(branch=branch)
+            if result is False:  # we failed to do this
+                # try to fetch it and try to checkout again
+                logger.info("We failed to checkout this branch, we'll try to fetch")
+                repo.fetch(branch=branch)
+                result = repo.checkout(branch=branch)
             repo.submodule("update", "--init", "--recursive")
         else:
             if repo.active_branch == dep["Branch"]:

edk2toolext/tests/test_repo_resolver.py

@@ -1,4 +1,4 @@
-## @file test_repo_resolver.py
+# @file test_repo_resolver.py
 # This contains unit tests for repo resolver
 #
 ##
@@ -87,14 +87,15 @@ def setUp(self):
     @classmethod
     def setUpClass(cls):
         logger = logging.getLogger('')
+        logger.setLevel(logging.DEBUG)
         logger.addHandler(logging.NullHandler())
         unittest.installHandler()
 
     @classmethod
-    def tearDownClass(cls):
+    def tearDown(cls):
         clean_workspace()
 
-        # check to make sure that we can clone a branch correctly
+    # check to make sure that we can clone a branch correctly
     def test_clone_branch_repo(self):
         # create an empty directory- and set that as the workspace
         repo_resolver.resolve(test_dir, branch_dependency)
@@ -167,7 +168,6 @@ def test_will_delete_dirty_repo(self):
             self.fail("We shouldn't fail when we are forcing")
 
     # check to make sure we can clone a commit correctly
-
     def test_clone_commit_repo(self):
         # create an empty directory- and set that as the workspace
         repo_resolver.resolve(test_dir, commit_dependency)
@@ -196,19 +196,22 @@ def test_fail_update(self):
 
     def test_does_update(self):
         # create an empty directory- and set that as the workspace
+        logging.info(f"Resolving at {test_dir}")
         repo_resolver.resolve(test_dir, commit_dependency)
         folder_path = os.path.join(test_dir, commit_dependency["Path"])
+        logging.info(f"Getting details at {folder_path}")
         details = repo_resolver.get_details(folder_path)
 
         self.assertEqual(details['Url'], commit_dependency['Url'])
         self.assertEqual(details['Commit'], commit_dependency['Commit'])
-        # first we checkout
+        # next we checkout and go to the later commit
         try:
             repo_resolver.resolve(
                 test_dir, commit_later_dependency, update_ok=True)
         except:
             self.fail("We are not supposed to throw an exception")
         details = repo_resolver.get_details(folder_path)
+        logging.info(f"Checking {folder_path} for current git commit")
 
         self.assertEqual(details['Url'], commit_later_dependency['Url'])
         self.assertEqual(details['Commit'], commit_later_dependency['Commit'])
@@ -263,6 +266,14 @@ def test_will_switch_urls(self):
         details = repo_resolver.get_details(folder_path)
         self.assertEqual(details['Url'], microsoft_branch_dependency['Url'])
 
+    def test_will_switch_branches(self):
+        repo_resolver.resolve(test_dir, branch_dependency)
+        folder_path = os.path.join(test_dir, branch_dependency["Path"])
+        repo_resolver.resolve(test_dir, sub_branch_dependency, force=True)
+        details = repo_resolver.get_details(folder_path)
+        self.assertEqual(details['Url'], branch_dependency['Url'])
+        self.assertEqual(details['Branch'], sub_branch_dependency['Branch'])
+
 
 if __name__ == '__main__':
     unittest.main()