From d3c76bb404df37db600749d5006b4556861249fc Mon Sep 17 00:00:00 2001 From: Michael Carlstrom Date: Thu, 18 Jul 2024 22:58:46 -0400 Subject: [PATCH 1/8] 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 12f2ef0cec20a287e79a4da1e153636aeca5bdbd Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 19 Jul 2024 15:17:08 -0400 Subject: [PATCH 2/8] 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 8a9ca8b1290238b648610604baa381241a6af657 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 19 Jul 2024 15:25:43 -0400 Subject: [PATCH 3/8] 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 206ca2ae3b2614548a281c4b9b03c6ecf7c20878 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 19 Jul 2024 15:37:03 -0400 Subject: [PATCH 4/8] 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 f7adff4e789c01bdf23768c5c484b6a59dd48e53 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 19 Jul 2024 15:37:53 -0400 Subject: [PATCH 5/8] 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 b440f45a808f28141d79ceaceb39c661a2de0f5c Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 19 Jul 2024 15:44:54 -0400 Subject: [PATCH 6/8] 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 94b6d14b669194cbbe45d1f79e6976200e34d691 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 19 Jul 2024 16:12:53 -0400 Subject: [PATCH 7/8] 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 4d50db336a7c4e02a1e021773a77df295d3fc76b Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 19 Jul 2024 16:13:35 -0400 Subject: [PATCH 8/8] 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. """