bpo-35081: Move Include/pyatomic.c to Include/internal/ by vstinner · Pull Request #10239 · python/cpython · GitHub
Skip to content

bpo-35081: Move Include/pyatomic.c to Include/internal/ - #10239

Merged
vstinner merged 2 commits into
python:masterfrom
vstinner:pyatomic
Oct 30, 2018
Merged

vstinner merged 2 commits into
python:masterfrom
vstinner:pyatomic

Conversation

@vstinner

@vstinner vstinner commented Oct 30, 2018

Copy link
Copy Markdown
Member

@vstinner

Copy link
Copy Markdown
Member Author

@serhiy-storchaka

Copy link
Copy Markdown
Member

Would not be better to move files with the content fully surrounded by #ifdef Py_BUILD_CORE out of the Include/ directory?

@serhiy-storchaka serhiy-storchaka 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.

Update Windows build files?

Comment thread Include/internal/pystate.h Outdated
@vstinner

Copy link
Copy Markdown
Member Author

Would not be better to move files with the content fully surrounded by #ifdef Py_BUILD_CORE out of the Include/ directory?

Right now, I'm not sure if third party projects require Py_BUILD_CORE code or not... I prefer to continue the previous work to move code into Include/internal/.

It seems like Include/internal/ is not installed by python3-devel on Fedora. So Include/internal/ is not usable by 3rd party modules on Fedora at least.

* Add pyatomic.h to the VS project (it wasn't referenced)
* Don't include pyatomic.c in internal/pystate.h
@vstinner
vstinner requested a review from a team as a code owner October 30, 2018 13:23
@vstinner

Copy link
Copy Markdown
Member Author

Update Windows build files?

pyatomic.h wasn't included in the VS project. I fixed that.

@vstinner
vstinner merged commit 31368a4 into python:master Oct 30, 2018
@vstinner
vstinner deleted the pyatomic branch October 30, 2018 14:14
@vstinner

Copy link
Copy Markdown
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants