pypa
Add support for building extensions using MinGW compilers
See individual commits for details on what this PR changes.
A new version of https://github.com/pypa/distutils/pull/78 Fixes https://github.com/pypa/distutils/issues/34
cc @lazka
I notice there are conflicts. I tried to resolve the conflicts, but I don't seem to have access to the fork, so I've pushed a couple of commits to the add-mingw-support-jaraco branch, resolving the conflict. Feel free to pull those changes in.
See individual commits for details on what this PR changes.
A new version of https://github.com/pypa/distutils/pull/78
Fixes https://github.com/pypa/distutils/issues/34
cc @lazka
@naveen521kk What is the status of this PR? I've been switching to PDM for a lot of my work, and installing a number of packages break with the good ol' --plat-name must be one of ('win32', 'win-amd64', 'win-arm32', 'win-arm64') error.
Since the workaround is going away, soon this means I won't be able to install packages anymore.
@naveen521kk What is the status of this PR? I've been switching to PDM for a lot of my work, and installing a number of packages break with the good ol' --plat-name must be one of ('win32', 'win-amd64', 'win-arm32', 'win-arm64') error.
Hey, I'm sorry, I kinda forgot about this PR. I'll spend some time this week to get this working.
@naveen521kk
Hey, I'm sorry, I kinda forgot about this PR.
No worries, I'm kinda happy you responded at all, tbh.
I'll spend some time this week to get this working.
Excellent, tyvm. I'm happy to test locally, although I'm not sure how I would replace the vendored version in my Python with this version for testing purposes. Right now I'm still pip installing pdm from the source tree b/c of the --plat-name errors (it works otherwise fine). Will be interested in whether this allows installing pdm from PyPI to succeed.
Should we also include https://github.com/msys2-contrib/cpython-mingw/pull/157 ?
Should we also include https://github.com/msys2-contrib/cpython-mingw/pull/157 ?
I think so, I'll include them.
naveen521kk: Thank you for your effort.
although I'm not sure how I would replace the vendored version in my Python with this version for testing purposes.
I have Python installed in a pdm/venv environment. Would you happen to know to how to replace the vendored version of distutils with the stdlib version so that I can test your PR? It would be great to get rid of the SETUPTOOLS_USE_DISTUTILS=stdlib workaround, even if via this patch for now.
Also, are the failing tests bad :P?
I've resolved the conflicts and I'd like to test the changes, but GitHub is rejecting me pushing the resolution.
distutils add-mingw-support @ git push
fatal: The upstream branch of your current branch does not match
the name of your current branch. To push to the upstream branch
on the remote, use
git push origin HEAD:refs/pull/184/head
To push to the branch of the same name on the remote, use
git push origin HEAD
To avoid automatically configuring an upstream branch when its name
won't match the local branch, see option 'simple' of branch.autoSetupMerge
in 'git help config'.
distutils add-mingw-support [128] @ git push origin HEAD:refs/pull/184/head
Enumerating objects: 99, done.
Counting objects: 100% (99/99), done.
Delta compression using up to 12 threads
Compressing objects: 100% (66/66), done.
Writing objects: 100% (68/68), 10.07 KiB | 10.07 MiB/s, done.
Total 68 (delta 51), reused 0 (delta 0), pack-reused 0 (from 0)
remote: Resolving deltas: 100% (51/51), completed with 25 local objects.
To https://github.com/pypa/distutils
! [remote rejected] HEAD -> refs/pull/184/head (deny updating a hidden ref)
error: failed to push some refs to 'https://github.com/pypa/distutils'
I'm not sure what "deny updating a hidden ref" means. I've pushed the commit (1b38bc903) to the add-mingw-support branch to see if tests pass.
That failed because there was a latent reference to setup.cfg. Corrected in 416ef6a75.
Tests are now failing during collection:
______________ ERROR collecting distutils/tests/test_install.py _______________
ImportError while importing test module 'D:/a/distutils/distutils/distutils/tests/test_install.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
distutils/tests/test_install.py:16: in <module>
from distutils.util import is_mingw
E ImportError: cannot import name 'is_mingw' from 'distutils.util' (D:/a/_temp/msys64/mingw64/lib/python3.11/distutils/util.py)
___________ ERROR collecting distutils/tests/test_mingwccompiler.py ___________
ImportError while importing test module 'D:/a/distutils/distutils/distutils/tests/test_mingwccompiler.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
distutils/tests/test_mingwccompiler.py:3: in <module>
from distutils.util import split_quoted, is_mingw
E ImportError: cannot import name 'is_mingw' from 'distutils.util' (D:/a/_temp/msys64/mingw64/lib/python3.11/distutils/util.py)
___________ ERROR collecting distutils/tests/test_unixccompiler.py ____________
ImportError while importing test module 'D:/a/distutils/distutils/distutils/tests/test_unixccompiler.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
distutils/tests/test_unixccompiler.py:10: in <module>
from distutils.util import _clear_cached_macosx_ver
E ImportError: cannot import name '_clear_cached_macosx_ver' from 'distutils.util' (D:/a/_temp/msys64/mingw64/lib/python3.11/distutils/util.py)
Clearly the wrong 'distutils' is getting imported. I don't know why.
@naveen521kk Can you pull 5fb38a1e1 over to your branch so we can get the CI to update here and then investigate why the wrong distutils is getting imported (but only for the mingw jobs)?
Clearly the wrong 'distutils' is getting imported. I don't know why.
What is the order in sys.meta_path? Anything weird there?
@naveen521kk Can you pull 5fb38a1 over to your branch so we can get the CI to update here and then investigate why the wrong distutils is getting imported (but only for the mingw jobs)?
From what I see you succeeded in pushing it before writing this (?? https://github.com/msys2-contrib/distutils/tree/add-mingw-support). I can also rebase the whole thing on top of main if wanted.
Clearly the wrong 'distutils' is getting imported. I don't know why.
I can reproduce locally. I'll try to figure out why.
What is the order in
sys.meta_path? Anything weird there?
distutils/tests/test_install.py:8: in <module>
assert 0, sys.meta_path
E AssertionError: [<_pytest.assertion.rewrite.AssertionRewritingHook object at 0x000001a442ffe510>, <_virtualenv._Finder object at 0x000...mportlib_external.PathFinder'>, <class '__editable___distutils_0_1_dev4112_g5fb38a1_d20240627_finder._EditableFinder'>]
Removing this line makes things work again: https://github.com/pypa/distutils/blob/4fd2d569e37268b8c23f3cc45879465f9c269d92/pytest.ini#L5
@naveen521kk Can you pull 5fb38a1 over to your branch so we can get the CI to update here and then investigate why the wrong distutils is getting imported (but only for the mingw jobs)?
I've pulled those changes. I'll also check out why the wrong distutils are imported.
What is the order in sys.meta_path? Anything weird there?
Not sure if there's anything weird there but this is what I get.
$ python
Python 3.11.9 (main, Apr 12 2024, 09:55:31) [GCC 13.2.0 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.meta_path
[<_virtualenv._Finder object at 0x000001745b6ffe50>, <class '_frozen_importlib.BuiltinImporter'>, <class '_frozen_importlib.FrozenImporter'>, <class '_frozen_importlib_external.PathFinder'>, <class '__editable___distutils_0_1_dev4112_g5fb38a1_d20240627_finder._EditableFinder'>]
>>>
I encountered similar errors in collection in #259, which I worked around by pinning pytest. And I can see the pin has taken effect, as pytest 8.0.2 is being used. Yet, somehow the import mode is still finding the wrong distutils. I did a deep dive on the issue in https://github.com/pytest-dev/pytest/issues/12490. I still have no clue how the msys environment is somehow reviving that issue.
How is that supposed to work anyway? With the pytest setting the current dir isn't in sys.path anymore. The stdlib pathfinder is before the editable finder in sys.meta_path, so I kinda expect it to pick up the wrong distutils, no?
I need to check in a Linux with Python 3.11 somehow, I only have 3.12 without distutils around.
When I was testing with pytest 8.2, where _pytest.pathlib.resolve_pkg_root_and_module_name, invoked when import-mode=importlib, doesn't rely on sys.path at all. On Pytest 8.0.2, the logic is very different. Let me see if I can trace why and how the import works on my macOS machine.
I now tried current main on Ubuntu 22.04, using Python 3.10, and it fails the same way, importing the wrong distutils.
python3 -m venv venv
source venv/bin/activate
pip install -e '.[test]'
pytest distutils/tests
I wonder what tox does differently there.
What about: ?
diff --git a/pytest.ini b/pytest.ini
index b53e0d93..35409603 100644
--- a/pytest.ini
+++ b/pytest.ini
@@ -4,6 +4,7 @@ addopts=
--doctest-modules
--import-mode importlib
consider_namespace_packages=true
+pythonpath= .
filterwarnings=
## upstream
this fixes things for both Ubuntu and mingw.
Aha. I see the difference now. It's the invocation of pytest distutils/tests.
I created this dockerfile to replicate the behavior.
from jaraco/multipy-tox:jammy
RUN git clone https://github.com/msys2-contrib/distutils
WORKDIR distutils
RUN git checkout add-mingw-support
RUN py -3.10 -m venv .venv
RUN py -m pip install -e .[test]
# simulate activating the venv:
ENV PATH=.venv/bin:$PATH
CMD pytest distutils/tests
If I run that docker image, it fails with the same issues in CI. If I instead run docker with the command bash -c pytest, the collection completes and the tests run (with a few failures not yet seen).
I think here's what's happening:
When passing distutils/tests to pytest, it causes distutils/* not to be collected, and thus falls back to sys.path for resolving the import distutils. If not passing distutils/tests, pytest will traverse ./distutils and because it's also run with --doctest-modules, the modules in distutils/* get imported during collection.
Indeed, if I disable the line with --doctest-modules and then run tests with pytest, the same collection failure occurs. The same problem exists even after restoring pytest 8.2+.
It seems that the reason I haven't encountered this issue previously or with other skeleton-based projects is because I rarely ever direct the collection by providing a subpath, but instead I use -k <pattern> to downselect tests after collection (so doctests are always collected even if not run).
What about [adding some config]?
I really try to avoid having to add config, especially to a single project. I'd like to be able to rely on defaults and best practices to make the project under test importable. I guess distutils is special here, because although it's pip-installed, a pip-installed package doesn't take precedence over a stdlib module.
How about for now, let's update the test execution to run the same way as it runs for other jobs (without specifying distutils/tests)?
How about for now, let's update the test execution to run the same way as it runs for other jobs (without specifying
distutils/tests)?
Done, see the commits for details.
It seems a bit brittle since this import side effect no longer seems to be there with pytest 8.2.0 for example, but fine with me. If you update pytest in the future I'd be happy to help debug Windows specific issues.
For future reference, this makes the tests pass with pytest 8.2.0 here:
diff --git a/pytest.ini b/pytest.ini
index b53e0d93..14188d07 100644
--- a/pytest.ini
+++ b/pytest.ini
@@ -3,7 +3,7 @@ norecursedirs=dist build .tox .eggs
addopts=
--doctest-modules
--import-mode importlib
-consider_namespace_packages=true
+pythonpath= .
filterwarnings=
## upstream