RESOLVED FIXED 153117
[GTK OSX] libGObjectDOMBindings.a is not linked into libwebkit2gtk-4.0.37.dylib 
https://bugs.webkit.org/show_bug.cgi?id=153117
Summary [GTK OSX] libGObjectDOMBindings.a is not linked into libwebkit2gtk-4.0.37.dylib
Jeremy Huddleston Sequoia
Reported 2016-01-14 22:05:04 PST
As reported at https://trac.macports.org/ticket/50334 We configure using: cmake -DCMAKE_INSTALL_PREFIX=/opt/local -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_COLOR_MAKEFILE=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_BUILD_WITH_INSTALL_RPATH=ON -DCMAKE_INSTALL_RPATH=/opt/local/lib -DCMAKE_INSTALL_NAME_DIR=/opt/local/lib -DCMAKE_SYSTEM_PREFIX_PATH="/opt/local;/usr" -DCMAKE_MODULE_PATH=/opt/local/share/cmake/Modules -DCMAKE_FIND_FRAMEWORK=LAST -Wno-dev -DPORT=GTK -DENABLE_X11_TARGET=ON -DENABLE_QUARTZ_TARGET=OFF -DENABLE_INTROSPECTION=OFF -DLLVM_CONFIG_EXE=/opt/local/bin/llvm-config-mp-3.7 -DENABLE_VIDEO=ON -DENABLE_PLUGIN_PROCESS_GTK2=ON -DCMAKE_C_FLAGS_RELEASE="-DNDEBUG" -DCMAKE_CXX_FLAGS_RELEASE="-DNDEBUG" -DCMAKE_OSX_ARCHITECTURES="x86_64" -DCMAKE_OSX_DEPLOYMENT_TARGET="10.11" -DCMAKE_OSX_SYSROOT="/" And install libwebkit2gtk-4.0.37.13.0.dylib by copying it from the lib directory to workaround bug #152651. The resulting libwebkit2gtk-4.0.37.13.0.dylib is missing some symbols, and dependent projects fail to link: Undefined symbols for architecture x86_64: "_webkit_dom_document_get_elements_by_tag_name", referenced from: _ephy_web_overview_document_loaded in libephywebextension_la-ephy-web-overview.o _ephy_web_dom_utils_get_application_title in libephymisc.a(libephymisc_la-ephy-web-dom-utils.o) _ephy_web_dom_utils_get_best_icon in libephymisc.a(libephymisc_la-ephy-web-dom-utils.o) "_webkit_dom_dom_window_webkit_message_handlers_post_message", referenced from: _request_decision_on_storing in libephywebextension_la-ephy-web-extension.o "_webkit_dom_html_input_element_is_edited", referenced from: _ephy_web_dom_utils_has_modified_forms in libephymisc.a(libephymisc_la-ephy-web-dom-utils.o) "_webkit_dom_html_text_area_element_is_edited", referenced from: _ephy_web_dom_utils_has_modified_forms in libephymisc.a(libephymisc_la-ephy-web-dom-utils.o) --- $ nm /opt/local/lib/libwebkit2gtk-4.0.dylib | egrep '(_webkit_dom_document_get_elements_by_tag_name|_webkit_dom_dom_window_webkit_message_handlers_post_message|_webkit_dom_html_input_element_is_edited|_webkit_dom_html_text_area_element_is_edited)' 0000000000d68f42 T _webkit_dom_document_get_elements_by_tag_name_as_html_collection 0000000000d6968f T _webkit_dom_document_get_elements_by_tag_name_ns_as_html_collection
Attachments
full build log (1.20 MB, application/x-bz2)
2016-01-14 22:24 PST, Jeremy Huddleston Sequoia 		no flags
Details
build log (1.34 MB, application/x-bz2)
2016-01-14 22:44 PST, Jeremy Huddleston Sequoia 		no flags
Details
patch to add -Wl,-all_load (1.43 KB, patch)
2016-01-15 01:23 PST, Jeremy Huddleston Sequoia 		mcatanzaro: review+
commit-queue: commit-queue-
DetailsFormatted DiffDiff
Updated patch with ChangeLog entry (2.08 KB, patch)
2016-01-15 23:16 PST, Jeremy Huddleston Sequoia 		jeremyhu: review+
DetailsFormatted DiffDiff
Updated patch to also include feedback for removal of now-irrelevant build hack for gobject-intorspection (3.39 KB, patch)
2016-01-15 23:29 PST, Jeremy Huddleston Sequoia 		jeremyhu: review+
mcatanzaro: commit-queue-
DetailsFormatted DiffDiff
Updated reviewers in patch (3.39 KB, patch)
2016-01-16 07:59 PST, Jeremy Huddleston Sequoia 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Jeremy Huddleston Sequoia
Comment 1 2016-01-14 22:24:31 PST
Created attachment 269033 [details] full build log
Jeremy Huddleston Sequoia
Comment 2 2016-01-14 22:33:39 PST
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
Jeremy Huddleston Sequoia
Comment 3 2016-01-14 22:44:42 PST
Created attachment 269035 [details] build log
Jeremy Huddleston Sequoia
Comment 4 2016-01-14 23:19:21 PST
Actually, libGObjectDOMBindings.a is on the link line, but we're missing -all_load.
Jeremy Huddleston Sequoia
Comment 5 2016-01-14 23:21:24 PST
This was caused by an incorrect fix for bug #144557
Jeremy Huddleston Sequoia
Comment 6 2016-01-15 01:23:02 PST
Created attachment 269043 [details] patch to add -Wl,-all_load
Michael Catanzaro
Comment 7 2016-01-15 07:06:28 PST
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?
Jeremy Huddleston Sequoia
Comment 8 2016-01-15 08:57:43 PST
> 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.
Philip Chimento
Comment 9 2016-01-15 09:17:11 PST
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)
Jeremy Huddleston Sequoia
Comment 10 2016-01-15 12:16:03 PST
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.
Michael Catanzaro
Comment 11 2016-01-15 13:37:59 PST
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?
Philip Chimento
Comment 12 2016-01-15 15:04:37 PST
Probably it can be removed if -all_load works as expected.
Jeremy Huddleston Sequoia
Comment 13 2016-01-15 15:08:03 PST
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.
WebKit Commit Bot
Comment 14 2016-01-15 19:17:16 PST
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
Jeremy Huddleston Sequoia
Comment 15 2016-01-15 23:16:27 PST
Created attachment 269157 [details] Updated patch with ChangeLog entry
Jeremy Huddleston Sequoia
Comment 16 2016-01-15 23:29:28 PST
Created attachment 269159 [details] Updated patch to also include feedback for removal of now-irrelevant build hack for gobject-intorspection
Michael Catanzaro
Comment 17 2016-01-16 06:43:42 PST
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.
Jeremy Huddleston Sequoia
Comment 18 2016-01-16 07:59:54 PST
Created attachment 269168 [details] Updated reviewers in patch
WebKit Commit Bot
Comment 19 2016-01-16 12:50:35 PST
Comment on attachment 269168 [details] Updated reviewers in patch Clearing flags on attachment: 269168 Committed r195174: <http://trac.webkit.org/changeset/195174>
WebKit Commit Bot
Comment 20 2016-01-16 12:50:39 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.