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

pyproject.toml: Add initial cibuildwheel configuration #157

Closed
wants to merge 24 commits into from

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Mar 23, 2022

Fixes #154

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 23, 2022

cibuildwheel --platform linux ends for me with

make[3]: Entering directory `/project/build/Cbc/2.10'
PKG_CONFIG_PATH=/usr/local/lib/pkgconfig:/usr/local/share/pkgconfig:/usr/local/lib64/pkgconfig:/usr/local/lib/pkgconfig:/usr/local/share/pkgconfig:/usr/local/lib/pkgconfig:/usr/local/lib/pkgconfig \
pkg-config --libs cbc > /usr/local/share/coin/doc/Cbc/cbc_addlibs.txt
make[3]: Leaving directory `/project/build/Cbc/2.10'
make[2]: Leaving directory `/project/build/Cbc/2.10'
make[1]: Leaving directory `/project/build/Cbc/2.10'

Running ldconfig to update library cache


                                                            ✕ 700.51s
Error: Command ['sh', '-c', '  apt-get install --yes wget || yum install -y wget\n  wget https://raw.githubusercontent.com/coin-or/coinbrew/master/coinbrew\n  sh coinbrew build Cbc@2.10 --no-third-party --parallel-jobs 16 --prefix=/usr/local --verbosity 4\n'] failed with code 127. 

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 23, 2022

From the build log:

checking whether -lblas has BLAS... no
checking for COIN-OR package Blas... not given: No package 'coinblas' found
checking whether -llapack has LAPACK... no
checking for COIN-OR package Lapack... not given: No package 'coinlapack' found

Should the wheels be built with a BLAS? Then OpenBLAS would be suitable: It is BSD-licensed, so it can be linked in without a problem.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 23, 2022

cibuildwheel --platform linux now succeeds locally:

24 wheels produced in 134 minutes:
  cylp-0.91.4-cp310-cp310-manylinux_2_17_i686.manylinux2014_i686.whl
  cylp-0.91.4-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
  cylp-0.91.4-cp310-cp310-musllinux_1_1_i686.whl
  cylp-0.91.4-cp310-cp310-musllinux_1_1_x86_64.whl
  cylp-0.91.4-cp36-cp36m-manylinux_2_17_i686.manylinux2014_i686.whl
  cylp-0.91.4-cp36-cp36m-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
  cylp-0.91.4-cp36-cp36m-musllinux_1_1_i686.whl
  cylp-0.91.4-cp36-cp36m-musllinux_1_1_x86_64.whl
  cylp-0.91.4-cp37-cp37m-manylinux_2_17_i686.manylinux2014_i686.whl
  cylp-0.91.4-cp37-cp37m-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
  cylp-0.91.4-cp37-cp37m-musllinux_1_1_i686.whl
  cylp-0.91.4-cp37-cp37m-musllinux_1_1_x86_64.whl
  cylp-0.91.4-cp38-cp38-manylinux_2_17_i686.manylinux2014_i686.whl
  cylp-0.91.4-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
  cylp-0.91.4-cp38-cp38-musllinux_1_1_i686.whl
  cylp-0.91.4-cp38-cp38-musllinux_1_1_x86_64.whl
  cylp-0.91.4-cp39-cp39-manylinux_2_17_i686.manylinux2014_i686.whl
  cylp-0.91.4-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
  cylp-0.91.4-cp39-cp39-musllinux_1_1_i686.whl
  cylp-0.91.4-cp39-cp39-musllinux_1_1_x86_64.whl
  cylp-0.91.4-pp37-pypy37_pp73-manylinux_2_17_i686.manylinux2014_i686.whl
  cylp-0.91.4-pp37-pypy37_pp73-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
  cylp-0.91.4-pp38-pypy38_pp73-manylinux_2_17_i686.manylinux2014_i686.whl
  cylp-0.91.4-pp38-pypy38_pp73-manylinux_2_17_x86_64.manylinux2014_x86_64.whl

@mkoeppe mkoeppe marked this pull request as ready for review March 23, 2022 04:36
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 23, 2022

Should the wheels be built with a BLAS? Then OpenBLAS would be suitable: It is BSD-licensed, so it can be linked in without a problem.

This could be lifted from https://github.com/numpy/numpy/blob/main/pyproject.toml#L79, https://github.com/numpy/numpy/blob/main/tools/wheels/cibw_before_build.sh

@tkralphs
Copy link
Member

@mkoeppe This looks amazing! Thank you so much.

Should the wheels be built with a BLAS? Then OpenBLAS would be suitable: It is BSD-licensed, so it can be linked in without a problem.

This could be lifted from https://github.com/numpy/numpy/blob/main/pyproject.toml#L79, https://github.com/numpy/numpy/blob/main/tools/wheels/cibw_before_build.sh

Yes, having Blas would be good. An option would be to use our third party wrapper, which would be easy. However, this uses the reference implementation, which is not the best. OpenBlas would be better, but it looks like it might be a little more work to set it up.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 24, 2022

Yes, let's defer the BLAS stuff to a follow-up PR.

…until it's merged. First, upload on every push and second, only publish to TestPyPI. Once everything is working, we can enable it as intended
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 25, 2022

... or just set COIN_INSTALL_DIR to something nonempty so that the setup.py doesn't complain ...

@tkralphs
Copy link
Member

or just set COIN_INSTALL_DIR to something nonempty so that the setup.py doesn't complain

To create the sdist, it first installs the package, which requires building it, so I didn't think this could be done without Cbc being present. Is that not right?

@tkralphs
Copy link
Member

I thought I might be able to get this into some kind of minimally running condition, so I could merge it and at least be able to have auto-generated Linux wheels with the coming release. But it looks like it will require a bit more trial and error to get all the pieces working, so I'll have to come back to it later. I guess I can just make the release fully manually. I would like to generate the Linux wheels locally, though, so I need the changes to the pyproject.toml. I may lift those over by hand.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 25, 2022

You don't need Cbc for building the sdist. It does not put Cython-generated files in the sdist. It's all static.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 25, 2022

Fixed some brokenness in setup.py, now it actually works

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 25, 2022

Users/runner/work/CyLP/CyLP/CoinUtils/CoinUtils/install-sh -c -m 644 config_coinutils.h /Users/runner/work/CyLP/CyLP/local/include/coin/CoinUtilsConfig.h
  test -z "/Users/runner/work/CyLP/CyLP/local/lib" || /Users/runner/work/CyLP/CyLP/CoinUtils/CoinUtils/install-sh -d "/Users/runner/work/CyLP/CyLP/local/lib"
  test -z "/Users/runner/work/CyLP/CyLP/local/include/coin" || /Users/runner/work/CyLP/CyLP/CoinUtils/CoinUtils/install-sh -d "/Users/runner/work/CyLP/CyLP/local/include/coin"
  mkdir: /Users/runner/work/CyLP/CyLP/local/include: File exists
  mkdir: /Users/runner/work/CyLP/CyLP/local/include/coin: File exists
  make[2]: *** [install-includecoinHEADERS] Error 1
  make[2]: *** Waiting for unfinished jobs....

parallelization bug in coin makefiles

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 25, 2022

green checkmark on macOS now.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 26, 2022

wheel build fails on musllinux platforms:

Building cp36-musllinux_x86_64 wheel
CPython 3.6 musllinux x86_64

Setting up build environment...
                                                              ✓ 0.04s
Building wheel...
  
      + rm -rf /tmp/cibuildwheel/built_wheel
      + mkdir -p /tmp/cibuildwheel/built_wheel
      + python -m pip wheel /project --wheel-dir=/tmp/cibuildwheel/built_wheel --no-deps
    ERROR: Command errored out with exit status 1:
     command: /opt/python/cp36-cp36m/bin/python /opt/python/cp36-cp36m/lib/python3.6/site-packages/pip/_vendor/pep517/in_process/_in_process.py prepare_metadata_for_build_wheel /tmp/tmpse3jculu
         cwd: /project
    Complete output (56 lines):
    Package cbc was not found in the pkg-config search path.
    Perhaps you should add the directory containing `cbc.pc'
    to the PKG_CONFIG_PATH environment variable
    Package 'cbc', required by 'virtual:world', not found
    Traceback (most recent call last):
      File "setup.py", line 50, in <module>
        CoinDir = os.environ['COIN_INSTALL_DIR']
      File "/opt/python/cp36-cp36m/lib/python3.6/os.py", line 669, in __getitem__
        raise KeyError(key) from None
    KeyError: 'COIN_INSTALL_DIR'

I'll disable it.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 26, 2022

OK, it works.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 26, 2022

The macOS build of CBC failed again as in #157 (comment), perhaps the parallelization needs to be reduced?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 26, 2022

Wheels build now, just checking them before uploading fails:

Checking dist/cylp-0.91.4.tar.gz: FAILED
  `long_description` has syntax errors in markup and would not be rendered on PyPI.
    line 28: Warning: Title underline too short.

    On Windows: Installation as a binary wheel

@tkralphs
Copy link
Member

Thank you so much for the help, @mkoeppe! I was trying to figure out what was wrong with the RST, but what's weird is that the twine check passes on the wheels I generated locally and everything looks precisely the same. RST is weird. It seems that PyPI now supports Markup, so I may just switch to that.

The last failure is an authentication issue, which I think is just because this is a PR from your fork and so you don't have access to my secrets. You would think that since it's a PR running in the his repo, the secrets would be available, but it seems not. So I will merge this now!

@tkralphs
Copy link
Member

I did an interactive rebase to clean things up a bit and merged this manually.

@tkralphs tkralphs closed this Mar 26, 2022
@tkralphs
Copy link
Member

tkralphs commented Mar 26, 2022

@tkralphs
Copy link
Member

I would still like to get Windows working, but I can upload Windows wheels manually for the time being.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 26, 2022

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 26, 2022

I would still like to get Windows working, but I can upload Windows wheels manually for the time being.

No idea here about native Windows builds. My knowledge about Windows ends with Windows NT 4.0

@tkralphs
Copy link
Member

Time to bump the version?

Yes, I'm getting ready to flip over to production PyPI and then bump the version number and make a release.

I have a pretty good idea on the Windows builds, since we're already doing Windows builds on GH Actions for Cbc. But it's a little more work. Basically, you can use the bash shell provided by MSys2, which is installed on the Windows nodes.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 26, 2022

The source tarball is missing in https://pypi.org/project/cylp/#files

@tkralphs
Copy link
Member

Sorry, it's there now. I had to upload the wheels manually, since the upload action didn't fire (and would have failed anyway, since I forgot to switch to a PyPI API token). I forgot to upload the source tarball.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 27, 2022

Thanks!

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

Successfully merging this pull request may close these issues.

Use cibuildwheel
2 participants