fix(pypi): rewrite RECORD file entries for extracted .data contents by rickeylev · Pull Request #4025 · bazel-contrib/rules_python

rickeylev · 2026-08-09T03:50:49Z

Extracting wheels with .data/ subdirectories moves files to their
target directories and deletes .data/, but left .dist-info/RECORD
referencing deleted .data/ paths. This causes tools like
importlib.metadata.files() to fail locating or reading files.

Per PEP 427 and PEP 376, rewrite .dist-info/RECORD entries during
wheel extraction to match installed locations relative to
site-packages.

Also adds Starlark unit tests for RECORD rewriting and expands
importlib_metadata_test.

Work towards #3024

…and are readable Expand importlib_metadata_test to assert that all files returned by importlib.metadata.files() can be found on disk and read both as binary and UTF-8 text. Update test fixture RECORD files to list installed paths.

…und and read Assert that each file returned by importlib.metadata.files() exists and can be read, reproducing the issue with unrewritten RECORD files.

Verify that the list of files returned by importlib.metadata.files() matches the expected set of paths from the distribution's RECORD.

Sort expected_paths list alphabetically in source for improved readability.

When extracting wheels containing .data directories (purelib, platlib, scripts, headers, data), the files are moved into their respective target scheme directories and .data is deleted. Rewrite .dist-info/RECORD entries so that archive member paths prefixed with .data/ are updated to their installed paths relative to site-packages, allowing tools like importlib.metadata.files() to locate and read them. Also add Starlark unit tests for the RECORD rewriting logic.

…tadata_files

Unify extraction destinations and RECORD path rewrite prefixes into a single _DATA_CATEGORIES mapping in whl_extract.bzl. Also add a news entry fragment for the bug fix in bazel-contrib#4025.

…_test On Windows, virtual environments have a 2-level directory depth (Lib/site-packages) compared to POSIX 3-level depth (lib/pythonX.Y/site-packages), so relative paths in RECORD starting with .. cannot resolve via locate() on Windows. Skip locate() on Windows for files outside site-packages.

Virtual environment directory structure depth differs between Windows (`Lib/site-packages`) and POSIX (`lib/pythonX.Y/site-packages`). Because the host platform during repo extraction cannot determine the target platform, RECORD files must be generated per target platform. Move extracted RECORD files to `rewrite-record/` and rewrite relative paths for `.data` directories at build time via `gen_wheel_record`.

Update wheel_record_rewriter_test.sh to run .ps1 scripts via powershell.exe when executing under Windows MSYS2 bash. Update importlib_metadata_test.py to assert data_prefix ('../../') for files originating in .data/data/bin/ and .data/data/include/ on Windows.

Convert input/output paths using cygpath -w when running powershell in wheel_record_rewriter_test.sh and ignore CRLF differences in diff. Handle .bat extension on Windows venv bin scripts when validating located files in importlib_metadata_test.py.

aignas · 2026-08-11T00:53:51Z

A few open questions:

Should we actually do this path manipulation where we extract it differently from the wheel layout? We are doing this because this was the layout before when we used to extract with installer.
Should we rename the RECORD file to something else, like RECORD.orig? We can generate a RECORD file from just the globs in a build action and that would mean it is always deterministic.

rickeylev · 2026-08-11T01:04:15Z

AFAIK, we're extracting the wheel the same as what other installers do, sans the later "install" part where it shows up into a binary's venv.

e.g. given https://github.com/bazel-contrib/rules_python/blob/main/tests/repos/whl_with_data1/whl_with_data1-1.0.dist-info/RECORD

platlib/ and purelib/ goes into site-packages
scripts goes into scripts; venv later puts it into $venv/bin/
headers/ goes into headers; venv later puts it into $venv/headers/
data/ goes into data; venv later puts it into $venv/ directly (e.g. data/XXX/foo.h will show up as $venv/XXX/foo.h

- Use PyRewriteWheelRecord mnemonic and progress message in gen_wheel_record.bzl - Use WINDOWS_CONSTRAINTS_ATTRS helper for _windows_constraints attribute - Move _DATA_CATEGORIES mapping to top of whl_extract.bzl - Document RECORD relative path conventions citing PEP 376 and PEP 427 in importlib_metadata_test.py

…rge_trees - Define WINDOWS_CONSTRAINTS_ATTRS using attr.label_list so rules can use dicts.add directly - Restore upstream merge_trees implementation in whl_extract.bzl

…n operator - Factor out WINDOWS_CONSTRAINTS_PLAIN_ATTRS in attributes.bzl for rules not using builders - Use | operator in gen_wheel_record.bzl instead of dicts.add

rickeylev · 2026-08-11T06:38:55Z

Ready, PTAL

aignas

Just to mention related code:

When patching the wheels we are rewriting a RECORD file. Since we are doing this here as well, we should stop doing it. Just patching the files is enough. That brings a question if we should rezip the patched wheel itself in a build action?

rickeylev · 2026-08-11T23:20:09Z

should rezip the patched wheel itself in a build action?

I would lean towards no, for two reasons: (1) keep our API smaller, and (2) does doing so benefit our code at all?

I don't entirely remember why re-zipping was required...did it stem from using piptools or something? Something that required a .whl to process?

TBH I'm not a fan of the :whl targets that let you reference the .whl file. They double the amount of disk used and I'm not sure they provide sufficient value.

- PackagePath.locate() returns os.PathLike which lacks attribute definitions in type checkers - Explicitly convert to pathlib.Path to satisfy pyrefly static analysis

aignas · 2026-08-12T05:56:59Z

TBH I'm not a fan of the :whl targets that let you reference the .whl file. They double the amount of disk used and I'm not sure they provide sufficient value.

The only internal usage is gazelle_python.yaml generation, but we should be able to use the extracted_whl_srcs for this.

For external usage, there is definitely something, but it's hard to know. I think patching + whl reziping is such a niche use-case, that we could just not support forward compatibility in there.

rickeylev added 4 commits August 9, 2026 03:39

test(venv): update importlib_metadata_test to verify all files are fo…

caeb209

…und and read Assert that each file returned by importlib.metadata.files() exists and can be read, reproducing the issue with unrewritten RECORD files.

test(venv): assert exact expected file paths in importlib_metadata_test

866c9eb

Verify that the list of files returned by importlib.metadata.files() matches the expected set of paths from the distribution's RECORD.

test(venv): sort expected paths in importlib_metadata_test

bd0da7e

Sort expected_paths list alphabetically in source for improved readability.

rickeylev mentioned this pull request Aug 9, 2026

chore: update default Python version to 3.14 #4023

Draft

rickeylev changed the title ~~test(venv): [repro] importlib_metadata_test fails to find/read wheel files~~ fix(pypi): rewrite RECORD file entries for extracted .data contents Aug 10, 2026

rickeylev added 7 commits August 10, 2026 05:51

Merge remote-tracking branch 'upstream/main' into verify_importlib_me…

f57b25f

…tadata_files

refactor(pypi): unify wheel .data extraction and RECORD rewrite mapping

cd2f6e4

Unify extraction destinations and RECORD path rewrite prefixes into a single _DATA_CATEGORIES mapping in whl_extract.bzl. Also add a news entry fragment for the bug fix in bazel-contrib#4025.

fix(pypi): emit lf line endings in powershell wheel record rewriter

ccb8660

rickeylev added 4 commits August 11, 2026 04:07

refactor(pypi): use direct attr dict in attributes.bzl and restore me…

3534a13

…rge_trees - Define WINDOWS_CONSTRAINTS_ATTRS using attr.label_list so rules can use dicts.add directly - Restore upstream merge_trees implementation in whl_extract.bzl

refactor(pypi): add WINDOWS_CONSTRAINTS_PLAIN_ATTRS and use dict unio…

a94faba

…n operator - Factor out WINDOWS_CONSTRAINTS_PLAIN_ATTRS in attributes.bzl for rules not using builders - Use | operator in gen_wheel_record.bzl instead of dicts.add

Merge branch 'upstream/main' into verify_importlib_metadata_files

203e233

rickeylev marked this pull request as ready for review August 11, 2026 06:38

rickeylev requested a review from aignas as a code owner August 11, 2026 06:38

aignas approved these changes Aug 11, 2026

View reviewed changes

aignas reviewed Aug 11, 2026

View reviewed changes

Comment thread python/private/pypi/BUILD.bazel

aignas added this pull request to the merge queue Aug 11, 2026