Bug 240289 - Need to add DYLD_LIBRARY_PATH for XPC services
Summary: Need to add DYLD_LIBRARY_PATH for XPC services
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-05-10 13:57 PDT by Michael Saboff
Modified: 2022-05-11 14:42 PDT (History)
3 users (show)

See Also:

Attachments
Speculative Fix (1.73 KB, patch)
2022-05-10 14:09 PDT, Michael Saboff 		no flags Details | Formatted Diff | Diff
Updated patch after discussing with David Quesada offline (1.94 KB, patch)
2022-05-10 15:36 PDT, Michael Saboff 		no flags Details | Formatted Diff | Diff
Updated Patch (3.13 KB, patch)
2022-05-11 10:11 PDT, Michael Saboff 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2022-05-10 13:57:11 PDT 
In addition to frameworks, there can be dylibs in the secondary path.  Therefore in addition to updating DYLD_VERSIONED_FRAMEWORK_PATH, we need to update DYLD_VERSIONED_LIBRARY_PATH.
Comment 1 Michael Saboff 2022-05-10 13:57:32 PDT 
<rdar://90858923>
Comment 2 Michael Saboff 2022-05-10 14:09:43 PDT 
Created attachment 459133 [details]
Speculative Fix
Comment 3 Michael Saboff 2022-05-10 15:36:56 PDT 
Created attachment 459137 [details]
Updated patch after discussing with David Quesada offline
Comment 4 David Quesada 2022-05-10 17:03:43 PDT 
Comment on attachment 459137 [details]
Updated patch after discussing with David Quesada offline

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

It's not 100% related (since it's not specific to the staging install path configuration), but along then theme of loading *libraries* in addition to frameworks, could you also update registerDaemonWithLaunchD() in WebPushToolMain.mm? That builds a dictionary of environment variables that currently includes DYLD_FRAMEWORK_PATH, but also needs to include DYLD_LIBRARY_PATH to make sure the whole stack loads all the right code.

> Source/WebKit/Configurations/BaseTarget.xcconfig:129
>  OTHER_LDFLAGS_VERSIONED_FRAMEWORK_PATH_YES[arch=arm64*][sdk=macosx11*] = -Wl,-dyld_env,DYLD_FRAMEWORK_PATH=$(WK_INSTALL_PATH_PREFIX)/System/Library/StagedFrameworks/Safari;

This should set DYLD_LIBRARY_PATH.
Comment 5 Michael Saboff 2022-05-11 09:37:42 PDT 
Updated the bug for the setting of DYLD_LIBRARY_PATH and DLYD_VERSIONED_LIBRARY_PATH environment variables for XPC services.
Comment 6 Michael Saboff 2022-05-11 10:11:05 PDT 
Created attachment 459171 [details]
Updated Patch
Comment 7 David Quesada 2022-05-11 11:56:40 PDT 
Looks good to me.
Comment 8 Alexey Proskuryakov 2022-05-11 12:01:35 PDT 
Comment on attachment 459171 [details]
Updated Patch

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

> Source/WebKit/Configurations/BaseTarget.xcconfig:129
> +OTHER_LDFLAGS_VERSIONED_FRAMEWORK_PATH_YES[arch=arm64*][sdk=macosx11*] = -Wl,-dyld_env,DYLD_FRAMEWORK_PATH=$(WK_INSTALL_PATH_PREFIX)/System/Library/StagedFrameworks/Safari,-dyld_env,DYLD_LIBRARY_PATH=$(WK_INSTALL_PATH_PREFIX)/System/Library/StagedFrameworks/Safari;

I don't know why there is a special case for macosx11 on Apple Silicon only, but that's preexisting code, no change.
Comment 9 EWS 2022-05-11 14:42:52 PDT 
Committed r294072 (250470@main): <https://commits.webkit.org/250470@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 459171 [details].