Bug 32753 – REGRESSION(52325) Chromium build broken due to link failure

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Eric Seidel (no email)]

REGRESSION(52325) Chromium build broken due to link failure

http://build.chromium.org/buildbot/waterfall.fyi/builders/Webkit%20Mac%20(webkit.org)/builds/11702/steps/compile/logs/stdio

Ld /b/slave/webkit-rel-mac-webkit-org/build/src/webkit/tools/test_shell/../../../xcodebuild/Release/test_shell_tests normal i386
    cd /b/slave/webkit-rel-mac-webkit-org/build/src/webkit/tools/test_shell
    setenv MACOSX_DEPLOYMENT_TARGET 10.5
    /Developer/usr/bin/g++-4.2 -arch i386
[snip]
Undefined symbols:
  "_wkNoteOpenPanelFiles", referenced from:
      _wkNoteOpenPanelFiles$non_lazy_ptr in libwebcore.a(WebSystemInterface.o)
  "_WKNoteOpenPanelFiles", referenced from:
      _WKNoteOpenPanelFiles$non_lazy_ptr in libwebcore.a(WebSystemInterface.o)
ld: symbol(s) not found

The failure does not occur on the webkit.org builders yet, because it doesn't link against anything yet.

The solution is to include platform.h from WebSystemInterface.m to make sure that we get the right symbol on Leopard.

I'm going to make a quick-and-dirty include platform.h fix.  Eventually we might wish to use a config.h wrapper around platform.h instead.

[Peter Kasting]

Edited IRC log for posterity:

<pkasting> othermaciej mentioned the idea of adding a config.h for this section of the project and just having it #include Platform.h
<othermaciej> WebCore and JavaScriptCore have a rule that config.h must be the first include in every file
<othermaciej> it seems reasonable to me to do that for WebKit too
<eseidel> sounds like othermaciej's idea is a good one, as usual :)
<eseidel> I think the utility of a non-PCH build has not at least historically been well understood
<eseidel> with XCode eventually moving to distcc 3, everyone will want a non-PCH build eventually
<pkasting> So is the resolution here that we should, in fact, add a config.h to WebKit, have it #include Platform.h, and add #include config.h to all the source files?
<eseidel> I would just add it to the files you need to un-break the build
<eseidel> and we can let webkit.org bake on that policy for a while
<eseidel> I think it's important to un-break the build, but we can figure out what the final full-scale policy change looks like later

[Eric Seidel (no email)]

Created attachment 45202 [details]
Patch

[Adam Barth]

Comment on attachment 45202 [details]
Patch

ok

[Peter Kasting]

Comment on attachment 45202 [details]
Patch

> +// Neded builds not using PCH to expose BUILDING_ macros, see bug 32753.

"Neded" -> "Needed for"

[Eric Seidel (no email)]

Created attachment 45204 [details]
Now with moar betr smelling

[Eric Seidel (no email)]

Created attachment 45205 [details]
I should re-enroll in 3rd grade, clearly.

[Adam Barth]

Comment on attachment 45205 [details]
I should re-enroll in 3rd grade, clearly.

ok.  I spelt goood to

[Eric Seidel (no email)]

Committed r52356: <http://trac.webkit.org/changeset/52356>