RESOLVED FIXED 42722
WebKit on Windows should build optionally with an unversioned ICU DLL 
https://bugs.webkit.org/show_bug.cgi?id=42722
Summary WebKit on Windows should build optionally with an unversioned ICU DLL
Steve Falkenburg
Reported 2010-07-20 20:01:58 PDT
WebKit on Windows should build optionally with an unversioned ICU DLL
Attachments
Patch (94.32 KB, patch)
2010-07-20 20:24 PDT, Steve Falkenburg 		no flags
DetailsFormatted DiffDiff
Patch (30.46 KB, patch)
2010-07-20 21:47 PDT, Steve Falkenburg 		aroben: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Steve Falkenburg
Comment 1 2010-07-20 20:24:26 PDT
Created attachment 62142 [details] Patch
Steve Falkenburg
Comment 2 2010-07-20 21:47:08 PDT
Created attachment 62143 [details] Patch
Mark Rowe (bdash)
Comment 3 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?
Steve Falkenburg
Comment 4 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.
Adam Roben (:aroben)
Comment 5 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
Adam Barth
Comment 6 2010-08-10 22:41:00 PDT
Comment on attachment 62143 [details] Patch Is this patch still current?
Adam Roben (:aroben)
Comment 7 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.
Note You need to log in before you can comment on or make changes to this bug.