-
Notifications
You must be signed in to change notification settings - Fork 144
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
Update to Xcode 12.2 recommended settings #742
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Actually, decline most of the suggestions and just update the LastUpgradeCheck so that Xcode stops nagging us for updates. - Keep building "arm64e" architecture for iOS. Xcode 12 wants to set ARCHS -- the architectures to build for -- to just $(ARCHS_STANDARD). However, keep building the arm64e slice contrary to Xcode suggestions. arm64e is an architecture for Apple A12 chips which supports 'pointer authentication' [1]. Keep building Themis for this architecture to ensure that it is supported. This architecture is only supported on the real iOS devices, iOS Simulator does not support it (even on Apple Silicon). It is also not supported on macOS. [1]: https://developer.apple.com/documentation/security/preparing_your_app_to_work_with_pointer_authentication - Build for all architectures only in Release mode. Xcode 12.2 suggests enabling ONLY_ACTIVE_ARCH in Debug mode in order to improve build times. Release builds -- such as when archiving -- will still build all architectures. However, make sure that arm64 builds are disabled for iOS Simulator in test targets too, just like they are disabled in the framework. - Ignore warnings about quotes includes in framework headers. Set CLANG_WARN_QUOTED_INCLUDE_IN_FRAMEWORK_HEADER to NO. This new warning (enabled by default since Xcode 12) is known to break umbrella headers of frameworks. CocoaPods currently disables it [2]. We do this as well since all those #import "objcthemis.h" in "src/wrappers/themis/Obj-C/Themis/themis.h" are causing this warning. Apparently, Xcode wants this to be changed to this: #import <objcthemis.h> but then in fails due to a missing file because the framework is not in the search path for itself. We *do intend* to include a header file from the current directory -- the framework header directory. It's not clear what Xcode wants from us here, so just follow the CocoaPods guidance for now and disable the warning. This does not affect compilation and later use of the framework in apps. [2]: CocoaPods/CocoaPods#9902 - Keep iOS target at 10.0. Xcode 12 suggests to update the minimum supported iOS to 12.0, but we'll still support iOS 10 for this release so keep the old value. - Keep Swift 4 in tests for Swift 4. Do not upgrade SWIFT_VERSION in the targets which test Swift 4 compatibility specifically, in spite Xcode wanting to do this.
A lot of Xcode settings for targets are actually shared by all of them. Having common settings specified for individual targets makes it hard to update them as you can easily forget to update them everywhere. Let's move all shared settings to the project level so that they can be easily updated everywhere at once. Leave per-target overrides where necessary. - Build for iOS and iPadOS simultaneously. Keep TARGETED_DEVICE_FAMILY for iOS framework as "1,2" which marks both iOS and iPadOS as supported by default for all targets. - Use common versioning. Instead of iOS and macOS frameworks using separate versions, move the version variables to the project level: CURRENT_PROJECT_VERSION and MARKETING_VERSION. We also have no use for VERSION_INFO_PREFIX. Since we release iOS and macOS frameworks simultaneously, their versions should be updated simultaneously as well. - Use common warning settings. Move some of the additional compiler warnings to the project level so that they are enabled for all targets by default. This includes the "treat warnings as errors" policy which is now enabled for all targets. - Use common Swift compiler settings. All our targets use the same Swift compiler settings (except for Swift version). Move all of them to the project level, but keep the test targets using SWIFT_VERSION. - Use default code signing settings. Since we're building a framework here, it's not normally signed. The signature is applied only when archiving the complete app. Default settings use ad-hoc signing for integrity protection, but we should not hardcode development teams, etc. - Enable bitcode by default. Make sure that bitcode is enabled for all (iOS) targets by default. However, keep it disabled for tests. Test targets are built as bundles and they are not compatible with bitcode, causing ld errors: "-bundle and -bitcode_bundle (Xcode setting ENABLE_BITCODE=YES) cannot be used together". - Move miscellaneous settings to project level. Such as CLANG_ENABLE_MODULES. We don't really use Clang modules in our source code, but system headers use them and this somewhat speeds up compilation.
Remove the settings which we don't really need. - Ignore high-DPI images. Remove COMBINE_HIDPI_IMAGES as none of our frameworks contains any image resources whatsoever. Xcode's default is fine for us. - Ignore user paths. Remove ALWAYS_SEARCH_USER_PATHS as it's marked deprecated since Xcode 8 and may be removed. It has no effect on out builds. - Remove unused Metal settings. Older Xcodes insisted on including MTL_ENABLE_DEBUG_INFO and MTL_FAST_MATH into new projects, but Themis does not use Metal so we don't really need those settings.
Since now all warnings are treated as errors, those warnings in tests are no longer tolerated. The tests intentionally pass NULL values to parameters markes as "nonnull". Suppress the warnings here to test this invalid behavior.
ilammy
added
O-iOS 📱
Operating system: iOS
O-macOS 💻
Operating system: macOS
W-SwiftThemis 🔶
Wrapper: SwiftThemis, Swift API
refactoring
W-ObjCThemis 🎨
Wrapper: ObjCThemis, Objective-C API
M-Carthage
Package manager: Carthage, Objective-C and Swift, iOS and macOS
labels
Nov 4, 2020
vixentael
approved these changes
Nov 4, 2020
ONLY_ACTIVE_ARCH = YES; | ||
SUPPORTS_MACCATALYST = NO; | ||
TARGETED_DEVICE_FAMILY = 1; | ||
SWIFT_ACTIVE_COMPILATION_CONDITIONS = DEBUG; | ||
SWIFT_OPTIMIZATION_LEVEL = "-Onone"; |
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.
yeah
@@ -129,6 +129,10 @@ - (void)encryptDecryptWithPrivateKey:(NSData *)privateKeyData publicKey:(NSData | |||
XCTAssertNil(emptyMessageDe, @"decrypting data with empty message should return nil message"); | |||
|
|||
// empty keys | |||
// Key parameters here are marked with "nonnull" and normally issue warnings. | |||
// Suppress the warnings and verify that we don't crash if they are ignored. |
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.
👍
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Bitcode 🐙
Bitcode weirdness
M-Carthage
Package manager: Carthage, Objective-C and Swift, iOS and macOS
O-iOS 📱
Operating system: iOS
O-macOS 💻
Operating system: macOS
refactoring
W-ObjCThemis 🎨
Wrapper: ObjCThemis, Objective-C API
W-SwiftThemis 🔶
Wrapper: SwiftThemis, Swift API
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update Xcode project used by Carthage and for local testing to the settings recommended by Xcode 12.2. Also, clean up the settings to improve maintainability.
Update to Xcode 12.2 recommended settings
Actually, decline most of the suggestions and just update the LastUpgradeCheck so that Xcode stops nagging us for updates.
arm64e
architecture for iOS.Keep building
arm64e
architecture for iOS.Xcode 12 wants to set
ARCHS
– the architectures to build for – to just$(ARCHS_STANDARD)
. However, keep building the arm64e slice contrary to Xcode suggestions.arm64e is an architecture for Apple A12 chips which supports 'pointer authentication'. Keep building Themis for this architecture to ensure that it is supported.
This architecture is only supported on the real iOS devices, iOS Simulator does not support it (even on Apple Silicon). It is also not supported on macOS.
Build for all architectures only in Release mode.
Xcode 12.2 suggests enabling
ONLY_ACTIVE_ARCH
in Debug mode in order to improve build times. Release builds – such as when archiving – will still build all architectures.However, make sure that arm64 builds are disabled for iOS Simulator in test targets too, just like they are disabled in the framework.
Ignore warnings about quotes includes in framework headers.
Set
CLANG_WARN_QUOTED_INCLUDE_IN_FRAMEWORK_HEADER
to NO. This new warning (enabled by default since Xcode 12) is known to break umbrella headers of frameworks. CocoaPods currently disables it. We do this as well since all thosein
src/wrappers/themis/Obj-C/Themis/themis.h
are causing this warning. Apparently, Xcode wants this to be changed to this:but then in fails due to a missing file because the framework is not in the search path for itself. We do intend to include a header file from the current directory – the framework header directory. It's not clear what Xcode wants from us here, so just follow the CocoaPods guidance for now and disable the warning. This does not affect compilation and later use of the framework in apps.
Keep iOS target at 10.0.
Xcode 12 suggests to update the minimum supported iOS to 12.0, but we'll still support iOS 10 for this release so keep the old value.
Keep Swift 4 in tests for Swift 4.
Do not upgrade
SWIFT_VERSION
in the targets which test Swift 4 compatibility specifically, in spite Xcode wanting to do this.Move common settings to project level
A lot of Xcode settings for targets are actually shared by all of them. Having common settings specified for individual targets makes it hard to update them as you can easily forget to update them everywhere. Let's move all shared settings to the project level so that they can be easily updated everywhere at once. Leave per-target overrides where necessary.
Build for iOS and iPadOS simultaneously.
Keep
TARGETED_DEVICE_FAMILY
for iOS framework as1,2
which marks both iOS and iPadOS as supported by default for all targets.Use common versioning.
Instead of iOS and macOS frameworks using separate versions, move the version variables to the project level:
CURRENT_PROJECT_VERSION
andMARKETING_VERSION
. We also have no use forVERSION_INFO_PREFIX
.Since we release iOS and macOS frameworks simultaneously, their versions should be updated simultaneously as well.
Use common warning settings.
Move some of the additional compiler warnings to the project level so that they are enabled for all targets by default.
This includes the "treat warnings as errors" policy which is now enabled for all targets.
Use common Swift compiler settings.
All our targets use the same Swift compiler settings (except for Swift version). Move all of them to the project level, but keep the test targets using
SWIFT_VERSION
.Use default code signing settings.
Since we're building a framework here, it's not normally signed. The signature is applied only when archiving the complete app. Default settings use ad-hoc signing for integrity protection, but we should not hardcode development teams, etc.
Enable bitcode by default.
Make sure that bitcode is enabled for all (iOS) targets by default. However, keep it disabled for tests. Test targets are built as bundles and they are not compatible with bitcode, causing linker errors:
Move miscellaneous settings to project level.
Such as
CLANG_ENABLE_MODULES
. We don't really use Clang modules in our source code, but system headers use them and this somewhat speeds up compilation.Drop deprecated settings
Remove the settings which we don't really need.
Ignore high-DPI images.
Remove
COMBINE_HIDPI_IMAGES
as none of our frameworks contains any image resources whatsoever. Xcode's default is fine for us.Ignore user paths.
Remove
ALWAYS_SEARCH_USER_PATHS
as it's marked deprecated since Xcode 8 and may be removed. It has no effect on out builds.Remove unused Metal settings.
Older Xcodes insisted on including
MTL_ENABLE_DEBUG_INFO
andMTL_FAST_MATH
into new projects, but Themis does not use Metal so we don't really need those settings.Suppress non-null warnings in tests
Since now all warnings are treated as errors, those warnings in tests are no longer tolerated. The tests intentionally pass NULL values to parameters marked as
nonnull
. Suppress the warnings here to test this invalid behavior.Checklist
Example projects and code samples are up-to-date(already done in Update iOS/macOS examples to use Themis 0.13.4 #740)