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

[FR] CIBW_BUILD_ISOLATION or similar #1580

Closed
abravalheri opened this issue Aug 19, 2023 · 7 comments · Fixed by #1588
Closed

[FR] CIBW_BUILD_ISOLATION or similar #1580

abravalheri opened this issue Aug 19, 2023 · 7 comments · Fixed by #1588

Comments

@abravalheri
Copy link
Contributor

abravalheri commented Aug 19, 2023

It would be nice if we could have a CIBW_BUILD_ISOLATION=false, equivalent to python -m build --no-isolation and pip wheel --no-deps --no-build-isolation.

The context for this FR is testing: some projects (like build-backends or build-backend plugins, e.g. setuptools-rust) have integration tests that use cibuildwheel; and during those tests we want to install the working versions of one of the build requirements from the source directory instead of fetching from PyPI.

The only alternative that we have nowadays is PIP_NO_BUILD_ISOLATION (no build equivalent as far as I know). However, I found that PIP_NO_BUILD_ISOLATION is tricky to use (in PyO3/setuptools-rust#352, it took me a long time to figure out how to make it work and solve the errors). I can highlight the following reasons:

  1. It is not supported by build

  2. You have to set it inside CIBW_ENVIRONMENT (forgetting to do that is a silly mistake if you understand how cibuildwheel works, but nevertheless, very easy to forget and let it happen/difficult for beginners to figure it out)

  3. PIP_NO_BUILD_ISOLATION is counter-intuitive: see PIP_NO_CACHE_DIR and PIP_NO_BUILD_ISOLATION behave opposite to how they read pip#5735

  4. When PIP_NO_BUILD_ISOLATION=false is set, every call to pip install (e.g. the ones in CIBW_BEFORE_BUILD to install the build dependencies) will also be subject to the "no isolation" rule.
    This make a supposedly simple operation (i.e., installing build dependencies using a pip install command) error prone. For example, the following CIBW_BEFORE_BUILD may result in errors (depending on the versions of Python/setuptools/wheel/pip already present on the build container/VM):

    CIBW_BEFORE_BUILD: pip install -U pip>=23.2.1 setuptools>=68.0.0 wheel>=0.41.1 -e .

    (Instead to make it work, after a few ours of debugging, you might have to split the command into a succession of independent pip installs).

    It would be better if CIBW_BEFORE_BUILD is not affected by PIP_NO_BUILD_ISOLATION (it gives less margins for surprise/errors).
    CIBW_BUILD_ISOLATION=false would be better because it would not affect CIBW_BEFORE_BUILD (less margin for surprise/errors).

An alternative to CIBW_BUILD_ISOLATION=true, would be allowing customising the build command CIBW_BUILD_CMD, as suggested in #442 (however that comes with a different set of considerations/issues as discussed in the other issue).

@Czaki
Copy link
Contributor

Czaki commented Aug 19, 2023

Hm. Maybe we should also allow set environment variable only for build wheel steep?
Also opening issues in the build repository to allow control isolation via environment variables may be a good idea.

@joerick
Copy link
Contributor

joerick commented Aug 22, 2023

  • CIBW_BUILD_ISOLATION, to me seems like a sledgehammer to crack a nut, I rather not expose a whole new option for an uncommon use-cases (I don't want to maintain a tool with thousands of options like ffmpeg!)
  • CIBW_BUILD_COMMAND has a few issues, as noted in [idea] pypa-build support #442 and on the discord[^1].

I wonder if a middle ground here, between CIBW_BUILD_ISOLATION and CIBW_BUILD_CMD would be CIBW_BUILD_FRONTEND_FLAGS, which lets users set flags to pass at the command line to either pip or build.

it would give users the freedom to do change an option like build-isolation, and have some advantages over CIBW_BUILD_FRONTEND_FLAGS - cibuildwheel is still responsible for installing the pinned version of pip/build, and setting the correct flags on the tool, so stuff like CIBW_CONFIG_SETTINGS and CIBW_BUILD_VERBOSITY would still work.

The downsides of CIBW_BUILD_FRONTEND_FLAGS are the API-leakage aspect of such an option (pip/build's API becomes part of our API). But I think we could alleviate this somewhat with documentation, "this option is intended for experimentation, we don't envisage this option being used as part of a production config".


[1]: my thoughts on CIBW_BUILD_COMMAND from the discord, pasted here so it's all in one place

From my perspective, I'm kinda +0 on such a feature - it would be great to allow more customisability, but it also opens up cibuildwheel to supporting many more use-cases that I'm not totally sure I'm interested in. My hunch is that the limited options here subtly push packagers to create more standards-compliant wheel builds, which benefits the community and makes it easier to support our users.

that said, perhaps some of those concerns could be alleviated by making clear in the docs that if you do this, you're taking the less trodden path and 'on your own'

--
edit: I mistakenly said CIBW_BUILD_BACKEND_FLAGS above, I've fixed it, I meant CIBW_BUILD_FRONTEND_FLAGS

@abravalheri
Copy link
Contributor Author

Hi @joerick, CIBW_BUILD_BACKEND_FLAGS works for me!

It allows to get the job done while avoiding the mental confusion that is to use PIP_NO_BUILD_ISOLATION.

@joerick
Copy link
Contributor

joerick commented Aug 22, 2023

The other thought I have on this is that at some point we're gonna make build the default build backend, here are the options you can pass:

joerick@joerick5 ~> pipx run build -h      
usage: build [-h] [--version] [--sdist] [--wheel] [--outdir OUTDIR] [--skip-dependency-check] [--no-isolation]
             [--config-setting CONFIG_SETTING]
             [srcdir]

    A simple, correct Python build frontend.

    By default, a source distribution (sdist) is built from {srcdir}
    and a binary distribution (wheel) is built from the sdist.
    This is recommended as it will ensure the sdist can be used
    to build wheels.

    Pass -s/--sdist and/or -w/--wheel to build a specific distribution.
    If you do this, the default behavior will be disabled, and all
    artifacts will be built from {srcdir} (even if you combine
    -w/--wheel with -s/--sdist, the wheel will be built from {srcdir}).

positional arguments:
  srcdir                source directory (defaults to current directory)

options:
  -h, --help            show this help message and exit
  --version, -V         show program's version number and exit
  --sdist, -s           build a source distribution (disables the default behavior)
  --wheel, -w           build a wheel (disables the default behavior)
  --outdir OUTDIR, -o OUTDIR
                        output directory (defaults to {srcdir}/dist)
  --skip-dependency-check, -x
                        do not check that build dependencies are installed
  --no-isolation, -n    do not isolate the build in a virtual environment
  --config-setting CONFIG_SETTING, -C CONFIG_SETTING
                        pass options to the backend.  options which begin with a hyphen must be in the form of "--config-setting=--opt(=value)" or "-C--opt(=value)"

The only two options that might be useful that we don't already expose are --skip-dependency-check and --no-isolation! Pip has a few more things, but little else that would be useful in a wheel-building context. So there's not a huge amount of utility/flexibility in that option either, which isn't leaning me towards it.

Hm. Maybe we should also allow set environment variable only for build wheel step? Also opening issues in the build repository to allow control isolation via environment variables may be a good idea.

These are good ideas. But I'm not sure if they're really solving the root cause, which is that people occasionally want to customise the actual wheel build step. (Part of me is still wondering if we just bite the bullet and implement CIBW_BUILD_COMMAND - that would provide the most flexibility, as long as it's clear that using such an option is experimental/outside of python packaging norms.)

@joerick
Copy link
Contributor

joerick commented Aug 22, 2023

Sorry, I'll shut up in a minute, but perhaps a better idea than CIBW_BUILD_FRONTEND_FLAGS would be to take the same approach we took with the container-engine option, which is to nest the extra flags within the CIBW_BUILD_FRONTEND option. I kinda like that better, as it makes it clear that these are options to pip/build specifically, and it avoids creating another option.

So I guess that would look like CIBW_BUILD_FRONTEND="pip; args: --no-build-isolation" or CIBW_BUILD_FRONTEND="build; args: --no-isolation". We also already have all the parsing code for that approach :)

@joerick
Copy link
Contributor

joerick commented Aug 23, 2023

FYI, i've started work on a PR to do the above, the CIBW_BUILD_FRONTEND="pip; args: --no-build-isolation" syntax. We can see how it looks when it's implemented.

@abravalheri
Copy link
Contributor Author

Thank you very much @joerick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants