Bug 106558 - [Windows] Compile fix.
Summary: [Windows] Compile fix.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-10 05:59 PST by peavo
Modified: 2013-07-10 11:15 PDT (History)
8 users (show)

See Also:


Attachments
Patch (3.16 KB, patch)
2013-01-10 06:11 PST, peavo
no flags Details | Formatted Diff | Diff
Patch (3.18 KB, patch)
2013-01-11 07:05 PST, peavo
no flags Details | Formatted Diff | Diff
Patch (4.02 KB, patch)
2013-06-22 04:12 PDT, peavo
no flags Details | Formatted Diff | Diff
Patch (1.24 KB, patch)
2013-07-02 07:13 PDT, peavo
no flags Details | Formatted Diff | Diff
Patch (1.21 KB, patch)
2013-07-09 04:33 PDT, peavo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description peavo 2013-01-10 05:59:37 PST
The Windows CF build fails with:

CFNetwork/CFURLConnection.h(336) : error C2061: syntax error : identifier 'dispatch_queue_t'
Comment 1 peavo 2013-01-10 06:11:27 PST
Created attachment 182124 [details]
Patch
Comment 2 WebKit Review Bot 2013-01-10 06:13:15 PST
Attachment 182124 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebKit/win/WebDownload.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit/win/DefaultDownloadDelegate.h:32:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 peavo 2013-01-11 07:05:11 PST
Created attachment 182340 [details]
Patch
Comment 4 peavo 2013-01-11 07:12:57 PST
The reason for these compile errors might be that the function

void CFURLConnectionSetDelegateDispatchQueue(CFURLConnectionRef conn, dispatch_queue_t q)

which uses the type dispatch_queue_t, has been added to WebKitLibraries/win/include/CFNetwork/CFURLConnection.h.
It seems my WebKitSupportLibraryVersion is 4.
Comment 5 Darin Adler 2013-03-06 09:17:25 PST
Comment on attachment 182340 [details]
Patch

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

Fine to add these includes. But none of these includes should be inside #if. They are all safe to do on non-Windows platforms so they should be unconditional.

> Source/WebCore/platform/network/cf/AuthenticationCF.cpp:39
> +#endif
>  // This header must come before all other CFNetwork headers to work around a CFNetwork bug. It can

Missing blank line here.
Comment 6 Darin Adler 2013-03-06 09:18:13 PST
(In reply to comment #4)
> The reason for these compile errors might be that the function
> 
> void CFURLConnectionSetDelegateDispatchQueue(CFURLConnectionRef conn, dispatch_queue_t q)
> 
> which uses the type dispatch_queue_t, has been added to WebKitLibraries/win/include/CFNetwork/CFURLConnection.h.
> It seems my WebKitSupportLibraryVersion is 4.

The correct fix is for that header file to include the correct header. We should see if we can get this fixed instead of working around it in WebKit.
Comment 7 peavo 2013-06-22 04:12:37 PDT
Created attachment 205244 [details]
Patch
Comment 8 peavo 2013-06-22 04:15:06 PDT
(In reply to comment #5)
> (From update of attachment 182340 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=182340&action=review
> 

Thanks for reviewing ;)

I removed the platform ifdefs in the new patch.

> Fine to add these includes. But none of these includes should be inside #if. They are all safe to do on non-Windows platforms so they should be unconditional.
> 
> > Source/WebCore/platform/network/cf/AuthenticationCF.cpp:39
> > +#endif
> >  // This header must come before all other CFNetwork headers to work around a CFNetwork bug. It can
> 
> Missing blank line here.
Comment 9 Brent Fulgham 2013-06-24 13:16:24 PDT
Is this something you see on a WinCairo build? Because the Windows builds all seem to be working properly without this patch.
Comment 10 peavo 2013-06-25 11:39:05 PDT
(In reply to comment #9)
> Is this something you see on a WinCairo build? Because the Windows builds all seem to be working properly without this patch.

No, this is in the WinApple build. I haven't been able to build WinApple for a long time because of these compile errors. I'm not sure why I am the only one with these problems. Maybe I have the wrong version of the file WebKitLibraries/win/include/CFNetwork/CFURLConnection.h, as I mentioned in an earlier comment. The function void CFURLConnectionSetDelegateDispatchQueue(CFURLConnectionRef conn, dispatch_queue_t q) uses the type dispatch_queue_t, which is not declared at this point. I'm on  WebKitSupportLibraryVersion 4, it seems.
Comment 11 Roger Fong 2013-06-25 11:59:39 PDT
I updated the downloadable support libraries recently (yesterday actually), on developer.apple.com/opensource.

Care to try those new ones out?
Comment 12 peavo 2013-06-25 12:20:33 PDT
(In reply to comment #11)
> I updated the downloadable support libraries recently (yesterday actually), on developer.apple.com/opensource.
> 
> Care to try those new ones out?

Thanks, I'll try them out ;)
Comment 13 Vivek Galatage 2013-06-30 21:59:11 PDT
(In reply to comment #11)
> I updated the downloadable support libraries recently (yesterday actually), on developer.apple.com/opensource.
> 
> Care to try those new ones out?

I tried to downloading the WebKitSupportLibs.zip and tried extracting it using the script ./Tools/Scripts/update-webkit-support-libs. It throws out-of-date error to re-download it again. So when I tried to cross verify what the error was, I got this:

The WebKitSupportLibraryVersion included in the zip file has the value '5' whereas https://developer.apple.com/opensource/internet/WebKitSupportLibraryVersion return '4' due to which the installation fails.

Also I am getting a lot of errors while building (due to new missing headers in the WebKitSupportLibs) as -

Cannot open include file: 'Security/Security.h': No such file or directory
Cannot open include file: 'ImageIO/ImageIOBase.h': No such file or directory

I think there is some issue with the version.
Comment 14 Vivek Galatage 2013-06-30 23:45:13 PDT
(In reply to comment #13)
> (In reply to comment #11)
> > I updated the downloadable support libraries recently (yesterday actually), on developer.apple.com/opensource.
> > 
> > Care to try those new ones out?
> 
> I tried to downloading the WebKitSupportLibs.zip and tried extracting it using the script ./Tools/Scripts/update-webkit-support-libs. It throws out-of-date error to re-download it again. So when I tried to cross verify what the error was, I got this:
> 
> The WebKitSupportLibraryVersion included in the zip file has the value '5' whereas https://developer.apple.com/opensource/internet/WebKitSupportLibraryVersion return '4' due to which the installation fails.
> 
> Also I am getting a lot of errors while building (due to new missing headers in the WebKitSupportLibs) as -
> 
> Cannot open include file: 'Security/Security.h': No such file or directory
> Cannot open include file: 'ImageIO/ImageIOBase.h': No such file or directory
> 
> I think there is some issue with the version.


Created a separate bug for the above mentioned error here: https://bugs.webkit.org/show_bug.cgi?id=118229
Comment 15 Alexey Proskuryakov 2013-07-01 09:11:22 PDT
Comment on attachment 205244 [details]
Patch

Can it be added to some shared prefix header instead, to better match Mac?
Comment 16 peavo 2013-07-02 07:13:49 PDT
Created attachment 205910 [details]
Patch
Comment 17 peavo 2013-07-02 07:15:09 PDT
(In reply to comment #15)
> (From update of attachment 205244 [details])
> Can it be added to some shared prefix header instead, to better match Mac?

Thanks, that's a good idea, updated patch.
Comment 18 Alexey Proskuryakov 2013-07-02 09:15:31 PDT
Why did you choose config.h, and not WebCore/WebCorePrefix.h?

Just curious, I'll leave final review to Brent and Roger.
Comment 19 peavo 2013-07-09 04:33:04 PDT
Created attachment 206307 [details]
Patch
Comment 20 peavo 2013-07-09 04:37:05 PDT
(In reply to comment #18)
> Why did you choose config.h, and not WebCore/WebCorePrefix.h?
> 

I agree, WebCorePrefix.h is a better place for it, updated patch ;)
Comment 21 Brent Fulgham 2013-07-09 09:02:23 PDT
Comment on attachment 206307 [details]
Patch

r=me
Comment 22 WebKit Commit Bot 2013-07-09 09:23:23 PDT
Comment on attachment 206307 [details]
Patch

Clearing flags on attachment: 206307

Committed r152495: <http://trac.webkit.org/changeset/152495>
Comment 23 WebKit Commit Bot 2013-07-09 09:23:26 PDT
All reviewed patches have been landed.  Closing bug.
Comment 24 peavo 2013-07-10 11:15:03 PDT
(In reply to comment #21)
> (From update of attachment 206307 [details])
> r=me

Thanks for reviewing :)