Bump GitHub Action pypa/cibuildwheel#211
Bump GitHub Action pypa/cibuildwheel#211DimitriPapadopoulos wants to merge 6 commits intoofek:masterfrom
Conversation
92efe26 to
93a3172
Compare
7232663 to
b5c86ec
Compare
|
Unliek the previous version, the new version of pypa/cibuildwheel attempts to build free threading wheels, and fails. We do need to fix that failure, but for now disable free threading. I created issue #212 to address free threading compatibility issues. |
2f22042 to
6e8e074
Compare
|
It looks like the |
|
After bumping cibuildwheel, CMake cannot find the I cannot find a way to fix this error, even after finding these:
This might an issue with |
|
The pkg-config installation did not seem to be picked up It seems the As for if it is needed or not, it seems that the |
|
I understand that the I can see that I'll try |
Warning: cibuildwheel: Invalid skip selector: 'pp*'. This selector matches a group that wasn't enabled. Enable it using the `enable` option or remove this selector. This selector will have no effect.
ac43981 to
3ed5a17
Compare
.github/workflows/build.yml
Outdated
| COINCURVE_SECP256K1_STATIC | ||
| COINCURVE_CROSS_HOST | ||
| CIBW_BEFORE_ALL_MACOS: ./.github/scripts/install-macos-build-deps.sh | ||
| CIBW_BEFORE_ALL_WINDOWS: choco install -y --no-progress --no-color pkgconfiglite |
There was a problem hiding this comment.
Not sure why we need both:
- Installing dependencies with
CIBW_BEFORE_ALL_MACOS/CIBW_BEFORE_ALL_WINDOWS(added by 6153647 / FEAT: Transition to CMake #152 on 11 March 2024)
and...
.github/workflows/build.yml
Outdated
| if: runner.os == 'macOS' | ||
| run: ./.github/scripts/install-macos-build-deps.sh | ||
| if: runner.os == 'Windows' | ||
| run: choco install -y --no-progress --no-color pkgconfiglite |
There was a problem hiding this comment.
- Explicitly installing dependencies again, conditionally to
runner.os(added by f0a68dc / Various updates #176 on 2 March 2025)
There was a problem hiding this comment.
@MementoRC and @ofek Do you recall why ./.github/scripts/install-macos-build-deps.sh is run twice, first using CIBW_BEFORE_ALL_MACOS and then explicitly and conditionally to runner.os?
There was a problem hiding this comment.
No, I don't recall. Creating this build was a bit challenging for me, perhaps this is remnants of various trial/error. My understanding at the time was that CIBW was an isolated build and did not seem to be registering the packages
There was a problem hiding this comment.
My understanding at the time was that CIBW was an isolated build and did not seem to be registering the packages
Only for linux. No real ways to make a container equivalent on windows or macos, so the Github runners are used as provided.
Ah right, I confused it for the Otherwise constructing the appropriate hint could be quite tricky, at which point using the "system" pkg-config would be more reliable. From the |
pyproject.toml
Outdated
| "cffi", | ||
| "scikit-build-core>=0.9.0", | ||
| "pkgconf; sys_platform == 'win32'", | ||
| "pkgconfig; sys_platform == 'win32'", |
There was a problem hiding this comment.
Vaguely remembering that we specifically chose 'pkgconf', though not why we did at the time
There was a problem hiding this comment.
Correct we shouldn't do this, I did this for Windows support and now it appears like that pkgconfig package is not well maintained. The existing pkgconf package is maintained by well-known folks in the scientific Python community.
There was a problem hiding this comment.
Unrelated to this PR, I'm interested in using that library on all platforms now for reproducibility purposes. I like being able to control as much of the build inputs as possible and I didn't realize the benefits of that until recently.
There was a problem hiding this comment.
Yet pkg-config is failing. I'll keep investigating if pkgconf is indeed installed, and is so, why pkg-config is failing.
There was a problem hiding this comment.
Yet
pkg-configis failing. I'll keep investigating ifpkgconfis indeed installed, and is so, whypkg-configis failing.
Most likely because of the hints not being available on CMake side. The pkg-config executable is installed in a build-isolated environment which is not available to PATH. Extracting the build-isolated environment that pip uses is a pain in the a**, unlike uv or other that use proper venv. I don't think we would have simple ways to add that to PATH even if we wanted to, it would be much easier if pkgconf provided a useful hint.
There was a problem hiding this comment.
Thanks! I am unfamiliar, do you mind explaining what exactly needs to be done to provide that hint?
Some more details about the CMake hints. The issue is that there is no real way of injecting the appropriate environment variables for CMake to pick it up. So the best option right now is to use what scikit-build-core defines and forwards to CMake. So far we do not support either editing PATH, mostly because of the annoyance above (although sometimes that manages to leak, don't quite remember the details), or other *_EXECUTABLE. The ones we support right now are documented here.
It just so happens that because of how CMake's find_program works, 2 hints could work: CMAKE_PREFIX_PATH and PkgConfig_ROOT. If the GNUInstallDirs path structure is preserved, then the upstream change would be as simple as adding
[project.entry-points."cmake.root"]
PkgConfig = "pkgconf"Unfortunately they put the executable under pkgconf/.bin so some minor adjustments would need to be made.
Another way would be to pass the build-issolated site-packages which would then be able to pick up the python script pkg-config, but I am not sure if CMake would complain if it tries to use that, or we are hitting something else, because I think we are doing that already doing that (intentionally or not) 1 and it doesn't seem to pass
Footnotes
-
To verify this, please run with
SKBUILD_LOGGING_LEVEL: "DEBUG"and it should show up in the log underSITE_PACKAGESandExtra SITE_PACKAGES↩
Just to be clear, are you referring to the build dependency https://pypi.org/project/pkgconf/? |
3ed5a17 to
52ac479
Compare
Originally the |
|
Thanks! I am unfamiliar, do you mind explaining what exactly needs to be done to provide that hint? |
|
@DimitriPapadopoulos I thought of an idea when reading Cristian's comment about how the issue is I understand that for Conda (if the issue exists there?) we might not want to build with UV but we can at least prevent the main pipeline from having hacks. What do you think? Here's an example of how to integrate UV into the workflow pypa/cibuildwheel#2630 |
|
Tried your suggestion. I don't know enough about this part of the build process to be useful. |
|
Very close! Like I wrote in the example I linked, I think you just need this part i.e. install UV on macOS and Windows: - name: Install UV
if: runner.os != 'Linux'
uses: astral-sh/setup-uv@v7 |
445e94a to
ddc240b
Compare
ddc240b to
0690e2d
Compare
|
Try to add |
|
Could it be that this needs to be updated by testing CMake This variable should be set by |
|
Argh, windows is always a challenge. The But the executables are installed in Best option is to fix it in if(SKBUILD AND WIN32)
foreach(prefix IN LIST CMAKE_PREFIX_PATH)
cmake_path(APPEND prefix Scripts pkg-config.exe OUTPUT_VARIABLE test_pkg_config)
if(EXISTS $test_pkg_config)
set(PKG_CONFIG_EXECUTABLE $test_pkg_config)
break()
endif()
endforeach()
endif() |
|
I don't know enough about scikit-build-core or CMake to find where to add the above code. It might be easier to fix |
The snippet above can fit just before the
Seems like I should have said But as I yet again think about this again, it does not really make sense why it failed when you And then there is another mystery, is the |
|
@LecrisUT Full transparency: @MementoRC rewrote the build system to be far more maintainable but I am unfamiliar with CMake and haven't had time to read the learning resources that @henryiii shared. I have almost no knowledge of how builds work here off the top of my head without looking deeply. |
You could also combine it in: |
The whole On another note, the |
Not quite. It must be before the
Ok, that explains But it would probably make more sense to for now skip these CI, and take a crack at fixing that CI in a different PR, since this has got quite off-topic from the original PR title. |
Do you mean that we only need to execute pkg-config.exe and thus could simply find it in the path since it is |
But conda CI has a whole different workflow file that doea not fail or use choco or such. It does not cover macos or windows, but it would be better to move them to that file instead in order to get a more accurate build that matches conda environment, i.e. using the packages from conda, not from choco or from pip. To that end cibuildwheel would not be a good tool to test the conda integration because it uses the PyPI packages, not the conda ones. To be absolutely compatible, you would need to duplicate the |
|
@LecrisUT First, correct, I mixed-up the verify_conda and verify_shared. The latter is effectively to verify that if a user has a custom dynamic Agreed, we don't use The whole I agree that I could investigate this in the recipe, when I get around it |
There are scripts in the conda-forge that make the build differ between the build instructions in upstream and downstream. There are automations that allow you to instead mimic the flow one-to-one, homebrew provides it via their github action and you just need to have an equivalent
Note that building with
Well it is more complicated. Have it installed by what makes a difference. That project can be built either via CMake or via Autotools, in which case it should try to pick up both methods in order. Best thing for those is to just try to build from the major packaging managers and see what they provide and in what form (shared/static library for example). Stuff like |
This should help fix today's CI failures.
Edit: Actually, it doesn't help. Quay is currently down with 502 errors (confirmed by status.redhat.com).
Still, it's a good idea to update!