From f03937fbc8f1c4fce6749ff8db9ed67c31e5e6bf Mon Sep 17 00:00:00 2001 From: Joey Vagedes Date: Wed, 10 May 2023 08:50:20 -0700 Subject: [PATCH 1/4] Revert "Add ability to skip verification of external dependencies (#419)" This reverts commit ff69ffd9e265148dbaf6f26a0e35c0f20522b1d9. --- .../environment/extdeptypes/git_dependency.py | 12 +------ .../environment/external_dependency.py | 31 +++++++++---------- edk2toolext/invocables/edk2_platform_build.py | 4 --- tests.unit/test_edk2_update.py | 2 +- .../test_self_describing_environment.py | 29 ----------------- 5 files changed, 17 insertions(+), 61 deletions(-) diff --git a/edk2toolext/environment/extdeptypes/git_dependency.py b/edk2toolext/environment/extdeptypes/git_dependency.py index 4cfa6a21..4e61d277 100644 --- a/edk2toolext/environment/extdeptypes/git_dependency.py +++ b/edk2toolext/environment/extdeptypes/git_dependency.py @@ -85,17 +85,7 @@ def clean(self): super().clean() def verify(self): - """Verifies the clone was successful. - - !!! Note - If verify is set to false in the dependencies state file, - it will always skip the verification process. - """ - state_data = self.get_state_file_data() - if state_data and state_data['verify'] is False: - logging.warning(f'{self.name} is unverified. Unexpected results may occur.') - return True - + """Verifies the clone was successful.""" result = True details = repo_resolver.repo_details(self._local_repo_root_path) diff --git a/edk2toolext/environment/external_dependency.py b/edk2toolext/environment/external_dependency.py index 0a38bd9f..ef2aec35 100644 --- a/edk2toolext/environment/external_dependency.py +++ b/edk2toolext/environment/external_dependency.py @@ -65,7 +65,7 @@ def __init__(self, descriptor): self.contents_dir = os.path.join( self.descriptor_location, self.name + "_extdep") self.state_file_path = os.path.join( - self.contents_dir, "extdep_state.yaml") + self.contents_dir, "extdep_state.json") self.published_path = self.compute_published_path() def set_global_cache_path(self, global_cache_path): @@ -177,8 +177,19 @@ def copy_to_global_cache(self, source_path: str): def verify(self): """Verifies the dependency was successfully downloaded.""" result = True - state_data = self.get_state_file_data() + state_data = None + # See whether or not the state file exists. + if not os.path.isfile(self.state_file_path): + result = False + + # Attempt to load the state file. + if result: + with open(self.state_file_path, 'r') as file: + try: + state_data = yaml.safe_load(file) + except Exception: + pass if state_data is None: result = False @@ -191,27 +202,15 @@ def verify(self): def report_version(self): """Reports the version of the external dependency.""" - state_data = self.get_state_file_data() - version = self.version - if state_data and state_data['verify'] is False: - version = "UNVERIFIED" version_aggregator.GetVersionAggregator().ReportVersion(self.name, - version, + self.version, version_aggregator.VersionTypes.INFO, self.descriptor_location) def update_state_file(self): """Updates the file representing the state of the dependency.""" with open(self.state_file_path, 'w+') as file: - yaml.dump({'version': self.version, 'verify': True}, file) - - def get_state_file_data(self): - """Loads the state file data into a json file and returns it.""" - try: - with open(self.state_file_path, 'r') as file: - return yaml.safe_load(file) - except Exception: - return None + yaml.dump({'version': self.version}, file) def ExtDepFactory(descriptor): diff --git a/edk2toolext/invocables/edk2_platform_build.py b/edk2toolext/invocables/edk2_platform_build.py index 9f9107e5..722f2ea1 100644 --- a/edk2toolext/invocables/edk2_platform_build.py +++ b/edk2toolext/invocables/edk2_platform_build.py @@ -69,14 +69,10 @@ def AddCommandLineOptions(self, parserObj): except (TypeError): raise RuntimeError(f"UefiBuild not found in module:\n{dir(self.PlatformModule)}") - parserObj.add_argument('-nv', '-NV', '--noverify', '--NOVERIFY', '--NoVerify', - dest="verify", default=True, action='store_false', - help='Skip verifying external dependencies before build.') self.PlatformBuilder.AddPlatformCommandLineOptions(parserObj) def RetrieveCommandLineOptions(self, args): """Retrieve command line options from the argparser.""" - self.verify = args.verify self.PlatformBuilder.RetrievePlatformCommandLineOptions(args) def AddParserEpilog(self) -> str: diff --git a/tests.unit/test_edk2_update.py b/tests.unit/test_edk2_update.py index 5413c784..0f7a12bf 100644 --- a/tests.unit/test_edk2_update.py +++ b/tests.unit/test_edk2_update.py @@ -84,7 +84,7 @@ def test_one_level_recursive(self): updater = self.invoke_update(tree.get_settings_provider_path()) # make sure it worked self.assertTrue(os.path.exists(os.path.join(WORKSPACE, "Edk2TestUpdate_extdep", - "NuGet.CommandLine_extdep", "extdep_state.yaml"))) + "NuGet.CommandLine_extdep", "extdep_state.json"))) build_env, shell_env, failure = updater.PerformUpdate() # we should have no failures self.assertEqual(failure, 0) diff --git a/tests.unit/test_self_describing_environment.py b/tests.unit/test_self_describing_environment.py index fda206f7..9ca754ef 100644 --- a/tests.unit/test_self_describing_environment.py +++ b/tests.unit/test_self_describing_environment.py @@ -10,8 +10,6 @@ import unittest import tempfile import git -import yaml -import os from edk2toolext.environment import self_describing_environment from uefi_tree import uefi_tree from edk2toolext.environment import version_aggregator @@ -137,33 +135,6 @@ def test_git_worktree(self): repo.git.worktree("add", "my_worktree") self_describing_environment.BootstrapEnvironment(self.workspace, ('global',)) - def test_no_verify_extdep(self): - tree = uefi_tree(self.workspace, create_platform=False) - tree.create_ext_dep(dep_type="git", - scope="global", - name="HelloWorld", - source="https://github.com/octocat/Hello-World.git", - version="7fd1a60b01f91b314f59955a4e4d4e80d8edf11d") - - # Bootstrap the environment - self_describing_environment.BootstrapEnvironment(self.workspace, ("global",)) - self_describing_environment.UpdateDependencies(self.workspace, scopes=("global",)) - self_describing_environment.VerifyEnvironment(self.workspace, scopes=("global",)) - - # Delete the readme to make the repo dirty then verify it fails - readme = os.path.join(tree.get_workspace(), "HelloWorld_extdep", "HelloWorld", "README") - os.remove(readme) - self.assertFalse(self_describing_environment.VerifyEnvironment(self.workspace, scopes=("global",))) - - # Update the state file to not verify the specific external dependency then verify it passes - state_file = os.path.join(tree.get_workspace(), "HelloWorld_extdep", "extdep_state.yaml") - with open(state_file, 'r+') as f: - content = yaml.safe_load(f) - f.seek(0) - content["verify"] = False - yaml.safe_dump(content, f) - self.assertTrue(self_describing_environment.VerifyEnvironment(self.workspace, scopes=("global",))) - if __name__ == '__main__': unittest.main() From 02368a23c8e7797c5122744c598c63ab48e4457a Mon Sep 17 00:00:00 2001 From: Joey Vagedes Date: Wed, 10 May 2023 08:50:42 -0700 Subject: [PATCH 2/4] continue to use .yaml --- edk2toolext/environment/external_dependency.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/edk2toolext/environment/external_dependency.py b/edk2toolext/environment/external_dependency.py index ef2aec35..e0bddd69 100644 --- a/edk2toolext/environment/external_dependency.py +++ b/edk2toolext/environment/external_dependency.py @@ -65,7 +65,7 @@ def __init__(self, descriptor): self.contents_dir = os.path.join( self.descriptor_location, self.name + "_extdep") self.state_file_path = os.path.join( - self.contents_dir, "extdep_state.json") + self.contents_dir, "extdep_state.yaml") self.published_path = self.compute_published_path() def set_global_cache_path(self, global_cache_path): From 232e041927b4e96bbb8c1d52e629e72b5e560da2 Mon Sep 17 00:00:00 2001 From: Joey Vagedes Date: Wed, 10 May 2023 08:52:42 -0700 Subject: [PATCH 3/4] update --- edk2toolext/invocables/edk2_platform_build.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/edk2toolext/invocables/edk2_platform_build.py b/edk2toolext/invocables/edk2_platform_build.py index 722f2ea1..ae985c00 100644 --- a/edk2toolext/invocables/edk2_platform_build.py +++ b/edk2toolext/invocables/edk2_platform_build.py @@ -103,19 +103,6 @@ def AddParserEpilog(self) -> str: return custom_epilog + epilog - def GetVerifyCheckRequired(self) -> bool: - """Will call self_describing_environment.VerifyEnvironment if this returns True. - - !!! hint - Optional override in a subclass - - Returns: - (bool): whether verify check is required or not - """ - if not self.verify: - logging.warning("Skipping Environment Verification. Unexpected results may occur.") - return self.verify - def GetSettingsClass(self): """Returns the BuildSettingsManager class. From 45b981d81419846aa8a93ca5dd805806153abfb6 Mon Sep 17 00:00:00 2001 From: Joey Vagedes Date: Wed, 10 May 2023 08:53:34 -0700 Subject: [PATCH 4/4] Update --- tests.unit/test_edk2_update.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests.unit/test_edk2_update.py b/tests.unit/test_edk2_update.py index 0f7a12bf..5413c784 100644 --- a/tests.unit/test_edk2_update.py +++ b/tests.unit/test_edk2_update.py @@ -84,7 +84,7 @@ def test_one_level_recursive(self): updater = self.invoke_update(tree.get_settings_provider_path()) # make sure it worked self.assertTrue(os.path.exists(os.path.join(WORKSPACE, "Edk2TestUpdate_extdep", - "NuGet.CommandLine_extdep", "extdep_state.json"))) + "NuGet.CommandLine_extdep", "extdep_state.yaml"))) build_env, shell_env, failure = updater.PerformUpdate() # we should have no failures self.assertEqual(failure, 0)