Bug 153117

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:

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	Flags
full build log	none
build log	none
patch to add -Wl,-all_load	mcatanzaro: review+, commit-queue: commit-queue-
Updated patch with ChangeLog entry	jeremyhu: review+
Updated patch to also include feedback for removal of now-irrelevant build hack for gobject-intorspection	jeremyhu: review+, mcatanzaro: commit-queue-
Updated reviewers in patch	none

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-	Details Formatted Diff Diff
Updated patch with ChangeLog entry (2.08 KB, patch) 2016-01-15 23:16 PST, Jeremy Huddleston Sequoia	jeremyhu: review+	Details Formatted Diff Diff
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-	Details Formatted Diff Diff
Updated reviewers in patch (3.39 KB, patch) 2016-01-16 07:59 PST, Jeremy Huddleston Sequoia	no flags	Details Formatted Diff Diff
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.