importlib.metadata.files() doesn't work because RECORD is missing

[rickeylev]

It appears that importlib.metadata.files() is broken with the pypi integration because the RECORD file gets ommited.

Per docs at https://docs.python.org/3/library/importlib.metadata.html#distribution-files

In the case where the metadata file listing files (RECORD or SOURCES.txt) is missing, files() will return

This came to my attention via @mering trying to use the rtree package, which tries to use its distribution metadata to find some C libraries to load.

Excluding RECORD seems somewhat intentional, though? The code has this comment:

https://github.com/bazel-contrib/rules_python/blob/main/python/private/pypi/whl_library_targets.bzl#L315-L318

RECORD is known to contain sha256 checksums of files which might include the checksums
of generated files produced when wheels are installed. The file is ignored to avoid
Bazel caching issues.

What's the caching issue, exactly? For a downloaded whl, what generated files is it referring to? If a whl is built at repo time from an sdist, then files shouldn't be changing, and the hash should be stable, so how does a cache issue come up?

[groodt]

I agree. I never liked removal of it.

However, I think it's to support sdist. Wheel building outside specialized tools like bazel can be non-reproducible.

My solution to that problem is to drop support for sdist in lockfiles, but that doesn't make me very fun at parties. 😜

[aignas]

To make it reproducible one could run:

• repairwheel
• repack the wheel with our script that attempts to make the RECORD reproducible.

[groodt]

I guess we could also create an invalid RECORD tbh? Just listing of the files without hashes.

I don't understand how repacking the wheel would work tbh (unless we are saying the same thing) because if we attempt to create a valid RECORD file, the source of the non-determinism is the file hashes. And guaranteeing determinism of arbitrary files from an sdist is the problem.

A thought that I've had is that maybe instead of hiding the problem, we should rather document the behaviour. So if somebody notices they see cache misses, they should correct the source of non-determinism. Either through creating wheels upfront and locking to those, or some other environmental changes to squeeze out non-determinism in the sdist builds.

[rickeylev]

I still don't understand why building from sdist (in repo phase) is a problem.

If the sdist-built-during-repo-phase files are non-deterministic, how does ignoring the RECORD file change that? Those files are still inputs into the build. The RECORD is basically just recording what bazel already will (content hash).

Once RECORD file is created, why does it matter if the hashes in it aren't correct? It's not like it's being re-packed into a wheel (right?).

[aignas]

I think the problem could be when sdist building depends on system libraries and files are generated referring absolute paths. That said, I agree with you, we should probably include the RECORD file, we can include it using a build flag.

[groodt]

I think it will bust the inputs @rickeylev

If the hash of the RECORD changes, which it will if anything in the sdist->wheel conversion changes, then all targets downstream are busted.

Your point is that if this happens the cache is busted anyway is true as far as I can tell. We do exclude some other files, but it seems unlikely the exclusion of RECORD helps a lot here.

I seem to remember a PR or issue related to this, but I don't have it to hand. I'd probably just include it by default tbh. I don't know why it should ever be optout.

[rickeylev]

unanticipatedly related: #2782 (wheels with BUILD files have to have them removed, and thus the record file is no longer accurate)