Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert skip verification #541

Merged
merged 4 commits into from
May 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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()