From bc1aa6fa7f8278756f808708e85499008d46d28f Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sat, 19 Oct 2024 17:08:10 -0400 Subject: [PATCH 1/5] Restore expectation that mkpath returns a list of the items it created. Closes #305 --- distutils/dir_util.py | 2 +- distutils/tests/test_dir_util.py | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/distutils/dir_util.py b/distutils/dir_util.py index 3b22839d27..3fa4d7f4c3 100644 --- a/distutils/dir_util.py +++ b/distutils/dir_util.py @@ -58,7 +58,7 @@ def mkpath(name: pathlib.Path, mode=0o777, verbose=True, dry_run=False): log.info("creating %s", name) ancestry = itertools.chain((name,), name.parents) - missing = (path for path in ancestry if not path.is_dir()) + missing = list(path for path in ancestry if not path.is_dir()) try: dry_run or name.mkdir(mode=mode, parents=True, exist_ok=True) diff --git a/distutils/tests/test_dir_util.py b/distutils/tests/test_dir_util.py index 12e643ab74..0a6ff431ac 100644 --- a/distutils/tests/test_dir_util.py +++ b/distutils/tests/test_dir_util.py @@ -52,6 +52,17 @@ def test_mkpath_with_custom_mode(self): mkpath(self.target2, 0o555) assert stat.S_IMODE(os.stat(self.target2).st_mode) == 0o555 & ~umask + def test_mkpath_returns_as_described(self, tmp_path): + """ + Per the docstring, mkpath should return the list of directories created. + + pypa/distutils#305 revealed that the return value is always the empty + string and no one complained. Consider removing this expectation. + """ + target = tmp_path / 'foodir' + res = mkpath(target) + assert res == [str(target)] + def test_create_tree_verbosity(self, caplog): create_tree(self.root_target, ['one', 'two', 'three'], verbose=False) assert caplog.messages == [] From ce4b760e88c9eca60ae21f57d58ec8f1ac88ac50 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sat, 19 Oct 2024 17:13:20 -0400 Subject: [PATCH 2/5] Remove support for returning the directories created in mkpath, unused. Closes #306 --- distutils/dir_util.py | 8 +------- distutils/tests/test_dir_util.py | 11 ----------- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/distutils/dir_util.py b/distutils/dir_util.py index 3fa4d7f4c3..5696359cc4 100644 --- a/distutils/dir_util.py +++ b/distutils/dir_util.py @@ -44,7 +44,7 @@ def wrapper(path, *args, **kwargs): @functools.singledispatch @wrapper -def mkpath(name: pathlib.Path, mode=0o777, verbose=True, dry_run=False): +def mkpath(name: pathlib.Path, mode=0o777, verbose=True, dry_run=False) -> None: """Create a directory and any missing ancestor directories. If the directory already exists (or if 'name' is the empty string, which @@ -52,21 +52,15 @@ def mkpath(name: pathlib.Path, mode=0o777, verbose=True, dry_run=False): Raise DistutilsFileError if unable to create some directory along the way (eg. some sub-path exists, but is a file rather than a directory). If 'verbose' is true, log the directory created. - Return the list of directories actually created. """ if verbose and not name.is_dir(): log.info("creating %s", name) - ancestry = itertools.chain((name,), name.parents) - missing = list(path for path in ancestry if not path.is_dir()) - try: dry_run or name.mkdir(mode=mode, parents=True, exist_ok=True) except OSError as exc: raise DistutilsFileError(f"could not create '{name}': {exc.args[-1]}") - return list(map(str, missing)) - @mkpath.register def _(name: str, *args, **kwargs): diff --git a/distutils/tests/test_dir_util.py b/distutils/tests/test_dir_util.py index 0a6ff431ac..12e643ab74 100644 --- a/distutils/tests/test_dir_util.py +++ b/distutils/tests/test_dir_util.py @@ -52,17 +52,6 @@ def test_mkpath_with_custom_mode(self): mkpath(self.target2, 0o555) assert stat.S_IMODE(os.stat(self.target2).st_mode) == 0o555 & ~umask - def test_mkpath_returns_as_described(self, tmp_path): - """ - Per the docstring, mkpath should return the list of directories created. - - pypa/distutils#305 revealed that the return value is always the empty - string and no one complained. Consider removing this expectation. - """ - target = tmp_path / 'foodir' - res = mkpath(target) - assert res == [str(target)] - def test_create_tree_verbosity(self, caplog): create_tree(self.root_target, ['one', 'two', 'three'], verbose=False) assert caplog.messages == [] From b1e746c2f25b2380cfa45105aafa2852a47f4f41 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sat, 19 Oct 2024 16:55:22 -0400 Subject: [PATCH 3/5] Add test capturing missed expectation. Ref #304 --- distutils/tests/test_dir_util.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/distutils/tests/test_dir_util.py b/distutils/tests/test_dir_util.py index 12e643ab74..ee76d05316 100644 --- a/distutils/tests/test_dir_util.py +++ b/distutils/tests/test_dir_util.py @@ -1,6 +1,7 @@ """Tests for distutils.dir_util.""" import os +import pathlib import stat import unittest.mock as mock from distutils import dir_util, errors @@ -110,3 +111,25 @@ def test_copy_tree_exception_in_listdir(self): ): src = self.tempdirs[-1] dir_util.copy_tree(src, None) + + @pytest.mark.xfail(reason="#304") + def test_mkpath_exception_uncached(self, monkeypatch, tmp_path): + """ + Caching should not remember failed attempts. + + pypa/distutils#304 + """ + + class FailPath(pathlib.Path): + def mkdir(self, *args, **kwargs): + raise OSError("Failed to create directory") + + target = tmp_path / 'foodir' + + with pytest.raises(errors.DistutilsFileError): + mkpath(FailPath(target)) + + assert not target.exists() + + mkpath(target) + assert target.exists() From 5904204aa175d9b4219742a137608eed190cc51b Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sat, 19 Oct 2024 16:59:42 -0400 Subject: [PATCH 4/5] Correct the order of operations to ensure that failed attempts aren't cached and are thus retried on subsequent operations. Closes #304 --- distutils/dir_util.py | 3 ++- distutils/tests/test_dir_util.py | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/distutils/dir_util.py b/distutils/dir_util.py index 5696359cc4..d9782602cf 100644 --- a/distutils/dir_util.py +++ b/distutils/dir_util.py @@ -32,8 +32,9 @@ def wrap(self, func): def wrapper(path, *args, **kwargs): if path.absolute() in self: return + result = func(path, *args, **kwargs) self.add(path.absolute()) - return func(path, *args, **kwargs) + return result return wrapper diff --git a/distutils/tests/test_dir_util.py b/distutils/tests/test_dir_util.py index ee76d05316..fcc37ac568 100644 --- a/distutils/tests/test_dir_util.py +++ b/distutils/tests/test_dir_util.py @@ -112,7 +112,6 @@ def test_copy_tree_exception_in_listdir(self): src = self.tempdirs[-1] dir_util.copy_tree(src, None) - @pytest.mark.xfail(reason="#304") def test_mkpath_exception_uncached(self, monkeypatch, tmp_path): """ Caching should not remember failed attempts. From 42e06250ce712839cfa437a5460ef4bb7b13ec0c Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Wed, 6 Nov 2024 20:33:47 -0500 Subject: [PATCH 5/5] Add news fragment. --- newsfragments/+f0b61194.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/+f0b61194.bugfix.rst diff --git a/newsfragments/+f0b61194.bugfix.rst b/newsfragments/+f0b61194.bugfix.rst new file mode 100644 index 0000000000..597165c3a0 --- /dev/null +++ b/newsfragments/+f0b61194.bugfix.rst @@ -0,0 +1 @@ +Merge with pypa/distutils@251797602, including fix for dirutil.mkpath handling in pypa/distutils#304. \ No newline at end of file