feat: remove test frameworks in Frameworks and add device local references as rpath for real devices #780
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
KazuCocoa
changed the title
KazuCocoa
changed the title
KazuCocoa
commented
Sep 24, 2023
| @@ -2414,7 +2414,6 @@ | |||
| 641EE6C02240C5CA00173FCB /* XCUIApplication+FBHelpers.h in Headers */, | |||
| 641EE6C12240C5CA00173FCB /* _XCTestObservationCenterImplementation.h in Headers */, | |||
| 714EAA0E2673FDFE005C5B47 /* FBCapabilities.h in Headers */, | |||
| 716F0DA22A16CA1000CDD977 /* NSDictionary+FBUtf8SafeDictionary.h in Headers */, | |||
There was a problem hiding this comment.
it seems like Xcode generated this diff automatically...
There was a problem hiding this comment.
I noticed this file is not built in tvOS, so added it in tvOS as well
KazuCocoa
changed the title
KazuCocoa
changed the title
KazuCocoa
changed the title
|
CI is green right now |
| GCC_C_LANGUAGE_STANDARD = gnu11; | ||
| INFOPLIST_FILE = WebDriverAgentRunner/Info.plist; | ||
| LD_RUNPATH_SEARCH_PATHS = ( | ||
| "$(inherited)", | ||
| "@executable_path/Frameworks", | ||
| "@loader_path/Frameworks", | ||
| /System/Developer/Library/Frameworks, |
There was a problem hiding this comment.
What happens if these paths are not accessible?
There was a problem hiding this comment.
Nothing happens. I mean just do not search under this path in my current research
mykola-mokhnach
approved these changes
Sep 25, 2023
github-actions bot
pushed a commit
that referenced
this pull request
Sep 25, 2023
## [5.10.0](v5.9.1...v5.10.0) (2023-09-25) ### Features * remove test frameworks in Frameworks and add device local references as rpath for real devices ([#780](#780)) ([ae6c842](ae6c842))
|
🎉 This PR is included in version 5.10.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
it seems like packages built with Xcode 14.5 do not work on iOS 17 real devices as XCTest framework's internal changes. The app process crashed then. Xcode 15 build with embedded test frameworks, or below method with Xcode 14.5 works.
Then, once we remove test packages in
WebDriverAgentRunner-Runner.app/Frameworkswork. It seems like test packages refer to the system local's one then instead of the embedded ones. It makes sense to keep referencing the device local's XCTest framework for real devices. For tvOS, the removal was not necessary. It seems like we should add the device local references asrpath. iOS is not necessary in my testing, but lets add the same to reduce diffs.Thus, this pr:
Frameworksfor real devicesuse Xcode 15.0 in building packages<= no matched Xcode found error came by GH. will do something as another pr if neededI have tested iOS 14, 15, 16 and 17 and tvOS 16 with https://github.com/appium/WebDriverAgent/actions/runs/6291729735 , or locally built with Xcode 14.3.1 packages (as the same, they do not have XCTest frameworks' references), thus it should be ok...
Additional node:
Initially I thought #775 was necessary, but seems not for iPhone. Probably this is because the below option in Xcode could be related. The value is resolved as YES so let me explicitly set it as YES as well.
[Update]
It seems like the system references are necessary for TVs. I guess iOS take care about some references automatically probably for historical reasons? so maybe not necessary to add system references explicitly.