Bug 117294 – [WinCairo] WTF.dll is linking with CoreFoundation.lib in VS2010.

• Component: Web Template Framework
• Status: RESOLVED FIXED

The WinCairo build should not use CoreFoundation.

Created attachment 203933 [details]
Patch

Created attachment 204008 [details]
Patch

Merged patch with changes in tree.

[WebKit Commit Bot]

Comment on attachment 204008 [details]
Patch

Clearing flags on attachment: 204008

Committed r151318: <http://trac.webkit.org/changeset/151318>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 117356

[Brent Fulgham]

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.

[Brent Fulgham]

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.

[Brent Fulgham]

Comment on attachment 205239 [details]
Patch

Looks good.  Thanks.

[WebKit Commit Bot]

Comment on attachment 205239 [details]
Patch

Clearing flags on attachment: 205239

Committed r151915: <http://trac.webkit.org/changeset/151915>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.