From b6bc525d0ad3d2bac31265840d5976e02a259e4d Mon Sep 17 00:00:00 2001 From: Matthew Carlson Date: Tue, 7 Jul 2020 13:27:11 -0700 Subject: [PATCH] Fix the Repo Resolver (#209) This does more specific cloning of branches and does --single-branch --- edk2toolext/edk2_git.py | 18 +++++++--- edk2toolext/environment/repo_resolver.py | 46 ++++++++++++++++-------- edk2toolext/tests/test_repo_resolver.py | 21 ++++++++--- 3 files changed, 61 insertions(+), 24 deletions(-) diff --git a/edk2toolext/edk2_git.py b/edk2toolext/edk2_git.py index 674b1d43..8bb2360e 100644 --- a/edk2toolext/edk2_git.py +++ b/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) diff --git a/edk2toolext/environment/repo_resolver.py b/edk2toolext/environment/repo_resolver.py index 6151f528..a03f5095 100644 --- a/edk2toolext/environment/repo_resolver.py +++ b/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"]: diff --git a/edk2toolext/tests/test_repo_resolver.py b/edk2toolext/tests/test_repo_resolver.py index d4a3c337..7ebc6162 100644 --- a/edk2toolext/tests/test_repo_resolver.py +++ b/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()