Summary: | run-safari --ios-simulator is no longer working with Xcode 9 | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Frédéric Wang (:fredw) <fred.wang> | ||||||
Component: | Tools / Tests | Assignee: | Frédéric Wang (:fredw) <fred.wang> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ap, dbates, dean_johnson, fred.wang, jbedard, lforschler, mitz, simon.fraser, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=199852 | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 178203 | ||||||||
Attachments: |
|
Description
Frédéric Wang (:fredw)
2017-09-27 23:54:18 PDT
> /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator11.0.sdk/Applications/MobileSafari.app.
It's now here: /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/Library/CoreSimulator/Profiles/Runtimes/iOS.simruntime/Contents/Resources/RuntimeRoot/Applications/MobileSafari.app
Given that we no longer support older OS versions, the expedient fix will be to change the hardcoded path. Frédéric, would you be willing to post a patch after confirming that this works?
(In reply to Alexey Proskuryakov from comment #1) > It's now here: > /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/ > Developer/Library/CoreSimulator/Profiles/Runtimes/iOS.simruntime/Contents/ > Resources/RuntimeRoot/Applications/MobileSafari.app > > Given that we no longer support older OS versions, the expedient fix will be > to change the hardcoded path. Frédéric, would you be willing to post a patch > after confirming that this works? I would be happy to do so. However, as I said in my first comment after changing the hardcoded path I got a different message: "This app was unable to be installed.". So maybe more is needed. I won't be able to try again in upcoming days. Sorry, I didn't realize that you changed it to the same path (just misread the bug description). I wonder why we are trying to install MobileSafari in the first place. Should we even be using the SDK to determine the app bundle ID? It seems like it would be better to just launch using com.apple.MobileSafari directly instead of going through the SDK to compute the bundle ID. Also, given Frédéric's comment, it seems like that code-path attempts to install the app as well. Which it shouldn't since, it's a system installed app...that would likely mean that the path returned by iosSimulatorApplicationsPath(...) is not correct. (In reply to Frédéric Wang (:fredw) from comment #2) > I would be happy to do so. However, as I said in my first comment after > changing the hardcoded path I got a different message: "This app was unable > to be installed.". So maybe more is needed. I won't be able to try again in > upcoming days. (In reply to Jonathan Bedard from comment #4) > Also, given Frédéric's comment, it seems like that code-path attempts to > install the app as well. Which it shouldn't since, it's a system installed > app...that would likely mean that the path returned by > iosSimulatorApplicationsPath(...) is not correct. Hi. I just went back to Paris and had time to check again. Indeed, hardcoding the new path in iosSimulatorApplicationsPath rather than runIOSWebKitAppInSimulator does allow to launch Safari again, thanks. I'll try and upload a patch. Created attachment 323303 [details]
Patch
Comment on attachment 323303 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323303&action=review Minor nit-pick, generally, looks good. (Note that I'm not a reviewer) > Tools/Scripts/webkitdirs.pm:2315 > + my $iphoneOSPlatformPath = `xcrun --sdk iphoneos --show-sdk-platform-path`; > + die 'Failed to get iphoneOS SDK platform path from xcrun' if $?; > + chomp $iphoneOSPlatformPath; Take a look at the code in Tools/Scripts/configure-xcode-for-ios-development, line 223. That does this same sort of thing a little bit more concisely. Credit to Dean Johnson for pointing this out: could we retain compatibility with Xcode 7/8? This code won't work for Xcode 8 (I just checked) and there are a few tools which use this code-path, we can't guarantee we won't run with old Xcode versions. webkitdirs.pm has a function, determineXcodeVersion(), which should do the trick. @Jonathan: Thanks for the feedback, I'll upload a new version tomorrow. What are the situations where you expect having Xcode 7/8? I think that only Xcode 9 works with iOS 11 simulator, and we only support building for iOS 11. Created attachment 323398 [details]
Patch
I added Jonathan's suggestion to check Xcode version. If that's not needed, it is straightforward to remove the corresponding code.
Unofficial r+ on attachment 323398 [details].
It seems reasonable to not deliberately break Xcode <9.0, even if we don't currently support iOS WebKit on Xcode <9.0, although, I will note that after looking through how we use this code path, breaking Xcode <9.0 shouldn't have any effect on CI infrastructure.
Jonathan and Dean, have you come to the agreement that the check is needed after all? Comment on attachment 323398 [details]
Patch
If we do not support Xcode 9 for iOS WebKit development/we no longer have any Xcode 9 iOS simulator builders on build.webkit.org/no longer have any Xcode 9 iOS Simulator EWS bots then we should remove the conditional code in webkitdirs::iosSimulatorApplicationsPath().
Comment on attachment 323398 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323398&action=review > Tools/ChangeLog:8 > + In XCode 9, the path of the simulator application is now in a new CoreSimulator subdirectory XCode => Xcode (Please fix all occurrences throughout this change log) > Tools/ChangeLog:13 > + * Scripts/configure-xcode-for-ios-development: Move sdkDirectory and sdkPlatformDirectory sdkDirectory => sdkDirectory() sdkPlatformDirectory => sdkPlatformDirectory() (As they are functions, please fix all occurrences throughout this change log) > Tools/ChangeLog:20 > + (XcodeSDKPath): Rely on sdkDirectory to implement this function. Ditto. > Tools/ChangeLog:22 > + by relying on sdkPlatformDirectory. Ditto. > Tools/Scripts/webkitdirs.pm:85 > &iosVersion I know you did not add this line. I suggest moving this line such that this export list is in sorted order according to the hr UNIX sort command. Committed r223234: <https://trac.webkit.org/changeset/223234> |