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

CIBW_BEFORE_BUILD: Changes do not propagate #2084

Open
mrmundt opened this issue Nov 15, 2024 · 12 comments
Open

CIBW_BEFORE_BUILD: Changes do not propagate #2084

mrmundt opened this issue Nov 15, 2024 · 12 comments

Comments

@mrmundt
Copy link

mrmundt commented Nov 15, 2024

Description

We recently updated to 2.21.3 in order to start building Python 3.13 wheels for Pyomo. We have always utilized CIBW_BEFORE_BUILD to install some dependencies (pip install cython pybind11), and that step ran; however, as soon as it attempted to actually build the wheels, a failure was hit because it could not find cython and pybind11.

Build log

https://github.com/mrmundt/pyomo/actions/runs/11860593427/job/33056184668

CI config

https://github.com/mrmundt/pyomo/blob/ee0c2b39b9a6663cb33ffeb5fe2f4549ac60a04b/.github/workflows/release_wheel_creation.yml

@Czaki
Copy link
Contributor

Czaki commented Nov 15, 2024

Why you do not use pyproject.toml for define build dependencies?

@mrmundt
Copy link
Author

mrmundt commented Nov 15, 2024

We are an old-style Python package and do not use pyproject.toml. Per the tool's documentation, pyproject.toml is not required (and, in fact, it is implied that CIBW_BEFORE_BUILD is meant to help with exactly this).

@Czaki
Copy link
Contributor

Czaki commented Nov 15, 2024

Hm. The only change that I see and may have such impact is bumping pip version.

However I see some successfull CI job with most recent cibuildwheel.

@henryiii
Copy link
Contributor

I would highly recommend adding a pyproject.toml. That will allow you to declare you need these, so even SDist builds will benefit. Adding dependencies like cython and pybind11 is why pyproject.toml was developed in the first place. And every version of Python you support (3.8+) should have excellent pyproject.toml support. The only thing you need to watch out for is that setuptools no longer makes the current directory available by default with the modern build backend; if you need to import from the local directory (say to grab a helper), then you'll need to manipulate the path manually, as it's now considered an anti pattern. But otherwise, you should be able to drop a three-line pyproject.toml in and get started.

It looks like you are getting isolated builds for some reason. Not sure why. It's just installing setuptools (since you don't have a pyproject.toml telling it what you do actually require) in an isolated environment.

@henryiii
Copy link
Contributor

I see you added this in CI - is there a reason not to add it to repo and benefit anyone building an SDist? And you should not add "wheel", that's never been required for pyproject.toml, setuptools adds it for you (and now doesn't because it's not used anymore by modern setuptools). And I highly recommend adding the explicit modern build backend setting, build-system.build-backend = "setuptools.build_meta". See https://learn.scientific-python.org/development/guides/packaging-classic/

What was the last version of cibuildwheel you were using? v2.16.5? I didn't see anything in the pip logs that would have been likely to cause isolated builds to start being used, though that's a pretty big jump. I'd recommend using dependabot to keep that a bit more up to date, you can also pin to minor version instead of patch version. Quite a bit happened which makes it harder to pin down what might have changed between the two versions. It would be helpful to know which version this happened in.

@henryiii
Copy link
Contributor

2.17 -> 2.18 is when this started happening.

@henryiii
Copy link
Contributor

We stopped injecting setuptools and wheel in every environment in 2.18. If you add setuptools and wheel to the before-build libraries, it works. https://github.com/henryiii/pyomo/actions/runs/11865436281

I guess pip makes an isolated env of setuptools is missing?

If you used pyproject.toml and put the cibw config there, I could have built these locally much faster.

@mrmundt
Copy link
Author

mrmundt commented Nov 18, 2024

We stopped injecting setuptools and wheel in every environment in 2.18. If you add setuptools and wheel to the before-build libraries, it works. https://github.com/henryiii/pyomo/actions/runs/11865436281

I guess pip makes an isolated env of setuptools is missing?

If you used pyproject.toml and put the cibw config there, I could have built these locally much faster.

Thanks, we will try this.

Re why we aren't using a pyproject.toml: We have not found a compelling reason to add yet another file into the root dir of Pyomo. Our main philosophies are "maintain backwards compatibility" and "keep it clean."

@Czaki
Copy link
Contributor

Czaki commented Nov 18, 2024

If you shift with static metadata from setup.cfg to pyproject.toml you will use universal, not backend specific file format.

The pyproject toml is widely supported by multiple tools, ex for project management. So doing such migration may benefit your users.

@henryiii
Copy link
Contributor

You could remove your .coveragerc file and place the config in pyproject.toml. You could also get rid of setup.cfg1; I always configure pytest from pyproject.toml too. Ruff and uv can be configured from pyproject.toml too. And, if you configure cibuildwheel via pyproject.toml, you'll be able to run locally as well as switch/use a different CI provider easily. Check-sdist, repo-review, and hundreds of other tools support pyproject.toml.

There is no backward compatibility issue. If you somehow had a version of pip that didn't support pyproject.toml but also did support Python 3.8, then it would just build the same as it does today. However, for any version that does support pyproject.toml, it will build more reliably since you no longer need anything preinstalled.

Footnotes

  1. You currently have license_files set there; that looks like it's actually part of PEP 639, which is only partially, incorrectly supported by setuptools currently (based on an old draft, and doesn't bump the metadata to 2.4)

@golobor
Copy link

golobor commented Nov 18, 2024

Hi, all,
@mrmundt , thank you for raising this issue! We are running into an exact same issue.
In our case, we do use pyproject.toml, but we have to install one package (pysam) with a --no-binary option, so that it re-compiles against libraries installed via yum.
We could not find any way to specify the --no-binary flag in the "requires" field and thus had to use pip via before_build. However, the library installed via pip this way is not discovered during wheel building, likely, because it gets installed into a different environment.
Any advice would be highly appreciated! :)
Thanks!
Anton.

@henryiii
Copy link
Contributor

henryiii commented Nov 18, 2024

Can't you set PIP_NO_BINARY=pysam?

Something like:

[tool.cibw.linux.environment]
PIP_NO_BINARY = "pysam"

Or is that not what you meant?

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

4 participants