-
Notifications
You must be signed in to change notification settings - Fork 2
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 of iOS simulator libraries. #58
base: main
Are you sure you want to change the base?
Conversation
ios-arm64: | ||
GOOS=ios \ | ||
GOARCH=arm64 \ | ||
CLANGARCH=arm64 \ |
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.
What is this variable for?
TARGET=arm64-apple-ios16 \ | ||
SDK=iphoneos \ | ||
SDK_PATH=$(IOS_SDK_PATH) \ |
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.
What is this variable for? If I understand what is going on, it supposed to be redefined in clangwrap.sh. I don't see how this value used.
lipo $(IOS_OUT)/libpolygonid-ios.a $(IOS_OUT)/libpolygonid-ios-simulator-x86_64.a -create -output $(IOS_OUT)/libpolygonid-sim.a | ||
cp $(IOS_OUT)/libpolygonid-ios.h $(IOS_OUT)/libpolygonid.h | ||
|
||
ios-device: ios-arm64 |
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.
I can't understand the purpose of this target. ios-arm64
already includes the line cp $(IOS_OUT)/libpolygonid-ios.h $(IOS_OUT)/libpolygonid.h
. What ios-device
should do that ios-arm64
doesn't?
Makefile
Outdated
@@ -43,14 +54,17 @@ darwin-arm64: | |||
|
|||
# Build a legacy multi-architecture version of libpolygonid.a with iOS Device arm64 & iOS Simulator x86_64 | |||
ios-old: ios-arm64 ios-simulator-x86_64 | |||
lipo $(IOS_OUT)/libpolygonid-ios.a $(IOS_OUT)/libpolygonid-ios-simulator-x86_64.a -create -output $(IOS_OUT)/libpolygonid.a | |||
lipo $(IOS_OUT)/libpolygonid-ios.a $(IOS_OUT)/libpolygonid-ios-simulator-x86_64.a -create -output $(IOS_OUT)/libpolygonid-sim.a |
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 target ios-old
is supposed to maintain backward compatibility and create 'legacy' library libpolygonid.a
. You have renamed it, so we have broken the backward compatibility. If we agree to do this, then why do we need the ios-old
target at all?
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.
I think this was a mistake. I'll roll it back.
cp $(IOS_OUT)/libpolygonid-ios.h $(IOS_OUT)/libpolygonid.h | ||
|
||
ios-simulator: ios-simulator-x86_64 ios-simulator-arm64 | ||
lipo $(IOS_OUT)/libpolygonid-ios-simulator-x86_64.a $(IOS_OUT)/libpolygonid-ios-simulator-arm64.a -create -output $(IOS_OUT)/libpolygonid-ios-simulator.a | ||
cp $(IOS_OUT)/libpolygonid-ios-simulator-arm64.h $(IOS_OUT)/libpolygonid.h | ||
|
||
ios: ios-old ios-arm64 ios-simulator | ||
ios: ios-device ios-simulator |
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.
It was expected that by calling the ios
target, we would get the 'legacy' library libpolygonid.a
. Now we do not. Is this the desired behavior, and does everyone (including @demonsh) agree?
Some issues with the linker when I was trying to use this with native iOS