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

[confusion report] CIBW_ENVIRONMENT seems to leak into the test stage #1666

Closed
webknjaz opened this issue Nov 19, 2023 · 6 comments
Closed

Comments

@webknjaz
Copy link
Member

Description

The docs @ https://cibuildwheel.readthedocs.io/en/stable/options/#environment say:

Set environment variables needed during the build

And that "during the build" wording is reiterated a few times in the paragraph. So I set environment = { PIP_CONSTRAINT="requirements/cython.txt" } in the YARL project to pin the build deps (composed manually so far, pending the jazzband/pip-tools#1681 release to make the automation possible).

The file contains just one line — cython==3.0.5. And this is the only place where this constraint is defined.

cibuildwheel is also configured to run tests (not by me). So it does that after the build. I was surprised that it failed with a pip install errorring out on attempt to build PyYAML from sdist:

        ERROR: Cannot install Cython<3.0 because these package versions have conflicting dependencies.
        
        The conflict is caused by:
            The user requested Cython<3.0
            The user requested (constraint) cython==3.0.5
        
        To fix this you could try to:
        1. loosen the range of package versions you've specified
        2. remove package versions to allow pip attempt to solve the dependency conflict
        
        ERROR: ResolutionImpossible: for help visit https://pip.pypa.io/en/latest/topics/dependency-resolution/#dealing-with-dependency-conflicts
        [end of output]
    
    note: This error originates from a subprocess, and is likely not a problem with pip.
  error: subprocess-exited-with-error
  
  × pip subprocess to install build dependencies did not run successfully.
  │ exit code: 1
  ╰─> See above for output.
  
  note: This error originates from a subprocess, and is likely not a problem with pip.

This is because PyYAML caps Cython under 3.0 since yaml/pyyaml@27f9a99#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711R2. Which is fine normally, since PEP 517 builds are isolated.

The problem is that PIP_CONSTRAINT="requirements/cython.txt" set via CIBW_ENVIRONMENT doesn't seem to actually be scoped to just the build stage as the docs advertise, but looks like it remains in the environment.

I think that cibuildwheel should either apply stage-specific settings in isolation, or at least document this surprise side effect...

Build log

https://github.com/aio-libs/yarl/actions/runs/6918060335/job/18819918206#step:6:10219

CI config

https://github.com/aio-libs/yarl/blob/560dd38/.github/workflows/reusable-build-wheel.yml + https://github.com/aio-libs/yarl/blob/560dd38/pyproject.toml#L68

@webknjaz
Copy link
Member Author

So I confirmed that the env vars set by that param are injected into the test stage, including provisioning the deps: https://github.com/pypa/cibuildwheel/blob/77c57bd/cibuildwheel/windows.py#L549.

Using

before-test = [                                                                                                                                        
  "unset PIP_CONSTRAINT",  # https://github.com/pypa/cibuildwheel/issues/1666                                                                          
]

doesn't work on windows. Still trying to find proper syntax for using a windows-specific command...

@joerick
Copy link
Contributor

joerick commented Nov 19, 2023

Yes, I understand the confusion. Unfortunately the project grew in such a way that tests were added as part of the 'build' phase, as config options like CIBW_BUILD choose which pythons to build-and-test. Additionally there is no way to just test a wheel without building it first, so the confusion never really came up. Maybe there's some clarification we can do to the environment option though.

As for your specific question, it does suggest the need for an additional CIBW_TEST_ENVIRONMENT option. However, I notice that you are setting PIP_CONSTRAINT. That is actually an option that cibuildwheel sets itself to ensure determinism in the build env that build creates. So it might actually not be having an effect. Is your intention to pin your build backend dependencies? We might have to look at creating a feature for that.

@webknjaz
Copy link
Member Author

Yep. Your assessment is correct. We used to run the cythonize program before invoking any Python packaging. That way, we were in control of this specific build dep. Now that I moved that into an in-tree build backend, that control is kinda lost and the current standard way of regaining it is setting the PIP_CONSTRAINT env var, since the CLI flags don't reach the ephemeral PEP 517 build env.

As for setting your own var, I think it's fine for your deps, but might be an undesired side effect for the end-users. Especially, when undocumented... I'd suggest scoping it more carefully.

@webknjaz
Copy link
Member Author

So I finally came up with a hack to bridge the gap for us — I injected an early PIP_CONSTRAINT= pip install PyYAML command that ends up building a wheel from sdist with a Cython version matching what PyYAML declares. Pip caches that wheel and any further attempts to install it, just pick it up from cache:
https://github.com/aio-libs/yarl/blob/c907c8e/pyproject.toml#L69-L77

I've made an exclusion for Windows since it always picks up wheels as historically, win32 and win64 wheels have been published by PyYAML, unlike *nix:
https://github.com/aio-libs/yarl/blob/c907c8e/pyproject.toml#L87

I hope to drop this workaround as soon as cibuildwheel starts providing an adequate replacement...

@joerick
Copy link
Contributor

joerick commented Nov 20, 2023

A clever hack! I did add a +1 to the pypa/build features request for an alternative to PIP_CONSTRAINTS, as I'd like to expose an option for build-dep pinning through CIBW_BUILD_FRONTEND. pypa/build#292 (comment)

In the meantime, it remains an open question over the need for more specific build/test env var options.

On the one hand, the reason this is needed is that PIP_CONSTRAINTS affects the inner workings of both build and cibuildwheel, and that's the source of the issue. If build can support a constraint option, we avoid extra complexity and more options here.

On the other hand, practicality beats purity. Comments/opinions welcome.

@joerick
Copy link
Contributor

joerick commented Jan 26, 2024

The original issue was solved, so closing this out.

@joerick joerick closed this as completed Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants