Bug 190121 - [WTF] r236617 broke JSCOnly build on Windows
Summary: [WTF] r236617 broke JSCOnly build on Windows
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks: 189947
  Show dependency treegraph
 
Reported: 2018-10-01 02:46 PDT by Koby
Modified: 2018-10-01 05:34 PDT (History)
4 users (show)

See Also:


Attachments
Patch (1.47 KB, patch)
2018-10-01 04:46 PDT, Koby
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Koby 2018-10-01 02:46:44 PDT
Hi,
After updating my copy of WebKit, the Windows build of JSCOnly port fails with the following error:
c:\togithub\webkit-original\webkit\source\wtf\wtf\generic\mainthreadgeneric.cpp(33): fatal error C1083: Cannot open include file: 'pthread.h': No such file or directory [C
:\togithub\WebKit-Original\webkit\WebKitBuild\Release\Source\WTF\wtf\WTF.vcxproj]

Looking at the file, it seems that since r236617 MainThreadGeneric.cpp always includes pthread.h, which is a problem since it isn't available on Windows (on the JSCOnly port uses MainThreadGeneric.cpp on all platforms).

What do you think? should this be fixed in MainThreadGeneric or should the JSCOnly use the Windows implementation of MainThread?

Thanks,
Koby
Comment 1 Yusuke Suzuki 2018-10-01 03:13:23 PDT
(In reply to Koby from comment #0)
> Hi,
> After updating my copy of WebKit, the Windows build of JSCOnly port fails
> with the following error:
> c:\togithub\webkit-original\webkit\source\wtf\wtf\generic\mainthreadgeneric.
> cpp(33): fatal error C1083: Cannot open include file: 'pthread.h': No such
> file or directory [C
> :\togithub\WebKit-Original\webkit\WebKitBuild\Release\Source\WTF\wtf\WTF.
> vcxproj]
> 
> Looking at the file, it seems that since r236617 MainThreadGeneric.cpp
> always includes pthread.h, which is a problem since it isn't available on
> Windows (on the JSCOnly port uses MainThreadGeneric.cpp on all platforms).
> 
> What do you think? should this be fixed in MainThreadGeneric or should the
> JSCOnly use the Windows implementation of MainThread?
> 
> Thanks,
> Koby

Use MainThreadWin instead in Window.
Comment 2 Koby 2018-10-01 03:30:20 PDT
Thanks! it seems to compile. I'll test it and upload a patch here.
If I'm moving MainThread to the Windows implementation, is there anything else I need to change? Like the WorkQueueGeneric which is currently used (vs WorkQueueWin)?
Comment 3 Yusuke Suzuki 2018-10-01 03:33:21 PDT
(In reply to Koby from comment #2)
> Thanks! it seems to compile. I'll test it and upload a patch here.
> If I'm moving MainThread to the Windows implementation, is there anything
> else I need to change? Like the WorkQueueGeneric which is currently used (vs
> WorkQueueWin)?

Just using MainThreadWin.cpp instead of MainThreadGeneric.cpp is OK. It should work.
Comment 4 Koby 2018-10-01 04:46:49 PDT
Created attachment 351236 [details]
Patch
Comment 5 Koby 2018-10-01 04:47:11 PDT
Added a patch :)
Comment 6 Yusuke Suzuki 2018-10-01 04:50:39 PDT
Comment on attachment 351236 [details]
Patch

r=me
Comment 7 WebKit Commit Bot 2018-10-01 05:33:12 PDT
Comment on attachment 351236 [details]
Patch

Clearing flags on attachment: 351236

Committed r236655: <https://trac.webkit.org/changeset/236655>
Comment 8 WebKit Commit Bot 2018-10-01 05:33:14 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2018-10-01 05:34:24 PDT
<rdar://problem/44906628>