◐ Shell
reader mode source ↗
Skip to content

bpo-33407: Implement Py_DEPRECATED() on Windows#8980

Merged
vstinner merged 6 commits into
python:masterfrom
ZackerySpytz:bpo-33407-Py_DEPRECATED-win
May 28, 2019
Merged

bpo-33407: Implement Py_DEPRECATED() on Windows#8980
vstinner merged 6 commits into
python:masterfrom
ZackerySpytz:bpo-33407-Py_DEPRECATED-win

Conversation

@ZackerySpytz

@ZackerySpytz ZackerySpytz commented Aug 28, 2018

Copy link
Copy Markdown
Contributor

@ZackerySpytz ZackerySpytz force-pushed the bpo-33407-Py_DEPRECATED-win branch from 64de78b to 72324ab Compare August 28, 2018 23:25

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

LGTM.

@serhiy-storchaka, @zooba: Would you mind to double check?

@serhiy-storchaka

Copy link
Copy Markdown
Member

This breaks compatibility.

@vstinner

Copy link
Copy Markdown
Member

This breaks compatibility.

How? Would you mind to elaborate?

@serhiy-storchaka

Copy link
Copy Markdown
Member

The current usage:

extern int foo(int) Py_DEPRECATED(3.2);

This PR changes it to

Py_DEPRECATED(3.2) extern int foo(int);

It requires to rewrite all code that uses Py_DEPRECATED. This is a large breakage. Py_DEPRECATED is public and can be used in user code. We shouldn't break it in such way.

@vstinner

vstinner commented Sep 22, 2018 via email

Copy link
Copy Markdown
Member

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

@ZackerySpytz: Can you please explain that Py_DEPRECATED() macro must now be used at the start of the function definition, rather than at the end, in a new "Changes in the C API" section at https://docs.python.org/dev/whatsnew/3.8.html#porting-to-python-3-8 ?

See https://docs.python.org/dev/whatsnew/3.7.html#changes-in-the-c-api for an example of the Python 3.7 documentation.

@ZackerySpytz: Can you please confirm that using Py_DEPRECATED() at the start of the function (rather than the end) already works in Python 3.7?

@bedevere-bot

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@ZackerySpytz

Copy link
Copy Markdown
Contributor Author

@vstinner I have added a What's New entry for this change.

Can you please confirm that using Py_DEPRECATED() at the start of the function (rather than the end) already works in Python 3.7?

Yes, I can confirm using Py_DEPRECATED() at the start of the function already worked before this change.
I have made the requested changes; please review again.

@bedevere-bot

Copy link
Copy Markdown

Thanks for making the requested changes!

@vstinner: please review the changes made to this pull request.

@ZackerySpytz

ZackerySpytz commented May 28, 2019

Copy link
Copy Markdown
Contributor Author

@vstinner, I have implemented all of your requested changes, and Serhiy Storchaka has also now approved this PR. Please have a look.

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

I have requests on the documentation part.

@bedevere-bot

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@ZackerySpytz

Copy link
Copy Markdown
Contributor Author

Two deprecation warnings are emitted on MSVC because of this PR; they can be seen on AppVeyor (e.g. https://ci.appveyor.com/project/python/cpython/builds/24580662#L125):

_winapi.c
c:\projects\cpython\modules\_winapi.c(505): warning C4996: 'PyErr_SetFromWindowsErrWithUnicodeFilename': deprecated in 3.3 [C:\projects\cpython\PCbuild\pythoncore.vcxproj]
  c:\projects\cpython\include\cpython\pyerrors.h(108): note: see declaration of 'PyErr_SetFromWindowsErrWithUnicodeFilename'
c:\projects\cpython\modules\_winapi.c(1382): warning C4996: 'PyErr_SetFromWindowsErrWithUnicodeFilename': deprecated in 3.3 [C:\projects\cpython\PCbuild\pythoncore.vcxproj]
  c:\projects\cpython\include\cpython\pyerrors.h(108): note: see declaration of 'PyErr_SetFromWindowsErrWithUnicodeFilename'

I had opened https://bugs.python.org/issue36935 and GH-13355 to fix these warnings.

@ZackerySpytz

ZackerySpytz commented May 28, 2019

Copy link
Copy Markdown
Contributor Author

@vstinner I have made the requested changes; please review again.

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

LGTM.

@vstinner vstinner merged commit 3c8724f into python:master May 28, 2019
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
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.

5 participants