Skip to content

Commit

Permalink
Revert skip verification (#541)
Browse files Browse the repository at this point in the history
The ability to skip external dependency verification was added with the anticipation of additional developer workflows, however those workflows have evolved over time and the ability to skip external dependency verification is no longer necessary. As it is no longer necessary, it only adds additional complexity to the codebase and confusion to developers attempting to use it. Removing this functionality is the best way forward, but can always be reconsidered or reworked in the future if necessary.
  • Loading branch information
Javagedes authored May 11, 2023
1 parent 0a2e20c commit 751ebf4
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 72 deletions.
12 changes: 1 addition & 11 deletions edk2toolext/environment/extdeptypes/git_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
29 changes: 14 additions & 15 deletions edk2toolext/environment/external_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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):
Expand Down
17 changes: 0 additions & 17 deletions edk2toolext/invocables/edk2_platform_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -107,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.
Expand Down
29 changes: 0 additions & 29 deletions tests.unit/test_self_describing_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()

0 comments on commit 751ebf4

Please sign in to comment.