Use --config-settings instead of deprecated --global-option #7171

radarhere · 2023-05-20T05:57:15Z

Resolves #7167

https://github.com/python-pillow/Pillow/actions/runs/4999985216/jobs/8956875899#step:7:23

DEPRECATION: --build-option and --global-option are deprecated. pip 23.3 will enforce this behaviour change. A possible replacement is to use --config-settings. Discussion can be found at pypa/pip#11859

--config-settings is described at https://peps.python.org/pep-0517/#config-settings

So I think our build options should change from

python3 -m pip install --upgrade Pillow --global-option="build_ext" --global-option="--enable-[feature]"

to

python3 -m pip install --upgrade Pillow -C enable=[feature]

However, setuptools does not currently pass the config settings through to the backend - pypa/setuptools#2491

Inspired by https://setuptools.pypa.io/en/latest/build_meta.html#dynamic-build-dependencies-and-other-build-meta-tweaks, my suggestion is to instead have a custom backend that translates the config-settings into a form that setuptools will understand. This should work until setuptools resolve their issue.

For pyproject.toml, I used the fallback setting for the require value - https://pip.pypa.io/en/stable/reference/build-system/pyproject-toml/#fallback-behaviour

_custom_build/backend.py

nulano · 2023-06-03T17:48:25Z

Would it be clearer to use freetype=enable instead of enable=freetype?

radarhere · 2023-06-04T13:23:09Z

Ok, sure, I've swapped the order.

hugovk · 2023-06-14T07:42:55Z

Would it be clearer to use freetype=enable instead of enable=freetype?

I don't mind too much either way, but will note we're flipping the order from the current: --disable-zlib to -C zlib=disable.

Trying to think of precedent, pip install --help has:

  --use-feature <feature>     Enable new functionality, that may be backward incompatible.
  --use-deprecated <feature>  Enable deprecated functionality, that will be removed in the future.

Which is more like the existing, but is a bit different. Are there any more direct comparisons?

radarhere · 2023-06-24T09:28:52Z

I wasn't able to find any comparisons.

Just for my personal thoughts, I think freetype=enable is better than enable=freetype because

enable=freetype might make people think there should only be one enable argument, which wouldn't have been the plan
it more clearly implies that you shouldn't try enabling and disabling freetype simultaneously

hugovk · 2023-06-24T09:43:05Z

Fair enough, thank you!

This implements a custom build backend, inspired by [the solution used by Pillow](python-pillow/Pillow#7171). With this change, argument flags now need to be specified as key-value pairs instead of key-only as `--config-settings`. Every argument can either be left unspecified or have a value that is either "enabled" or "disabled". For example: ``` --config-settings --global-option=--cpp_ext --global-option --cpp_ext ``` becomes ``` --config-settings cpp_ext=enabled ``` The setuptools version requirement of 40.8.0 was also adopted from Pillow. It is the version that introduced the `build_meta:__legacy__` backend.

This implements a custom build backend, inspired by [the solution used by Pillow](python-pillow/Pillow#7171). The setuptools version requirement of 40.8.0 was also adopted from Pillow. It is the version that introduced the `build_meta:__legacy__` backend.

This implements a custom build backend, inspired by the [solution used by Pillow.](python-pillow/Pillow#7171) install-option is deprecated and was removed with pip 23.1. The comonly accepted solution seems to be to define a custom build backend for now pypa/setuptools#2491 (comment) This commit changes the usage from `--install-option` to `--config-settings`.

webknjaz · 2023-12-23T03:54:01Z

pyproject.toml

@@ -0,0 +1,4 @@
+[build-system]
+requires = ["setuptools >= 40.8.0", "wheel"]


@radarhere @hugovk wheel shouldn't actually be depended on: pypa/setuptools#3056.

This has since been removed in #7248.

Ah, I should've checked. Thanks!

webknjaz · 2023-12-23T03:56:05Z

_custom_build/backend.py

+                sys.argv = sys.argv[:1] + ["build_ext"] + flags + sys.argv[1:]
+        return super().run_setup(setup_script)
+
+    def build_wheel(


@hugovk @radarhere this omits build_editable() which is almost the same as build_wheel(). You'll likely want to wrap it as well.

I've attempted to add build_editable in 290a2d1, but for some reason it is not working properly on Windows: https://github.com/nulano/Pillow/actions/runs/7308594440/job/19915362677#step:25:728

It looks like the build script calls something different from what's called on the main branch: I see "Running command Getting requirements to build editable" vs. "Running command Getting requirements to build wheel". The former is from your log, the latter is from the main branch.

Yes, I added 2446fd8 to test editable installing on GHA. For some reason, it seems the include paths are not passed in properly when building editable on Windows (specifically the OpenJpeg path is found and added for part of the build, but not all of it).

Oh, interesting, I didn't see where it's defined. It's often hard to distinguish outputs of different commands if they are squashed in the same CI step, so I tend to recommend putting each command in a separate step or using the corresponding shell's capabilities to improve such logging. I also recently saw a case where Powershell would proceed to execute commands after the previous ones fail.

Anyway, this may indicate dependency on some weird pre-existing state.

One way of exploring GHA jobs is https://github.com/marketplace/actions/debugging-with-tmate, by the way.

Sorry, I should have mentioned that it also fails for me locally.

It's the first command in that step that is failing, the second one just runs a simple test.
The path is added in setup.py in the build_extensions method here:

Pillow/setup.py

Lines 687 to 691 in 41e45b5

if best_version and _find_library_file(self, "openjp2"):

# Add the directory to the include path so we can include

# <openjpeg.h> rather than having to cope with the versioned

# include path

_add_directory(self.compiler.include_dirs, best_path, 0)

From the GHA log, it looks almost as if it first does a regular build, then a second build again skipping that method, but I'm not sure why or where to look.

Yes, I have used tmate before.

I've created #7658 for this.

webknjaz · 2023-12-23T03:57:58Z

.github/workflows/test-mingw.yml

@@ -80,7 +80,7 @@ jobs:
          pushd depends && ./install_extra_test_images.sh && popd

      - name: Build Pillow
-        run: SETUPTOOLS_USE_DISTUTILS="stdlib" CFLAGS="-coverage" python3 -m pip install --global-option="build_ext" .
+        run: SETUPTOOLS_USE_DISTUTILS="stdlib" CFLAGS="-coverage" python3 -m pip install .


I got to the conclusion that it's also nice to configure coverage through the config setting. Here's an example of the interface I ended up with: https://yarl.aio-libs.org/en/latest/changes/#released-versions

We're already providing make install-coverage. I don't see the need to provide yet another way of enabling coverage.

Yeah, I see. Just thought it's less integrated into the Python interfaces, but into tooling of external ecosystems.

radarhere added 2 commits May 20, 2023 15:38

Use --config-settings instead of deprecated --global-option

4f734d2

Removed inplace target

18da2d0

radarhere added the Build label May 20, 2023

radarhere mentioned this pull request May 20, 2023

--global-option is deprecated #7167

Closed

hugovk reviewed Jun 3, 2023

View reviewed changes

_custom_build/backend.py Show resolved Hide resolved

_custom_build/backend.py Outdated Show resolved Hide resolved

radarhere added 2 commits June 4, 2023 22:17

Merge branch 'main' into build

8780772

Swapped config key and value

2d0b13b

hugovk mentioned this pull request Jun 13, 2023

Setuptools does not pass config_settings through backend pypa/setuptools#2491

Open

Do not import internal class

38d6386

radarhere force-pushed the build branch from cd61f72 to 38d6386 Compare June 14, 2023 05:47

hugovk approved these changes Jun 21, 2023

View reviewed changes

hugovk added this to the 10.0.0 milestone Jun 23, 2023

radarhere mentioned this pull request Jun 24, 2023

Specify build backend via pyproject.toml #7190

Closed

hugovk merged commit c482634 into python-pillow:main Jun 24, 2023

radarhere deleted the build branch June 24, 2023 09:43

This was referenced Jun 24, 2023

Use --config-settings python-pillow/pillow-wheels#389

Merged

Use --config-settings python-pillow/docker-images#186

Merged

radarhere mentioned this pull request Jul 2, 2023

Remove redundant wheel dep from pyproject.toml #7248

Merged

janEbert mentioned this pull request Jul 6, 2023

Add custom build backend to support build args NVIDIA/apex#1692

Open

rossbar mentioned this pull request Jul 8, 2023

Add pyproject.toml pygraphviz/pygraphviz#471

Merged

tobiasah mentioned this pull request Oct 2, 2023

Add custom build backend to support build args capnproto/pycapnp#328

Merged

hugovk mentioned this pull request Oct 27, 2023

Move config from setup.cfg to pyproject.toml #7484

Merged

nulano mentioned this pull request Nov 20, 2023

Use --config-settings in external libraries list in installation.rst #7564

Merged

webknjaz reviewed Dec 23, 2023

View reviewed changes

nulano mentioned this pull request Dec 30, 2023

Fix editable installation with custom build backend and configuration options #7658

Merged

nulano mentioned this pull request Jan 31, 2024

python setup.py and the use of setup.py as a command line tool are deprecated hpyproject/hpy#473

Closed

radarhere mentioned this pull request Mar 21, 2024

Failing to load Image from PIL in macOS with 9.5.0 #7889

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use --config-settings instead of deprecated --global-option #7171

Use --config-settings instead of deprecated --global-option #7171

radarhere commented May 20, 2023

nulano commented Jun 3, 2023

radarhere commented Jun 4, 2023

hugovk commented Jun 14, 2023

radarhere commented Jun 24, 2023

hugovk commented Jun 24, 2023

webknjaz Dec 23, 2023

nulano Dec 23, 2023

webknjaz Dec 23, 2023

webknjaz Dec 23, 2023

nulano Dec 23, 2023

webknjaz Dec 23, 2023

nulano Dec 23, 2023

webknjaz Dec 23, 2023

nulano Dec 23, 2023

nulano Dec 30, 2023

webknjaz Dec 23, 2023

radarhere Dec 23, 2023

webknjaz Dec 23, 2023

		@@ -0,0 +1,4 @@
		[build-system]
		requires = ["setuptools >= 40.8.0", "wheel"]

	if best_version and _find_library_file(self, "openjp2"):
	# Add the directory to the include path so we can include
	# <openjpeg.h> rather than having to cope with the versioned
	# include path
	_add_directory(self.compiler.include_dirs, best_path, 0)

Use --config-settings instead of deprecated --global-option #7171

Use --config-settings instead of deprecated --global-option #7171

Conversation

radarhere commented May 20, 2023

nulano commented Jun 3, 2023

radarhere commented Jun 4, 2023

hugovk commented Jun 14, 2023

radarhere commented Jun 24, 2023

hugovk commented Jun 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment