Fix race condition in FileSystemPackageRepository directory creation, modernize usage of os.makedirs

src/rez/bind/_utils.py

@@ -26,8 +26,7 @@ def log(msg):
 
 def make_dirs(*dirs):
     path = os.path.join(*dirs)
-    if not os.path.exists(path):
-        os.makedirs(path)
+    os.makedirs(path, exist_ok=True)
     return path
 
 

src/rez/package_cache.py

@@ -23,8 +23,7 @@
 from rez.exceptions import PackageCacheError
 from rez.vendor.lockfile import LockFile, NotLocked
 from rez.vendor.progress.spinner import PixelSpinner
-from rez.utils.filesystem import safe_listdir, safe_makedirs, safe_remove, \
-    forceful_rmtree
+from rez.utils.filesystem import forceful_rmtree, safe_listdir, safe_remove
 from rez.utils.colorize import ColorizedStreamHandler
 from rez.utils.logging_ import print_warning
 from rez.packages import get_variant
@@ -99,9 +98,9 @@ def __init__(self, path):
         self.path = path
 
         # make dirs for internal use
-        safe_makedirs(self._log_dir)
-        safe_makedirs(self._pending_dir)
-        safe_makedirs(self._remove_dir)
+        os.makedirs(self._log_dir, exist_ok=True)
+        os.makedirs(self._pending_dir, exist_ok=True)
+        os.makedirs(self._remove_dir, exist_ok=True)
 
     def get_cached_root(self, variant):
         """Get location of variant payload copy.
@@ -171,7 +170,6 @@ def add_variant(self, variant, force=False, wait_for_copying=False, logger=None)
             - int: One of VARIANT_FOUND, VARIANT_CREATED, VARIANT_COPYING, VARIANT_COPY_STALLED
         """
         from rez.utils.base26 import get_next_base26
-        from rez.utils.filesystem import safe_makedirs
 
         # do some sanity checking on variant to cache
         package = variant.parent
@@ -262,7 +260,7 @@ def add_variant(self, variant, force=False, wait_for_copying=False, logger=None)
 
         # 1.
         path = self._get_hash_path(variant)
-        safe_makedirs(path)
+        os.makedirs(path, exist_ok=True)
 
         # construct data to store to json file
         data = {

src/rez/package_copy.py

@@ -17,7 +17,7 @@
 from rez.utils.sourcecode import IncludeModuleManager
 from rez.utils.logging_ import print_info, print_warning
 from rez.utils.filesystem import replacing_symlink, replacing_copy, \
-    safe_makedirs, additive_copytree, make_path_writable, get_existing_path
+    additive_copytree, make_path_writable, get_existing_path
 
 
 def copy_package(package, dest_repository, variants=None, shallow=False,
@@ -287,7 +287,7 @@ def _copy_variant_payload(src_variant, dest_pkg_repo, shallow=False,
 
     # copy the variant payload
     with ctxt:
-        safe_makedirs(variant_install_path)
+        os.makedirs(variant_install_path, exist_ok=True)
 
         # determine files not to copy
         skip_files = []
@@ -362,7 +362,7 @@ def _copy_variant_payload(src_variant, dest_pkg_repo, shallow=False,
                 src_package.config.variant_shortlinks_dirname
             )
 
-            safe_makedirs(base_shortlinks_path)
+            os.makedirs(base_shortlinks_path, exist_ok=True)
 
             # shortlink
             rel_variant_path = os.path.relpath(
@@ -425,5 +425,5 @@ def _copy_package_include_modules(src_package, dest_pkg_repo, overrides=None):
         ctxt = with_noop()
 
     with ctxt:
-        safe_makedirs(dest_include_modules_path)
+        os.makedirs(dest_include_modules_path, exist_ok=True)
         additive_copytree(src_include_modules_path, dest_include_modules_path)

src/rez/package_maker.py

@@ -220,15 +220,13 @@ def make_package(name, path, make_base=None, make_root=None, skip_existing=True,
 
             base = variant_.base
             if make_base and base:
-                if not os.path.exists(base):
-                    os.makedirs(base)
+                os.makedirs(base, exist_ok=True)
                 os.chdir(base)
                 make_base(variant_, base)
 
             root = variant_.root
             if make_root and root:
-                if not os.path.exists(root):
-                    os.makedirs(root)
+                os.makedirs(root, exist_ok=True)
                 os.chdir(root)
                 make_root(variant_, root)
 

src/rez/pip.py

@@ -386,8 +386,7 @@ def make_root(variant, path):
                 src = os.path.join(targetpath, rel_src)
                 dest = os.path.join(path, rel_dest)
 
-                if not os.path.exists(os.path.dirname(dest)):
-                    os.makedirs(os.path.dirname(dest))
+                os.makedirs(os.path.dirname(dest), exist_ok=True)
 
                 shutil.copyfile(src, dest)
 

src/rez/suite.py

@@ -441,7 +441,7 @@ def save(self, path, verbose=False):
                 raise SuiteError("Cannot save, path exists: %r" % path)
 
         contexts_path = os.path.join(path, "contexts")
-        os.makedirs(contexts_path)
+        os.makedirs(contexts_path, exist_ok=True)
 
         # write suite data
         data = self.to_dict()
@@ -460,7 +460,7 @@ def save(self, path, verbose=False):
 
         # create alias wrappers
         tools_path = os.path.join(path, "bin")
-        os.makedirs(tools_path)
+        os.makedirs(tools_path, exist_ok=True)
         if verbose:
             print("creating alias wrappers in %r..." % tools_path)
 

src/rez/tests/test_packages.py

@@ -347,8 +347,7 @@ def test_variant_iteration(self):
     def test_variant_install(self):
         """test variant installation."""
         repo_path = os.path.join(self.root, "packages")
-        if not os.path.exists(repo_path):
-            os.makedirs(repo_path)
+        os.makedirs(repo_path, exist_ok=True)
 
         def _data(obj):
             d = obj.validated_data()

src/rez/utils/filesystem.py

@@ -173,19 +173,6 @@ def safe_listdir(path):
         raise
 
 
-def safe_makedirs(path):
-    """Safe makedirs.
-
-    Works in a multithreaded scenario.
-    """
-    if not os.path.exists(path):
-        try:
-            os.makedirs(path)
-        except OSError:
-            if not os.path.exists(path):
-                raise
-
-
 def safe_remove(path):
     """Safely remove the given file or directory.
 
@@ -303,8 +290,7 @@ def replace_file_or_dir(dest, source):
 def additive_copytree(src, dst, symlinks=False, ignore=None):
     """Version of `copytree` that merges into an existing directory.
     """
-    if not os.path.exists(dst):
-        os.makedirs(dst)
+    os.makedirs(dst, exist_ok=True)
 
     for item in os.listdir(src):
         s = os.path.join(src, item)
@@ -437,8 +423,7 @@ def copy(srcname, dstname):
     else:
         copy = shutil.copy2
 
-    if not os.path.isdir(dst):
-        os.makedirs(dst)
+    os.makedirs(dst, exist_ok=True)
 
     errors = []
     for name in names:

src/rez/utils/py_dist.py

@@ -16,8 +16,7 @@
 
 def _mkdirs(*dirs):
     path = os.path.join(*dirs)
-    if not os.path.exists(path):
-        os.makedirs(path)
+    os.makedirs(path, exist_ok=True)
     return path
 
 

src/rezplugins/build_process/local.py

@@ -14,8 +14,8 @@
 from rez.utils.logging_ import print_warning
 from rez.utils.base26 import create_unique_base26_symlink
 from rez.utils.colorize import Printer, warning
-from rez.utils.filesystem import safe_makedirs, copy_or_replace, \
-    make_path_writable, get_existing_path, forceful_rmtree
+from rez.utils.filesystem import copy_or_replace, get_existing_path, \
+    forceful_rmtree, make_path_writable
 from rez.utils.sourcecode import IncludeModuleManager
 from rez.utils.filesystem import TempDirs
 from rez.package_test import PackageTestRunner, PackageTestResults
@@ -148,7 +148,7 @@ def _build_variant_base(self, variant, build_type, install_path=None,
         if clean and os.path.exists(variant_build_path):
             self._rmtree(variant_build_path)
 
-        safe_makedirs(variant_build_path)
+        os.makedirs(variant_build_path, exist_ok=True)
 
         # find last dir of installation path that exists, and possibly make it
         # writable during variant installation
@@ -167,8 +167,7 @@ def _build_variant_base(self, variant, build_type, install_path=None,
                 pkg_repo = package_repository_manager.get_repository(install_path)
                 pkg_repo.pre_variant_install(variant.resource)
 
-                if not os.path.exists(variant_install_path):
-                    safe_makedirs(variant_install_path)
+                os.makedirs(variant_install_path, exist_ok=True)
 
                 # if hashed variants are enabled, create the variant shortlink
                 if variant.parent.hashed_variants:
@@ -179,7 +178,7 @@ def _build_variant_base(self, variant, build_type, install_path=None,
                             variant.parent.config.variant_shortlinks_dirname
                         )
 
-                        safe_makedirs(base_shortlinks_path)
+                        os.makedirs(base_shortlinks_path, exist_ok=True)
 
                         # create the shortlink
                         rel_variant_path = os.path.relpath(
@@ -293,7 +292,7 @@ def _install_include_modules(self, install_path):
         base_path = self.get_package_install_path(install_path)
 
         path = os.path.join(base_path, IncludeModuleManager.include_modules_subpath)
-        safe_makedirs(path)
+        os.makedirs(path, exist_ok=True)
 
         definition_python_path = self.package.config.package_definition_python_path
 

src/rezplugins/package_repository/filesystem.py

@@ -10,7 +10,6 @@
 import os.path
 import os
 import stat
-import errno
 import time
 import shutil
 
@@ -677,10 +676,7 @@ def ignore_package(self, pkg_name, pkg_version, allow_missing=False):
             return 0
 
         # create .ignore{ver} file
-        try:
-            os.makedirs(fam_path)
-        except OSError:  # already exists
-            pass
+        os.makedirs(fam_path, exist_ok=True)
 
         with open(filepath, 'w'):
             pass
@@ -861,8 +857,7 @@ def pre_variant_install(self, variant_resource):
         path = self.location
 
         family_path = os.path.join(path, variant_resource.name)
-        if not os.path.isdir(family_path):
-            os.makedirs(family_path)
+        os.makedirs(family_path, exist_ok=True)
 
         filename = self.building_prefix + str(variant_resource.version)
         filepath = os.path.join(family_path, filename)
@@ -931,13 +926,12 @@ def install_variant(self, variant_resource, dry_run=False, overrides=None):
         path = self.location
 
         try:
-            os.makedirs(path)
+            os.makedirs(path, exist_ok=True)
         except OSError as e:
-            if e.errno != errno.EEXIST:
-                raise PackageRepositoryError(
-                    "Package repository path %r could not be created: %s: %s"
-                    % (path, e.__class__.__name__, e)
-                )
+            raise PackageRepositoryError(
+                "Package repository path %r could not be created: %s: %s"
+                % (path, e.__class__.__name__, e)
+            )
 
         # install the variant
         def _create_variant():
@@ -1194,8 +1188,7 @@ def _get_file(self, path, package_filename=None):
 
     def _create_family(self, name):
         path = os.path.join(self.location, name)
-        if not os.path.exists(path):
-            os.makedirs(path)
+        os.makedirs(path, exist_ok=True)
 
         self._on_changed(name)
         return self.get_package_family(name)
@@ -1397,8 +1390,7 @@ def _remove_build_keys(obj):
             pkg_base_path = os.path.join(family_path, str(variant_version))
         else:
             pkg_base_path = family_path
-        if not os.path.exists(pkg_base_path):
-            os.makedirs(pkg_base_path)
+        os.makedirs(pkg_base_path, exist_ok=True)
 
         # Apply overrides.
         #