giotto-ai / pyflagser

Python bindings and API for the flagser C++ library (https://github.com/luetge/flagser).
Other
13 stars 14 forks source link

Update CI from azure to github actions #70

Closed reds-heig closed 1 year ago

reds-heig commented 3 years ago

Reference Issues/PRs

None

What does this implement/fix? Explain your changes.

This PR moves CI from azure to github actions.

Any other comments?

It's a first draft, let's discuss our needs. Also when this works as he would like it to work, then we can move to update giotto-tda pipeline too.

Main changes in the CI compared to previous version
MonkeyBreaker commented 3 years ago

I don't know why I cannot see this PR building in the github actions directly here. I go directly in my fork to see right now the status of the pipeline: https://github.com/reds-heig/flagser-pybind/actions

ulupo commented 3 years ago

My main question moving forward with this is: is there going to be any way to support at least manylinux2014?

MonkeyBreaker commented 3 years ago

Well for the wheels, if you look in the README of cibuildwheel, here, it seems it is already supported. If it's better to go with manylinux2014, we could enable it. Did I understood you right ?

ulupo commented 3 years ago

I wasn't able to see where the wheels are built for the manylinux platform.

MonkeyBreaker commented 3 years ago

The wheels are managed in file wheels.yml. Currently it needs to be triggered manually, but you can only trigger a job manually when it is located in main branch.

To build the wheels we use an action called cibuildwheel, see link, it indicates that they use manylinux to build images.

For the manylinux images supported by the package, you have more details in the documentation. I let default values, I did not indicate which image to use. but looking at the text:

default images, quay.io/pypa/manylinux2010_x86_64, quay.io/pypa/manylinux2010_i686, pypywheels/manylinux2010-pypy_x86_64, quay.io/pypa/manylinux2014_aarch64, quay.io/pypa/manylinux2014_ppc64le, and quay.io/pypa/manylinux2014_s390x.

Meaning that depending on the architecture it uses 2010 or 2014.

hope it helps ^^

ulupo commented 2 years ago

@MonkeyBreaker I'm happy to look at this PR again soon. For now, there's a merge conflicts that must be resolved.

MonkeyBreaker commented 2 years ago

@ulupo resolved ! I didn't see we had a merge conflict 😅

ulupo commented 1 year ago

@MonkeyBreaker, I implemented the changes made in giotto-ph in ed0dd8a. Would be good to see if the wheels build and test fine.

MonkeyBreaker commented 1 year ago

When the last build is done, check the artifact if it contains the documentation. Then we can proceed with review + merge ;)