Summary: | [GTK OSX] libGObjectDOMBindings.a is not linked into libwebkit2gtk-4.0.37.dylib | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jeremy Huddleston Sequoia <jeremyhu> | ||||||||||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | bugs-noreply, commit-queue, mcatanzaro, philip.chimento | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Mac | ||||||||||||||||
OS: | OS X 10.11 | ||||||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=144557 | ||||||||||||||||
Bug Depends on: | |||||||||||||||||
Bug Blocks: | 126492 | ||||||||||||||||
Attachments: |
|
Description
Jeremy Huddleston Sequoia
2016-01-14 22:05:04 PST
Created attachment 269033 [details]
full build log
Those symbols are found in: Source/WebCore/bindings/gobject/WebKitDOMDeprecated.cpp Source/WebCore/bindings/gobject/WebKitDOMCustom.cpp which are linked into libGObjectDOMBindings.a, which is not linked into libwebkit2gtk-4.0.37.13.0.dylib Created attachment 269035 [details]
build log
Actually, libGObjectDOMBindings.a is on the link line, but we're missing -all_load. This was caused by an incorrect fix for bug #144557 Created attachment 269043 [details]
patch to add -Wl,-all_load
Comment on attachment 269043 [details] patch to add -Wl,-all_load View in context: https://bugs.webkit.org/attachment.cgi?id=269043&action=review Thanks for your patch! In the future, when you want a review for your patch, please set the r? flag for your patch. If you want us to commit your patch (which you do if you don't have committer access), set the cq? flag. You'll want to set cq? after answering this question: > Source/cmake/OptionsGTK.cmake:477 > + list(APPEND ${_list_name} -Wl,-all_load) Should this be -Wl,-all_load ${library} -Wl,-no-all_load, or does it not work the same as --whole-archive? > Should this be -Wl,-all_load ${library} -Wl,-no-all_load No. > or does it not work the same as --whole-archive? It does not. There is nothing exactly analogous. There are two similar options in ld64: -all_load Loads all members of static archive libraries. -force_load path_to_archive Loads all members of the specified static archive library. Note: -all_load forces all members of all archives to be loaded. This option allows you to target a specific archive. -force_load would be ideal if we had a contract that the arguments to the macro were always static archives that we built for inclusion in the library. I tried that first, and it didn't work because the macro is called with arguments other than just static archives that we had built. -all_load is thus the ideal choice, and it's also used elsewhere in the build already. Do -all_load and/or -force_load work for you? For me, using either one did allow the build to complete, but MiniBrowser segfaulted on startup (see https://bugs.webkit.org/show_bug.cgi?id=144557#c1) yes, they work for me. They do what they are supposed to do and symbols are included in the library. Without this change, clients fail to link due to the missing symbols. With this change in place, clients are able to link, and we're seeing crashes at launch (https://trac.macports.org/ticket/50339) which we are starting to look at now. I suspect your crash is likely the same as that one. Hm, I just found this bit of code in Source/WebKit2/PlatformGTK.cmake: # Manually add some libraries on OSX because we don't have the --whole-archive flag if (CMAKE_SYSTEM_NAME MATCHES "Darwin") set(INTROSPECTION_ADDITIONAL_LIBRARIES --library=c++) set(INTROSPECTION_ADDITIONAL_LDFLAGS -lGObjectDOMBindings) endif () Is that still needed? Probably it can be removed if -all_load works as expected. Yeah, I saw that as well. I believe that is specific to the gobject-introspection code, but that is currently not working on OSX anyways. We've disabled it in our (MacPorts) build. See bug #152650. Comment on attachment 269043 [details] patch to add -Wl,-all_load Rejecting attachment 269043 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 269043, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: 153117. Found no modified ChangeLogs, cannot create a commit message. All changes require a ChangeLog. See: http://webkit.org/coding/contributing.html Found no modified ChangeLogs, cannot create a commit message. All changes require a ChangeLog. See: http://webkit.org/coding/contributing.html Found no modified ChangeLogs, cannot create a commit message. All changes require a ChangeLog. See: http://webkit.org/coding/contributing.html Updating OpenSource Current branch master is up to date. Full output: http://webkit-queues.webkit.org/results/696742 Created attachment 269157 [details]
Updated patch with ChangeLog entry
Created attachment 269159 [details]
Updated patch to also include feedback for removal of now-irrelevant build hack for gobject-intorspection
Comment on attachment 269159 [details] Updated patch to also include feedback for removal of now-irrelevant build hack for gobject-intorspection View in context: https://bugs.webkit.org/attachment.cgi?id=269159&action=review > Source/WebKit2/ChangeLog:7 > + Reviewed by Philip Chimento. Philip is not a reviewer; this line has official significance, so please don't list people here unless they set the r+ flag in Bugzilla. > Source/cmake/OptionsGTK.cmake:475 > macro(ADD_WHOLE_ARCHIVE_TO_LIBRARIES _list_name) It occurs to me that this macro does not belong in OptionsGTK.cmake as it has nothing to do with build options. Once we manage to land this patch, I will move it in a follow-up patch. Created attachment 269168 [details]
Updated reviewers in patch
Comment on attachment 269168 [details] Updated reviewers in patch Clearing flags on attachment: 269168 Committed r195174: <http://trac.webkit.org/changeset/195174> All reviewed patches have been landed. Closing bug. |