Bug 117294 - [WinCairo] WTF.dll is linking with CoreFoundation.lib in VS2010.
Summary: [WinCairo] WTF.dll is linking with CoreFoundation.lib in VS2010.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 117356
Blocks:
  Show dependency treegraph
 
Reported: 2013-06-06 07:07 PDT by peavo
Modified: 2013-06-24 09:25 PDT (History)
2 users (show)

See Also:


Attachments
Patch (4.26 KB, patch)
2013-06-06 07:17 PDT, peavo
no flags Details | Formatted Diff | Diff
Patch (4.24 KB, patch)
2013-06-07 00:32 PDT, peavo
no flags Details | Formatted Diff | Diff
Patch (3.52 KB, patch)
2013-06-21 04:43 PDT, peavo
no flags Details | Formatted Diff | Diff
Patch (5.44 KB, patch)
2013-06-22 02:08 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-06-06 07:07:17 PDT
The WinCairo build should not use CoreFoundation.
Comment 1 peavo 2013-06-06 07:17:23 PDT
Created attachment 203933 [details]
Patch
Comment 2 peavo 2013-06-07 00:32:35 PDT
Created attachment 204008 [details]
Patch
Comment 3 peavo 2013-06-07 00:34:14 PDT
Merged patch with changes in tree.
Comment 4 WebKit Commit Bot 2013-06-07 07:43:12 PDT
Comment on attachment 204008 [details]
Patch

Clearing flags on attachment: 204008

Committed r151318: <http://trac.webkit.org/changeset/151318>
Comment 5 WebKit Commit Bot 2013-06-07 07:43:14 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 WebKit Commit Bot 2013-06-07 11:15:12 PDT
Re-opened since this is blocked by bug 117356
Comment 7 Brent Fulgham 2013-06-07 11:27:22 PDT
The patch as currently written breaks the Apple Windows build.  We need to compartmentalize this a bit better.
Comment 8 peavo 2013-06-09 02:37:08 PDT
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?
Comment 9 peavo 2013-06-14 05:56:26 PDT
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...
Comment 10 peavo 2013-06-21 04:43:32 PDT
Created attachment 205171 [details]
Patch
Comment 11 peavo 2013-06-21 05:18:57 PDT
(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 12 Brent Fulgham 2013-06-21 09:48:09 PDT
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.
Comment 13 peavo 2013-06-22 02:08:07 PDT
Created attachment 205239 [details]
Patch
Comment 14 peavo 2013-06-22 02:14:59 PDT
(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 15 Brent Fulgham 2013-06-24 09:04:04 PDT
Comment on attachment 205239 [details]
Patch

Looks good.  Thanks.
Comment 16 WebKit Commit Bot 2013-06-24 09:25:56 PDT
Comment on attachment 205239 [details]
Patch

Clearing flags on attachment: 205239

Committed r151915: <http://trac.webkit.org/changeset/151915>
Comment 17 WebKit Commit Bot 2013-06-24 09:25:58 PDT
All reviewed patches have been landed.  Closing bug.