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.

Sign up for GitHub

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

Jump to bottom

Fix arm wheels on macOS #4017

Merged
merged 13 commits into from Nov 6, 2023
Merged

Fix arm wheels on macOS #4017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
69c8910
Attempt to fix arm wheels on macOS by toggling hatch settings
hauntsaninja Nov 3, 2023
3380098
tmp macos
hauntsaninja Nov 3, 2023
b7cb820
no thanks
hauntsaninja Nov 3, 2023
fcc91fe
work around hatch's inner platform
hauntsaninja Nov 3, 2023
75bd6e7
.
hauntsaninja Nov 3, 2023
757430b
.
hauntsaninja Nov 3, 2023
33073bf
.
hauntsaninja Nov 3, 2023
bdf4105
.
hauntsaninja Nov 3, 2023
e7f14d7
.
hauntsaninja Nov 3, 2023
7650f6e
.
hauntsaninja Nov 3, 2023
05eb557
changelog
hauntsaninja Nov 3, 2023
5f3e84d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 3, 2023
f6a3033
.
hauntsaninja Nov 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions .github/workflows/pypi_upload.yml
View file
Open in desktop
Expand Up @@ -68,9 +68,10 @@ jobs:
- name: generate matrix (PR)
if: github.event_name == 'pull_request'
run: |
cibuildwheel --print-build-identifiers --platform linux \
| pyp 'json.dumps({"only": x, "os": "ubuntu-latest"})' \
| pyp 'json.dumps(list(map(json.loads, lines)))' > /tmp/matrix
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this change anything? I'd expect it to work the same as before.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No change, I was just messing with this logic to give me macOS wheel builds I could test (linked above)

cibuildwheel --print-build-identifiers --platform linux \
| pyp 'json.dumps({"only": x, "os": "ubuntu-latest"})'
} | pyp 'json.dumps(list(map(json.loads, lines)))' > /tmp/matrix
env:
CIBW_BUILD: "cp38-* cp311-*"
CIBW_ARCHS_LINUX: x86_64
Expand Down
2 changes: 2 additions & 0 deletions CHANGES.md
View file
Open in desktop
Expand Up @@ -38,6 +38,8 @@

<!-- Changes that improve Black's performance. -->

- Fix mypyc builds on arm64 on macOS (#4017)

### Output

<!-- Changes to Black's terminal output and error messages -->
Expand Down
13 changes: 12 additions & 1 deletion pyproject.toml
View file
Open in desktop
Expand Up @@ -113,6 +113,8 @@ exclude = ["/profiling"]
[tool.hatch.build.targets.wheel]
only-include = ["src"]
sources = ["src"]
# Note that we change the behaviour of this flag below
macos-max-compat = true

[tool.hatch.build.targets.wheel.hooks.mypyc]
enable-by-default = false
Expand Down Expand Up @@ -175,9 +177,18 @@ before-build = [
HATCH_BUILD_HOOKS_ENABLE = "1"
MYPYC_OPT_LEVEL = "3"
MYPYC_DEBUG_LEVEL = "0"
AIOHTTP_NO_EXTENSIONS = "1"

# Black needs Clang to compile successfully on Linux.
CC = "clang"
AIOHTTP_NO_EXTENSIONS = "1"

[tool.cibuildwheel.macos]
build-frontend = { name = "build", args = ["--no-isolation"] }
# Unfortunately, hatch doesn't respect MACOSX_DEPLOYMENT_TARGET
before-build = [
"python -m pip install 'hatchling==1.18.0' hatch-vcs hatch-fancy-pypi-readme 'hatch-mypyc>=0.16.0' 'mypy==1.5.1' 'click==8.1.3'",
Copy link
Collaborator

Choose a reason for hiding this comment

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

We'll have to remember to update this pin when we upgrade mypy.

Also, any specific reason we need to pin click and not our other dependencies here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We pinned click because 8.1.4 and 8.1.5 broke mypyc builds. I copied all the pins here from elsewhere in this file, with the exception of hatchling because we patch it

"""sed -i '' -e "600,700s/'10_16'/os.environ['MACOSX_DEPLOYMENT_TARGET'].replace('.', '_')/" $(python -c 'import hatchling.builders.wheel as h; print(h.__file__)') """,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the actual logic for this is https://github.com/pypa/wheel/blob/main/src/wheel/macosx_libfile.py#L392

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ofek I think the thing to upstream would be this monkeypatch.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We should have a way to set the sdk version in the tag to something based on MACOSX_DEPLOYMENT_TARGET, maybe this pseudocode:

sdk_ver = sdk_ver_from_narrowest_tag(next(sys_tags()))
if "MACOSX_DEPLOYMENT_TARGET" in os.environ:
    sdk_ver = tuple(map(int, os.environ["MACOSX_DEPLOYMENT_TARGET"].split(".")))
assert sdk_ver >= sysconfig.get_config_var("MACOSX_DEPLOYMENT_TARGET")
# then do equivalent of code from wheel^ to turn sdk_ver into a valid tag, can assert at the end that it is indeed in packaging's sys_tags

]

[tool.isort]
atomic = true
Expand Down