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

Feat: macos support #68

Merged
merged 3 commits into from
Aug 10, 2024
Merged

Conversation

amangupta17
Copy link
Contributor

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
Copy link
Collaborator

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

@dancergraham
Copy link
Collaborator

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

@dancergraham
Copy link
Collaborator

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

@amangupta17
Copy link
Contributor Author

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
Copy link
Collaborator

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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 dancergraham mentioned this pull request Aug 7, 2024
@dancergraham dancergraham merged commit dbe04bb into davidcaron:master Aug 10, 2024
35 checks passed
@dancergraham
Copy link
Collaborator

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

@dancergraham
Copy link
Collaborator

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 😄

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.

2 participants