ruslo / polly

:wrench: Collection of CMake toolchain files and scripts for cross-platform build and CI testing (GCC, Visual Studio, iOS, Android, Clang analyzer, sanitizers etc.)
BSD 2-Clause "Simplified" License
862 stars 191 forks source link

Fix Xcode 11+ nocodesign #335

Closed Einh06 closed 4 years ago

Einh06 commented 4 years ago

From Xcode 11+ onward, Apple enforces by default Code Signing. This prevents cmake to perform the compiler check. To work around that we force disable code signing.

All credit goes to @jdonald: https://github.com/rrbpalm/polly/commit/f9bbec83a11b283cbfcdaf36dbf55a688362f817

ruslo commented 4 years ago

From Xcode 11+ onward, Apple enforces by default Code Signing. This prevents cmake to perform the compiler check. To work around that we force disable code signing.

What problem exactly you're experiencing? iOS toolchain with Xcode 11 is part of the CI testing and currently is working fine, e.g. ios-nocodesign-13-0-dep-9-3-arm64 :

If you can reproduce it on Travis, that will be the best since I can extend the testing matrix.

jdonald commented 4 years ago

The CMAKE_TRY_COMPILE error is typically:

    Check dependencies
    Code Signing Error: Signing for "cmTC_a9590" requires a development team. Select a development team in the Signing & Capabilities editor.

Back when I did rrbpalm/polly@f9bbec8 on October 2nd, Travis didn't have any Xcode 11.x coverage. It does today though, so it surprised me to hear mention of Travis still passing without this fix.

Today I've investigated further with Xcode 11.3.1. I reproduced both the original error in my setup and the passing one similar to what Travis shows. I've narrowed it down to a difference betweenios-nocodesign-13-0-dep-9-3-arm64.cmake vs the config used by my earlier team and apparently SoundCloud here.

The 13-0-dep-9-3 one has the line:

include("${CMAKE_CURRENT_LIST_DIR}/flags/ios_nocodesign.cmake")

and in 9f78e12 on October 4th, flags/ios_nocodesign.cmake started setting CMAKE_XCODE_ATTRIBUTE_CODE_SIGN_IDENTITY to an empty string, an alternate way to deal with the signing error.

On the tip of master, I count 26 other ios-nocodesign.cmake toolchain files that directly include flags/ios_nocodesign.cmake and should work with Xcode 11.x, but that leaves for 44 remaining ios-nocodesign.cmake configs broken, including ios-nocodesign-arm64.cmake which my project was based on. They just aren't included in Travis's tests yet.

ruslo commented 4 years ago

I've narrowed it down to a difference betweenios-nocodesign-13-0-dep-9-3-arm64.cmake vs the config used by my earlier team and apparently SoundCloud here.

Okay, I see, thanks for the summary.

that leaves for 44 remaining ios-nocodesign*.cmake configs broken, including ios-nocodesign-arm64.cmake which my project was based on. They just aren't included in Travis's tests yet.

Pull request with the fix is welcome, no need to update everything. If you want, you can make fixes only for the toolchains you're interested in.

jdonald commented 4 years ago

Rather than selectively edit some toolchain files, wouldn't it be simpler--and arguably more correct--to accept the one-line fix to NoCodeSign.xcconfig in this pull request?

ruslo commented 4 years ago

and arguably more correct

Thanks to your analysis, we know that the problem is about not using ios_nocodesign.cmake in some places. The solution is to do include(ios_nocodesign) everywhere. From this point of view, the patch definitely falls into the "quick hack" category. Despite looking trivial, the iOS nocodesign part already caused the troubles in the past, and it's better to avoid adding unnecessary complexity.

ruslo commented 4 years ago

Closing inactive pull request.