From 886d7bf2bb7bc76902edae69ed67b78af695c01d Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 15 Aug 2023 16:26:30 +0200 Subject: [PATCH 1/4] Add `makedirs(..., exits_ok)` for Python2 --- easybuild/tools/py2vs3/py2.py | 10 ++++++++++ easybuild/tools/py2vs3/py3.py | 1 + test/framework/filetools.py | 11 +++++++++++ 3 files changed, 22 insertions(+) diff --git a/easybuild/tools/py2vs3/py2.py b/easybuild/tools/py2vs3/py2.py index 99f8cb0f4a..08a3b52b72 100644 --- a/easybuild/tools/py2vs3/py2.py +++ b/easybuild/tools/py2vs3/py2.py @@ -33,8 +33,10 @@ """ # these are not used here, but imported from here in other places import ConfigParser as configparser # noqa +import errno import imp import json +import os import subprocess import time import urllib2 as std_urllib # noqa @@ -115,3 +117,11 @@ def sort_looseversions(looseversions): # with Python 2, we can safely use 'sorted' on LooseVersion instances # (but we can't in Python 3, see https://bugs.python.org/issue14894) return sorted(looseversions) + + +def makedirs(name, mode=0o777, exist_ok=False): + try: + os.makedirs(name, mode) + except OSError as e: + if not exist_ok or e.errno != errno.EEXIST: + raise diff --git a/easybuild/tools/py2vs3/py3.py b/easybuild/tools/py2vs3/py3.py index 77f786cbec..8d9145179d 100644 --- a/easybuild/tools/py2vs3/py3.py +++ b/easybuild/tools/py2vs3/py3.py @@ -44,6 +44,7 @@ from html.parser import HTMLParser # noqa from itertools import zip_longest from io import StringIO # noqa +from os import makedirs # noqa from string import ascii_letters, ascii_lowercase # noqa from urllib.request import HTTPError, HTTPSHandler, Request, URLError, build_opener, urlopen # noqa from urllib.parse import urlencode # noqa diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 32d72c7b83..14011e2cbc 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -45,6 +45,7 @@ from unittest import TextTestRunner from easybuild.tools import run import easybuild.tools.filetools as ft +import easybuild.tools.py2vs3 as py2vs3 from easybuild.tools.build_log import EasyBuildError from easybuild.tools.config import IGNORE, ERROR, build_option, update_build_option from easybuild.tools.multidiff import multidiff @@ -3389,6 +3390,16 @@ def test_set_gid_sticky_bits(self): self.assertEqual(dir_perms & stat.S_ISGID, stat.S_ISGID) self.assertEqual(dir_perms & stat.S_ISVTX, stat.S_ISVTX) + def test_compat_makedirs(self): + """Test compatibility layer for Python3 os.makedirs""" + name = os.path.join(self.test_prefix, 'folder') + self.assertNotExists(name) + py2vs3.makedirs(name) + self.assertExists(name) + self.assertErrorRegex(Exception, os.path.basename(name), py2vs3.makedirs, name) + py2vs3.makedirs(name, exist_ok=True) # No error + self.assertExists(name) + def test_create_unused_dir(self): """Test create_unused_dir function.""" path = ft.create_unused_dir(self.test_prefix, 'folder') From 22def640ae0bfb96280389e96347a015837aecc3 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 15 Aug 2023 16:32:51 +0200 Subject: [PATCH 2/4] Don't rely on errno Same as the Python3 implementation which has a comment: > Cannot rely on checking for EEXIST, since the operating system could give priority to other errors like EACCES or EROFS --- easybuild/tools/py2vs3/py2.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/easybuild/tools/py2vs3/py2.py b/easybuild/tools/py2vs3/py2.py index 08a3b52b72..1fec4650b1 100644 --- a/easybuild/tools/py2vs3/py2.py +++ b/easybuild/tools/py2vs3/py2.py @@ -33,7 +33,6 @@ """ # these are not used here, but imported from here in other places import ConfigParser as configparser # noqa -import errno import imp import json import os @@ -122,6 +121,6 @@ def sort_looseversions(looseversions): def makedirs(name, mode=0o777, exist_ok=False): try: os.makedirs(name, mode) - except OSError as e: - if not exist_ok or e.errno != errno.EEXIST: + except OSError: + if not exist_ok or not os.path.isdir(name): raise From 204814347b05ce0b5977783aae59e83ee2e219ef Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 15 Aug 2023 16:36:02 +0200 Subject: [PATCH 3/4] Remove use of Python3 `FileExistsError` Use the new py2vpy3.makedirs compat layer. --- easybuild/tools/filetools.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index d16d4f4778..a5e5f63ec6 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -65,7 +65,7 @@ from easybuild.tools.config import DEFAULT_WAIT_ON_LOCK_INTERVAL, ERROR, GENERIC_EASYBLOCK_PKG, IGNORE, WARN from easybuild.tools.config import build_option, install_path from easybuild.tools.output import PROGRESS_BAR_DOWNLOAD_ONE, start_progress_bar, stop_progress_bar, update_progress_bar -from easybuild.tools.py2vs3 import HTMLParser, load_source, std_urllib, string_type +from easybuild.tools.py2vs3 import HTMLParser, load_source, makedirs, std_urllib, string_type from easybuild.tools.utilities import natural_keys, nub, remove_unwanted_chars, trace_msg try: @@ -1918,15 +1918,9 @@ def mkdir(path, parents=False, set_gid=None, sticky=None): # climb up until we hit an existing path or the empty string (for relative paths) while existing_parent_path and not os.path.exists(existing_parent_path): existing_parent_path = os.path.dirname(existing_parent_path) - os.makedirs(path) + makedirs(path, exist_ok=True) else: os.mkdir(path) - except FileExistsError as err: - if os.path.exists(path): - # This may happen if a parallel build creates the directory after we checked for its existence - _log.debug("Directory creation aborted as it seems it was already created: %s", err) - else: - raise EasyBuildError("Failed to create directory %s: %s", path, err) except OSError as err: raise EasyBuildError("Failed to create directory %s: %s", path, err) From 40f9fe761c1810e45ec407c9343647a3283982e8 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Tue, 15 Aug 2023 18:37:21 +0200 Subject: [PATCH 4/4] use liberal pattern to check whether makedirs wrapper raises an exception if directory already exists, so the check passes with both Python 2 and 3 --- test/framework/filetools.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 14011e2cbc..b87b9359b0 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -3396,7 +3396,8 @@ def test_compat_makedirs(self): self.assertNotExists(name) py2vs3.makedirs(name) self.assertExists(name) - self.assertErrorRegex(Exception, os.path.basename(name), py2vs3.makedirs, name) + # exception is raised because file exists (OSError in Python 2, FileExistsError in Python 3) + self.assertErrorRegex(Exception, '.*', py2vs3.makedirs, name) py2vs3.makedirs(name, exist_ok=True) # No error self.assertExists(name)