Phasing Out Distutils And Bringing the Build System Up To PEP 517 #4198
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
illume
reviewed
Apr 6, 2024
There was a problem hiding this comment.
Thanks for this epic amount of cleanup.
Would you be able to send smaller changes you are confident in already as separate PRs?
For the commit messages, I'm trying to follow a git / linux kernel way where the context is first, and the module is too.
buildconfig: appveyor: Fix old way of doing something to new way
More description, details, why, how, etc...
| @@ -168,8 +168,9 @@ install: | |||
| - "%PYTHON% -m pip install Sphinx==4.5.0 sphinxcontrib-applehelp==1.0.2 pip install sphinxcontrib-devhelp==1.0.2 sphinxcontrib-htmlhelp==2.0.0 sphinxcontrib-qthelp==1.0.3 sphinxcontrib-serializinghtml==1.1.5" | |||
| - "%PYTHON% setup.py docs" | |||
| - "%WITH_COMPILER% %PYTHON% -m buildconfig" | |||
| - "%WITH_COMPILER% %PYTHON% setup.py build -j4 %USE_ANALYZE%" | |||
| - "%WITH_COMPILER% %PYTHON% setup.py -setuptools %DISTRIBUTIONS%" | |||
| # - "%WITH_COMPILER% %PYTHON% setup.py build -j4 %USE_ANALYZE%" | |||
There was a problem hiding this comment.
This is used to turn on the compiler static analysis. I guess there will be some new way to pass flags through or extend the compiler?
buildconfig/config_emsdk.py
Outdated
| @@ -183,7 +183,7 @@ def configure(self, incdirs, libdirs): | |||
| except ImportError: | |||
| self.found = 0 | |||
| if self.found and self.header: | |||
| fullpath = os.path.join(get_python_inc(0), self.header) | |||
| fullpath = os.path.join(get_path('include'), self.header) | |||
There was a problem hiding this comment.
This whole file isn't used, so it can be removed.
illume
reviewed
Apr 6, 2024
pyproject.toml
Outdated
| Twitter = "https://twitter.com/pygame_org" | ||
|
|
||
| [build-system] | ||
| requires = ["setuptools>=42", "wheel", "cython~=3.0.0"] |
There was a problem hiding this comment.
I think it's good to require specific versions here. Especially with Cython. Things can sometimes go eerie when folks use slightly different versions.
This is to fix an issue with pip not grabbing the metadata from the project table of the pyproject.toml on python 3.6
Specified latest cached versions except for cython==3.0.0
Changed version requirement to grab latest version for each python environment. This allows python 3.6 to use version 59.6.0 while allowing python 3.12 to use a later version that supports 3.12
Changed the setup.py script to search for the USE_ANALYZE env variable rather than arguments. This is because you cannot pass args to the setup.py script with pip wheel.
This file is not longer used
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With the deprecation of
distutilsin Python 3.10, my senior design team thought it would be helpful to bring pygame's build system up to the PEP 517 standards. This is still a work on progress, but we are submitting this PR to run our changes through AppVeyor and get feedback from the core development team.We opted to use a
pyproject.tomlto house the metadata while using thesetup.pyas the build backend to retain the complex build logic.The
setup.pyhas been reorganized to for maintainability. Additionally, most instances ofdistutilshave been replaced by theirsetuptoolscounterparts. Our goal is to reduce the length and complexity of thesetup.pyscript by leveraging the encapsulation offered bysetuptools.We removed all but 1 instance of
distutilsfrom thebuildconfigmodule.We have also updated the AppVeyor script to use
pip wheelinstead of callingsetup.pydirectly per PEP 517 standards.Overall, we want to bring the pygame build system up to date to ensure the longevity of the project. We will continue to polish out these changes to fully eliminate
distutils. We are open to any feedback, suggestions, or questions you have about our work.