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

Use CPython 3.8.0 mechanism to find msvc 14+ #1904

Merged
merged 1 commit into from Mar 7, 2020
Merged

Conversation

mayeut
Copy link
Member

@mayeut mayeut commented Nov 11, 2019

Summary of changes

Update msvc.py to use CPython 3.8.0 mechanism to find msvc 14+ as recommended in this comment.

Closes #1903

This PR has been tested as part of cibuildwheel tests using various CI providers.
Please note that I can't test it more thoroughly cause I don't have any Windows environment at hand (except for CI which requires very long iterations).

Pull Request Checklist

  • Changes have tests
  • News fragment added in changelog.d. See documentation for details

@JGoutin
Copy link
Contributor

@JGoutin JGoutin commented Nov 11, 2019

Using a Python 3.8 back port instead of the current mechanism seem to be the good way to do.

This also allow to completely drop the old mechanism that become unused. This will make this file far more easy to maintain. Note that this will remove the following:

  • VC 2010 support, only used by Python 3.4 that is deprecated.
  • Microsoft Windows SDK 6.1 and 7.0 support, tools that are deprecated and where removed from download by Microsoft. (And there is still the "VC for python 2.7" that replace them)

I sent you a PR with the following changes:

  • Drop the old mechanism.
  • Fixed a missing import.
  • PEP8 + Add python 3.8 backport comment.

I do not have a Windows right now to do more tests on it.

There are maybe tests with Python 3.8 that can be back-ported too.

@mayeut
Copy link
Member Author

@mayeut mayeut commented Nov 11, 2019

Thanks @JGoutin, this is much cleaner indeed. All CI tests of cibuildwheel are passing with your version of msvc.py. I will correct the missing import in the meantime (itertools.count() right ?)

If @pypa/setuptools-developers are ok with your notes (Microsoft Windows SDK 6.1 and 7.0, Python 3.4 support dropped), I'll merge that in this PR.

@mayeut
Copy link
Member Author

@mayeut mayeut commented Nov 11, 2019

There are maybe tests with Python 3.8 that can be back-ported too.

Indeed, I'll check if I can add those:
https://github.com/python/cpython/blob/master/Lib/distutils/tests/test_msvccompiler.py

@JGoutin
Copy link
Contributor

@JGoutin JGoutin commented Nov 12, 2019

(itertools.count() right ?)

It's exactly that.

@mayeut mayeut force-pushed the msvc-python3.5 branch 2 times, most recently from 299bc07 to 20126c8 Compare Nov 16, 2019
@JGoutin
Copy link
Contributor

@JGoutin JGoutin commented Nov 18, 2019

Concerning the drop of the old mechanism (and VC2010 support), Python 3.4 will be dropped with #1908.

This will also fix the issue #1902

@JGoutin
Copy link
Contributor

@JGoutin JGoutin commented Jan 2, 2020

If @pypa/setuptools-developers are ok with your notes (Microsoft Windows SDK 6.1 and 7.0, Python 3.4 support dropped), I'll merge that in this PR.

#1908 is merged, Python 3.4 support is dropped from setuptools.

@mayeut
Copy link
Member Author

@mayeut mayeut commented Jan 10, 2020

I will drop python 3.4 support in msvc.py from the PR

Edit: I won't drop python 3.4

Since python 3.4 uses the same code base as python 2.7, I'd rather not change it. It'll be easier to drop as a whole once python 2.7 support is really removed from setuptools.

@JGoutin
Copy link
Contributor

@JGoutin JGoutin commented Feb 12, 2020

Edit: I won't drop python 3.4

Since python 3.4 uses the same code base as python 2.7, I'd rather not change it. It'll be easier to drop as a whole once python 2.7 support is really removed from setuptools.

Removing the python 3.4 support will not remove the Python 2.7 support. It will only remove the
Microsoft Windows SDK 6.1 and 7.0 support, that are deprecated and not available anymore.

Python 2.7 requires msvc9_find_vcvarsall & msvc9_query_vcvarsall functions.
In the PR code, theses functions are still available: https://github.com/JGoutin/setuptools/blob/msvc-python3.5/setuptools/msvc.py#L56.

@mayeut
Copy link
Member Author

@mayeut mayeut commented Feb 12, 2020

@JGoutin,
I'd rather not change python 2.7 support at all to prevent any regressions, even if unlikely. So, in my opinion, removing Microsoft Windows SDK 6.1 and 7.0 support is not an option.

@mayeut
Copy link
Member Author

@mayeut mayeut commented Mar 7, 2020

@jaraco,
can you review this PR please ?

jaraco
jaraco approved these changes Mar 7, 2020
Copy link
Member

@jaraco jaraco left a comment

Looks good to me. Thanks!

@jaraco jaraco merged commit 42eb813 into pypa:master Mar 7, 2020
23 of 24 checks passed
@jaraco
Copy link
Member

@jaraco jaraco commented Mar 7, 2020

Released as 45.3.0

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.

3 participants