Summary: | [GTK] Broken build with undefined references to shm_open / shm_unlink | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kalev Lember <kalevlember> | ||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cgarcia, dbates, dominique-webkit.org, mrobinson, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Kalev Lember
2012-11-24 18:39:05 PST
Created attachment 175871 [details]
Patch
Comment on attachment 175871 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175871&action=review > configure.ac:1128 > + save_LIBS="$LIBS" > + AC_SEARCH_LIBS([shm_open], [rt], [SHM_LIBS="-lrt"]) > + AC_SUBST(SHM_LIBS) > + LIBS="$save_LIBS" What's going on here with save_LIBS? AC_CHECK_LIB and AC_SEARCH_LIBS write the result to the LIBS variable and I didn't want to clobber the contents of the variable here. I'm using save_LIBS to restore the original LIBS after calling AC_SEARCH_LIBS(). Perhaps it doesn't make much of a difference right here and right now, but it can avoid surprises in the future. I guess the macros were written with small build systems in mind where only one LIBS variable is sufficient; in webkitgtk with several libraries and executables produced, it seems better to avoid using the global LIBS variable. For what it's worth, cairo does the same thing with save_LIBS: http://cgit.freedesktop.org/cairo/tree/configure.ac#n796 Or would it be better to do something like this? (untested) save_LIBS="$LIBS" LIBS='' AC_SEARCH_LIBS([shm_open], [rt]) SHM_LIBS="$LIBS" AC_SUBST(SHM_LIBS) LIBS="$save_LIBS" My preference is that we use this work-around for all instances of AC_SEARCH_LIBS or none of them. So far we haven't had an issue with it. Perhaps we can avoid it here and just be careful not to use a variable named LIBS? Let me know if I'm misunderstanding you. Created attachment 176015 [details]
Patch v2
Fair enough, uploaded a new version with the save_LIBS workaround removed. I agree, should be fine as long as we avoid using LIBS.
Comment on attachment 176015 [details]
Patch v2
Thanks for fixing this!
Comment on attachment 176015 [details] Patch v2 Rejecting attachment 176015 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ls/Scripts/update-webkit line 108. Failed to run "['Tools/Scripts/update-webkit', '--chromium', '--force-update']" exit_code: 2 hile loading dependencies of All.gyp while trying to load All.gyp Error: 'depot_tools/gclient sync --force --reset --delete_unversioned_trees' failed 3 tries and returned 512 at Tools/Scripts/update-webkit-chromium line 96. Re-trying 'depot_tools/gclient sync --force --reset --delete_unversioned_trees' No such file or directory at Tools/Scripts/update-webkit line 108. Full output: http://queues.webkit.org/results/14986694 Comment on attachment 176015 [details] Patch v2 Clearing flags on attachment: 176015 Committed r135761: <http://trac.webkit.org/changeset/135761> All reviewed patches have been landed. Closing bug. *** Bug 104118 has been marked as a duplicate of this bug. *** |