ziglang / zig-pypi

The Zig programming language, packaged for PyPI
https://pypi.org/project/ziglang/
MIT License
148 stars 16 forks source link

allow newer wheel dependency #16

Closed csachs closed 12 months ago

csachs commented 12 months ago

The error mentioned in #13 is due to the changes introduced by https://github.com/pypa/wheel/pull/511 which changed the RECORD file creation from creating a ZipInfo in ẀheelFile.close() to just using WheelFile.writestr(<name>, <contents>), which led to the error in the overridden method in ReproducibleWheelFile.

This PR allows usage of newer wheel.

However, in general, it seems the standard for files in wheels is 664 (rw-rw-r--) not 644 (rw-r--r--), furthermore the PR mentioned above was specifically added to set the S_IFREG bit, which the current code does not set.

To address this in the future, I'd propose for a version jump at which changes in the produced wheels are okay to just use wheel's writestr() along setting env var SOURCE_DATE_EPOCH which wheel will respect and use for all otherwise unset times, yielding reproducible wheels. An idea how to switch to that would be in https://github.com/csachs/zig-pypi/tree/simplerWheel (this would yield binary different wheels tho).

whitequark commented 12 months ago

I don't really see the motivation. If it works why touch it?

In particular I think reproducibility of older wheels is obviously valuable, and being able to build with newer wheel is not obviously valuable.

csachs commented 12 months ago

Reproducibility is certainly very important, but makes only sense taking into account the producer version as well ... otherwise changes like https://github.com/ziglang/zig-pypi/commit/fdb570f019b1d4f0ca15f0e1e6669a401e5fca5d would need to be considered breaking it as well (and one could never improve anything in regard to the packaging...). Maybe the tool could be versioned and the version be embedded in the wheels.

With respect to the motivation, I find maintaining things always easiest if they are kept current in all aspects; if this is not desired then this PR is unnecessary.

whitequark commented 12 months ago

You made a good argument. Thanks for the PR!

csachs commented 12 months ago

Thank you