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

Adopted the bdist_wheel command from "wheel" #4369

Merged
merged 31 commits into from
May 22, 2024

Conversation

agronholm
Copy link
Contributor

@agronholm agronholm commented May 19, 2024

Summary of changes

This adopts the bdist_wheel command and vendors most of the wheel code.

Closes #1386.

Pull Request Checklist

NOTE: Coverage reports missing lines in test_bdist_wheel but those would be handled with combined coverage reporting.

@agronholm agronholm marked this pull request as draft May 19, 2024 10:18
@agronholm
Copy link
Contributor Author

@jaraco I'll need some assistance, perhaps during the sprints, to figure out why there are so many errors/failures. The test suite didn't pass for me locally even with a pristine copy of the repo without my changes, but these changes seemed to have introduced tons of additional errors and failures.

@abravalheri
Copy link
Contributor

abravalheri commented May 20, 2024

I'll need some assistance, perhaps during the sprints, to figure out why there are so many errors/failures. The test suite didn't pass for me locally even with a pristine copy of the repo without my changes, but these changes seemed to have introduced tons of additional errors and failures.

Just a wild guess, but maybe it also needs to be bootstrapped in https://github.com/pypa/setuptools/blob/main/bootstrap.egg-info/entry_points.txt?

@agronholm
Copy link
Contributor Author

I'll need some assistance, perhaps during the sprints, to figure out why there are so many errors/failures. The test suite didn't pass for me locally even with a pristine copy of the repo without my changes, but these changes seemed to have introduced tons of additional errors and failures.

Just a wild guess, but maybe it also needs to be bootstrapped in https://github.com/pypa/setuptools/blob/main/bootstrap.egg-info/entry_points.txt?

I have no clue. Can you make sense of the errors in CI? It's like at least some of them have nothing to do with this change.

Co-authored-by: Anderson Bravalheri <andersonbravalheri+github@gmail.com>
@abravalheri
Copy link
Contributor

abravalheri commented May 20, 2024

Can you make sense of the errors in CI? It's like at least some of them have nothing to do with this change.

For some reason when you run tox -e vendor some directories ended up excluded. I am not sure why that happened, but when I run tox -e vendor in my machine, as a result it creates these new files that are not added to the PR yet:

# Python 3.12 (pipx run --python python3.12 tox -e vendor)
        new file:   pkg_resources/_vendor/backports/tarfile/__init__.py
        new file:   pkg_resources/_vendor/backports/tarfile/__main__.py
        new file:   pkg_resources/_vendor/backports/tarfile/compat/__init__.py
        new file:   pkg_resources/_vendor/backports/tarfile/compat/py38.py
        new file:   setuptools/_vendor/backports/tarfile/__init__.py
        new file:   setuptools/_vendor/backports/tarfile/__main__.py
        new file:   setuptools/_vendor/backports/tarfile/compat/__init__.py
        new file:   setuptools/_vendor/backports/tarfile/compat/py38.py
# Python 3.8 (pipx run --python python3.8 tox -e vendor)
        new file:   pkg_resources/_vendor/backports/tarfile/__init__.py
        new file:   pkg_resources/_vendor/backports/tarfile/__main__.py
        new file:   pkg_resources/_vendor/backports/tarfile/compat/__init__.py
        new file:   pkg_resources/_vendor/backports/tarfile/compat/py38.py
        new file:   setuptools/_vendor/backports/tarfile/__init__.py
        new file:   setuptools/_vendor/backports/tarfile/__main__.py
        new file:   setuptools/_vendor/backports/tarfile/compat/__init__.py
        new file:   setuptools/_vendor/backports/tarfile/compat/py38.py

I think these files need to be included.

There also seems to be a problems with static check:

.tox/py/lib/python3.12/site-packages/pytest_mypy.py:212: MypyWarning
_________________________________ setuptools/tests/test_bdist_wheel.py _________________________________
[gw4] linux -- Python 3.12.3 /home/abravalheri/workspace/setuptools/.tox/py/bin/python
458: error: Cannot find implementation or library stub for module named "wheel"  [import-not-found]
458: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports

Maybe this solves the problem:

diff --git i/setuptools/tests/test_bdist_wheel.py w/setuptools/tests/test_bdist_wheel.py
index 161fb88e2..1f8e76e30 100644
--- i/setuptools/tests/test_bdist_wheel.py
+++ w/setuptools/tests/test_bdist_wheel.py
@@ -455,6 +455,6 @@ def test_no_ctypes(monkeypatch) -> None:
         if module.startswith("wheel"):
             monkeypatch.delitem(sys.modules, module)

-    from wheel import bdist_wheel
+    from setuptools.command import bdist_wheel

     assert bdist_wheel

I am still having a look on the other errors that appeared.

setuptools/tests/test_bdist_wheel.py Outdated Show resolved Hide resolved
setuptools/tests/test_bdist_wheel.py Outdated Show resolved Hide resolved
@agronholm
Copy link
Contributor Author

I have one final problem, with the mypy exclude not working for bdist_wheel_testdata.

@agronholm
Copy link
Contributor Author

We debugged this with @webknjaz and concluded that we need to prevent files from setuptools/tests/bdist_wheel_testdata from being collected by pytest, but we couldn't yet figure out any working way to accomplish that.

@agronholm agronholm marked this pull request as ready for review May 21, 2024 14:36
@agronholm
Copy link
Contributor Author

Should be a-ok now. The missing coverage lines reported by collateral/diffcov are due to conditional imports and would be covered by a combined coverage report.

tools/vendored.py Outdated Show resolved Hide resolved
@agronholm
Copy link
Contributor Author

Adapted to changes in main, all good again.

@jaraco
Copy link
Member

jaraco commented May 22, 2024

Thanks Alex for the work here. I've added @agronholm as a Setuptools Developer. Please feel free to merge.

@agronholm agronholm merged commit 52d7324 into pypa:main May 22, 2024
20 of 22 checks passed
@agronholm agronholm deleted the vendored-wheel branch May 22, 2024 14:36
@Avasam Avasam mentioned this pull request May 22, 2024
2 tasks
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 this pull request may close these issues.

Adopt bdist_wheel from wheel project
3 participants