Bug 140702 – [iOS] build-webkit fails to build jsc command-line tool with external SDK

• Component: Tools / Tests
• Status: CLOSED DUPLICATE

[David Kilzer (:ddkilzer)]

The libreadline.dylib library is not included in the external iOS SDK (for either iphoneos or iphonesimucator SDKs), so we need to set an explicit target when building JavaScriptCore with build-webkit so that the command-line tools are not built.

This is another fix needed to bring up the ios-ews queue.

[David Kilzer (:ddkilzer)]

Created attachment 245034 [details]
Patch v1

[Daniel Bates]

Comment on attachment 245034 [details]
Patch v1

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

r=me

> Tools/Scripts/build-webkit:260
> +            # Can't build jsc command-line tool with external SDK.
> +            push @local_options, qw(-target JavaScriptCore) if ($project eq "JavaScriptCore") && !hasIPhoneInternalSDK() && !$clean;
> +            # iOS has a separate target than OS X for WebKitTestRunner.
>              push @local_options, qw(-target WebKitTestRunnerApp) if ($project eq "WebKitTestRunner") && !$clean;

Notice that we check for !$clean on both line 258 and line 260. I suggest we move the !$check for the individual source code lines to the if-statement condition for the block (line 256) to avoid duplicate checks of the same variable. If you feel that it improves the readable of the code to explicitly check !$clean on line 258 and line 260 then on I suggest swapping the order of the conjuncts !hasIPhoneInternalSDK() and !$clean on line 258 such that we first check !$clean before evaluating hasIPhoneInternalSDK() (since it's more computationally expensive given that it runs an external program and waits for its exit status).

[David Kilzer (:ddkilzer)]

Comment on attachment 245034 [details]
Patch v1

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

> Tools/Scripts/build-webkit:258
> +            push @local_options, qw(-target JavaScriptCore) if ($project eq "JavaScriptCore") && !hasIPhoneInternalSDK() && !$clean;

Note to self:  Need to change the test for hasIPhoneInternalSDK() to check if the current SDK is an internal or external SDK.

This is tricky because iphoneos and iphoneos.internal are separate SDKs, but iphonesimulator is a single SDK with internal bits overlaid when iphoneos.internal is installed.

[David Kilzer (:ddkilzer)]

Actually, I think the better fix may be to only define HAVE_READLINE in Source/WTF/wtf/Platform.h if USE(APPLE_INTERNAL_SDK) is defined.

[David Kilzer (:ddkilzer)]

Comment on attachment 245034 [details]
Patch v1

Here's the build error when building for "iphoneos" SDK:

Source/JavaScriptCore/jsc.cpp:68:10: fatal error: 'readline/history.h' file not found
#include <readline/history.h>
         ^
1 error generated.

So the headers are missing from the iphoneos SDK, but they are present in the iphonesimulator SDK.  Strangely, jsc does not actually link to the libreadline.dylib library, and the jsc built from the external SDK still works fine.

So this may be as simple as copying the missing readline headers into the iphoneos SDK, which would be covered by a change to the patch for Bug 140704.

[David Kilzer (:ddkilzer)]

(In reply to comment #5)
> So this may be as simple as copying the missing readline headers into the
> iphoneos SDK, which would be covered by a change to the patch for Bug 140704.

Yep.  I'm going to dupe this to Bug 140704.

*** This bug has been marked as a duplicate of bug 140704 ***

[David Kilzer (:ddkilzer)]

Reopening to attach new patch.

[David Kilzer (:ddkilzer)]

Created attachment 245055 [details]
Patch v2 (for posterity)

[David Kilzer (:ddkilzer)]

*** This bug has been marked as a duplicate of bug 140704 ***