Bug 177595

Summary: run-safari --ios-simulator is no longer working with Xcode 9
Product: WebKit Reporter: Frédéric Wang (:fredw) <fred.wang>
Component: Tools / TestsAssignee: 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 Flags
Patch
none
Patch dbates: review+

Description Frédéric Wang (:fredw) 2017-09-27 23:54:18 PDT
I've recently upgraded XCode to version 9 (released 2017/09/19) and rebuilt Safari but when I try running Safari on the simulator, I get the following errors:

    Tools/Scripts/build-safari --ios-simulator --debug
    Tools/Scripts/run-safari --ios-simulator
    Quitting and launching iOS Simulator...
    Installing
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator11.0.sdk/Applications/MobileSafari.app.
    An error was encountered processing the command
(domain=NSPOSIXErrorDomain, code=2):
    Failed to install the requested application
    An application bundle was not found at the provided path.
    Provide a valid path to the desired application bundle.
    Died at /Users/fred/WebKit/Tools/Scripts/webkitdirs.pm line 2546.

 can not find any MobileSafari.app in my WebKitBuild directory nor in
the /Applications/ directory calculated by webkitdirs.pm ; trying to
hardcode a different path in webkitdirs.pm I instead obtain:

    Quitting and launching iOS Simulator...
    Installing
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/Library/CoreSimulator/Profiles/Runtimes/iOS.simruntime/Contents/Resources/RuntimeRoot/Applications/MobileSafari.app.
    An error was encountered processing the command
(domain=IXUserPresentableErrorDomain, code=2):
    This app was unable to be installed.
    Died at /Users/fred/WebKit/Tools/Scripts/webkitdirs.pm line 2546.

Note that run-webkit-tests --ios-simulator works as expected.
Comment 1 Alexey Proskuryakov 2017-09-28 09:15:24 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?
Comment 2 Frédéric Wang (:fredw) 2017-09-28 09:27:29 PDT
(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.
Comment 3 Alexey Proskuryakov 2017-09-28 09:34:58 PDT
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.
Comment 4 Jonathan Bedard 2017-09-28 09:36:49 PDT
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.
Comment 5 Frédéric Wang (:fredw) 2017-10-10 06:12:13 PDT
(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.
Comment 6 Frédéric Wang (:fredw) 2017-10-10 06:49:29 PDT
Created attachment 323303 [details]
Patch
Comment 7 Jonathan Bedard 2017-10-10 13:18:27 PDT
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.
Comment 8 Jonathan Bedard 2017-10-10 13:55:17 PDT
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.
Comment 9 Frédéric Wang (:fredw) 2017-10-10 13:59:08 PDT
@Jonathan: Thanks for the feedback, I'll upload a new version tomorrow.
Comment 10 Alexey Proskuryakov 2017-10-10 13:59:16 PDT
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.
Comment 11 Frédéric Wang (:fredw) 2017-10-11 02:09:08 PDT
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.
Comment 12 Jonathan Bedard 2017-10-11 08:43:04 PDT
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.
Comment 13 Alexey Proskuryakov 2017-10-11 12:11:41 PDT
Jonathan and Dean, have you come to the agreement that the check is needed after all?
Comment 14 Daniel Bates 2017-10-11 22:37:14 PDT
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 15 Daniel Bates 2017-10-11 22:43:05 PDT
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.
Comment 16 Frédéric Wang (:fredw) 2017-10-12 01:37:58 PDT
Committed r223234: <https://trac.webkit.org/changeset/223234>
Comment 17 Radar WebKit Bug Importer 2017-10-12 01:38:48 PDT
<rdar://problem/34951701>