Specify build backend via pyproject.toml by hugovk · Pull Request #7190 · python-pillow/Pillow

hugovk · 2023-05-27T21:03:06Z

#7188 added setuptools to the CI installation for Python 3.12, because 3.12 dropped setuptools:

https://docs.python.org/3.12/whatsnew/3.12.html#removed

This is actually a bit bigger problem than the CI because users could theoretically have a system that does not have setuptools installed.

With pyproject.toml, we can specify the "build backend". In our case, this is setuptools. (The "build frontend" is what reads this file and does the install, often pip.)

So when installing, the frontend will install the required backend, so we don't need to worry whether users have already installed it. We can also specify a minimum version. I just went with the current latest, and it also seems we can remove the extra PILLOW_VERSION double quotes hack we needed for Windows on Python 3.8.

It's also possible to move more config from setup.cfg to pyproject.toml, but I ran into some problems when moving [options] and [options.extras_require] over, so let's leave those for later. We can also move the tools config, but that can also wait to keep the diff smaller here and the PR focused.

About winbuild/build_prepare.py: it had a direct invocation of setup.py which is a no-go because it assumes setuptools is already installed. So we need pip install instead (reminder of https://blog.ganssle.io/tag/setuptools.html).

I went with --global-option for now, to follow the current approach elsewhere, but that will need changing as part of #7167.

I did hit one blocker though: test-windows.yml builds a wheel, it calls:

winbuild\\build\\build_pillow.cmd --disable-imagequant bdist_wheel

Before, that was handled by this:

        r'"{python_dir}\{python_exe}" setup.py build_ext --vendor-raqm --vendor-fribidi %*',  # noqa: E501

Which wrote out a command that was called like:

2023-04-28T14:37:47.1413330Z D:\a\Pillow\Pillow>"C:\hostedtoolcache\windows\Python\3.8.10\x86\python.exe" setup.py build_ext --vendor-raqm --vendor-fribidi --disable-imagequant bdist_wheel

Now, those two args --disable-imagequant bdist_wheel each need wrapping as --global-option="--disable-imagequant" --global-option="bdist_wheel"

But this PR does:

        r'"{python_dir}\{python_exe}" -m pip install --upgrade pip',
        r'"{python_dir}\{python_exe}" -m pip install . '
        r'--global-option="--vendor-raqm" '
        r'--global-option="--vendor-fribidi" '
        r'--global-option="%*"',

And those two args were both passed into %* meaning they both were wrapped in a single global-option which doesn't work, so it doesn't build the wheel:

2023-05-27T14:56:57.1800310Z D:\a\Pillow\Pillow>"C:\hostedtoolcache\windows\Python\3.8.10\x86\python.exe" -m pip install . --global-option="--vendor-raqm" --global-option="--vendor-fribidi" --global-option="--disable-imagequant bdist_wheel"

Any ideas?

Maybe replace the build_pillow.cmd call entirely, and use something like https://pypi.org/project/build/ to call python -m build --wheel?

This reverts commit e6d7f1f.

aclark4life · 2023-05-27T21:40:29Z

@hugovk Is there any other reasonable option besides jumping ship to build? Even if you could fix the wheel build with build_pillow.cmd the entire build_prepare.py is hard to follow like setup.py … not sure how much of that can be cleaned up now, but that's an attractive goal at least.

nulano

Maybe replace the build_pillow.cmd call entirely, and use something like https://pypi.org/project/build/ to call python -m build --wheel?

Does this build a wheel from the previously compiled Pillow without invoking the compiler?
If so, that sounds like a great solution!
(I expect the --disable-imagequant flag will no longer be needed when building the wheel if that is the case.)

Edit to clarify: The first build_pillow.cmd call when building Pillow is needed to make sure the correct compiler is used. The second one is needed because setuptools requires passing the same parameters to avoid rebuilding Pillow before making a wheel. If the second call can be avoided in some way, it would greatly simplify building wheels from an already built and tested build. However, I can't quite understand how build works in the time I have available at the moment, so it is possible I am misunderstanding it.

Note that I have a ton of work this week so I cannot look into this in detail until next week.

winbuild/build_prepare.py

.github/workflows/test-windows.yml

hugovk · 2023-05-28T16:48:58Z

@hugovk Is there any other reasonable option besides jumping ship to build? Even if you could fix the wheel build with build_pillow.cmd the entire build_prepare.py is hard to follow like setup.py … not sure how much of that can be cleaned up now, but that's an attractive goal at least.

Switching to build might be the simpler choice - creating wheels from existing binaries that have already been built using build_prepare.py, rather than calling it a second time.

Edit: actually build does build again (the clue is in the name!), but in isolation:

This will build the package in an isolated environment, generating a source-distribution and wheel in the directory dist/.

Otherwise, we can keep the current build_prepare.py system, it's a matter of getting the existing parameters passed through properly, shouldn't be too hard. I don't have Windows so it's a bit slow to test via CI pushes :)

hugovk · 2023-05-28T17:03:07Z

Maybe replace the build_pillow.cmd call entirely, and use something like pypi.org/project/build to call python -m build --wheel?

Does this build a wheel from the previously compiled Pillow without invoking the compiler? If so, that sounds like a great solution! (I expect the --disable-imagequant flag will no longer be needed when building the wheel if that is the case.)

Edit to clarify: The first build_pillow.cmd call when building Pillow is needed to make sure the correct compiler is used. The second one is needed because setuptools requires passing the same parameters to avoid rebuilding Pillow before making a wheel. If the second call can be avoided in some way, it would greatly simplify building wheels from an already built and tested build. However, I can't quite understand how build works in the time I have available at the moment, so it is possible I am misunderstanding it.

No, it actually does rebuild (hence the name!) but does so in an isolated environment. It's a "build frontend" so it can do a full build, like pip.

We might want to replace build_pillow.cmd with build at some point, but I'm less clear on the full Windows build process so it's out of scope here and we should probably to stick with build_pillow.cmd so it can do the correct compiler selection, and fix passing the parameters.

Note that I have a ton of work this week so I cannot look into this in detail until next week.

Thanks! We're not in a rush for this. The July release will have beta wheels for Python 3.12, it would be nice this in there to get feedback before releasing with full support for 3.12 in the October release. Otherwise, it can likely wait until October.

Co-authored-by: nulano <nulano@nulano.eu>

radarhere · 2023-06-24T08:18:11Z

We're not in a rush for this. The July release will have beta wheels for Python 3.12, it would be nice this in there to get feedback before releasing with full support for 3.12 in the October release. Otherwise, it can likely wait until October.

The essential part of this - adding pyproject.toml - is part of #7171, which we do plan to release in July.

So I would think this should also be part of that release?

Wrap arguments before passing in documentation

nulano · 2023-06-24T15:14:14Z

With #7171 this fails to pass the --global-option values to setup.py.
I've created #7230 to use the -C raqm=vendor format instead.

hugovk added 6 commits May 27, 2023 22:40

Revert "Install setuptools on Windows"

26e0c81

This reverts commit e6d7f1f.

Add minimal pyproject.toml

454dfcc

Use latest setuptools 67.8

18960e8

Replace direct invocation of setup.py

9ea7721

Disable extra quotes

3a0881d

Upgrade pip

58b8d6c

nulano reviewed May 27, 2023

View reviewed changes

winbuild/build_prepare.py Outdated Show resolved Hide resolved

winbuild/build_prepare.py Outdated Show resolved Hide resolved

.github/workflows/test-windows.yml Show resolved Hide resolved

hugovk and others added 2 commits May 28, 2023 21:14

Move pip upgrade to .appveyor.yml

7f0c49a

Wrap arguments before passing

d5e03cc

Co-authored-by: nulano <nulano@nulano.eu>

hugovk marked this pull request as ready for review May 29, 2023 11:46

radarhere added 3 commits June 24, 2023 13:48

Wrap arguments before passing in documentation

bd2fdd6

Updated pyproject-fmt to 0.12.1

0e1f86a

Removed unused code

600b823

radarhere mentioned this pull request Jun 24, 2023

Wrap arguments before passing in documentation hugovk/Pillow#103

Merged

radarhere added the Needs Rebase label Jun 24, 2023

Merge pull request #103 from radarhere/add-pyproject.toml

ddd5df7

Wrap arguments before passing in documentation

nulano mentioned this pull request Jun 24, 2023

Fix Windows build with pyproject.toml #7230

Merged

hugovk closed this in #7230 Jun 30, 2023

hugovk deleted the add-pyproject.toml branch July 5, 2023 20:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Specify build backend via pyproject.toml#7190

Specify build backend via pyproject.toml#7190
hugovk wants to merge 12 commits intopython-pillow:mainfrom
hugovk:add-pyproject.toml

hugovk commented May 27, 2023 •

edited

Loading

Uh oh!

aclark4life commented May 27, 2023

Uh oh!

nulano left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hugovk commented May 28, 2023 •

edited

Loading

Uh oh!

hugovk commented May 28, 2023

Uh oh!

radarhere commented Jun 24, 2023

Uh oh!

nulano commented Jun 24, 2023 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

hugovk commented May 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aclark4life commented May 27, 2023

Uh oh!

nulano left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hugovk commented May 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hugovk commented May 28, 2023

Uh oh!

radarhere commented Jun 24, 2023

Uh oh!

nulano commented Jun 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hugovk commented May 27, 2023 •

edited

Loading

nulano left a comment •

edited

Loading

hugovk commented May 28, 2023 •

edited

Loading

nulano commented Jun 24, 2023 •

edited

Loading