Summary: | [WinCairo] WTF.dll is linking with CoreFoundation.lib in VS2010. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | peavo | ||||||||||
Component: | Web Template Framework | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | bfulgham, commit-queue | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 117356 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
peavo
2013-06-06 07:07:17 PDT
Created attachment 203933 [details]
Patch
Created attachment 204008 [details]
Patch
Merged patch with changes in tree. Comment on attachment 204008 [details] Patch Clearing flags on attachment: 204008 Committed r151318: <http://trac.webkit.org/changeset/151318> All reviewed patches have been landed. Closing bug. Re-opened since this is blocked by bug 117356 The patch as currently written breaks the Apple Windows build. We need to compartmentalize this a bit better. I'm not sure where I went wrong here. According to http://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/49413 the Apple Win Release build was successful? Or is this not the build we're talking about? Could it be some problems with applying the patch? I see from the logs (https://webkit-queues.appspot.com/patch/204008) that there are some issues when applying... Created attachment 205171 [details]
Patch
(In reply to comment #10) > Created an attachment (id=205171) [details] > Patch Simplified the patch slightly by leaving out the new prop file created in the first patch. Comment on attachment 205171 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=205171&action=review I don't think this patch is quite right. The SchedulePair header and implementations both refer to CoreFoundation types. It may be that you can build without CoreFoundation in cases where the underlying code using these headers do not happen to call code that actually needs access to CoreFoundation symbols, but this really isn't right as some seemingly unrelated future change could suddenly cause the build to break. Instead, we need to make Debug_WinCairo and Release_WinCairo targets for WTF that link to CFLite.lib instead of CoreFoundation.lib so that the calls to CFEqual and use of various CF types will resolve to real symbols at link time. > Source/WTF/WTF.vcxproj/WTFCommon.props:26 > + <AdditionalDependencies>winmm.lib;libicuuc$(DebugSuffix).lib;libicuin$(DebugSuffix).lib;%(AdditionalDependencies)</AdditionalDependencies> While it is good to remove the CoreFoundation.lib dependency here, we should be using CFLite on the WinCairo build. > Source/WTF/WTF.vcxproj/WTFDebug.props:15 > + </Link> This property sheet still requires CoreFoundation.lib for WinCairo builds. Created attachment 205239 [details]
Patch
(In reply to comment #12) > (From update of attachment 205171 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=205171&action=review Thanks for reviewing ;) I readded the SchedulePair implementation file to the project, and added the property sheets WTFCFLite.props and WTFCoreFoundation.props to make WinCairo link with CFLite.lib and WinApple link with CoreFoundation.lib. > > I don't think this patch is quite right. The SchedulePair header and implementations both refer to CoreFoundation types. It may be that you can build without CoreFoundation in cases where the underlying code using these headers do not happen to call code that actually needs access to CoreFoundation symbols, but this really isn't right as some seemingly unrelated future change could suddenly cause the build to break. > > Instead, we need to make Debug_WinCairo and Release_WinCairo targets for WTF that link to CFLite.lib instead of CoreFoundation.lib so that the calls to CFEqual and use of various CF types will resolve to real symbols at link time. > > > Source/WTF/WTF.vcxproj/WTFCommon.props:26 > > + <AdditionalDependencies>winmm.lib;libicuuc$(DebugSuffix).lib;libicuin$(DebugSuffix).lib;%(AdditionalDependencies)</AdditionalDependencies> > > While it is good to remove the CoreFoundation.lib dependency here, we should be using CFLite on the WinCairo build. > > > Source/WTF/WTF.vcxproj/WTFDebug.props:15 > > + </Link> > > This property sheet still requires CoreFoundation.lib for WinCairo builds. Comment on attachment 205239 [details]
Patch
Looks good. Thanks.
Comment on attachment 205239 [details] Patch Clearing flags on attachment: 205239 Committed r151915: <http://trac.webkit.org/changeset/151915> All reviewed patches have been landed. Closing bug. |