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

CMake, scikit-build, and Apple Silicon #582

Closed
YannickJadoul opened this issue Feb 7, 2021 · 6 comments
Closed

CMake, scikit-build, and Apple Silicon #582

YannickJadoul opened this issue Feb 7, 2021 · 6 comments

Comments

@YannickJadoul
Copy link
Member

YannickJadoul commented Feb 7, 2021

After a few hours of wasted time learning opportunities, I managed to strangle some arm64 and universal2 wheels out of my setup. But this was less straightforward than hoped for.

Thing I had to change:

  • Added MACOSX_DEPLOYMENT_TARGET to CIBW_ENVIRONMENT. Could be fixed by a solution to set MACOSX_DEPLOYMENT_TARGET for all stages? #563.
  • Added CMAKE_OSX_ARCHITECTURES to CIBW_ENVIRONMENT. It is kind of unfortunate that CMake doesn't seem to pick up on ARCHFLAGS (or maybe it would, but it gets overridden by CMAKE_OSX_ARCHITECTURES passed by scikit-learn). We could of course set this environment variable (and I'd be happy with that), but this is also tricky to add something very build-system specific?
  • Teach scikit-build about the universal2 <-> CMAKE_OSX_ARCHITECTURES=x86_64;arm64 link, and teach it to respect _PYTHON_HOST_PLATFORM if that's already set (so, yeah, scikit-build is to blame here, @joerick): YannickJadoul/scikit-build@vs2019...YannickJadoul:vs2019-apple-silicon

@henryiii, how do you judge about the scikit-build changes? They seem reasonably ad-hoc and very brittle, so not I'm wondering what else I missed. Also, worth a PR on scikit-build, according to you?

And what should we do in cibuildwheel? Just some docs?

Main commit with the changes: YannickJadoul/Parselmouth@f3b61e0

See discussion on #579 (comment) thread.

@henryiii
Copy link
Contributor

henryiii commented Feb 7, 2021

I'm rather thinking (though I haven't been thinking about it too heavily) that CIBW_ENVIRONMENT should likely affect the entire CIBW process, rather than just the build phase. At least the BEFORE_ALL part; adding it to TEST could cause issues.

I wouldn't add anything specific to CMake, I'd just note it in the docs (and it should be fixed in scikit-build; ideally scikit-build should read and use everything from setuptools).

I'll look at the changes soon. scikit-build is going to need some work I expect.

@YannickJadoul
Copy link
Member Author

YannickJadoul commented Feb 7, 2021

I'm rather thinking (though I haven't been thinking about it too heavily) that CIBW_ENVIRONMENT should likely affect the entire CIBW process, rather than just the build phase.

CIBW_ENVIRONMENT is already happening, even for CIBW_BEFORE_ALL. It just so happens that MACOSX_DEPLOYMENT_TARGET gets added separately, but not for CIBW_BEFORE_ALL (though that would make sense to me, I think).

I wouldn't add anything specific to CMake, I'd just note it in the docs (and it should be fixed in scikit-build; ideally scikit-build should read and use everything from setuptools).

Not the easiest solution for me, but I agree that that's the better principle, yes :-)

@henryiii
Copy link
Contributor

I think I'd make that PR to scikit-build, wouldn't hurt, and needs to start somewhere. :)

@YannickJadoul
Copy link
Member Author

Good point! I'd almost forgotten. Done at scikit-build/scikit-build#530.

@henryiii
Copy link
Contributor

henryiii commented Aug 18, 2021

I think this is closable now that Scikit-Build 0.12 is out. Everything should work similarly to setuptools now.

@joerick
Copy link
Contributor

joerick commented Aug 18, 2021

Agreed!

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

No branches or pull requests

3 participants