Bug 42722 - WebKit on Windows should build optionally with an unversioned ICU DLL
Summary: WebKit on Windows should build optionally with an unversioned ICU DLL
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Steve Falkenburg
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-20 20:01 PDT by Steve Falkenburg
Modified: 2010-08-16 07:16 PDT (History)
1 user (show)

See Also:


Attachments
Patch (94.32 KB, patch)
2010-07-20 20:24 PDT, Steve Falkenburg
no flags Details | Formatted Diff | Diff
Patch (30.46 KB, patch)
2010-07-20 21:47 PDT, Steve Falkenburg
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Falkenburg 2010-07-20 20:01:58 PDT
WebKit on Windows should build optionally with an unversioned ICU DLL
Comment 1 Steve Falkenburg 2010-07-20 20:24:26 PDT
Created attachment 62142 [details]
Patch
Comment 2 Steve Falkenburg 2010-07-20 21:47:08 PDT
Created attachment 62143 [details]
Patch
Comment 3 Mark Rowe (bdash) 2010-07-20 21:50:39 PDT
Comment on attachment 62143 [details]
Patch

Is there a reason to force the include of ICUVersion.h rather than having the project prefix header include it explicitly?
Comment 4 Steve Falkenburg 2010-07-20 22:00:59 PDT
It could probably be moved to <wtf/Platform.h>. That's where we include QuartzCorePresent.h, which is somewhat similar to this.

The reason the patch puts it in vcproj/vsprops files is that I needed to edit those files for this patch anyway, so I figured I'd keep the changes localized. This change will be rolled out once we have the new unversioned ICU in the tree.
Comment 5 Adam Roben (:aroben) 2010-07-21 04:57:22 PDT
Comment on attachment 62143 [details]
Patch

> +        Since the versioned and unversioned ICU have different filenames (libicuuc.lib vs icuuc.lib)
> +        we copy the ICU lib to an intermediate location under obj with a common name.

I think you meant "under lib".

It would be nicer to add a new ICU.vsprops file that contains the ICU-specific settings (AdditionalDependencies, AdditionalLibraryDirectories, VCPreLinkEventTool, ForcedIncludes). Then each project that links against ICU could just include it.

r=me
Comment 6 Adam Barth 2010-08-10 22:41:00 PDT
Comment on attachment 62143 [details]
Patch

Is this patch still current?
Comment 7 Adam Roben (:aroben) 2010-08-16 07:16:06 PDT
(In reply to comment #6)
> (From update of attachment 62143 [details])
> Is this patch still current?

No, it was landed in r63833.