Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release wheel 51.1.0 packs to many #2505

Closed
gaborbernat opened this issue Dec 23, 2020 · 14 comments · Fixed by LibreELEC/LibreELEC.tv#4952
Closed

Release wheel 51.1.0 packs to many #2505

gaborbernat opened this issue Dec 23, 2020 · 14 comments · Fixed by LibreELEC/LibreELEC.tv#4952

Comments

@gaborbernat
Copy link
Contributor

I think it should be yanked. Look at the file size of the wheel before and after:

setuptools-51.1.0-py3-none-any.whl (2.0 MB)
setuptools-51.0.0-py3-none-any.whl (785.2 kB)

The reason for the extra size (more than double of the previous) is that somehow you now include all test files in the wheel. I don't think test files should be in wheel (only in sdist).

Discovered as this is causing now Windows long path limit violation for the virtualenv CI pypa/virtualenv#2036, which is unsupported by PyPy3 on Windows https://foss.heptapod.net/pypy/pypy/-/issues/3363.

@gaborbernat
Copy link
Contributor Author

@jaraco you made a new release, but still ignored this ticket that's causing headaches for setuptools, can you take a look at it, please?

@pombredanne
Copy link

FWIW the last OK (without test code) release seems to be v51.0.0 but I cannot see what changes would have triggered that would be the culprit though there are a lot of changes (in part because of adopting a skeleton)
This has not changed v51.0.0...v51.1.0#diff-fa602a8a75dc9dcc92261bac5f533c2a85e34fcceaff63b3a3a81d9acde2fc52R32 :|

Happy new year!

@jaraco
Copy link
Member

jaraco commented Jan 8, 2021

Apologies for the delay in responding. I have looked into this, either as part of this report or possibly as part of another report. I don't see that I made a response or status update, so I'll try to do better.

Possibly related to #1461 and the introduction of include_package_data as part of adopting jaraco/skeleton. The "tests" are considered package data, so get included.

@jaraco
Copy link
Member

jaraco commented Jan 8, 2021

I'm actually a little surprised that adding tests would add 1.2MB to the wheel. From my estimation, there's only 500k additional uncompressed in the source for those test directories:

setuptools main $ du -h setuptools/tests pkg_resources/tests
4.0K    setuptools/tests/indexes/test_links_priority/simple/foobar
4.0K    setuptools/tests/indexes/test_links_priority/simple
8.0K    setuptools/tests/indexes/test_links_priority
8.0K    setuptools/tests/indexes
404K    setuptools/tests
4.0K    pkg_resources/tests/data/my-test-package_zipped-egg
 20K    pkg_resources/tests/data/my-test-package_unpacked-egg/my_test_package-1.0-py3.7.egg/EGG-INFO
 20K    pkg_resources/tests/data/my-test-package_unpacked-egg/my_test_package-1.0-py3.7.egg
 20K    pkg_resources/tests/data/my-test-package_unpacked-egg
4.0K    pkg_resources/tests/data/my-test-package-source
 28K    pkg_resources/tests/data
 96K    pkg_resources/tests

Maybe the wheel gets compiled artifacts?

@jaraco
Copy link
Member

jaraco commented Jan 8, 2021

So there are several bugs implicit in this report:

  • Including tests in the wheel results in file paths unsupported by PyPy
  • Including tests in the wheel results in bloat of the wheel distribution
  • Tests are installed even though it's intended that they not be

Obviously, addressing the third issue will work around the others.

@jaraco
Copy link
Member

jaraco commented Jan 8, 2021

After some inspection, I found that expanding the wheel, the setuptools/tests and pkg_resources/tests directories still only account for ~500k of space, so removing the tests from the install may not address the space issue.

@gaborbernat
Copy link
Contributor Author

So the question is what else got included to fill up the rest of the space?

@pombredanne
Copy link

Apologies for the delay in responding. I have looked into this, either as part of this report or possibly as part of another report. I don't see that I made a response or status update, so I'll try to do better.

Thanks for chiming in and Happy New Year! You do not "owe us" anything and that's the holidays, so it's cool :)

@benoit-pierre
Copy link
Member

 60K setuptools/_distutils/command/wininst-6.0.exe
 60K setuptools/_distutils/command/wininst-8.0.exe
 64K setuptools/_distutils/command/wininst-7.1.exe
188K setuptools/_distutils/command/wininst-10.0.exe
192K setuptools/_distutils/command/wininst-9.0.exe
220K setuptools/_distutils/command/wininst-10.0-amd64.exe
220K setuptools/_distutils/command/wininst-9.0-amd64.exe
448K setuptools/_distutils/command/wininst-14.0.exe
576K setuptools/_distutils/command/wininst-14.0-amd64.exe
  2M total

@jaraco
Copy link
Member

jaraco commented Jan 8, 2021

Oh, my. So the fact that those exes are missing from the install on older installs is probably another bug... and likely will manifest if/when wininst is invoked with SETUPTOOLS_USE_DISTUTILS=local. Thanks for finding that, Benoit!

@gaborbernat
Copy link
Contributor Author

Do we need all those versions of it? Can we strip some of it, maybe have different wheels per OS compatibility?

@jaraco jaraco closed this as completed in fbc9bd6 Jan 9, 2021
@pombredanne
Copy link

Could we drop wininst entirely to another package?

@jaraco
Copy link
Member

jaraco commented Jan 9, 2021

Could we drop wininst entirely to another package?

I was thinking just that, but I think it's more likely that Setuptools will drop bdist_wininst support alongside Python 3.10, obviating the need altogether.

heitbaum added a commit to heitbaum/LibreELEC.tv that referenced this issue Jan 14, 2021
@abravalheri
Copy link
Contributor

abravalheri commented Apr 29, 2022

As per #3284, it seems that recent versions of setuptools do not include the necessary wininst-*.exe binaries. So we still have the problems mentioned in #2505 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants