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 <[email protected]>

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())

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

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.

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"]))

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
 

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