Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from
Mar 30, 2024
2 changes: 2 additions & 0 deletions .github/workflows/wheels.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ on:
paths:
- ".ci/requirements-cibw.txt"
- ".github/workflows/wheel*"
- "setup.py"
- "wheels/*"
- "winbuild/build_prepare.py"
- "winbuild/fribidi.cmake"
Expand All @@ -14,6 +15,7 @@ on:
paths:
- ".ci/requirements-cibw.txt"
- ".github/workflows/wheel*"
- "setup.py"
- "wheels/*"
- "winbuild/build_prepare.py"
- "winbuild/fribidi.cmake"
Expand Down
51 changes: 16 additions & 35 deletions _custom_build/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,41 +11,12 @@
class _CustomBuildMetaBackend(backend_class):
def run_setup(self, setup_script="setup.py"):
if self.config_settings:
for key, values in self.config_settings.items():
if not isinstance(values, list):
values = [values]
for value in values:
sys.argv.append(f"--pillow-configuration={key}={value}")

def config_has(key, value):
settings = self.config_settings.get(key)
if settings:
if not isinstance(settings, list):
settings = [settings]
return value in settings

flags = []
for dependency in (
"zlib",
"jpeg",
"tiff",
"freetype",
"raqm",
"lcms",
"webp",
"webpmux",
"jpeg2000",
"imagequant",
"xcb",
):
if config_has(dependency, "enable"):
flags.append("--enable-" + dependency)
elif config_has(dependency, "disable"):
flags.append("--disable-" + dependency)
for dependency in ("raqm", "fribidi"):
if config_has(dependency, "vendor"):
flags.append("--vendor-" + dependency)
if self.config_settings.get("platform-guessing") == "disable":
flags.append("--disable-platform-guessing")
if self.config_settings.get("debug") == "true":
flags.append("--debug")
if flags:
sys.argv = sys.argv[:1] + ["build_ext"] + flags + sys.argv[1:]
return super().run_setup(setup_script)

def build_wheel(
Expand All @@ -54,5 +25,15 @@ def build_wheel(
self.config_settings = config_settings
return super().build_wheel(wheel_directory, config_settings, metadata_directory)

def build_editable(
self, wheel_directory, config_settings=None, metadata_directory=None
):
self.config_settings = config_settings
return super().build_editable(
wheel_directory, config_settings, metadata_directory
)


build_wheel = _CustomBuildMetaBackend().build_wheel
_backend = _CustomBuildMetaBackend()
build_wheel = _backend.build_wheel
build_editable = _backend.build_editable
radarhere marked this conversation as resolved.
Show resolved Hide resolved
14 changes: 7 additions & 7 deletions docs/installation/building-from-source.rst
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,10 @@ After navigating to the Pillow directory, run::
Build Options
^^^^^^^^^^^^^

* Environment variable: ``MAX_CONCURRENCY=n``. Pillow can use
multiprocessing to build the extension. Setting ``MAX_CONCURRENCY``
sets the number of CPUs to use, or can disable parallel building by
* Config setting: ``-C parallel=n``. Can also be given
with environment variable: ``MAX_CONCURRENCY=n``. Pillow can use
multiprocessing to build the extension. Setting ``-C parallel=n``
sets the number of CPUs to use to ``n``, or can disable parallel building by
using a setting of 1. By default, it uses 4 CPUs, or if 4 are not
available, as many as are present.

Expand All @@ -293,14 +294,13 @@ Build Options
used to compile the standard Pillow wheels. Compiling libraqm requires
a C99-compliant compiler.

* Build flag: ``-C platform-guessing=disable``. Skips all of the
* Config setting: ``-C platform-guessing=disable``. Skips all of the
platform dependent guessing of include and library directories for
automated build systems that configure the proper paths in the
environment variables (e.g. Buildroot).

* Build flag: ``-C debug=true``. Adds a debugging flag to the include and
library search process to dump all paths searched for and found to
stdout.
* Config setting: ``-C debug=true``. Adds a debugging flag to the include and
library search process to dump all paths searched for and found to stdout.


Sample usage::
Expand Down
26 changes: 22 additions & 4 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ def get_version():
return locals()["__version__"]


configuration = {}


PILLOW_VERSION = get_version()
FREETYPE_ROOT = None
HARFBUZZ_ROOT = None
Expand Down Expand Up @@ -333,15 +336,24 @@ def __iter__(self):
+ [("add-imaging-libs=", None, "Add libs to _imaging build")]
)

@staticmethod
def check_configuration(option, value):
return True if value in configuration.get(option, []) else None

def initialize_options(self):
self.disable_platform_guessing = None
self.disable_platform_guessing = self.check_configuration(
"platform-guessing", "disable"
)
self.add_imaging_libs = ""
build_ext.initialize_options(self)
for x in self.feature:
setattr(self, f"disable_{x}", None)
setattr(self, f"enable_{x}", None)
setattr(self, f"disable_{x}", self.check_configuration(x, "disable"))
setattr(self, f"enable_{x}", self.check_configuration(x, "enable"))
for x in ("raqm", "fribidi"):
setattr(self, f"vendor_{x}", None)
setattr(self, f"vendor_{x}", self.check_configuration(x, "vendor"))
if self.check_configuration("debug", "true"):
self.debug = True
self.parallel = configuration.get("parallel", [None])[-1]

def finalize_options(self):
build_ext.finalize_options(self)
Expand Down Expand Up @@ -984,6 +996,12 @@ def debug_build():
Extension("PIL._imagingmorph", ["src/_imagingmorph.c"]),
]


# parse configuration from _custom_build/backend.py
while sys.argv[-1].startswith("--pillow-configuration="):
_, key, value = sys.argv.pop().split("=", 2)
configuration.setdefault(key, []).append(value)

try:
setup(
cmdclass={"build_ext": pil_build_ext},
Expand Down
9 changes: 8 additions & 1 deletion winbuild/build.rst
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,18 @@ are set by running ``winbuild\build\build_env.cmd`` and install Pillow with pip:
winbuild\build\build_env.cmd
python.exe -m pip install -v -C raqm=vendor -C fribidi=vendor .

To build a wheel instead, run::
You can also install Pillow in `editable mode`_::

winbuild\build\build_env.cmd
python.exe -m pip install -v -C raqm=vendor -C fribidi=vendor -e .
Copy link

Choose a reason for hiding this comment

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

I think that for pip install, the config settings should go after the user-requested package. I know that for some options pip attached them to the package install requests that they follow. I think it might do the same for PEP 517 config settings as pip install may have many install requests in the same command. As in pip install pkg-X --config-setting=some=thing pkg-Y --config-setting=some=other-thing. Needs checking, though.

But I still recommend having package settings coupled with the package name/spec this way.

Suggested change
python.exe -m pip install -v -C raqm=vendor -C fribidi=vendor -e .
python.exe -m pip install -v -e . -C raqm=vendor -C fribidi=vendor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't find any recommendations about the order in the pip documentation, so I'll take your word for it. Testing locally, I find that either order works the same with a single package. I've created #7686 for your suggestion.


To build a binary wheel instead, run::

winbuild\build\build_env.cmd
python.exe -m pip wheel -v -C raqm=vendor -C fribidi=vendor .

.. _editable mode: https://setuptools.pypa.io/en/stable/userguide/development_mode.html

Testing Pillow
--------------

Expand Down
Loading