-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
add support for CMAKE_OSX_ARCHITECTURES when detecting SIZEOF variables #3945
add support for CMAKE_OSX_ARCHITECTURES when detecting SIZEOF variables #3945
Conversation
I don't get how my changes could reduce the coverage by 8%. And the other error seems to be a timeout in the CI. |
@JonasVautherin no, you're right. The coveralls service acts up like this every now and then, so in this particular case we'll just ignore it. And the cirrus-ci freebsd test sometimes times out and that's another one to ignore... |
@JonasVautherin pardon me... But why |
@JonasVautherin After some investigation, I've discovered your point. |
@snikulov surprised me also but CheckTypeSize documentation makes it clear now. Nevertheless, it feels pretty disconnected but I don't know if there's a way to make it easier to read. |
@jzakrzewski perhaps @bradking could shed some light on this. |
Judging from the documentation I''d say it is correct. But there's this "Where the heck is that coming from?" moment. I guess for an expert it may be obvious. I haven't written (yet) enough CMake to know it from the first glance. |
Maybe an added comment with the link to the documentation for it is enough? |
I updated with a description before the |
For me personally that'd be enough. |
LGTM. |
Thanks! |
When cross-compiling for iOS, toolchains set
CMAKE_OSX_ARCHITECTURES
. As per the doc:In this case, e.g. ${SIZEOF_LONG} gets set to "0", and therefore
#cmakedefine SIZEOF_LONG ${SIZEOF_LONG}
becomes "/* #undef SIZEOF_LONG */" incurl_config.h
, resulting in a broken build.However, and still described in the docs, when ${SIZEOF_LONG} is "0", CMake also defines other values, including ${SIZEOF_LONG_CODE}, which gives, e.g. for
CMAKE_OSX_ARCHITECTURES=armv7;armv7s;arm64
:Instead of using
#cmakedefine
and${SIZEOF_LONG}
, I believe it always works to use${SIZEOF_LONG_CODE}
directly, while enabling support for multiple architectures inCMAKE_OSX_ARCHITECTURES
.It is working for me on Linux, and on macos when cross-compiling for iOS (
armv7;armv7s;arm64
).