search

davidcaron / pye57

Read and write e57 point clouds from Python
MIT License
70 stars 41 forks source link

Feat: macos support #68

Closed amangupta17 closed 2 months ago

amangupta17 commented 3 months ago

What was the problem/requirement? (What/Why)

pye57 library does not support macOS builds at the moment.

What was the solution? (How)

Updated xerces-c installation as a static dependency which is now referenced by the libe57 binary.

How was this change tested?

sh scripts/install_xerces_c.sh python3 setup.py bdist_wheel

Updated github actions to test the newly created wheels for each python version on all the supported operating systems.

Github actions report: https://github.com/amangupta17/pye57/actions/runs/10173256405

Ran: pip install dist/{wheel} import pye57 executes without an error.

dancergraham commented 3 months ago

Wow well done - it looks like you managed to reproduce the errors I was seeing on mac and Windows and fixed them ?

dancergraham commented 3 months ago

oh wow - you added a pretty big installation step to the ci - did you try adding a test command environment variable to cibuildwheel ?

dancergraham commented 3 months ago

Your wheel seems to install OK for me on Windows...

amangupta17 commented 3 months ago

Yeah the issue was xerces binaries were not linked correctly and the build wheels command was succeeding because libe57 was reading the xerces from local installation. Added a new test step so we can test the built wheels on a fresh platform.

Test suite would still pass if we added the cibw test variable to build wheels job since libe57 might read from the locally installed xerces.

We can look into running the test suite on the test built wheels step in the future.

dancergraham commented 3 months ago

The macos build doesn't work on my mac - my mac reports it is version 10.9 universal2 and I think these wheels are 11.0 arm64 wheels - is that Apple Silicon ? I guess as a minimum we should specify in the readme that this is Apple Silicon (arm cpus) only, and ideally add x86 / unerversal2 wheels later

amangupta17 commented 3 months ago

Yes that's correct. Only Apple Silicon is supported at the moment. We can add Mac Intel support later. I will update the description.

amangupta17 commented 3 months ago

We need to set this enviroment variable while building wheels for macos. CIBW_ARCHS_MACOS: "x86_64 universal2 arm64" to enable other architecture builds.

dancergraham commented 2 months ago

Great - I've released it - can you test the new release on your mac? it seems to work well on my pc

dancergraham commented 2 months ago

I think just universal2 would be enough - that can be installed on both architectures. It is larger because it duplicates everything but people using pye57 are used to large file sizes 😄