From 94fced54d8ed443441bf8c921bb01da3b25aed2d Mon Sep 17 00:00:00 2001 From: Mike Foster Date: Mon, 3 Jun 2024 19:31:43 +0200 Subject: [PATCH 01/14] replace `with suppress(FileNotFoundError)` by `.unlink(missing_ok=True)` for macos --- cibuildwheel/macos.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cibuildwheel/macos.py b/cibuildwheel/macos.py index 977f7f203..f3d5c73e3 100644 --- a/cibuildwheel/macos.py +++ b/cibuildwheel/macos.py @@ -641,8 +641,7 @@ def build(options: Options, tmp_path: Path) -> None: # we're all done here; move it to output (overwrite existing) if compatible_wheel is None: - with contextlib.suppress(FileNotFoundError): - (build_options.output_dir / repaired_wheel.name).unlink() + (build_options.output_dir / repaired_wheel.name).unlink(missing_ok=True) shutil.move(str(repaired_wheel), build_options.output_dir) built_wheels.append(build_options.output_dir / repaired_wheel.name) From 5f9e4ddac7c5a63fc6d46117eb396a1d4288a34b Mon Sep 17 00:00:00 2001 From: Mike Foster Date: Mon, 3 Jun 2024 19:33:11 +0200 Subject: [PATCH 02/14] also use `.unlink(missing_ok=True)` in pyodide and windows for consistency --- cibuildwheel/pyodide.py | 3 +-- cibuildwheel/windows.py | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/cibuildwheel/pyodide.py b/cibuildwheel/pyodide.py index 30345f7bb..530dac194 100644 --- a/cibuildwheel/pyodide.py +++ b/cibuildwheel/pyodide.py @@ -395,8 +395,7 @@ def build(options: Options, tmp_path: Path) -> None: # we're all done here; move it to output (overwrite existing) if compatible_wheel is None: - with contextlib.suppress(FileNotFoundError): - (build_options.output_dir / repaired_wheel.name).unlink() + (build_options.output_dir / repaired_wheel.name).unlink(missing_ok=True) shutil.move(str(repaired_wheel), build_options.output_dir) built_wheels.append(build_options.output_dir / repaired_wheel.name) diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index 3c0b87792..077747bd6 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -543,8 +543,7 @@ def build(options: Options, tmp_path: Path) -> None: # we're all done here; move it to output (remove if already exists) if compatible_wheel is None: - with suppress(FileNotFoundError): - (build_options.output_dir / repaired_wheel.name).unlink() + (build_options.output_dir / repaired_wheel.name).unlink(missing_ok=True) shutil.move(str(repaired_wheel), build_options.output_dir) built_wheels.append(build_options.output_dir / repaired_wheel.name) From cbf5afddc140501b14030054282c318473cfcea1 Mon Sep 17 00:00:00 2001 From: Mike Foster Date: Mon, 3 Jun 2024 19:37:16 +0200 Subject: [PATCH 03/14] remove contextlib imports which are no longer required --- cibuildwheel/macos.py | 1 - cibuildwheel/pyodide.py | 1 - cibuildwheel/windows.py | 1 - 3 files changed, 3 deletions(-) diff --git a/cibuildwheel/macos.py b/cibuildwheel/macos.py index f3d5c73e3..99e9b3ed1 100644 --- a/cibuildwheel/macos.py +++ b/cibuildwheel/macos.py @@ -1,6 +1,5 @@ from __future__ import annotations -import contextlib import functools import os import platform diff --git a/cibuildwheel/pyodide.py b/cibuildwheel/pyodide.py index 530dac194..c0a7f0ff9 100644 --- a/cibuildwheel/pyodide.py +++ b/cibuildwheel/pyodide.py @@ -1,6 +1,5 @@ from __future__ import annotations -import contextlib import os import shutil import sys diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index 077747bd6..fc634fdfe 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -7,7 +7,6 @@ import sys import textwrap from collections.abc import MutableMapping, Sequence, Set -from contextlib import suppress from dataclasses import dataclass from functools import lru_cache from pathlib import Path From 872009bc0238fe70ce74ee88b7cf3e4a2cf4d057 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 3 Jun 2024 16:33:44 -0400 Subject: [PATCH 04/14] Apply suggestions from code review --- cibuildwheel/macos.py | 2 +- cibuildwheel/pyodide.py | 2 +- cibuildwheel/windows.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cibuildwheel/macos.py b/cibuildwheel/macos.py index 99e9b3ed1..2b2d65fdd 100644 --- a/cibuildwheel/macos.py +++ b/cibuildwheel/macos.py @@ -640,7 +640,7 @@ def build(options: Options, tmp_path: Path) -> None: # we're all done here; move it to output (overwrite existing) if compatible_wheel is None: - (build_options.output_dir / repaired_wheel.name).unlink(missing_ok=True) + build_options.output_dir.joinpath(repaired_wheel.name).unlink(missing_ok=True) shutil.move(str(repaired_wheel), build_options.output_dir) built_wheels.append(build_options.output_dir / repaired_wheel.name) diff --git a/cibuildwheel/pyodide.py b/cibuildwheel/pyodide.py index c0a7f0ff9..85504d2ea 100644 --- a/cibuildwheel/pyodide.py +++ b/cibuildwheel/pyodide.py @@ -394,7 +394,7 @@ def build(options: Options, tmp_path: Path) -> None: # we're all done here; move it to output (overwrite existing) if compatible_wheel is None: - (build_options.output_dir / repaired_wheel.name).unlink(missing_ok=True) + build_options.output_dir.joinpath(repaired_wheel.name).unlink(missing_ok=True) shutil.move(str(repaired_wheel), build_options.output_dir) built_wheels.append(build_options.output_dir / repaired_wheel.name) diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index fc634fdfe..1ed186cd4 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -542,7 +542,7 @@ def build(options: Options, tmp_path: Path) -> None: # we're all done here; move it to output (remove if already exists) if compatible_wheel is None: - (build_options.output_dir / repaired_wheel.name).unlink(missing_ok=True) + build_options.output_dir.joinpath(repaired_wheel.name).unlink(missing_ok=True) shutil.move(str(repaired_wheel), build_options.output_dir) built_wheels.append(build_options.output_dir / repaired_wheel.name) From dfe87378ec4f97f5c77ae6176f60bbe92949285f Mon Sep 17 00:00:00 2001 From: Mike Foster Date: Tue, 4 Jun 2024 11:46:49 +0200 Subject: [PATCH 05/14] explicity resolve and create output location --- cibuildwheel/macos.py | 14 ++++++++++---- cibuildwheel/windows.py | 14 ++++++++++---- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/cibuildwheel/macos.py b/cibuildwheel/macos.py index 2b2d65fdd..1e59fbd60 100644 --- a/cibuildwheel/macos.py +++ b/cibuildwheel/macos.py @@ -640,10 +640,16 @@ def build(options: Options, tmp_path: Path) -> None: # we're all done here; move it to output (overwrite existing) if compatible_wheel is None: - build_options.output_dir.joinpath(repaired_wheel.name).unlink(missing_ok=True) - - shutil.move(str(repaired_wheel), build_options.output_dir) - built_wheels.append(build_options.output_dir / repaired_wheel.name) + output_dir = build_options.output_dir.resolve() + output_wheel = output_dir.joinpath(repaired_wheel.name).resolve() + output_wheel.unlink(missing_ok=True) + + # os.move() will rename the file to what we were expecting to be the parent directory if we don't ensure it exists + output_dir.mkdir(parents=True, exist_ok=True) + + # using os.move() as Path.rename() is not guaranteed to work across filesystem boundaries + shutil.move(repaired_wheel, output_wheel) + built_wheels.append(output_wheel) # clean up shutil.rmtree(identifier_tmp_dir) diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index 1ed186cd4..6c5cc4cf2 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -542,10 +542,16 @@ def build(options: Options, tmp_path: Path) -> None: # we're all done here; move it to output (remove if already exists) if compatible_wheel is None: - build_options.output_dir.joinpath(repaired_wheel.name).unlink(missing_ok=True) - - shutil.move(str(repaired_wheel), build_options.output_dir) - built_wheels.append(build_options.output_dir / repaired_wheel.name) + output_dir = build_options.output_dir.resolve() + output_wheel = output_dir.joinpath(repaired_wheel.name).resolve() + output_wheel.unlink(missing_ok=True) + + # os.move() will rename the file to what we were expecting to be the parent directory if we don't ensure it exists + output_dir.mkdir(parents=True, exist_ok=True) + + # using os.move() as Path.rename() is not guaranteed to work across filesystem boundaries + shutil.move(repaired_wheel, output_wheel) + built_wheels.append(output_wheel) # clean up # (we ignore errors because occasionally Windows fails to unlink a file and we From 9202c83eefa1b6c8195dd06c17b22140e393cdf3 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 4 Jun 2024 09:47:18 +0000 Subject: [PATCH 06/14] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- cibuildwheel/macos.py | 2 +- cibuildwheel/windows.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cibuildwheel/macos.py b/cibuildwheel/macos.py index 1e59fbd60..caacc6cea 100644 --- a/cibuildwheel/macos.py +++ b/cibuildwheel/macos.py @@ -646,7 +646,7 @@ def build(options: Options, tmp_path: Path) -> None: # os.move() will rename the file to what we were expecting to be the parent directory if we don't ensure it exists output_dir.mkdir(parents=True, exist_ok=True) - + # using os.move() as Path.rename() is not guaranteed to work across filesystem boundaries shutil.move(repaired_wheel, output_wheel) built_wheels.append(output_wheel) diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index 6c5cc4cf2..b2c42b79e 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -548,7 +548,7 @@ def build(options: Options, tmp_path: Path) -> None: # os.move() will rename the file to what we were expecting to be the parent directory if we don't ensure it exists output_dir.mkdir(parents=True, exist_ok=True) - + # using os.move() as Path.rename() is not guaranteed to work across filesystem boundaries shutil.move(repaired_wheel, output_wheel) built_wheels.append(output_wheel) From 6dd397e1a5a4624c97f03a2432b081b7bffeec14 Mon Sep 17 00:00:00 2001 From: Mike Foster Date: Tue, 4 Jun 2024 11:59:59 +0200 Subject: [PATCH 07/14] use explicit str for move --- cibuildwheel/macos.py | 2 +- cibuildwheel/windows.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cibuildwheel/macos.py b/cibuildwheel/macos.py index caacc6cea..96cfc6c58 100644 --- a/cibuildwheel/macos.py +++ b/cibuildwheel/macos.py @@ -648,7 +648,7 @@ def build(options: Options, tmp_path: Path) -> None: output_dir.mkdir(parents=True, exist_ok=True) # using os.move() as Path.rename() is not guaranteed to work across filesystem boundaries - shutil.move(repaired_wheel, output_wheel) + shutil.move(str(repaired_wheel), str(output_wheel)) # explicit str() needed for mypy3.8 built_wheels.append(output_wheel) # clean up diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index b2c42b79e..170969578 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -550,7 +550,7 @@ def build(options: Options, tmp_path: Path) -> None: output_dir.mkdir(parents=True, exist_ok=True) # using os.move() as Path.rename() is not guaranteed to work across filesystem boundaries - shutil.move(repaired_wheel, output_wheel) + shutil.move(str(repaired_wheel), str(output_wheel)) # explicit str() needed for mypy3.8 built_wheels.append(output_wheel) # clean up From 16b96ca499db7c05a3021d49055076af93cb4bdd Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 4 Jun 2024 10:01:06 +0000 Subject: [PATCH 08/14] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- cibuildwheel/macos.py | 4 +++- cibuildwheel/windows.py | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/cibuildwheel/macos.py b/cibuildwheel/macos.py index 96cfc6c58..17e884377 100644 --- a/cibuildwheel/macos.py +++ b/cibuildwheel/macos.py @@ -648,7 +648,9 @@ def build(options: Options, tmp_path: Path) -> None: output_dir.mkdir(parents=True, exist_ok=True) # using os.move() as Path.rename() is not guaranteed to work across filesystem boundaries - shutil.move(str(repaired_wheel), str(output_wheel)) # explicit str() needed for mypy3.8 + shutil.move( + str(repaired_wheel), str(output_wheel) + ) # explicit str() needed for mypy3.8 built_wheels.append(output_wheel) # clean up diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index 170969578..ae18f8630 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -550,7 +550,9 @@ def build(options: Options, tmp_path: Path) -> None: output_dir.mkdir(parents=True, exist_ok=True) # using os.move() as Path.rename() is not guaranteed to work across filesystem boundaries - shutil.move(str(repaired_wheel), str(output_wheel)) # explicit str() needed for mypy3.8 + shutil.move( + str(repaired_wheel), str(output_wheel) + ) # explicit str() needed for mypy3.8 built_wheels.append(output_wheel) # clean up From df77e9790a4193d6a6e26564901b93e5cf9ebe3f Mon Sep 17 00:00:00 2001 From: Mike Foster Date: Wed, 5 Jun 2024 17:18:23 +0200 Subject: [PATCH 09/14] update comments based on review feedback --- cibuildwheel/macos.py | 6 +++--- cibuildwheel/windows.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cibuildwheel/macos.py b/cibuildwheel/macos.py index 17e884377..a29faffac 100644 --- a/cibuildwheel/macos.py +++ b/cibuildwheel/macos.py @@ -644,13 +644,13 @@ def build(options: Options, tmp_path: Path) -> None: output_wheel = output_dir.joinpath(repaired_wheel.name).resolve() output_wheel.unlink(missing_ok=True) - # os.move() will rename the file to what we were expecting to be the parent directory if we don't ensure it exists + # shutil.move() will rename the file to what we were expecting to be the parent directory if we don't ensure it exists output_dir.mkdir(parents=True, exist_ok=True) - # using os.move() as Path.rename() is not guaranteed to work across filesystem boundaries + # using shutil.move() as Path.rename() is not guaranteed to work across filesystem boundaries shutil.move( str(repaired_wheel), str(output_wheel) - ) # explicit str() needed for mypy3.8 + ) # explicit str() needed for Python 3.8 - can change to Paths when we drop 3.8 support built_wheels.append(output_wheel) # clean up diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index ae18f8630..4eb610219 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -546,13 +546,13 @@ def build(options: Options, tmp_path: Path) -> None: output_wheel = output_dir.joinpath(repaired_wheel.name).resolve() output_wheel.unlink(missing_ok=True) - # os.move() will rename the file to what we were expecting to be the parent directory if we don't ensure it exists + # shutil.move() will rename the file to what we were expecting to be the parent directory if we don't ensure it exists output_dir.mkdir(parents=True, exist_ok=True) - # using os.move() as Path.rename() is not guaranteed to work across filesystem boundaries + # using shutil.move() as Path.rename() is not guaranteed to work across filesystem boundaries shutil.move( str(repaired_wheel), str(output_wheel) - ) # explicit str() needed for mypy3.8 + ) # explicit str() needed for Python 3.8 - can change to Paths when we drop 3.8 support built_wheels.append(output_wheel) # clean up From e04bc49052b3a43a54f5d2b2eea2a7ba86825070 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Wed, 5 Jun 2024 11:24:55 -0400 Subject: [PATCH 10/14] Apply suggestions from code review --- cibuildwheel/macos.py | 5 ++--- cibuildwheel/windows.py | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/cibuildwheel/macos.py b/cibuildwheel/macos.py index a29faffac..2e4404a4d 100644 --- a/cibuildwheel/macos.py +++ b/cibuildwheel/macos.py @@ -648,9 +648,8 @@ def build(options: Options, tmp_path: Path) -> None: output_dir.mkdir(parents=True, exist_ok=True) # using shutil.move() as Path.rename() is not guaranteed to work across filesystem boundaries - shutil.move( - str(repaired_wheel), str(output_wheel) - ) # explicit str() needed for Python 3.8 - can change to Paths when we drop 3.8 support + # explicit str() needed for Python 3.8 + shutil.move(str(repaired_wheel), str(output_wheel)) built_wheels.append(output_wheel) # clean up diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index 4eb610219..b6c8840d9 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -550,9 +550,8 @@ def build(options: Options, tmp_path: Path) -> None: output_dir.mkdir(parents=True, exist_ok=True) # using shutil.move() as Path.rename() is not guaranteed to work across filesystem boundaries - shutil.move( - str(repaired_wheel), str(output_wheel) - ) # explicit str() needed for Python 3.8 - can change to Paths when we drop 3.8 support + # explicit str() needed for Python 3.8 + shutil.move(str(repaired_wheel), str(output_wheel)) built_wheels.append(output_wheel) # clean up From db4abe26fbb9bcd0391836bcbb00cbba4bc3ba2f Mon Sep 17 00:00:00 2001 From: Mike Foster Date: Thu, 6 Jun 2024 17:34:31 +0200 Subject: [PATCH 11/14] Break out functionality to move files to util.py --- cibuildwheel/macos.py | 15 +++++---------- cibuildwheel/pyodide.py | 9 ++++++--- cibuildwheel/util.py | 35 +++++++++++++++++++++++++++++++++++ cibuildwheel/windows.py | 15 +++++---------- 4 files changed, 51 insertions(+), 23 deletions(-) diff --git a/cibuildwheel/macos.py b/cibuildwheel/macos.py index 2e4404a4d..521a42f80 100644 --- a/cibuildwheel/macos.py +++ b/cibuildwheel/macos.py @@ -36,6 +36,7 @@ get_build_verbosity_extra_flags, get_pip_version, install_certifi_script, + move_file, prepare_command, read_python_configs, shell, @@ -640,16 +641,10 @@ def build(options: Options, tmp_path: Path) -> None: # we're all done here; move it to output (overwrite existing) if compatible_wheel is None: - output_dir = build_options.output_dir.resolve() - output_wheel = output_dir.joinpath(repaired_wheel.name).resolve() - output_wheel.unlink(missing_ok=True) - - # shutil.move() will rename the file to what we were expecting to be the parent directory if we don't ensure it exists - output_dir.mkdir(parents=True, exist_ok=True) - - # using shutil.move() as Path.rename() is not guaranteed to work across filesystem boundaries - # explicit str() needed for Python 3.8 - shutil.move(str(repaired_wheel), str(output_wheel)) + output_wheel = build_options.output_dir.joinpath(repaired_wheel.name) + moved_wheel = move_file(repaired_wheel, output_wheel) + if moved_wheel != output_wheel.resolve(): + log.warning("{repaired_wheel} was moved to {moved_wheel} instead of {output_wheel}") built_wheels.append(output_wheel) # clean up diff --git a/cibuildwheel/pyodide.py b/cibuildwheel/pyodide.py index 85504d2ea..b56bec7a3 100644 --- a/cibuildwheel/pyodide.py +++ b/cibuildwheel/pyodide.py @@ -26,6 +26,7 @@ extract_zip, find_compatible_wheel, get_pip_version, + move_file, prepare_command, read_python_configs, shell, @@ -394,9 +395,11 @@ def build(options: Options, tmp_path: Path) -> None: # we're all done here; move it to output (overwrite existing) if compatible_wheel is None: - build_options.output_dir.joinpath(repaired_wheel.name).unlink(missing_ok=True) + output_wheel = build_options.output_dir.joinpath(repaired_wheel.name) + moved_wheel = move_file(repaired_wheel, output_wheel) + if moved_wheel != output_wheel.resolve(): + log.warning("{repaired_wheel} was moved to {moved_wheel} instead of {output_wheel}") + built_wheels.append(output_wheel) - shutil.move(str(repaired_wheel), build_options.output_dir) - built_wheels.append(build_options.output_dir / repaired_wheel.name) finally: pass diff --git a/cibuildwheel/util.py b/cibuildwheel/util.py index 22e2c212b..75d1d63e6 100644 --- a/cibuildwheel/util.py +++ b/cibuildwheel/util.py @@ -6,6 +6,7 @@ import os import re import shlex +import shutil import ssl import subprocess import sys @@ -359,6 +360,40 @@ def extract_tar(tar_src: Path, dest: Path) -> None: tar_.extractall(dest) +def move_file(src_file: Path, dst_file: Path) -> Path: + """Moves a file safely while avoiding potential semantic confusion: + + 1. `dst_file` must point to the target filename, not a directory + 2. `dst_file` will be overwritten if it already exists + 3. any missing parent directories will be created + + Returns the fully resolved Path of the resulting file. + + Raises: + NotADirectoryError: If any part of the intermediate path to `dst_file` is an existing file + IsADirectoryError: If `dst_file` points directly to an existing directory + """ + + # Importing here as logger needs various functions from util -> circular imports + from .logger import log + + src_file = src_file.resolve() + dst_file = dst_file.resolve() + + if dst_file.is_dir(): + # Cannot overwrite a directory with a file + raise IsADirectoryError + dst_file.unlink(missing_ok=True) + dst_file.parent.mkdir(parents=True, exist_ok=True) + + # using shutil.move() as Path.rename() is not guaranteed to work across filesystem boundaries + # explicit str() needed for Python 3.8 + resulting_file = shutil.move(str(src_file), str(dst_file)) + resulting_file = Path(resulting_file).resolve() + log.notice(f"Moved {src_file} to {resulting_file}") + return Path(resulting_file) + + class DependencyConstraints: def __init__(self, base_file_path: Path): assert base_file_path.exists() diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index b6c8840d9..dbd43f158 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -33,6 +33,7 @@ find_compatible_wheel, get_build_verbosity_extra_flags, get_pip_version, + move_file, prepare_command, read_python_configs, shell, @@ -542,16 +543,10 @@ def build(options: Options, tmp_path: Path) -> None: # we're all done here; move it to output (remove if already exists) if compatible_wheel is None: - output_dir = build_options.output_dir.resolve() - output_wheel = output_dir.joinpath(repaired_wheel.name).resolve() - output_wheel.unlink(missing_ok=True) - - # shutil.move() will rename the file to what we were expecting to be the parent directory if we don't ensure it exists - output_dir.mkdir(parents=True, exist_ok=True) - - # using shutil.move() as Path.rename() is not guaranteed to work across filesystem boundaries - # explicit str() needed for Python 3.8 - shutil.move(str(repaired_wheel), str(output_wheel)) + output_wheel = build_options.output_dir.joinpath(repaired_wheel.name) + moved_wheel = move_file(repaired_wheel, output_wheel) + if moved_wheel != output_wheel.resolve(): + log.warning("{repaired_wheel} was moved to {moved_wheel} instead of {output_wheel}") built_wheels.append(output_wheel) # clean up From 8a98f74006d000392fe3ba2df950620f49d0685d Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 6 Jun 2024 15:36:04 +0000 Subject: [PATCH 12/14] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- cibuildwheel/macos.py | 4 +++- cibuildwheel/pyodide.py | 4 +++- cibuildwheel/util.py | 6 +++--- cibuildwheel/windows.py | 4 +++- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/cibuildwheel/macos.py b/cibuildwheel/macos.py index 521a42f80..813531b09 100644 --- a/cibuildwheel/macos.py +++ b/cibuildwheel/macos.py @@ -644,7 +644,9 @@ def build(options: Options, tmp_path: Path) -> None: output_wheel = build_options.output_dir.joinpath(repaired_wheel.name) moved_wheel = move_file(repaired_wheel, output_wheel) if moved_wheel != output_wheel.resolve(): - log.warning("{repaired_wheel} was moved to {moved_wheel} instead of {output_wheel}") + log.warning( + "{repaired_wheel} was moved to {moved_wheel} instead of {output_wheel}" + ) built_wheels.append(output_wheel) # clean up diff --git a/cibuildwheel/pyodide.py b/cibuildwheel/pyodide.py index b56bec7a3..e9e75a5fe 100644 --- a/cibuildwheel/pyodide.py +++ b/cibuildwheel/pyodide.py @@ -398,7 +398,9 @@ def build(options: Options, tmp_path: Path) -> None: output_wheel = build_options.output_dir.joinpath(repaired_wheel.name) moved_wheel = move_file(repaired_wheel, output_wheel) if moved_wheel != output_wheel.resolve(): - log.warning("{repaired_wheel} was moved to {moved_wheel} instead of {output_wheel}") + log.warning( + "{repaired_wheel} was moved to {moved_wheel} instead of {output_wheel}" + ) built_wheels.append(output_wheel) finally: diff --git a/cibuildwheel/util.py b/cibuildwheel/util.py index 75d1d63e6..781c5328a 100644 --- a/cibuildwheel/util.py +++ b/cibuildwheel/util.py @@ -362,7 +362,7 @@ def extract_tar(tar_src: Path, dest: Path) -> None: def move_file(src_file: Path, dst_file: Path) -> Path: """Moves a file safely while avoiding potential semantic confusion: - + 1. `dst_file` must point to the target filename, not a directory 2. `dst_file` will be overwritten if it already exists 3. any missing parent directories will be created @@ -373,7 +373,7 @@ def move_file(src_file: Path, dst_file: Path) -> Path: NotADirectoryError: If any part of the intermediate path to `dst_file` is an existing file IsADirectoryError: If `dst_file` points directly to an existing directory """ - + # Importing here as logger needs various functions from util -> circular imports from .logger import log @@ -392,7 +392,7 @@ def move_file(src_file: Path, dst_file: Path) -> Path: resulting_file = Path(resulting_file).resolve() log.notice(f"Moved {src_file} to {resulting_file}") return Path(resulting_file) - + class DependencyConstraints: def __init__(self, base_file_path: Path): diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index dbd43f158..f5daa28c6 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -546,7 +546,9 @@ def build(options: Options, tmp_path: Path) -> None: output_wheel = build_options.output_dir.joinpath(repaired_wheel.name) moved_wheel = move_file(repaired_wheel, output_wheel) if moved_wheel != output_wheel.resolve(): - log.warning("{repaired_wheel} was moved to {moved_wheel} instead of {output_wheel}") + log.warning( + "{repaired_wheel} was moved to {moved_wheel} instead of {output_wheel}" + ) built_wheels.append(output_wheel) # clean up From 2f18df5416f463afdadd54832adef25f19ceee6a Mon Sep 17 00:00:00 2001 From: Mike Foster Date: Thu, 6 Jun 2024 18:40:58 +0200 Subject: [PATCH 13/14] raise instance of IsADirectoryError with meaningful message --- cibuildwheel/util.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cibuildwheel/util.py b/cibuildwheel/util.py index 781c5328a..3e4f8b43f 100644 --- a/cibuildwheel/util.py +++ b/cibuildwheel/util.py @@ -382,7 +382,8 @@ def move_file(src_file: Path, dst_file: Path) -> Path: if dst_file.is_dir(): # Cannot overwrite a directory with a file - raise IsADirectoryError + msg = "dst_file must be a valid target filename, not an existing directory." + raise IsADirectoryError(msg) dst_file.unlink(missing_ok=True) dst_file.parent.mkdir(parents=True, exist_ok=True) From 03aeff3ffb38afd1eb0c4746efc884f4fe2fc7c8 Mon Sep 17 00:00:00 2001 From: Mike Foster Date: Thu, 6 Jun 2024 18:41:48 +0200 Subject: [PATCH 14/14] Don't need a comment and a exception message --- cibuildwheel/util.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cibuildwheel/util.py b/cibuildwheel/util.py index 3e4f8b43f..2d127f2bc 100644 --- a/cibuildwheel/util.py +++ b/cibuildwheel/util.py @@ -381,7 +381,6 @@ def move_file(src_file: Path, dst_file: Path) -> Path: dst_file = dst_file.resolve() if dst_file.is_dir(): - # Cannot overwrite a directory with a file msg = "dst_file must be a valid target filename, not an existing directory." raise IsADirectoryError(msg) dst_file.unlink(missing_ok=True)