From 42aa1345c34b05d518c289cd281cdd950be67967 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Fri, 12 Jun 2026 11:03:36 -0400 Subject: [PATCH] chore: minor cleanups and perf tweaks from code review (#2910) * chore: minor cleanups and perf tweaks from code review - util/file.py, util/python_build_standalone.py: use hashlib.file_digest for streaming SHA-256 verification instead of loading whole archives into memory with read_bytes() - logger.py: convert colors/symbols properties to functools.cached_property so the Colors/Symbols objects are constructed only once per Logger instance - platforms/windows.py: remove redundant .strip() on where_pip (already stripped at assignment) - util/python_build_standalone.py: remove unreachable python_base_dir.exists() guard (callers always pass a fresh temp subdirectory) - util/file.py: add comment explaining the getattr shim for tar_.extraction_filter and when it can be removed Assisted-by: ClaudeCode:claude-fable-5 * revert: restore assertion to check python_base_dir existence --- cibuildwheel/logger.py | 4 ++-- cibuildwheel/platforms/windows.py | 2 +- cibuildwheel/util/file.py | 5 ++++- cibuildwheel/util/python_build_standalone.py | 7 +++---- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/cibuildwheel/logger.py b/cibuildwheel/logger.py index 69314fac..aeb6c42f 100644 --- a/cibuildwheel/logger.py +++ b/cibuildwheel/logger.py @@ -375,11 +375,11 @@ class Logger: out.write("\n") return out.getvalue() - @property + @functools.cached_property def colors(self) -> Colors: return Colors(enabled=self.colors_enabled) - @property + @functools.cached_property def symbols(self) -> Symbols: return Symbols(unicode=self.unicode_enabled) diff --git a/cibuildwheel/platforms/windows.py b/cibuildwheel/platforms/windows.py index fd4aa65b..bd6cb3a5 100644 --- a/cibuildwheel/platforms/windows.py +++ b/cibuildwheel/platforms/windows.py @@ -348,7 +348,7 @@ def setup_python( assert (venv_path / "Scripts" / "pip.exe").exists() where_pip = call("where", "pip", env=env, capture_stdout=True).splitlines()[0].strip() print(where_pip) - if where_pip.strip() != str(venv_path / "Scripts" / "pip.exe"): + if where_pip != str(venv_path / "Scripts" / "pip.exe"): msg = "pip available on PATH doesn't match our installed instance. If you have modified PATH, ensure that you don't overwrite cibuildwheel's entry or insert pip above it." raise errors.FatalError(msg) call("pip", "--version", env=env) diff --git a/cibuildwheel/util/file.py b/cibuildwheel/util/file.py index 111b7d90..32b0a4fc 100644 --- a/cibuildwheel/util/file.py +++ b/cibuildwheel/util/file.py @@ -99,7 +99,8 @@ def download(url: str, dest: Path, *, sha256: str | None = None) -> None: time.sleep(3) if sha256: - computed = hashlib.sha256(dest.read_bytes()).hexdigest() + with dest.open("rb") as f: + computed = hashlib.file_digest(f, "sha256").hexdigest() if computed != sha256: dest.unlink(missing_ok=True) msg = f"SHA256 mismatch for {url}: expected {sha256!r}, got {computed!r}" @@ -130,6 +131,8 @@ def extract_tar(tar_src: Path, dest: Path) -> None: See: https://docs.python.org/3/library/tarfile.html#tarfile.tar_filter for filter details """ with tarfile.open(tar_src) as tar_: + # getattr shim needed while Python 3.11.0-3.11.3 are supported; + # once the minimum is 3.11.4+/3.12, replace with: tar_.extractall(dest, filter="tar") tar_.extraction_filter = getattr(tarfile, "tar_filter", (lambda member, _: member)) tar_.extractall(dest) diff --git a/cibuildwheel/util/python_build_standalone.py b/cibuildwheel/util/python_build_standalone.py index 2e8c7fb8..30cc0c06 100644 --- a/cibuildwheel/util/python_build_standalone.py +++ b/cibuildwheel/util/python_build_standalone.py @@ -154,7 +154,8 @@ def _download_or_get_from_cache( asset_cache_path = cache_dir / asset_filename if asset_cache_path.is_file(): if sha256: - computed = hashlib.sha256(asset_cache_path.read_bytes()).hexdigest() + with asset_cache_path.open("rb") as f: + computed = hashlib.file_digest(f, "sha256").hexdigest() if computed != sha256: print( f"Cached python_build_standalone SHA256 mismatch for {asset_cache_path}; redownloading." @@ -229,9 +230,7 @@ def create_python_build_standalone_environment( ) python_base_dir = temp_dir / "pbs" - if python_base_dir.exists(): - msg = f"python-build-standalone directory already exists: {python_base_dir}" - raise PythonBuildStandaloneError(msg) + assert not python_base_dir.exists() extract_tar(archive_path, python_base_dir) return _find_python_executable(python_base_dir)