Bug 188180

Summary: watchOS engineering build is broken after r234227
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: JavaScriptCoreAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, commit-queue, ews-watchlist, keith_miller, mark.lam, msaboff, saam, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Wenson Hsieh 2018-07-30 14:06:47 PDT 
In file included from /Volumes/main/Users/whsieh/Build/Release-watchos/DerivedSources/WebCore/unified-sources/UnifiedSource1-mm.mm:1:
In file included from ./Modules/plugins/QuickTimePluginReplacement.mm:55:
In file included from /Volumes/main/Users/whsieh/Build/Release-watchos/JavaScriptCore.framework/Headers/JavaScriptCore.h:35:
/Volumes/main/Users/whsieh/Build/Release-watchos/JavaScriptCore.framework/Headers/JSValue.h:50:189: error: 'JSC_IOS_VERSION_TBA' is not defined, evaluates to 0 [-Werror,-Wundef]
#if (defined(__MAC_OS_X_VERSION_MIN_REQUIRED) && __MAC_OS_X_VERSION_MIN_REQUIRED < JSC_MAC_VERSION_TBA) || (defined(__IPHONE_OS_VERSION_MIN_REQUIRED) && __IPHONE_OS_VERSION_MIN_REQUIRED < JSC_IOS_VERSION_TBA)
CompileC UnifiedSource1.o
CompileC UnifiedSource2-mm.o
CompileC UnifiedSource2.o
CompileC UnifiedSource3-mm.o
CompileC UnifiedSource3.o
CompileC UnifiedSource4-mm.o
CompileC UnifiedSource4.o
CompileC UnifiedSource5-mm.o
CompileC UnifiedSource5.o
CompileC UnifiedSource6-mm.o
In file included from /Volumes/main/Users/whsieh/Build/Release-watchos/DerivedSources/WebCore/unified-sources/UnifiedSource6-mm.mm:1:
In file included from ./bindings/js/ScriptControllerMac.mm:44:
In file included from /Volumes/main/Users/whsieh/Build/Release-watchos/JavaScriptCore.framework/PrivateHeaders/JSContextInternal.h:26:
In file included from /Volumes/main/Users/whsieh/Build/Release-watchos/JavaScriptCore.framework/Headers/JavaScriptCore.h:35:
/Volumes/main/Users/whsieh/Build/Release-watchos/JavaScriptCore.framework/Headers/JSValue.h:50:189: error: 'JSC_IOS_VERSION_TBA' is not defined, evaluates to 0 [-Werror,-Wundef]
#if (defined(__MAC_OS_X_VERSION_MIN_REQUIRED) && __MAC_OS_X_VERSION_MIN_REQUIRED < JSC_MAC_VERSION_TBA) || (defined(__IPHONE_OS_VERSION_MIN_REQUIRED) && __IPHONE_OS_VERSION_MIN_REQUIRED < JSC_IOS_VERSION_TBA)
1 error generated.
Comment 1 Wenson Hsieh 2018-07-30 14:33:59 PDT 
It seems that this snippet in postprocess-headers.sh:

    function rewrite_headers () {
        if [[ "${PLATFORM_NAME}" == "macosx" ]]; then
            [[ -n ${OSX_VERSION} ]] || OSX_VERSION=${MACOSX_DEPLOYMENT_TARGET}
            [[ -n ${IOS_VERSION} ]] || IOS_VERSION="NA"
            [[ -n ${OSX_VERSION_NUMBER} ]] || OSX_VERSION_NUMBER=${TARGET_MAC_OS_X_VERSION_MAJOR}
            [[ -n ${IOS_VERSION_NUMBER} ]] || IOS_VERSION_NUMBER="0"
        elif [[ "${PLATFORM_NAME}" =~ "iphone" ]]; then
            [[ -n ${IOS_VERSION} ]] || IOS_VERSION=${IPHONEOS_DEPLOYMENT_TARGET}
            [[ -n ${OSX_VERSION} ]] || OSX_VERSION="NA"
            [[ -n ${OSX_VERSION_NUMBER} ]] || OSX_VERSION_NUMBER="0"
            [[ -n ${IOS_VERSION_NUMBER} ]] || IOS_VERSION_NUMBER=${SDK_VERSION_MAJOR}
        fi

        …

    }

...doesn't take watchOS into account, since PLATFORM_NAME starts with "watch" rather than "iphone".
Comment 2 Wenson Hsieh 2018-07-30 14:53:45 PDT 
Hmm, I see now that this is modeled closely after rewrite_headers() in postprocess-framework-headers.sh in WebKit, and we also miss setting OSX_VERSION and IOS_VERSION there too, because WK_PLATFORM_NAME is "watch(os|simulator)".
Comment 3 Wenson Hsieh 2018-07-30 16:11:29 PDT 
(In reply to Wenson Hsieh from comment #2)
> Hmm, I see now that this is modeled closely after rewrite_headers() in
> postprocess-framework-headers.sh in WebKit, and we also miss setting
> OSX_VERSION and IOS_VERSION there too, because WK_PLATFORM_NAME is
> "watch(os|simulator)".

So in the case where this happens, we end up in a fallback path:

    if [[ -n "$OSX_VERSION" && -n "$IOS_VERSION" ]]; then
        SED_OPTIONS+=(
            -e s/JSC_MAC_TBA/${OSX_VERSION}/g
            -e s/JSC_IOS_TBA/${IOS_VERSION}/g
            -e s/JSC_MAC_VERSION_TBA/${OSX_VERSION_NUMBER}/g
            -e s/JSC_IOS_VERSION_TBA/${IOS_VERSION_NUMBER}/g
            -e s/JSC_API_AVAILABLE/API_AVAILABLE/
            -e s/JSC_API_DEPRECATED/API_DEPRECATED/
            -e "s/^JSC_CLASS_AVAILABLE/JSC_EXTERN API_AVAILABLE/"
            -e "s/^JSC_CLASS_DEPRECATED/JSC_EXTERN API_DEPRECATED/"
        )
    else
        SED_OPTIONS+=( -e 's/JSC_(API_|CLASS_)AVAILABLE\(.*\)\s*\)//g' -e 's/JSC_(API_|CLASS_)DEPRECATED(_WITH_REPLACEMENT)?\(.*\)\s*\)//g' )
    fi

...which effectively removes all uses of the JSC availability macro. Perhaps this should replace JSC_IOS_VERSION_TBA and JSC_MAC_VERSION_TBA as well with a fallback value of 0?
Comment 4 Wenson Hsieh 2018-07-30 16:34:33 PDT 
Created attachment 346122 [details]
Patch
Comment 5 Keith Miller 2018-07-30 16:54:46 PDT 
Comment on attachment 346122 [details]
Patch

r=me.
Comment 6 WebKit Commit Bot 2018-07-30 17:48:56 PDT 
Comment on attachment 346122 [details]
Patch

Clearing flags on attachment: 346122

Committed r234401: <https://trac.webkit.org/changeset/234401>
Comment 7 WebKit Commit Bot 2018-07-30 17:48:58 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2018-07-30 17:49:18 PDT 
<rdar://problem/42754208>