From f350a98aaf9f800859f8e83422ff056157beec5d Mon Sep 17 00:00:00 2001 From: Michael Carlstrom Date: Thu, 18 Jul 2024 22:58:46 -0400 Subject: [PATCH 01/18] Allow path objects --- distutils/command/install_data.py | 11 +++++++++++ distutils/tests/test_install_data.py | 26 +++++++++++++++++--------- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/distutils/command/install_data.py b/distutils/command/install_data.py index 624c0b90..6b7c9aef 100644 --- a/distutils/command/install_data.py +++ b/distutils/command/install_data.py @@ -6,6 +6,7 @@ # contributed by Bastian Kleineidam import os +from pathlib import Path from ..core import Command from ..util import change_root, convert_path @@ -56,6 +57,16 @@ def run(self): ) (out, _) = self.copy_file(f, self.install_dir) self.outfiles.append(out) + elif isinstance(f, Path): + # it's a simple file, so copy it + f = convert_path(str(f)) + if self.warn_dir: + self.warn( + "setup script did not provide a directory for " + f"'{f}' -- installing right in '{self.install_dir}'" + ) + (out, _) = self.copy_file(f, self.install_dir) + self.outfiles.append(out) else: # it's a tuple with path to install to and a list of files dir = convert_path(f[0]) diff --git a/distutils/tests/test_install_data.py b/distutils/tests/test_install_data.py index f34070b1..43fd98ed 100644 --- a/distutils/tests/test_install_data.py +++ b/distutils/tests/test_install_data.py @@ -1,6 +1,7 @@ """Tests for distutils.command.install_data.""" import os +from pathlib import Path from distutils.command.install_data import install_data from distutils.tests import support @@ -18,22 +19,27 @@ def test_simple_run(self): # data_files can contain # - simple files + # - a Path object # - a tuple with a path, and a list of file one = os.path.join(pkg_dir, 'one') self.write_file(one, 'xxx') inst2 = os.path.join(pkg_dir, 'inst2') two = os.path.join(pkg_dir, 'two') self.write_file(two, 'xxx') + three = Path(pkg_dir) / 'three' + self.write_file(three, 'xxx') - cmd.data_files = [one, (inst2, [two])] - assert cmd.get_inputs() == [one, (inst2, [two])] + cmd.data_files = [one, (inst2, [two]), three] + assert cmd.get_inputs() == [one, (inst2, [two]), three] # let's run the command cmd.ensure_finalized() cmd.run() # let's check the result - assert len(cmd.get_outputs()) == 2 + assert len(cmd.get_outputs()) == 3 + rthree = os.path.split(one)[-1] + assert os.path.exists(os.path.join(inst, rthree)) rtwo = os.path.split(two)[-1] assert os.path.exists(os.path.join(inst2, rtwo)) rone = os.path.split(one)[-1] @@ -46,21 +52,23 @@ def test_simple_run(self): cmd.run() # let's check the result - assert len(cmd.get_outputs()) == 2 + assert len(cmd.get_outputs()) == 3 + assert os.path.exists(os.path.join(inst, rthree)) assert os.path.exists(os.path.join(inst2, rtwo)) assert os.path.exists(os.path.join(inst, rone)) cmd.outfiles = [] # now using root and empty dir cmd.root = os.path.join(pkg_dir, 'root') - inst4 = os.path.join(pkg_dir, 'inst4') - three = os.path.join(cmd.install_dir, 'three') - self.write_file(three, 'xx') - cmd.data_files = [one, (inst2, [two]), ('inst3', [three]), (inst4, [])] + inst5 = os.path.join(pkg_dir, 'inst5') + four = os.path.join(cmd.install_dir, 'four') + self.write_file(four, 'xx') + cmd.data_files = [one, (inst2, [two]), ('inst5', [four]), (inst5, [])] cmd.ensure_finalized() cmd.run() # let's check the result - assert len(cmd.get_outputs()) == 4 + assert len(cmd.get_outputs()) == 5 + assert os.path.exists(os.path.join(inst, rthree)) assert os.path.exists(os.path.join(inst2, rtwo)) assert os.path.exists(os.path.join(inst, rone)) From 202ff81fb5adcbde9cf990f27a6c9fbdc4ad7602 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 19 Jul 2024 15:17:08 -0400 Subject: [PATCH 02/18] Need to include 'three' in the input. --- distutils/tests/test_install_data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/distutils/tests/test_install_data.py b/distutils/tests/test_install_data.py index 43fd98ed..e340c758 100644 --- a/distutils/tests/test_install_data.py +++ b/distutils/tests/test_install_data.py @@ -63,7 +63,7 @@ def test_simple_run(self): inst5 = os.path.join(pkg_dir, 'inst5') four = os.path.join(cmd.install_dir, 'four') self.write_file(four, 'xx') - cmd.data_files = [one, (inst2, [two]), ('inst5', [four]), (inst5, [])] + cmd.data_files = [one, (inst2, [two]), three, ('inst5', [four]), (inst5, [])] cmd.ensure_finalized() cmd.run() From c2d9fba3913a4394c9222199eb8518197df2ff06 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 19 Jul 2024 15:25:43 -0400 Subject: [PATCH 03/18] Consolidate str and Path handling. --- distutils/command/install_data.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/distutils/command/install_data.py b/distutils/command/install_data.py index 6b7c9aef..a53786b9 100644 --- a/distutils/command/install_data.py +++ b/distutils/command/install_data.py @@ -47,19 +47,9 @@ def finalize_options(self): def run(self): self.mkpath(self.install_dir) for f in self.data_files: - if isinstance(f, str): + if isinstance(f, (str, Path)): # it's a simple file, so copy it - f = convert_path(f) - if self.warn_dir: - self.warn( - "setup script did not provide a directory for " - f"'{f}' -- installing right in '{self.install_dir}'" - ) - (out, _) = self.copy_file(f, self.install_dir) - self.outfiles.append(out) - elif isinstance(f, Path): - # it's a simple file, so copy it - f = convert_path(str(f)) + f = convert_path(os.fspath(f)) if self.warn_dir: self.warn( "setup script did not provide a directory for " From 29281a6441f5c95baf451b8c6a05b5ac9dd95796 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 19 Jul 2024 15:37:03 -0400 Subject: [PATCH 04/18] Expand convert_path to also accept pathlib.Path objects. --- distutils/command/install_data.py | 2 +- distutils/tests/test_util.py | 5 +++++ distutils/util.py | 10 +++++++++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/distutils/command/install_data.py b/distutils/command/install_data.py index a53786b9..735adf2b 100644 --- a/distutils/command/install_data.py +++ b/distutils/command/install_data.py @@ -49,7 +49,7 @@ def run(self): for f in self.data_files: if isinstance(f, (str, Path)): # it's a simple file, so copy it - f = convert_path(os.fspath(f)) + f = convert_path(f) if self.warn_dir: self.warn( "setup script did not provide a directory for " diff --git a/distutils/tests/test_util.py b/distutils/tests/test_util.py index 0de4e1a5..a614a1da 100644 --- a/distutils/tests/test_util.py +++ b/distutils/tests/test_util.py @@ -5,6 +5,7 @@ import email.policy import io import os +import pathlib import sys import sysconfig as stdlib_sysconfig import unittest.mock as mock @@ -72,6 +73,7 @@ def _join(path): os.path.join = _join assert convert_path('/home/to/my/stuff') == '/home/to/my/stuff' + assert convert_path(pathlib.Path('/home/to/my/stuff')) == '/home/to/my/stuff' # win os.sep = '\\' @@ -85,8 +87,11 @@ def _join(*path): convert_path('/home/to/my/stuff') with pytest.raises(ValueError): convert_path('home/to/my/stuff/') + with pytest.raises(ValueError): + convert_path(pathlib.Path('/home/to/my/stuff')) assert convert_path('home/to/my/stuff') == 'home\\to\\my\\stuff' + assert convert_path(pathlib.Path('home/to/my/stuff')) == 'home\\to\\my\\stuff' assert convert_path('.') == os.curdir def test_change_root(self): diff --git a/distutils/util.py b/distutils/util.py index 9db89b09..635d715a 100644 --- a/distutils/util.py +++ b/distutils/util.py @@ -7,6 +7,7 @@ import functools import importlib.util import os +import pathlib import re import string import subprocess @@ -116,7 +117,14 @@ def split_version(s): return [int(n) for n in s.split('.')] -def convert_path(pathname): +def convert_path(pathname: str | pathlib.Path) -> str: + """ + Allow for pathlib.Path inputs and then make native. + """ + return make_native(os.fspath(pathname)) + + +def make_native(pathname: str) -> str: """Return 'pathname' as a name that will work on the native filesystem, i.e. split it on '/' and put it back together again using the current directory separator. Needed because filenames in the setup script are From 2a7122a885ddf34664ed9f6edb88c66a8510ef68 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 19 Jul 2024 15:37:53 -0400 Subject: [PATCH 05/18] Prefer simply 'pathlib' for import. --- distutils/command/install_data.py | 4 ++-- distutils/tests/test_install_data.py | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/distutils/command/install_data.py b/distutils/command/install_data.py index 735adf2b..cb03d0e5 100644 --- a/distutils/command/install_data.py +++ b/distutils/command/install_data.py @@ -6,7 +6,7 @@ # contributed by Bastian Kleineidam import os -from pathlib import Path +import pathlib from ..core import Command from ..util import change_root, convert_path @@ -47,7 +47,7 @@ def finalize_options(self): def run(self): self.mkpath(self.install_dir) for f in self.data_files: - if isinstance(f, (str, Path)): + if isinstance(f, (str, pathlib.Path)): # it's a simple file, so copy it f = convert_path(f) if self.warn_dir: diff --git a/distutils/tests/test_install_data.py b/distutils/tests/test_install_data.py index e340c758..4b15a269 100644 --- a/distutils/tests/test_install_data.py +++ b/distutils/tests/test_install_data.py @@ -1,12 +1,13 @@ """Tests for distutils.command.install_data.""" import os -from pathlib import Path -from distutils.command.install_data import install_data -from distutils.tests import support +import pathlib import pytest +from distutils.command.install_data import install_data +from distutils.tests import support + @pytest.mark.usefixtures('save_env') class TestInstallData( @@ -26,7 +27,7 @@ def test_simple_run(self): inst2 = os.path.join(pkg_dir, 'inst2') two = os.path.join(pkg_dir, 'two') self.write_file(two, 'xxx') - three = Path(pkg_dir) / 'three' + three = pathlib.Path(pkg_dir) / 'three' self.write_file(three, 'xxx') cmd.data_files = [one, (inst2, [two]), three] From 6099343a445935b13136da23a4b92d816dd5ee29 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 19 Jul 2024 15:44:54 -0400 Subject: [PATCH 06/18] Extract a singledispatchmethod _copy for handling the copy of each data file. --- distutils/command/install_data.py | 69 ++++++++++++++++++------------- 1 file changed, 40 insertions(+), 29 deletions(-) diff --git a/distutils/command/install_data.py b/distutils/command/install_data.py index cb03d0e5..e13b5ca6 100644 --- a/distutils/command/install_data.py +++ b/distutils/command/install_data.py @@ -5,13 +5,19 @@ # contributed by Bastian Kleineidam +import functools import os import pathlib +from typing import Tuple, Iterable + from ..core import Command from ..util import change_root, convert_path +StrPath = str | pathlib.Path + + class install_data(Command): description = "install data files" @@ -47,36 +53,41 @@ def finalize_options(self): def run(self): self.mkpath(self.install_dir) for f in self.data_files: - if isinstance(f, (str, pathlib.Path)): - # it's a simple file, so copy it - f = convert_path(f) - if self.warn_dir: - self.warn( - "setup script did not provide a directory for " - f"'{f}' -- installing right in '{self.install_dir}'" - ) - (out, _) = self.copy_file(f, self.install_dir) + self._copy(f) + + @functools.singledispatchmethod + def _copy(self, f: StrPath | Tuple[StrPath, Iterable[StrPath]]): + # it's a tuple with path to install to and a list of files + dir = convert_path(f[0]) + if not os.path.isabs(dir): + dir = os.path.join(self.install_dir, dir) + elif self.root: + dir = change_root(self.root, dir) + self.mkpath(dir) + + if f[1] == []: + # If there are no files listed, the user must be + # trying to create an empty directory, so add the + # directory to the list of output files. + self.outfiles.append(dir) + else: + # Copy files, adding them to the list of output files. + for data in f[1]: + data = convert_path(data) + (out, _) = self.copy_file(data, dir) self.outfiles.append(out) - else: - # it's a tuple with path to install to and a list of files - dir = convert_path(f[0]) - if not os.path.isabs(dir): - dir = os.path.join(self.install_dir, dir) - elif self.root: - dir = change_root(self.root, dir) - self.mkpath(dir) - - if f[1] == []: - # If there are no files listed, the user must be - # trying to create an empty directory, so add the - # directory to the list of output files. - self.outfiles.append(dir) - else: - # Copy files, adding them to the list of output files. - for data in f[1]: - data = convert_path(data) - (out, _) = self.copy_file(data, dir) - self.outfiles.append(out) + + @_copy.register + def _(self, f: StrPath): + # it's a simple file, so copy it + f = convert_path(f) + if self.warn_dir: + self.warn( + "setup script did not provide a directory for " + f"'{f}' -- installing right in '{self.install_dir}'" + ) + (out, _) = self.copy_file(f, self.install_dir) + self.outfiles.append(out) def get_inputs(self): return self.data_files or [] From 3a2d7a62e355db2d1e67af38ba2be03c63099480 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 19 Jul 2024 16:12:53 -0400 Subject: [PATCH 07/18] Use explicit registration for compatibility with older Pythons. Prior to 3.11, singledispatch[method] doesn't know about unions. --- distutils/command/install_data.py | 15 +++++++-------- distutils/util.py | 2 ++ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/distutils/command/install_data.py b/distutils/command/install_data.py index e13b5ca6..bd2932ab 100644 --- a/distutils/command/install_data.py +++ b/distutils/command/install_data.py @@ -5,19 +5,17 @@ # contributed by Bastian Kleineidam +from __future__ import annotations + import functools import os -import pathlib -from typing import Tuple, Iterable +from typing import Iterable from ..core import Command from ..util import change_root, convert_path -StrPath = str | pathlib.Path - - class install_data(Command): description = "install data files" @@ -56,7 +54,7 @@ def run(self): self._copy(f) @functools.singledispatchmethod - def _copy(self, f: StrPath | Tuple[StrPath, Iterable[StrPath]]): + def _copy(self, f: tuple[str | os.PathLike, Iterable[str | os.PathLike]]): # it's a tuple with path to install to and a list of files dir = convert_path(f[0]) if not os.path.isabs(dir): @@ -77,8 +75,9 @@ def _copy(self, f: StrPath | Tuple[StrPath, Iterable[StrPath]]): (out, _) = self.copy_file(data, dir) self.outfiles.append(out) - @_copy.register - def _(self, f: StrPath): + @_copy.register(str) + @_copy.register(os.PathLike) + def _(self, f: str | os.PathLike): # it's a simple file, so copy it f = convert_path(f) if self.warn_dir: diff --git a/distutils/util.py b/distutils/util.py index 635d715a..95503e95 100644 --- a/distutils/util.py +++ b/distutils/util.py @@ -4,6 +4,8 @@ one of the other *util.py modules. """ +from __future__ import annotations + import functools import importlib.util import os From a5e3e5f47e333b0c3d04a5f35cf8a3a67a4ffb62 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 19 Jul 2024 16:13:35 -0400 Subject: [PATCH 08/18] Prefer os.PathLike in convert_path --- distutils/util.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/distutils/util.py b/distutils/util.py index 95503e95..7751eb94 100644 --- a/distutils/util.py +++ b/distutils/util.py @@ -9,7 +9,6 @@ import functools import importlib.util import os -import pathlib import re import string import subprocess @@ -119,7 +118,7 @@ def split_version(s): return [int(n) for n in s.split('.')] -def convert_path(pathname: str | pathlib.Path) -> str: +def convert_path(pathname: str | os.PathLike) -> str: """ Allow for pathlib.Path inputs and then make native. """ From 87f77315d5fb2b00e88db0bd6931a2f94c10cdfd Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 19 Jul 2024 16:39:26 -0400 Subject: [PATCH 09/18] Convert needs to accept None for Setuptools' sake. --- distutils/util.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/distutils/util.py b/distutils/util.py index 7751eb94..68565534 100644 --- a/distutils/util.py +++ b/distutils/util.py @@ -16,6 +16,7 @@ import sysconfig import tempfile +from ._functools import pass_none from ._log import log from ._modified import newer from .errors import DistutilsByteCompileError, DistutilsPlatformError @@ -118,9 +119,16 @@ def split_version(s): return [int(n) for n in s.split('.')] +@pass_none def convert_path(pathname: str | os.PathLike) -> str: """ Allow for pathlib.Path inputs and then make native. + + Also if None is passed, will just pass it through as + Setuptools relies on this behavior. + + >>> convert_path(None) is None + True """ return make_native(os.fspath(pathname)) From 2e6a69e92aa2eb5a85e068d8aa8e92427855a9fc Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 19 Jul 2024 16:50:07 -0400 Subject: [PATCH 10/18] In test_convert_path, utilize posixpath.join and ntpath.join for maximum compatibility with other libraries. --- distutils/tests/test_util.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/distutils/tests/test_util.py b/distutils/tests/test_util.py index a614a1da..b24f1fb4 100644 --- a/distutils/tests/test_util.py +++ b/distutils/tests/test_util.py @@ -4,8 +4,10 @@ import email.generator import email.policy import io +import ntpath import os import pathlib +import posixpath import sys import sysconfig as stdlib_sysconfig import unittest.mock as mock @@ -66,22 +68,14 @@ def test_get_platform(self): def test_convert_path(self): # linux/mac os.sep = '/' - - def _join(path): - return '/'.join(path) - - os.path.join = _join + os.path.join = posixpath.join assert convert_path('/home/to/my/stuff') == '/home/to/my/stuff' assert convert_path(pathlib.Path('/home/to/my/stuff')) == '/home/to/my/stuff' # win os.sep = '\\' - - def _join(*path): - return '\\'.join(path) - - os.path.join = _join + os.path.join = ntpath.join with pytest.raises(ValueError): convert_path('/home/to/my/stuff') From 90cbfbe72f5ed07d1fd589bfb7c1660f66159756 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 19 Jul 2024 17:06:10 -0400 Subject: [PATCH 11/18] Wrap paths in PurePosixPath to ensure that any WindowsPaths don't get backslashes. --- distutils/util.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/distutils/util.py b/distutils/util.py index 68565534..f8e4fbb7 100644 --- a/distutils/util.py +++ b/distutils/util.py @@ -9,6 +9,7 @@ import functools import importlib.util import os +import pathlib import re import string import subprocess @@ -130,7 +131,9 @@ def convert_path(pathname: str | os.PathLike) -> str: >>> convert_path(None) is None True """ - return make_native(os.fspath(pathname)) + # wrap in PurePosixPath to retain forward slashes on Windows + # see https://github.com/pypa/distutils/pull/272#issuecomment-2240100013 + return make_native(os.fspath(pathlib.PurePosixPath(pathname))) def make_native(pathname: str) -> str: From e2cbf97dda5929ae835c52921461c09bda521a06 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 19 Jul 2024 17:48:30 -0400 Subject: [PATCH 12/18] convert_path no longer fails if passed a path with a trailing slash. Instead, trailing slashes are stripped just as they are with pathlib.Path. Ref https://github.com/pypa/distutils/pull/272#issuecomment-2240252653. --- distutils/tests/test_util.py | 2 -- distutils/util.py | 2 -- 2 files changed, 4 deletions(-) diff --git a/distutils/tests/test_util.py b/distutils/tests/test_util.py index b24f1fb4..c05df03a 100644 --- a/distutils/tests/test_util.py +++ b/distutils/tests/test_util.py @@ -79,8 +79,6 @@ def test_convert_path(self): with pytest.raises(ValueError): convert_path('/home/to/my/stuff') - with pytest.raises(ValueError): - convert_path('home/to/my/stuff/') with pytest.raises(ValueError): convert_path(pathlib.Path('/home/to/my/stuff')) diff --git a/distutils/util.py b/distutils/util.py index f8e4fbb7..50ebb1c1 100644 --- a/distutils/util.py +++ b/distutils/util.py @@ -151,8 +151,6 @@ def make_native(pathname: str) -> str: return pathname if pathname[0] == '/': raise ValueError(f"path '{pathname}' cannot be absolute") - if pathname[-1] == '/': - raise ValueError(f"path '{pathname}' cannot end with '/'") paths = pathname.split('/') while '.' in paths: From 9d6ca5fe777e7bc2ab611814adac989789d7f023 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sat, 20 Jul 2024 14:25:46 -0400 Subject: [PATCH 13/18] convert_path now converts to a platform-native path.Path, but then calls `.as_posix()` on it. This change will have the unintended effect of adding support for backslashes on Windows. Maybe that's fine, or maybe it should be prohibited. --- distutils/util.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/distutils/util.py b/distutils/util.py index 50ebb1c1..3163eb48 100644 --- a/distutils/util.py +++ b/distutils/util.py @@ -123,17 +123,17 @@ def split_version(s): @pass_none def convert_path(pathname: str | os.PathLike) -> str: """ - Allow for pathlib.Path inputs and then make native. + Allow for pathlib.Path inputs, coax to posix, and then make native. - Also if None is passed, will just pass it through as + If None is passed, will just pass it through as Setuptools relies on this behavior. >>> convert_path(None) is None True """ - # wrap in PurePosixPath to retain forward slashes on Windows + # Use .as_posix() to retain forward slashes on Windows # see https://github.com/pypa/distutils/pull/272#issuecomment-2240100013 - return make_native(os.fspath(pathlib.PurePosixPath(pathname))) + return make_native(pathlib.Path(pathname).as_posix()) def make_native(pathname: str) -> str: From 4eaa02f2fff8f8e6d4eac5442988dc586fe56004 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sat, 20 Jul 2024 14:45:06 -0400 Subject: [PATCH 14/18] Separate test_convert_path into two tests to avoid interactions in monkeypatching os.path. --- distutils/tests/test_util.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/distutils/tests/test_util.py b/distutils/tests/test_util.py index c05df03a..434c9b80 100644 --- a/distutils/tests/test_util.py +++ b/distutils/tests/test_util.py @@ -65,15 +65,14 @@ def test_get_platform(self): with mock.patch.dict('os.environ', {'VSCMD_ARG_TGT_ARCH': 'arm64'}): assert get_platform() == 'win-arm64' - def test_convert_path(self): - # linux/mac + def test_convert_path_unix(self): os.sep = '/' os.path.join = posixpath.join assert convert_path('/home/to/my/stuff') == '/home/to/my/stuff' assert convert_path(pathlib.Path('/home/to/my/stuff')) == '/home/to/my/stuff' - # win + def test_convert_path_windows(self): os.sep = '\\' os.path.join = ntpath.join From 5469f24a5978acb5c7397a4b2e2b2ffd78c7c9e6 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sat, 20 Jul 2024 15:16:56 -0400 Subject: [PATCH 15/18] Remove expectation that a ValueError is raised for data_files being drive-relative absolute on Windows. --- distutils/tests/test_util.py | 5 ----- distutils/util.py | 2 -- 2 files changed, 7 deletions(-) diff --git a/distutils/tests/test_util.py b/distutils/tests/test_util.py index 434c9b80..16fcfe3e 100644 --- a/distutils/tests/test_util.py +++ b/distutils/tests/test_util.py @@ -76,11 +76,6 @@ def test_convert_path_windows(self): os.sep = '\\' os.path.join = ntpath.join - with pytest.raises(ValueError): - convert_path('/home/to/my/stuff') - with pytest.raises(ValueError): - convert_path(pathlib.Path('/home/to/my/stuff')) - assert convert_path('home/to/my/stuff') == 'home\\to\\my\\stuff' assert convert_path(pathlib.Path('home/to/my/stuff')) == 'home\\to\\my\\stuff' assert convert_path('.') == os.curdir diff --git a/distutils/util.py b/distutils/util.py index 3163eb48..c7e73011 100644 --- a/distutils/util.py +++ b/distutils/util.py @@ -149,8 +149,6 @@ def make_native(pathname: str) -> str: return pathname if not pathname: return pathname - if pathname[0] == '/': - raise ValueError(f"path '{pathname}' cannot be absolute") paths = pathname.split('/') while '.' in paths: From 28e7431142f4bc380518f4691076af2bd81222b7 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sat, 20 Jul 2024 15:33:26 -0400 Subject: [PATCH 16/18] Simplify convert_path by simply relying on the logic in PurePath. Test for convert_path no longer runs on all operating systems as it's too difficult (impossible) to monkeypatch PurePath reliably. --- distutils/tests/test_util.py | 10 ++-------- distutils/util.py | 36 +++++++----------------------------- 2 files changed, 9 insertions(+), 37 deletions(-) diff --git a/distutils/tests/test_util.py b/distutils/tests/test_util.py index 16fcfe3e..25e26e01 100644 --- a/distutils/tests/test_util.py +++ b/distutils/tests/test_util.py @@ -4,10 +4,8 @@ import email.generator import email.policy import io -import ntpath import os import pathlib -import posixpath import sys import sysconfig as stdlib_sysconfig import unittest.mock as mock @@ -65,17 +63,13 @@ def test_get_platform(self): with mock.patch.dict('os.environ', {'VSCMD_ARG_TGT_ARCH': 'arm64'}): assert get_platform() == 'win-arm64' + @pytest.mark.skipif('platform.system() == "Windows"') def test_convert_path_unix(self): - os.sep = '/' - os.path.join = posixpath.join - assert convert_path('/home/to/my/stuff') == '/home/to/my/stuff' assert convert_path(pathlib.Path('/home/to/my/stuff')) == '/home/to/my/stuff' + @pytest.mark.skipif('platform.system() != "Windows"') def test_convert_path_windows(self): - os.sep = '\\' - os.path.join = ntpath.join - assert convert_path('home/to/my/stuff') == 'home\\to\\my\\stuff' assert convert_path(pathlib.Path('home/to/my/stuff')) == 'home\\to\\my\\stuff' assert convert_path('.') == os.curdir diff --git a/distutils/util.py b/distutils/util.py index c7e73011..4cc6bd28 100644 --- a/distutils/util.py +++ b/distutils/util.py @@ -122,43 +122,21 @@ def split_version(s): @pass_none def convert_path(pathname: str | os.PathLike) -> str: - """ - Allow for pathlib.Path inputs, coax to posix, and then make native. + r""" + Allow for pathlib.Path inputs, coax to a native path string. If None is passed, will just pass it through as Setuptools relies on this behavior. >>> convert_path(None) is None True - """ - # Use .as_posix() to retain forward slashes on Windows - # see https://github.com/pypa/distutils/pull/272#issuecomment-2240100013 - return make_native(pathlib.Path(pathname).as_posix()) - - -def make_native(pathname: str) -> str: - """Return 'pathname' as a name that will work on the native filesystem, - i.e. split it on '/' and put it back together again using the current - directory separator. Needed because filenames in the setup script are - always supplied in Unix style, and have to be converted to the local - convention before we can actually use them in the filesystem. Raises - ValueError on non-Unix-ish systems if 'pathname' either starts or - ends with a slash. - """ - if os.sep == '/': - return pathname - if not pathname: - return pathname - paths = pathname.split('/') - while '.' in paths: - paths.remove('.') - if not paths: - return os.curdir - return os.path.join(*paths) + Removes empty paths. - -# convert_path () + >>> convert_path('foo/./bar').replace('\\', '/') + 'foo/bar' + """ + return os.fspath(pathlib.PurePath(pathname)) def change_root(new_root, pathname): From 8cada19d07e286558e5193841f66483f631170b3 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sat, 20 Jul 2024 15:43:45 -0400 Subject: [PATCH 17/18] Harmonize convert_path tests across Unix and Windows. --- distutils/tests/test_util.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/distutils/tests/test_util.py b/distutils/tests/test_util.py index 25e26e01..d1254ca1 100644 --- a/distutils/tests/test_util.py +++ b/distutils/tests/test_util.py @@ -67,11 +67,12 @@ def test_get_platform(self): def test_convert_path_unix(self): assert convert_path('/home/to/my/stuff') == '/home/to/my/stuff' assert convert_path(pathlib.Path('/home/to/my/stuff')) == '/home/to/my/stuff' + assert convert_path('.') == os.curdir @pytest.mark.skipif('platform.system() != "Windows"') def test_convert_path_windows(self): - assert convert_path('home/to/my/stuff') == 'home\\to\\my\\stuff' - assert convert_path(pathlib.Path('home/to/my/stuff')) == 'home\\to\\my\\stuff' + assert convert_path('/home/to/my/stuff') == r'\home\to\my\stuff' + assert convert_path(pathlib.Path('/home/to/my/stuff')) == r'\home\to\my\stuff' assert convert_path('.') == os.curdir def test_change_root(self): From 8f2498a1b072f3206b85f786016b9defbb4b796c Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sat, 20 Jul 2024 15:52:05 -0400 Subject: [PATCH 18/18] Consolidate convert_path tests and just generate the expected value in a platform-sensitive way. Should fix failures on mingw. --- distutils/tests/test_util.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/distutils/tests/test_util.py b/distutils/tests/test_util.py index d1254ca1..00c9743e 100644 --- a/distutils/tests/test_util.py +++ b/distutils/tests/test_util.py @@ -63,16 +63,10 @@ def test_get_platform(self): with mock.patch.dict('os.environ', {'VSCMD_ARG_TGT_ARCH': 'arm64'}): assert get_platform() == 'win-arm64' - @pytest.mark.skipif('platform.system() == "Windows"') - def test_convert_path_unix(self): - assert convert_path('/home/to/my/stuff') == '/home/to/my/stuff' - assert convert_path(pathlib.Path('/home/to/my/stuff')) == '/home/to/my/stuff' - assert convert_path('.') == os.curdir - - @pytest.mark.skipif('platform.system() != "Windows"') - def test_convert_path_windows(self): - assert convert_path('/home/to/my/stuff') == r'\home\to\my\stuff' - assert convert_path(pathlib.Path('/home/to/my/stuff')) == r'\home\to\my\stuff' + def test_convert_path(self): + expected = os.sep.join(('', 'home', 'to', 'my', 'stuff')) + assert convert_path('/home/to/my/stuff') == expected + assert convert_path(pathlib.Path('/home/to/my/stuff')) == expected assert convert_path('.') == os.curdir def test_change_root(self):