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

Handle race condition caused by AppleDouble files when removing a directory tree #1716

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
7 changes: 4 additions & 3 deletions install.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
# though rez is not yet built.
#
from rez.utils._version import _rez_version # noqa: E402
from rez.utils.filesystem import safe_rmtree
from rez.utils.which import which # noqa: E402
from rez.cli._entry_points import get_specifications # noqa: E402
from rez.vendor.distlib.scripts import ScriptMaker # noqa: E402
Expand Down Expand Up @@ -106,7 +107,7 @@ def patch_rez_binaries(dest_dir):
# we don't want resolved envs accidentally getting the virtualenv's 'python'.
dest_bin_path = os.path.join(virtualenv_bin_path, "rez")
if os.path.exists(dest_bin_path):
shutil.rmtree(dest_bin_path)
safe_rmtree(dest_bin_path)
os.makedirs(dest_bin_path)

maker = ScriptMaker(
Expand Down Expand Up @@ -140,7 +141,7 @@ def copy_completion_scripts(dest_dir):
if completion_path:
dest_path = os.path.join(dest_dir, "completion")
if os.path.exists(dest_path):
shutil.rmtree(dest_path)
safe_rmtree(dest_path)
shutil.copytree(completion_path, dest_path)
return dest_path

Expand Down Expand Up @@ -263,7 +264,7 @@ def install_as_rez_package(repo_path):
finally:
# cleanup temp install
try:
shutil.rmtree(tmpdir)
safe_rmtree(tmpdir)
except:
pass

Expand Down
6 changes: 3 additions & 3 deletions src/rez/bind/hello_world.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@
from rez.version import Version
from rez.utils.lint_helper import env
from rez.utils.execution import create_executable_script, ExecutableScriptMode
from rez.utils.filesystem import safe_rmtree
from rez.vendor.distlib.scripts import ScriptMaker
from rez.bind._utils import make_dirs, check_version
import os.path
import shutil


def commands():
Expand Down Expand Up @@ -55,13 +55,13 @@ def make_root(variant, root):
py_script_mode=ExecutableScriptMode.single,
)

# We want to use ScriptMaker on all platofrms. This allows us to
# We want to use ScriptMaker on all platforms. This allows us to
# correctly setup the script to work everywhere, even on Windows.
# create_executable_script should be fixed to use ScriptMaker
# instead.
maker = ScriptMaker(binpathtmp, make_dirs(binpath))
maker.make("hello_world")
shutil.rmtree(binpathtmp)
safe_rmtree(binpathtmp)

with make_package("hello_world", path, make_root=make_root) as pkg:
pkg.version = version
Expand Down
4 changes: 2 additions & 2 deletions src/rez/cli/selftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import sys
import inspect
import argparse
import shutil
from rez.utils.filesystem import safe_rmtree
from pkgutil import iter_modules

try:
Expand Down Expand Up @@ -98,7 +98,7 @@ def command(opts, parser, extra_arg_groups=None):
else:
run_unittest(module_tests, opts.tests, opts.verbose)
finally:
shutil.rmtree(repo)
safe_rmtree(repo)


def run_unittest(module_tests, tests, verbosity):
Expand Down
3 changes: 2 additions & 1 deletion src/rez/pip.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from rez.vendor.packaging.specifiers import Specifier
from rez.resolved_context import ResolvedContext
from rez.utils.execution import Popen
from rez.utils.filesystem import safe_rmtree
from rez.utils.pip import get_rez_requirements, pip_to_rez_package_name, \
pip_to_rez_version
from rez.utils.logging_ import print_debug, print_info, print_error, \
Expand Down Expand Up @@ -461,7 +462,7 @@ def make_root(variant, path):
log_append_pkg_variants(pkg)

# cleanup
shutil.rmtree(targetpath)
safe_rmtree(targetpath)

# print summary
#
Expand Down
4 changes: 2 additions & 2 deletions src/rez/suite.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from rez.exceptions import SuiteError, ResolvedContextError
from rez.resolved_context import ResolvedContext
from rez.utils.data_utils import cached_property
from rez.utils.filesystem import safe_rmtree
from rez.utils.formatting import columnise, PackageRequest
from rez.utils.colorize import warning, critical, Printer, alias as alias_col
from rez.vendor import yaml
Expand All @@ -14,7 +15,6 @@
from collections import defaultdict
import os
import os.path
import shutil
import sys


Expand Down Expand Up @@ -436,7 +436,7 @@ def save(self, path, verbose=False):
print("saving over previous suite...")
for context_name in self.context_names:
self.context(context_name) # load before dir deleted
shutil.rmtree(path)
safe_rmtree(path)
else:
raise SuiteError("Cannot save, path exists: %r" % path)

Expand Down
31 changes: 28 additions & 3 deletions src/rez/utils/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from rez.utils.execution import Popen

is_windows = platform.system() == "Windows"
is_mac = platform.system() == "Darwin"


class TempDirs(object):
Expand Down Expand Up @@ -69,7 +70,7 @@ def clear(self):

for path in dirs:
if os.path.exists(path) and not os.getenv("REZ_KEEP_TMPDIRS"):
shutil.rmtree(path)
safe_rmtree(path)

@classmethod
def clear_all(cls):
Expand Down Expand Up @@ -196,7 +197,7 @@ def safe_remove(path):

try:
if os.path.isdir(path) and not os.path.islink(path):
shutil.rmtree(path)
safe_rmtree(path)
else:
os.remove(path)
except OSError:
Expand All @@ -213,9 +214,15 @@ def forceful_rmtree(path):
Also handled:
* path length over 259 char (on Windows)
* unicode path
* AppleDouble resource forks
"""

def _on_error(func, path, exc_info):
if is_mac and exc_info[0] is FileNotFoundError and os.path.basename(path).startswith("._"):
# Assume if we are on a mac and the file starts with a "._" it is a resource fork and the
# corresponding data fork has been removed. Since we are removing the whole directory tree
# it should not be a problem.
return
try:
if is_windows:
path = windows_long_path(path)
Expand All @@ -238,6 +245,24 @@ def _on_error(func, path, exc_info):
shutil.rmtree(path, onerror=_on_error)


def safe_rmtree(path):
"""Like shutil.rmtree, but handles race condition caused by AppleDouble files.

On Mac OSX files may consist of a data fork and a resource fork. On a foreign file system these files
are stored as AppleDouble files. The data fork is stored as "filename" and the resource fork is stored
as "._filename". When the data fork is removed the corresponding resource fork is also removed. This
results in a FileNotFoundError when `shutil.rmtree` tries to remove the resource fork. This is addressed
in Python 13.3 for another situation not related to AppleDouble files
(https://github.com/python/cpython/pull/14064)
"""

def _on_error(_func, path, exc_info):
if not is_mac or exc_info[0] is not FileNotFoundError or not os.path.basename(path).startswith("._"):
raise

shutil.rmtree(path, onerror=_on_error)


def replacing_symlink(source, link_name):
"""Create symlink that overwrites any existing target.
"""
Expand Down Expand Up @@ -479,7 +504,7 @@ def movetree(src, dst):
shutil.move(src, dst)
except:
copytree(src, dst, symlinks=True, hardlinks=True)
shutil.rmtree(src)
safe_rmtree(src)


def safe_chmod(path, mode):
Expand Down
7 changes: 3 additions & 4 deletions src/rezplugins/package_repository/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import stat
import errno
import time
import shutil

from rez.package_repository import PackageRepository
from rez.package_resources import PackageFamilyResource, VariantResourceHelper, \
Expand All @@ -29,7 +28,7 @@
from rez.utils.logging_ import print_warning, print_info
from rez.utils.memcached import memcached, pool_memcached_connections
from rez.utils.filesystem import make_path_writable, \
canonical_path, is_subdirectory
canonical_path, is_subdirectory, safe_rmtree
from rez.utils.platform_ import platform_
from rez.utils.yaml import load_yaml
from rez.config import config
Expand Down Expand Up @@ -728,7 +727,7 @@ def remove_package(self, pkg_name, pkg_version):

# delete the payload
pkg_dir = os.path.join(self.location, pkg_name, str(pkg_version))
shutil.rmtree(pkg_dir)
safe_rmtree(pkg_dir)

# unignore (just so the .ignore{ver} file is removed)
self.unignore_package(pkg_name, pkg_version)
Expand Down Expand Up @@ -760,7 +759,7 @@ def remove_package_family(self, pkg_name, force=False):

# delete the fam dir
fam_dir = os.path.join(self.location, pkg_name)
shutil.rmtree(fam_dir)
safe_rmtree(fam_dir)

self._on_changed(pkg_name)
return True
Expand Down