-
Notifications
You must be signed in to change notification settings - Fork 10
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
Build and test wheels for x86_64 & arm64 #102
Conversation
4273109
to
840d9c1
Compare
|
||
jobs: | ||
build_dependencies: | ||
strategy: | ||
fail-fast: true | ||
fail-fast: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was this intended for dev or production?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This made most sense when developing, but I see why it would make more sense to do a full stop if something fails in production. Lets flip this to true
.
fi | ||
python --version | ||
|
||
export PYTHON_EXECUTABLE=$(which python3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I wrote this python3
originally for some reason, but does it maybe make sense to just write python
instead of python3
here, if it has the same effect? Since we are sourcing the venv which is python3 anyway.
export _PYTHON_HOST_PLATFORM="macosx-11.0-$arch_ver" | ||
export ARCHFLAGS="-arch $arch_ver" | ||
|
||
if [ "$arch_ver" == "arm64" ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suppose this is necessary to make it work, when/if GH actions change their runner/arch combinations, will this still hold up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tools actually update the version number in the host platform, so this hints to the wheel build that we don't want universal
wheels, but rather arm64
or x86_64
wheels separate.
I will add a comment to this since the next person won't know these details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job! Some comments/questions to address, feeling pretty confident you can address them and then merge this
840d9c1
to
281e2a7
Compare
No description provided.