| Summary: | [PATCH] Add missing _WIN32_WINNT and WINVER definitions | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Kalev Lember <kalevlember> | ||||||
| Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | aroben, eric, erik-webkit, sfalken, webkit.review.bot | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows XP | ||||||||
| Attachments: |
|
||||||||
|
Description
Kalev Lember
2011-04-28 08:34:15 PDT
Created attachment 91498 [details]
Add missing _WIN32_WINNT and WINVER definitions
Additional note: the compile error mentioned here only occurs when using the mingw.org toolchain. If the mingw-w64 toolchain is used then this error doesn't happen as _WIN32_WINNT is set to 0x0500 in their headers by default Comment on attachment 91498 [details]
Add missing _WIN32_WINNT and WINVER definitions
I'm not sure this belongs in config.h.
For Apple's Windows port, we define these in a global vsprops file (common.vsprops). Also, these don't match the versions we're using in that file. We use: WINVER=0x502 _WIN32_WINNT=0x502 _WIN32_IE=0x603 For what it's worth, both WebCore/config.h and WebKit2/config.h already contain these definitions, so the patch makes it more similar to these two. Any other ideas where these definitions should go instead of config.h? I think defining it only in the vsprops file isn't enough, as that file is specific to Visual Studio build system, whereas there are ports that use other build systems, for instance autotools. Each individual port might choose to tighten the deps even more (e.g. requiring WINVER=502 as Apple's Windows port is doing), but there should still be _minimal_ WINVER/_WIN32_WINNT definitions somewhere in buildsystem-agnostic code. I am not entirely sure either where the right place for these definitions should be, but the attached patch at least brings JavaScriptCore in line with the rest of the projects and fixes a build failure with webkitgtk port. Any chance this patch could go in? We definitely need _WIN32_WINNT and WINVER defined also somewhere else than Visual Studio common.vsprops (the GTK+ port uses autotools instead), and in my opinion config.h is a good place for that. Comment on attachment 91498 [details] Add missing _WIN32_WINNT and WINVER definitions Rejecting attachment 91498 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-02', '--port..." exit_code: 2 Last 500 characters of output: (27.8%) => Tests that will only be fixed if they crash (WONTFIX) (8133): 1 test timed out ( 0.0%) 111 text diff mismatch ( 1.4%) 7972 skipped (98.0%) Unexpected flakiness: text diff mismatch (3) media/media-blocked-by-beforeload.html = TEXT PASS media/media-blocked-by-willsendrequest.html = TEXT PASS perf/array-reverse.html = TEXT PASS Regressions: Unexpected text diff mismatch : (1) inspector/styles/styles-url-linkify.html = TEXT Full output: http://queues.webkit.org/results/9003305 Created attachment 100118 [details]
Archive of layout-test-results from ec2-cq-02
The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: ec2-cq-02 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment on attachment 91498 [details]
Add missing _WIN32_WINNT and WINVER definitions
I don't understand how this can cause a regression in inspector/styles/styles-url-linkify.html; perhaps it was something unrelated? Could you please set the commit-queue flag again to retry?
Comment on attachment 91498 [details] Add missing _WIN32_WINNT and WINVER definitions Clearing flags on attachment: 91498 Committed r90649: <http://trac.webkit.org/changeset/90649> All reviewed patches have been landed. Closing bug. |