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

unintentional pure wheel created for ctypes wrapper, general usage question. #837

Closed
AndrewAnnex opened this issue Sep 21, 2021 · 21 comments

Comments

@AndrewAnnex
Copy link

AndrewAnnex commented Sep 21, 2021

I author/maintain a package https://github.com/AndrewAnnex/SpiceyPy that wraps a C shared library using ctypes that I am trying to adapt to use cibuildwheels in AndrewAnnex/SpiceyPy#421. I've supported building wheels for windows using a custom github workflow adapted from appveyor for a while now https://github.com/AndrewAnnex/SpiceyPy/blob/main/.github/workflows/publish-to-test-and-live-pypi.yml, but I want to upgrade things to cibuildwheel to support more platforms in a unified manner with cibuildwheels.

Based on the documentation I have seen, my library does not fit neatly into the paradigm of cibuildwheel and my ci builds currently fail because a pure python library is being produced erroneously, so I could use some advice to work through this. In the current builds it looks like cibuildwheel is not applying the --plat-name parameter for bdist_wheel, and I am not sure if there is a way to force it to/override it. I could also do some trickery https://stackoverflow.com/questions/35112511/pip-setup-py-bdist-wheel-no-longer-builds-forced-non-pure-wheels, but I would hope that cibuildwheel has the ability to do this. I am about to try switching the build frontend option to see if that works as it does for windows. <I misread what this actually does

For some background, I build the shared library during install time within my setup.py with a custom script that downloads the static library distribution and runs various shell commands to build a shared library on linux/macos/windows.

This also means that my source code is not actually dependent on the python version, as I write it to be compatible with 3.6+, so I really only need 1 build for each architecture (windows, macos, macos arm, etc), which maybe related to #569. So in the end the macos shared library for example could be used for any of the python versions on macos.

any tips would be appreciated, apologies if this is the wrong venue for general usage questions

@henryiii
Copy link
Contributor

henryiii commented Sep 21, 2021

I'd look at https://github.com/scikit-build/cmake-python-distributions and https://github.com/scikit-build/ninja-python-distributions where this is done using a post-build script. I'm going to work on pypa/wheel#407 soon, which should help make this easier (actually would have started if it wasn't for musllinux causing #799 to be bumped up in priority).

However, for your case, maybe look at https://github.com/ssciwr/clang-format-wheel - this has a very simple workaround in https://github.com/ssciwr/clang-format-wheel/blob/1abcee5a932961c342fa5462c7e2d004a21b7abb/setup.py#L7-L15. No special work needed after that.

In all cases, you should select just one version of python to build. Eventually we might have a way to run the other Python versiontests on a single Python version.

@AndrewAnnex
Copy link
Author

AndrewAnnex commented Sep 21, 2021

@henryiii thanks for the suggestions, I think the clang example could work for my needs (I actually used to have version of this in my setup.py in a earlier release but I never distributed the wheels because I was unsure if they would work generically) but I am unsure if [PEP425] (https://www.python.org/dev/peps/pep-0425/#python-tag) would work for the minimum python 3.6 version I had in mind. Looking at the pep it looks like py3 would specify any python 3 version, but that py36 would say only python 3.6 is compatible. I think I would need a python tag like py36.py37.py38.py39 to prevent 3.5 users from installing the package, but again maybe I am just not reading the pep carefully but I am not sure if that is supported. Alternatively, I could more closely follow that example by making a new package that just provides the compiled shared library and then refactor my main project to depend on it... arguably a more correct approach but I would hope it's not necessary.

@henryiii
Copy link
Contributor

You should use the Requires.Python metadata slot (python_requires= in setup.py/cfg) to control this, not the Python version tag. The wheel should have no Python tags associated with it if it is not compiled against a single version of CPython. A pure Python wheel also does not use py36.py37.py38.py39, but just uses the above mentioned metadata slot.

@henryiii
Copy link
Contributor

Also, "Python 3" means "3.6+" more or less nowadays, since 3.0 - 3.5 are end-of-life. So a human reading the wheels (pip will use Requires.Python) will assume py3 means Python 3.6+. No one expects anything to work on Python 3.0, 3.1, etc...

@AndrewAnnex
Copy link
Author

ahh. yes I do have python_requires set in both my setup.py and the pyproject.toml I am adding in that PR. I also just spotted pypa/wheel#336 that seems relevant to this. Pep 425 seems to imply that the abi tag specifies if the wheel is pure python (if it's set to none). I am going to try overriding get_tag to return "py3", "abi3" and the default platform to see if that works.

@AndrewAnnex
Copy link
Author

but then again, setting the abi shouldn't matter, but if I don't am I back to the issue of having a pure python wheel or does overriding the attribute infinalize_options fix that?

@henryiii
Copy link
Contributor

You are not making a limited API / stable ABI wheel. You are making a wheel that does not depend on CPython at all, so none is correct for the implementation. It would / should support PyPy as well, or any other implementation of the Python language, because it does not depend on the implementation.

Your names should in the end look just like cmake, ninja, clang-format, or any other package that does not touch the CPython interpreter.

@ccaprani
Copy link

Hi @AndrewAnnex , looking at the repo I guess you didn't solve it? I have a similar problem at https://github.com/MonashSmartStructures/ospgrillage

@henryiii forgive the ignorance, but what is wrong with cibuildwheel producing a pure-Python wheel? And although I have >=3.8 in the build config files, it still just builds the wheel as py3. I'm really stuck - thanks!

@Czaki
Copy link
Contributor

Czaki commented Oct 26, 2021

@henryiii forgive the ignorance, but what is wrong with cibuildwheel producing a pure-Python wheel?

Because You do not need multiple python environments to build pure-Python wheel and environment provided by CI by default is enough to build such a type of wheel.

@henryiii
Copy link
Contributor

henryiii commented Oct 26, 2021

@ccaprani See https://scikit-hep.org/developer/gha_pure for an example of a pure python GHA workflow. There's no need to setup up special environments to compile a pure Python wheel (like download special interpreters, start up large docker images, etc) - the host Python that runs cibuildwheel itself is perfectly capable of building a pure Python wheel. There's no need to run on multiple platforms, and since you should be making an sdist anyway, you should just add the wheel production to the sdist setup and then there's nothing at all for cibuildwheel to do. The PyPA's build tool is exactly what you what for this, there's no reason to add cibuildwheel. Heres' what it would look like in GHA:

dist:
  runs-on: ubuntu-latest
    steps:
    - uses: actions/checkout@v2

    - name: Build SDist and then wheel from SDist
      run: pipx run build

    - uses: actions/upload-artifact@v2
      with:
        path: dist/*

This will just take a few seconds, rather than minutes for cibuildwheel to setup environments, etc.

Remember you are just making one wheel!

@AndrewAnnex
Copy link
Author

AndrewAnnex commented Oct 26, 2021

@ccaprani no I did solve it by overriding build_wheel in my setup.py, so I should have closed the issue weeks ago. Currently I'm running into an unrelated issue with setuptools/peps 517-518 as I'm trying to update to the newer paradigms. However, I think more documentation in the RTD surrounding this topic would be helpful, some of the links @henryiii posted above are dead and since the C library I build doesn't use ninja/cmake (and I basically have no experience with either of those tools) so they didn't seem as relevant to me. I think a more generic "you have some c files and want to use gcc, but not distutils.Extension" example would be helpful.

On a side note, It also isn't clear to me how the docker images for cibuildwheel have the necessary state to build the C library for my project on Windows. For windows I need to use the https://github.com/ilammy/msvc-dev-cmd action to setup the environment on the GHA host to ensure cl.exe was available, but somehow cibuildwheels is able to use msvc within it? Maybe mvsc is already inside the windows images and I just misinterpreted the errors I saw at one point

@henryiii
Copy link
Contributor

Sorry about the dead links, they were from the old renamed repo for clang-format-wheel. I can try to fix them. The other links should all be fine.

you have some c files and want to use gcc, but not distutils.Extension

But you also don't want to use CMake, which is used for over 60% of all C/C++ projects? You need to use something, otherwise you'll be basically reinventing distutils/CMake/Meson/Basel/Autotools (yuck)/SCons (Simi-yuck), etc.

I think distutils/setuptools started to support this, there is a class that seems to have been made for this purpose, but it seems to be unfinished. It's not surprising; CPython devs want to get out of the business of maintaining build tools, distutils is deprecated and stated for removal in 3.12. PEP 517 and arbitrary build systems are the future. I'm working on a proposal to modernize Scikit-build; SciPy is supporting Meson, and SCons has encsons, the first true PEP 517 system capable of building binaries. (But uses SCons :'( )

how the docker images for cibuildwheel have the necessary state to build the C library for my project on Windows

Quite a few things wrong with this statement - docker images are linux, and they contain all the compilers needed, but using them on windows would build linux extensions, which is silly in CI, might as well use linux for the host OS. :)

The CI runners already have MSVC 2019 (and 2017 and 2015) installed. ilammy/msvc-dev-cmd simply "actives" the requested compiler; it already exists. Distutils and CMake both can find and use the correct compiler without needing to have it activated. Though both also work find if you do, and respect your choice. If you want to build this yourself, without using distutils or CMake, then you'll have to learn how to search for and activate compilers manually.

@henryiii
Copy link
Contributor

@AndrewAnnex
Copy link
Author

@henryiii I am not using cmake/meson etc firstly because I don't maintain the C library, and I don't work often enough with C/C++ to really need to know cmake. It is maintained and released by NASA JPL, and they only provide TCH scripts (not even makefiles) for their project. It's all ANSI C with no dependencies other than the math library, technically translated from FORTRAN77 with F2C ahead of time. You can read more about it here https://naif.jpl.nasa.gov/pub/naif/toolkit_docs/C/req/cspice.html.

They don't provide cmake scripts for the project and I don't want to maintain my own, as it only takes 2 commands with gcc to compile the shared library.

@henryiii
Copy link
Contributor

But you want to target Windows? Do you expect those same two lines to work on Windows?

@henryiii
Copy link
Contributor

henryiii commented Oct 26, 2021

What are the two GCC lines? If it's two GCC lines, it's likely not many lines in those other systems, and you'd get windows support, etc.

@AndrewAnnex
Copy link
Author

AndrewAnnex commented Oct 27, 2021 via email

@henryiii
Copy link
Contributor

I think the spaces got stripped from that file somehow (edit before sending: the main branch looks fine).

I'm interested because I'm preparing a proposal to work on scikit-build for the next three years, and I'd really like to have good solutions for builds like this one. You have 500 lines of custom code in addition to a 200 line setup.py, so I think a CMake file + Scikit-build could handle something like this quite well.

cmake_minimum_required(VERSION 3.12...3.21)
project(CSPICE LANGUAGES C)
file(GLOB CSPICE_SOURCES CONFIGURE_DEPENDS *.c)
add_library(cspice SHARED ${CSPICE_SOURCES})

find_library(MATH_LIBRARY m)
if(MATH_LIBRARY)
    target_link_libraries(cspice PUBLIC "${MATH_LIBRARY}")
endif()

You could add the download and patching here too. Should work on all platforms.

Scikit build doesn't really have a native way to make a python independent output though. Yet. Anyway, I'll keep this use case in mind.

@AndrewAnnex
Copy link
Author

@henryiii, okay cool! I mean sure I have 500 lines of code but it's all easy python and I didn't need to learn cmake's DSL as well. That said, trading for ~50 lines of cmake would be acceptable

@ccaprani
Copy link

@Czaki @AndrewAnnex Thanks indeed for getting back to me - the conversation here was very helpful!

@henryiii a special thank you for your time and for being so supportive! The developer notes you pointed me to (https://scikit-hep.org/developer) are really excellent for the modern python packaging and workflow in general. And now I am up and running...thanks again!

@AndrewAnnex
Copy link
Author

oh yeah forgot to close this issue, I worked out the issue is really in setuptools

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