Skip to content

binary-extensions: update Windows instructions #911

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

Merged
merged 6 commits into from
Jul 12, 2021
Merged

Conversation

henryiii
Copy link
Contributor

@henryiii henryiii commented Jun 1, 2021

About a month ago Microsoft permanently killed off https://www.microsoft.com/en-gb/download/details.aspx?id=8279 (MSVC 2008 for Python 2.7); it was officially at end-of-really-extended-life-for-python-2.7 since 2018. I don't think details about Python 3.4 are all that interesting to anyone anymore, either.

Included the caveat discovered by matplotlib for MSVC 2019.

Copy link
Contributor

@bhrutledge bhrutledge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a Windows user, so these are just suggested copy edits for clarity (plus one for spelling).

``VCRUNTIME140.dll`` that all previous versions back to 2015 depend on. This
will add an extra requirement to using your extension on versions of CPython
that do not include this extra file. To avoid this, you can add the
compile-time argument ``/d2FH4-``. Recent versions of Python may include this
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to put exact numbers, but I'm not sure which ones they are (it might have changed in patch releases, too?)

Copy link
Contributor

@bhrutledge bhrutledge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this reads much better. Thanks!

@webknjaz
Copy link
Member

I'm not a Windows user either so I'd feel much more comfortable merging the change if one could be found and posted a review. If nobody is available for some time, we'll merge this as is.

Copy link
Member

@pfmoore pfmoore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Windows user here!) This looks good to me, although I only very rarely build C extensions these days, so while the technical details of the content look accurate to me, I'm not an expert.

I'd say this can be merged as is, though - we can always iterate on the technical details if further nuances are reported.

@webknjaz
Copy link
Member

Thanks, Paul!

@webknjaz webknjaz merged commit 06bc33d into pypa:main Jul 12, 2021
@henryiii henryiii deleted the patch-5 branch July 12, 2021 17:49
@henryiii
Copy link
Contributor Author

Thanks for the review! I'll try to work on updating macOS and Linux next (though likely after SciPy).

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.

4 participants