Bug 54896 – [Qt] [WK2] WebKitTestRunner, QtWebProcess and WTRInjectBundle should fail to compile when there's undefined symbols

• Component: WebKit2
• Status: RESOLVED FIXED

[Kimmo Kinnunen]

WebKitTestRunner, QtWebProcess and WTRInjectBundle should fail to compile when there's undefined symbols.

Not failing the compile early leads to hard to debug situations, like webkit test runner failing all tests because WTRInjectBundle doesn't contain for example following symbols:
InjectedBundlePage.cpp:(.text+0x28d8): undefined reference to `WebCore::KURL::KURL(WebCore::ParsedURLStringTag, char const*)'
InjectedBundlePage.cpp:(.text+0x2906): undefined reference to `WebCore::KURL::host() const'
InjectedBundlePage.cpp:(.text+0x2a33): undefined reference to `WebCore::KURL::protocol() const'
InjectedBundlePage.cpp:(.text+0x2a62): undefined reference to `WebCore::KURL::protocol() const'
InjectedBundlePage.cpp:(.text+0x2a95): undefined reference to `WebCore::KURL::host() const'
InjectedBundlePage.cpp:(.text+0x2acb): undefined reference to `WebCore::KURL::host() const'
InjectedBundlePage.cpp:(.text+0x2bb3): undefined reference to `WebCore::KURL::host() const'

[Kimmo Kinnunen]

Created attachment 83197 [details]
fix

[Kimmo Kinnunen]

Created attachment 83198 [details]
fix2 (previous had a typo in diff command)

[Early Warning System Bot]

Attachment 83198 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7939852

[Csaba Osztrogonác]

The idea is good, catching a build break is always easier.
I added bug 54109 to the "depends on" list not to break the build now.

[Kimmo Kinnunen]

Comment on attachment 83198 [details]
fix2 (previous had a typo in diff command)

clearing to retry ews

[Kimmo Kinnunen]

Comment on attachment 83198 [details]
fix2 (previous had a typo in diff command)

requesting review + commit

[Kenneth Rohde Christiansen]

This seems to fail on Qt.

[Balazs Kelemen]

I think it would build on a fresh tree.
Could you try it once again, Kimmo?

[Kimmo Kinnunen]

(In reply to comment #8)
> I think it would build on a fresh tree.
> Could you try it once again, Kimmo?

Yeah, I tried to invoke buildbots again (there where i reset the review flags). However, it didn't seem to start the build. Any hints on how to restart it properly?

That Qt failure was probably due to buildbot trying to build too early with respect to the bug 54109 fix.

[Balazs Kelemen]

(In reply to comment #9)
> (In reply to comment #8)
> > I think it would build on a fresh tree.
> > Could you try it once again, Kimmo?
> 
> Yeah, I tried to invoke buildbots again (there where i reset the review flags). However, it didn't seem to start the build. Any hints on how to restart it properly?
> 

I don't know about any other way than resubmitting the patch.

[Kimmo Kinnunen]

Created attachment 86042 [details]
Rebased and resubmitted patch to get new try with buildbots

[Benjamin Poulain]

Comment on attachment 86042 [details]
Rebased and resubmitted patch to get new try with buildbots

I understand why it is needed for InjectedBundle but not for WebKitTestRunner and the WebProcess. Could you explain?

[Eric Seidel (no email)]

Comment on attachment 86042 [details]
Rebased and resubmitted patch to get new try with buildbots

OK.

[WebKit Commit Bot]

Comment on attachment 86042 [details]
Rebased and resubmitted patch to get new try with buildbots

Clearing flags on attachment: 86042

Committed r83416: <http://trac.webkit.org/changeset/83416>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.