Bug 173937 – Update configure-xcode-for-ios-development for iOS 11

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Jonathan Bedard]

Some headers have been moved, renamed or removed in iOS 11.  Update configure-xcode-for-ios-development to handle the iOS 11 SDK.

[Radar WebKit Bug Importer]

<rdar://problem/33122072>

[Jonathan Bedard]

Created attachment 319000 [details]
Patch

[Daniel Bates]

Comment on attachment 319000 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=319000&action=review

> Tools/Scripts/configure-xcode-for-ios-development:91
> +    my $macSDKDirectory = sdkDirectory("macosx");

How does this work if a person has more than one Mac SDK installed?

> Tools/Scripts/configure-xcode-for-ios-development:96
> +        my $macSDKPah = File::Spec->canonpath(File::Spec->catfile($macSDKDirectory, $header));

macSDKPah => macSDKPath
(Please fix throughout this patch.)

> Tools/Scripts/configure-xcode-for-ios-development:107
> +            system("/usr/bin/ditto", $macSDKPah, $iphonesimulatorSDKPath);

Eww, duplicate code. We should factor out the code to call ditto, die and print success to a shared function. I don't think the combined success message in line 111 is worth duplicating code.

[Jonathan Bedard]

Comment on attachment 319000 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=319000&action=review

>> Tools/Scripts/configure-xcode-for-ios-development:91
>> +    my $macSDKDirectory = sdkDirectory("macosx");
> 
> How does this work if a person has more than one Mac SDK installed?

It will use whichever SDK is returned from `xcrun --sdk macosx --show-sdk-path`.  Which seems like the correct approach.  This is the same way we generally look for the current SDK in WebKit tools.

[Jonathan Bedard]

Created attachment 319016 [details]
Patch

[Daniel Bates]

Comment on attachment 319000 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=319000&action=review

>> Tools/Scripts/configure-xcode-for-ios-development:107
>> +            system("/usr/bin/ditto", $macSDKPah, $iphonesimulatorSDKPath);
> 
> Eww, duplicate code. We should factor out the code to call ditto, die and print success to a shared function. I don't think the combined success message in line 111 is worth duplicating code.

Alternatively, we could making this function abstract: move @missingHeaders out of this function, have this function take a source SDK and destination SDK and call this function copyMissingHeadersFromSDKToSDK(). Then define a function called copyMissingHeadersToIPhoneOSSDKIfNeeded() and define a function called copyMissingHeadersToIOSSimulatorSDKIfNeeded() that turns around and calls copyMissingHeadersFromSDKToSDK("iphonesimulator", "iphoneos") and copyMissingHeadersFromSDKToSDK("macosx", "iphonesimulator"), respectively. Then this script would call copyMissingHeadersToIOSSimulatorSDKIfNeeded() first and then call copyMissingHeadersToIPhoneOSSDKIfNeeded(). The benefit of this approach is that we avoid duplication, unnecessary branches, and the names of the functions clearly describe their purpose.

[Daniel Bates]

Comment on attachment 319016 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=319016&action=review

> Tools/Scripts/configure-xcode-for-ios-development:115
> +            print "Successfully copied $macSDKPath to $iphonesimulatorSDKPath and $iphoneosSDKPath.\n";

You seem to have disregarded the last sentence of my remark in comment #3 without explanation.I don't think the combined success message here is worth much. Regardless, I suggest we take the approach I outlined in comment #4.

[Jonathan Bedard]

Comment on attachment 319016 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=319016&action=review

>> Tools/Scripts/configure-xcode-for-ios-development:115
>> +            print "Successfully copied $macSDKPath to $iphonesimulatorSDKPath and $iphoneosSDKPath.\n";
> 
> You seem to have disregarded the last sentence of my remark in comment #3 without explanation.I don't think the combined success message here is worth much. Regardless, I suggest we take the approach I outlined in comment #4.

I isolated the duplicated code and placed it in a function.  I would be open to moving the print statements to the function as well, but I don't feel like the print statements are really duplicated code.

As for the approach you suggest in comment #4, I think you may be suggesting some functions we don't need, but I like the general idea.  I'll update this bug shortly.

[Jonathan Bedard]

Created attachment 319018 [details]
Patch

[Daniel Bates]

Comment on attachment 319018 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=319018&action=review

> Tools/ChangeLog:9
> +        Xcode 9 has removed some headers required by WebKit builds for testing.

If it is straightforward to do so it would be great to list the headers that were removed and what SDK they were removed from. We should also explain what we are doing to fix this issue: we are copying the missing headers from the active macOS SDK.

> Tools/Scripts/configure-xcode-for-ios-development:71
> +copyMissingHeadersFromSDKToSDKIfNeeded("macosx", "iphonesimulator");

Out of curiosity what headers would actually be copied (i.e. what headers are missing from the iphonesimulator 11 SDK)?

[Jonathan Bedard]

(In reply to Daniel Bates from comment #10)
> Comment on attachment 319018 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=319018&action=review
> 
> > Tools/ChangeLog:9
> > +        Xcode 9 has removed some headers required by WebKit builds for testing.
> 
> If it is straightforward to do so it would be great to list the headers that
> were removed and what SDK they were removed from. We should also explain
> what we are doing to fix this issue: we are copying the missing headers from
> the active macOS SDK.
> 
> > Tools/Scripts/configure-xcode-for-ios-development:71
> > +copyMissingHeadersFromSDKToSDKIfNeeded("macosx", "iphonesimulator");
> 
> Out of curiosity what headers would actually be copied (i.e. what headers
> are missing from the iphonesimulator 11 SDK)?

I'll include this information in the changelog as well, the following headers were removed from the iOS 11 Simulator SDK:

crt_externs.h
mach_types.defs
machine_types.def
std_types.def
objc-class.h
objc-runtime.h
Protocol.h
history.h
readline.h

[Jonathan Bedard]

Created attachment 319028 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 319028 [details]
Patch for landing

Clearing flags on attachment: 319028

Committed r221168: <http://trac.webkit.org/changeset/221168>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.