From 800567dc11c7fc523c5b5d4b7e2a124384ff5e63 Mon Sep 17 00:00:00 2001 From: Sean Brogan Date: Wed, 25 Aug 2021 13:30:27 -0700 Subject: [PATCH] Update to use RemoveTree rather than one-off solutions (#271) A number of places were previously implementing their own redundancies to support cases where shutil.rmdir() fails. Now that more robust support is available under edk2-pytool-library, switch to using this in all places. Co-authored-by: Bret Barkelew --- .../extdeptypes/nuget_dependency.py | 16 ++++----------- .../environment/extdeptypes/web_dependency.py | 3 ++- .../environment/external_dependency.py | 20 ++----------------- edk2toolext/environment/repo_resolver.py | 15 ++------------ edk2toolext/environment/uefi_build.py | 9 ++++----- edk2toolext/tests/test_nuget_dependency.py | 15 ++------------ 6 files changed, 16 insertions(+), 62 deletions(-) diff --git a/edk2toolext/environment/extdeptypes/nuget_dependency.py b/edk2toolext/environment/extdeptypes/nuget_dependency.py index fda1ac89..40c95a25 100644 --- a/edk2toolext/environment/extdeptypes/nuget_dependency.py +++ b/edk2toolext/environment/extdeptypes/nuget_dependency.py @@ -11,7 +11,7 @@ import shutil from io import StringIO from edk2toolext.environment.external_dependency import ExternalDependency -from edk2toollib.utility_functions import RunCmd +from edk2toollib.utility_functions import RunCmd, RemoveTree from edk2toollib.utility_functions import GetHostInfo import pkg_resources from typing import List @@ -237,15 +237,7 @@ def fetch(self): source_dir = os.path.join(temp_directory, package_name) shutil.copytree(source_dir, self.contents_dir) - for _ in range(3): # rmtree doesn't always remove all files. Attempt delete up to 3 times. - try: - shutil.rmtree(source_dir) - except OSError as err: - logging.warning(f"Failed to fully remove {source_dir}: {err}") - else: - break - else: - raise RuntimeError(f"Failed to remove {source_dir}") + RemoveTree(source_dir) # # Add a file to track the state of the dependency. @@ -255,7 +247,7 @@ def fetch(self): # # Finally, delete the temp directory. # - self._clean_directory(temp_directory) + RemoveTree(temp_directory) # The published path may change now that the package has been unpacked. self.published_path = self.compute_published_path() @@ -266,4 +258,4 @@ def get_temp_dir(self): def clean(self): super(NugetDependency, self).clean() if os.path.isdir(self.get_temp_dir()): - self._clean_directory(self.get_temp_dir()) + RemoveTree(self.get_temp_dir()) diff --git a/edk2toolext/environment/extdeptypes/web_dependency.py b/edk2toolext/environment/extdeptypes/web_dependency.py index fe9f2f9e..10d6d6e6 100644 --- a/edk2toolext/environment/extdeptypes/web_dependency.py +++ b/edk2toolext/environment/extdeptypes/web_dependency.py @@ -16,6 +16,7 @@ import urllib.error import urllib.request from edk2toolext.environment.external_dependency import ExternalDependency +from edk2toollib.utility_functions import RemoveTree class WebDependency(ExternalDependency): @@ -137,7 +138,7 @@ def fetch(self): # If the unzipped directory still exists, delete it. if os.path.isdir(temp_folder): logging.debug(f"Cleaning up {temp_folder}") - shutil.rmtree(temp_folder) + RemoveTree(temp_folder) # If we just downloaded a file, we need to create a directory named self.contents_dir, # copy the file inside, and name it self.internal_path diff --git a/edk2toolext/environment/external_dependency.py b/edk2toolext/environment/external_dependency.py index 8b627e0f..568dde48 100644 --- a/edk2toolext/environment/external_dependency.py +++ b/edk2toolext/environment/external_dependency.py @@ -11,11 +11,9 @@ import os import logging -import shutil -import time import yaml from edk2toolext.environment import version_aggregator -from edk2toollib.utility_functions import GetHostInfo +from edk2toollib.utility_functions import GetHostInfo, RemoveTree class ExternalDependency(object): @@ -95,24 +93,10 @@ def compute_published_path(self): return new_published_path - def _clean_directory(self, dir_path): - retry = 1 - while True: - try: - shutil.rmtree(dir_path) - except OSError: - if not retry: - # If we're out of retries, bail. - raise - time.sleep(5) - retry -= 1 - continue - break - def clean(self): logging.debug("Cleaning dependency directory for '%s'..." % self.name) if os.path.isdir(self.contents_dir): - self._clean_directory(self.contents_dir) + RemoveTree(self.contents_dir) def fetch(self): # The base class does not implement a fetch. diff --git a/edk2toolext/environment/repo_resolver.py b/edk2toolext/environment/repo_resolver.py index 6616b96b..e6eab2c0 100644 --- a/edk2toolext/environment/repo_resolver.py +++ b/edk2toolext/environment/repo_resolver.py @@ -8,10 +8,9 @@ import os import logging -import shutil -import stat from edk2toolext import edk2_logging from edk2toolext.edk2_git import Repo +from edk2toollib.utility_functions import RemoveTree # this follows a documented flow chart @@ -152,20 +151,10 @@ def clear_folder(abs_file_system_path): logger = logging.getLogger("git") logger.warning("WARNING: Deleting contents of folder {0} to make way for Git repo".format( abs_file_system_path)) + RemoveTree(abs_file_system_path) - # spell-checker:ignore dorw - def dorw(action, name, exc): - os.chmod(name, stat.S_IWRITE) - if(os.path.isdir(name)): - os.rmdir(name) - else: - os.remove(name) - - shutil.rmtree(abs_file_system_path, onerror=dorw) # Clones the repo in the folder we need using the dependency object from the json - - def clone_repo(abs_file_system_path, DepObj): logger = logging.getLogger("git") logger.log(edk2_logging.get_progress_level(), "Cloning repo: {0}".format(DepObj["Url"])) diff --git a/edk2toolext/environment/uefi_build.py b/edk2toolext/environment/uefi_build.py index 071146ce..a3d7d466 100644 --- a/edk2toolext/environment/uefi_build.py +++ b/edk2toolext/environment/uefi_build.py @@ -14,13 +14,12 @@ from edk2toolext.environment.multiple_workspace import MultipleWorkspace from edk2toolext.environment import conf_mgmt import traceback -import shutil import time from edk2toolext.environment import shell_environment from edk2toollib.uefi.edk2.parsers.targettxt_parser import TargetTxtParser from edk2toollib.uefi.edk2.parsers.dsc_parser import DscParser from edk2toollib.uefi.edk2.parsers.fdf_parser import FdfParser -from edk2toollib.utility_functions import RunCmd +from edk2toollib.utility_functions import RunCmd, RemoveTree from edk2toolext import edk2_logging from edk2toolext.environment.plugintypes.uefi_build_plugin import IUefiBuildPlugin import datetime @@ -199,7 +198,7 @@ def CleanTree(self, RemoveConfTemplateFilesToo=False): logging.debug("Removing [%s]", d) # if the folder is opened in Explorer do not fail the entire Rebuild try: - shutil.rmtree(d) + RemoveTree(d) except WindowsError as wex: logging.debug(wex) @@ -210,15 +209,15 @@ def CleanTree(self, RemoveConfTemplateFilesToo=False): # this needs to be removed in case build flags changed d = os.path.join(self.ws, "Conf", ".cache") if(os.path.isdir(d)): - shutil.rmtree(d) logging.debug("Removing [%s]" % d) + RemoveTree(d) if(RemoveConfTemplateFilesToo): for a in ["target.txt", "build_rule.txt", "tools_def.txt"]: d = os.path.join(self.ws, "Conf", a) if(os.path.isfile(d)): - os.remove(d) logging.debug("Removing [%s]" % d) + os.remove(d) return ret diff --git a/edk2toolext/tests/test_nuget_dependency.py b/edk2toolext/tests/test_nuget_dependency.py index ba7f68e9..b766058a 100644 --- a/edk2toolext/tests/test_nuget_dependency.py +++ b/edk2toolext/tests/test_nuget_dependency.py @@ -10,11 +10,9 @@ import sys import unittest import logging -import shutil -import stat import tempfile import pkg_resources -from edk2toollib.utility_functions import RunCmd +from edk2toollib.utility_functions import RunCmd, RemoveTree from edk2toolext.environment import environment_descriptor_files as EDF from edk2toolext.environment.extdeptypes.nuget_dependency import NugetDependency from edk2toolext.environment import version_aggregator @@ -54,16 +52,7 @@ def clean_workspace(): return if os.path.isdir(test_dir): - - # spell-checker:ignore dorw - def dorw(action, name, exc): - os.chmod(name, stat.S_IWRITE) - if(os.path.isdir(name)): - os.rmdir(name) - else: - os.remove(name) - - shutil.rmtree(test_dir, onerror=dorw) + RemoveTree(test_dir) test_dir = None