From 6403b273cec3f4f9d596a251f46c9c7f6f41a677 Mon Sep 17 00:00:00 2001 From: loonghao Date: Mon, 19 Feb 2024 15:42:41 +0800 Subject: [PATCH] Add utility functions to rename a file or folder src to dst with retrying. Signed-off-by: loonghao --- src/rez/package_cache.py | 3 +- src/rez/tests/test_utils_filesystem.py | 77 ++++++++++++++++++++++++++ src/rez/utils/filesystem.py | 65 ++++++++++++++++++++-- 3 files changed, 140 insertions(+), 5 deletions(-) create mode 100644 src/rez/tests/test_utils_filesystem.py diff --git a/src/rez/package_cache.py b/src/rez/package_cache.py index 5a47333099..ccd619e523 100644 --- a/src/rez/package_cache.py +++ b/src/rez/package_cache.py @@ -28,6 +28,7 @@ from rez.utils.logging_ import print_warning from rez.packages import get_variant from rez.system import system +from rez.utils.filesystem import rename class PackageCache(object): @@ -337,7 +338,7 @@ def remove_variant(self, variant): os.chmod(rootpath, st.st_mode | stat.S_IWUSR) # actually a mv - os.rename(rootpath, dest_rootpath) + rename(rootpath, dest_rootpath) except OSError as e: if e.errno == errno.ENOENT: diff --git a/src/rez/tests/test_utils_filesystem.py b/src/rez/tests/test_utils_filesystem.py new file mode 100644 index 0000000000..be5395a14f --- /dev/null +++ b/src/rez/tests/test_utils_filesystem.py @@ -0,0 +1,77 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright Contributors to the Rez Project + + +""" +unit tests for 'rez.utils.filesystem' module +""" +import os.path +import shutil +import tempfile + +from rez.tests.util import TestBase +from rez.utils import filesystem +from rez.utils.platform_ import platform_ +import unittest + + +class TestFileSystem(TestBase): + + def setUp(cls): + super().setUp() + cls.temp_dir = tempfile.mkdtemp(prefix="RezTempDirtsTempDir_") + + def tearDown(self): + super().tearDown() + shutil.rmtree(self.temp_dir, ignore_errors=True) + + def test_windows_rename_folder_with_permission_error(self): + if platform_.name != 'windows': + self.skipTest('This test is on linux/macos, ' + '`os.rename` will definitely return PermissionError, so ignore it.') + src = tempfile.mkdtemp(dir=self.temp_dir) + dst = tempfile.mkdtemp(dir=self.temp_dir) + with unittest.mock.patch("os.rename") as mock_rename: + mock_rename.side_effect = PermissionError("Permission denied") + filesystem.rename(src, dst) + self.assertTrue(os.path.exists(dst)) + self.assertFalse(os.path.exists(src)) + + def test_rename_folder_with_permission_error_and_no_robocopy(self): + src = tempfile.mkdtemp(dir=self.temp_dir) + dst = tempfile.mkdtemp(dir=self.temp_dir) + with unittest.mock.patch("os.rename") as mock_rename: + mock_rename.side_effect = PermissionError("Permission denied") + with unittest.mock.patch("rez.utils.filesystem.which") as mock_which: + mock_which.return_value = False + with self.assertRaises(PermissionError) as err: + filesystem.rename(src, dst) + self.assertEqual(str(err.exception), "Permission denied") + + def test_rename_catch_error(self): + src = tempfile.mkdtemp(dir=self.temp_dir) + dst = tempfile.mkdtemp(dir=self.temp_dir) + with self.assertRaises(FileExistsError): + filesystem.rename(src, dst) + + def test_rename_folder_with_permission_error_and_src_is_file(self): + src = tempfile.mktemp(dir=self.temp_dir) + dst = tempfile.mktemp(dir=self.temp_dir) + with open(src, "w") as file_: + file_.write("content.") + with unittest.mock.patch("os.rename") as mock_rename: + mock_rename.side_effect = PermissionError("Permission denied") + with self.assertRaises(PermissionError) as err: + filesystem.rename(src, dst) + self.assertEqual(str(err.exception), "Permission denied") + self.assertFalse(os.path.exists(dst)) + self.assertTrue(os.path.exists(src)) + + def test_rename_file(self): + src = tempfile.mktemp(dir=self.temp_dir) + dst = tempfile.mktemp(dir=self.temp_dir) + with open(src, "w") as file_: + file_.write("content.") + filesystem.rename(src, dst) + self.assertTrue(os.path.exists(dst)) + self.assertFalse(os.path.exists(src)) diff --git a/src/rez/utils/filesystem.py b/src/rez/utils/filesystem.py index eb825f9729..3514745203 100644 --- a/src/rez/utils/filesystem.py +++ b/src/rez/utils/filesystem.py @@ -23,7 +23,8 @@ import uuid from rez.utils.platform_ import platform_ - +from rez.util import which +from rez.utils.execution import Popen is_windows = platform.system() == "Windows" @@ -213,6 +214,7 @@ def forceful_rmtree(path): * path length over 259 char (on Windows) * unicode path """ + def _on_error(func, path, exc_info): try: if is_windows: @@ -281,7 +283,7 @@ def replace_file_or_dir(dest, source): if not os.path.exists(dest): try: - os.rename(source, dest) + rename(source, dest) return except: if not os.path.exists(dest): @@ -294,8 +296,8 @@ def replace_file_or_dir(dest, source): pass with make_tmp_name(dest) as tmp_dest: - os.rename(dest, tmp_dest) - os.rename(source, dest) + rename(dest, tmp_dest) + rename(source, dest) def additive_copytree(src, dst, symlinks=False, ignore=None): @@ -690,3 +692,58 @@ def windows_long_path(dos_path): path = "\\\\?\\" + path return path + + +def rename(src, dst): + """Utility functions to rename a file or folder src to dst with retrying. + + This function uses the built-in 'os.rename()' method, but in case of a + 'PermissionError', it uses the 'robocopy' command line utility on Windows. + This is done to avoid the frequent 'Access is denied' exception raised by 'os.rename()' on Windows. + + Args: + src (str): The original name (path) of the file or folder. + dst (str): The new name (path) for the file or folder. + + Raises: + OSError: If renaming fails after all attempts. + + References: + - https://github.com/conan-io/conan/blob/develop2/conan/tools/files/files.py#L207 + + """ + try: + os.rename(src, dst) + except PermissionError as err: + if is_windows and which("robocopy") and os.path.isdir(src): + # https://learn.microsoft.com/en-us/windows-server/administration/windows-commands/robocopy + args = [ + "robocopy", + # /move Moves files and directories, and deletes them from the source after they are copied. + "/move", + # /e Copies subdirectories. Note that this option includes empty directories. + "/e", + # /ndl Specifies that directory names are not to be logged. + "/ndl", + # /nfl Specifies that file names are not to be logged. + "/nfl", + # /njs Specifies that there's no job summary. + "/njs", + # /njh Specifies that there's no job header. + "/njh", + # /np Specifies that the progress of the copying operation + # (the number of files or directories copied so far) won't be displayed. + "/np", + # /ns Specifies that file sizes aren't to be logged. + "/ns", + # /nc Specifies that file classes aren't to be logged. + "/nc", + src, + dst, + ] + process = Popen(args) + process.communicate() + if process.returncode > 7: # https://ss64.com/nt/robocopy-exit.html + raise OSError("rename {} to {} failed.".format(src, dst)) + else: + raise err