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

Fix build with XCode 10 #12

Closed
wants to merge 2 commits into from
Closed

Fix build with XCode 10 #12

wants to merge 2 commits into from

Conversation

@Roger-luo
Copy link

Roger-luo commented Oct 24, 2018

I was wondering whether this breaks the build on other platforms?

(That was why I didn't open a PR for this)

@iamed2
Copy link
Author

iamed2 commented Oct 24, 2018

I don't know if it's right either but I need it and Keno said it seemed fine on Slack

@ararslan
Copy link
Member

I was wondering whether this breaks the build on other platforms?

It shouldn't, since this repository is specific to macOS. On other platforms we use libunwind.

@ararslan
Copy link
Member

Unless you mean other macOS/Xcode versions. It would likely be worthwhile to condition on the version being used, since this wasn't a problem until recently. You should be able to use Apple's C macros to get versions.

@iamed2
Copy link
Author

iamed2 commented Oct 24, 2018

XCode 8+ will support this solution for sure, so we could condition on that, but I'm not sure what the macros are to check those things

@ararslan
Copy link
Member

I think this should do it.

#include <AvailabilityMacros.h>

#if MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_12
// stuff using -stdlib=libstdc++
#else
// stuff using -lstdc++
#endif

@iamed2
Copy link
Author

iamed2 commented Oct 24, 2018

It doesn't actually depend on your version of macOS though, it depends on your version of the XCode command line tools, which aren't directly tied.

@ararslan
Copy link
Member

Each version of Xcode CLT has a minimum and maximum macOS version it supports, so it seems roughly safe to condition on the macOS version if we set it recent enough, even though it isn't a perfect check.

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.

3 participants