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
Fix arm wheels on macOS #4017
Fix arm wheels on macOS #4017
Conversation
604e1cf
to
406d096
Compare
406d096
to
b7cb820
Compare
for more information, see https://pre-commit.ci
Okay, this works. Here's a successful build: https://github.com/psf/black/actions/runs/6741962086?pr=4017 cc @ofek thoughts on upstreaming something that lets us accomplish the same thing as this patch? |
# 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'", | ||
"""sed -i '' -e "600,700s/'10_16'/os.environ['MACOSX_DEPLOYMENT_TARGET'].replace('.', '_')/" $(python -c 'import hatchling.builders.wheel as h; print(h.__file__)') """, |
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 actual logic for this is https://github.com/pypa/wheel/blob/main/src/wheel/macosx_libfile.py#L392
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.
@ofek I think the thing to upstream would be this monkeypatch.
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.
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
Sure, can you please explain what I should do? |
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.
Thanks, definitely an interesting solution.
cibuildwheel --print-build-identifiers --platform linux \ | ||
| pyp 'json.dumps({"only": x, "os": "ubuntu-latest"})' \ | ||
| pyp 'json.dumps(list(map(json.loads, lines)))' > /tmp/matrix | ||
{ |
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.
Does this change anything? I'd expect it to work the same as before.
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.
No change, I was just messing with this logic to give me macOS wheel builds I could test (linked above)
# 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'", | ||
"""sed -i '' -e "600,700s/'10_16'/os.environ['MACOSX_DEPLOYMENT_TARGET'].replace('.', '_')/" $(python -c 'import hatchling.builders.wheel as h; print(h.__file__)') """, |
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.
@ofek I think the thing to upstream would be this monkeypatch.
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'", |
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.
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?
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.
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
Fixes #3981
Don't hate me for this