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

✨ Apple Silicon (arm64) support, fixes #36 #54

Conversation

AndreMiras
Copy link
Contributor

Note that we (cross) compile for both arm64 and the universal2. This may feel redundant, it can still be interesting to have both. While universal2 is more convenient arm64 is lighter.

Also drop Python 3.7 (end-of-life 2023-06-27).
Add Python 3.12 and pypy-3.10 support.

Bump CI actions versions as well.

Note that we (cross) compile for both arm64 and the universal2.
This may feel redundant, it can still be interesting to have both.
While universal2 is more convenient arm64 is lighter.

Also drop Python 3.7 (end-of-life 2023-06-27).
Add Python 3.12 and pypy-3.10 support.

Bump CI actions versions as well.
@AndreMiras AndreMiras force-pushed the feature/version_bump_and_apple_arm64_support branch from b368541 to 863c94e Compare October 15, 2023 20:32
@@ -1,16 +1,19 @@
name: Tests

on: [push]
on: [push, pull_request]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also sneaked that change in so we would have caught that issue earlier:
#49 (comment)

on: [push, pull_request]

env:
PYTHON_VERSIONS: ["3.8", "3.9", "3.10", "3.11", "3.12", "pypy-3.8", "pypy-3.9", "pypy-3.10"]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DRY fix, but also removed 3.7, and added 3.12 as well as pypy-3.10.
But if we prefer I can address these changes in another dedicated PR

Comment on lines +26 to +30
CIBW_ARCHS_MACOS: x86_64 arm64 universal2
CIBW_TEST_REQUIRES: .[test]
CIBW_TEST_COMMAND: pytest {project}
# arm64 cannot be tested on a x86_64 CI runner
CIBW_TEST_SKIP: "*-macosx_arm64"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the macOS arm64 support part, all the rest of the PR is just chore

@AndreMiras
Copy link
Contributor Author

Hi @amitdev, how can I help you merge this one?
Do you want me to split it up into smaller concerns or describe further what's doing and why it's needed?
I'm excited to see it merged as it would make life easier to Apple ARM/M1 developers

@amitdev
Copy link
Owner

amitdev commented Nov 1, 2023

Thanks @AndreMiras for the PR. I was away for sometime so couldn't get this merged. Will get a release out soon.

@amitdev amitdev merged commit 1487017 into amitdev:master Nov 1, 2023
5 checks passed
@AndreMiras AndreMiras deleted the feature/version_bump_and_apple_arm64_support branch November 1, 2023 07:34
@amitdev
Copy link
Owner

amitdev commented Nov 1, 2023

@AndreMiras can you fix this issue with tests btw? If not I'll take a look when I get time later:

[Invalid workflow file: .github/workflows/tests.yml#L6](https://github.com/amitdev/lru-dict/actions/runs/6526334419/workflow)
The workflow is not valid. .github/workflows/tests.yml (Line: 6, Col: 20): A sequence was not expected .github/workflows/tests.yml (Line: 13, Col: 17): Unrecognized named-value: 'env'. Located at position 10 within expression: fromJson(env.PYTHON_VERSIONS)

@AndreMiras
Copy link
Contributor Author

OK I was confused how I missed that one, but it's clear now. Since the workflow had a syntax error it didn't even run so the PR was still marked green.
Of course I'll fix that. I'll keep you posted.

AndreMiras added a commit to AndreMiras/lru-dict that referenced this pull request Nov 1, 2023
@AndreMiras
Copy link
Contributor Author

Here's the fix #55, thanks for the heads up.

RF-Tar-Railt pushed a commit to RF-Tar-Railt/lru-dict that referenced this pull request Nov 3, 2023
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