Bug 88873 – [Qt][Win] Symbols are not exported in QtWebKit5.dll

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

[Simon Hausmann]

In the WebKit2 static library we Q_DECL_EXPORT a bunch of classes/functions, but when linking QtWebKit5.dll those symbols are not exported when compiling with MSVC. This is how the MSVC toolchain behaves.

The suggested workaround is to run a little script over the static libraries we create to extract all the symbols that are marked for export and make sure that they are actually exported (and not discarded!) in the final dll.

[Simon Hausmann]

Created attachment 147084 [details]
Script to facilitate symbol exports

The attached scripts implements the basic functionality needed. Symbols marked for export get a /EXPORT:<name of mangled symbol> in the linker directive section of the .obj file (and thus .lib file). We can extract all these with the script and generate a new file with linker pragmas that should be compiled into QtWebKit5.dll and makes sure that the symbols we want to export are also referenced.

[Simon Hausmann]

After the changes for bug #78109 this is now needed for the the Windows build altogether, because the WK1 API is now also in a separate static library.

[Simon Hausmann]

Created attachment 149504 [details]
Patch

[Jocelyn Turcotte]

Comment on attachment 149504 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=149504&action=review

> Tools/Scripts/mkexports:43
> +print "Found %s symbols in %s" % (len(symbols), sys.argv[1])

This could describe the action made on this symbol for those not knowing what this is about, something like "Forwarding the export of symbol %s from static lib %s".

> Tools/qmake/mkspecs/features/win32/default_post.prf:25
> +contains(TARGET, $$resolveFinalLibraryName($${QT.webkit.name})) {

Just a nitpick but it's a bit weird to place it in default_post and then add a condition on TARGET.
A different way of doing it could be to put a function in functions.prf and call it from api.pri and WebKit1.pro.

> Tools/qmake/mkspecs/features/win32/default_post.prf:35
> +    exportgen.commands = python $${ROOT_WEBKIT_DIR}$${QMAKE_DIR_SEP}Tools$${QMAKE_DIR_SEP}Scripts$${QMAKE_DIR_SEP}mkexports ${QMAKE_FILE_IN} ${QMAKE_FILE_OUT}

If we have to put the script in Tools/Scripts we could name it a bit more like the local script name style. Something like "generate-win32-export-forwards" maybe.

[Simon Hausmann]

Comment on attachment 149504 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=149504&action=review

>> Tools/Scripts/mkexports:43
>> +print "Found %s symbols in %s" % (len(symbols), sys.argv[1])
> 
> This could describe the action made on this symbol for those not knowing what this is about, something like "Forwarding the export of symbol %s from static lib %s".

Good idea, will change that.

>> Tools/qmake/mkspecs/features/win32/default_post.prf:25
>> +contains(TARGET, $$resolveFinalLibraryName($${QT.webkit.name})) {
> 
> Just a nitpick but it's a bit weird to place it in default_post and then add a condition on TARGET.
> A different way of doing it could be to put a function in functions.prf and call it from api.pri and WebKit1.pro.

True. Either that or we simply don't do that check and always implement this behaviour when TEMPLATE = lib and contains(CONFIG, shared)

Essentially this is adding the missing bits on win32 that --whole-archive is on linux for example.

>> Tools/qmake/mkspecs/features/win32/default_post.prf:35
>> +    exportgen.commands = python $${ROOT_WEBKIT_DIR}$${QMAKE_DIR_SEP}Tools$${QMAKE_DIR_SEP}Scripts$${QMAKE_DIR_SEP}mkexports ${QMAKE_FILE_IN} ${QMAKE_FILE_OUT}
> 
> If we have to put the script in Tools/Scripts we could name it a bit more like the local script name style. Something like "generate-win32-export-forwards" maybe.

Alright, I like export-forwards in the name.

[Simon Hausmann]

Created attachment 149523 [details]
Patch

[Simon Hausmann]

Created attachment 149525 [details]
Patch

[Simon Hausmann]

Created attachment 149526 [details]
Patch

[Simon Hausmann]

Committed r121261: <http://trac.webkit.org/changeset/121261>