Bug 233080 - [webkitpy] Symlink daemons into simulator runtime root
Summary: [webkitpy] Symlink daemons into simulator runtime root
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-11-12 16:52 PST by Jonathan Bedard
Modified: 2021-11-16 07:50 PST (History)
5 users (show)

See Also:


Attachments
Patch (8.76 KB, patch)
2021-11-12 16:59 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (4.99 KB, patch)
2021-11-15 10:58 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Bedard 2021-11-12 16:52:43 PST
WebKit daemons live inside the WebKit framework. However, we can't start these daemons during testing because they live outside the runtime root of the simulator. If we symlink our daemons into the simulator runtime root, we can safely test them.
Comment 1 Radar WebKit Bug Importer 2021-11-12 16:53:01 PST
<rdar://problem/85362551>
Comment 2 Jonathan Bedard 2021-11-12 16:59:10 PST
Created attachment 444125 [details]
Patch
Comment 3 Jonathan Bedard 2021-11-12 16:59:36 PST
Pull-request: https://github.com/WebKit/WebKit/pull/38
Comment 4 Brady Eidson 2021-11-12 17:06:33 PST
Comment on attachment 444125 [details]
Patch

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

> Tools/Scripts/webkitpy/api_tests/manager.py:156
> +            # Daemons can't be called from outside the runtime root

I would phrase a bit differently, like:
"A Daemons executable path must be located within the runtime root"
Comment 5 Alexey Proskuryakov 2021-11-12 17:25:36 PST
Comment on attachment 444125 [details]
Patch

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

> Tools/ChangeLog:3
> +        [webkitpy] Symlink daemons into simulator runtime root

Does this modify what's inside /Applications/Xcode.app? If so, this seems very undesirable.
Comment 6 Jonathan Bedard 2021-11-12 17:31:12 PST
(In reply to Alexey Proskuryakov from comment #5)
> Comment on attachment 444125 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=444125&action=review
> 
> > Tools/ChangeLog:3
> > +        [webkitpy] Symlink daemons into simulator runtime root
> 
> Does this modify what's inside /Applications/Xcode.app? If so, this seems
> very undesirable.

Yes, not sure there is another way to do this, though. Simulators can't interact with daemons outside of their runtime root. We're sticking the binaries in a place that shouldn't effect anything.
Comment 7 Brady Eidson 2021-11-12 18:42:39 PST
(In reply to Jonathan Bedard from comment #6)
> (In reply to Alexey Proskuryakov from comment #5)
> > Comment on attachment 444125 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=444125&action=review
> > 
> > > Tools/ChangeLog:3
> > > +        [webkitpy] Symlink daemons into simulator runtime root
> > 
> > Does this modify what's inside /Applications/Xcode.app? If so, this seems
> > very undesirable.
> 
> Yes, not sure there is another way to do this, though. Simulators can't
> interact with daemons outside of their runtime root. We're sticking the
> binaries in a place that shouldn't effect anything.

Right.

We chose a symlink location that has zero chance of conflicting with any other built-in file that might end up in a future runtime root, and a near-zero chance of affecting anything else with the runtime.

To test our daemons (which is a quite desirable thing to do) we have no other identified options.
Comment 8 Jonathan Bedard 2021-11-12 20:47:42 PST
Landed https://commits.webkit.org/244216@main (r285772)!

If we come up with a better technique in the coming weeks, happy to remove this. I think it's important to unblock testing for now, though.
Comment 9 Brady Eidson 2021-11-12 23:44:18 PST
(In reply to Jonathan Bedard from comment #8)
> Landed https://commits.webkit.org/244216@main (r285772)!
> 
> If we come up with a better technique in the coming weeks, happy to remove
> this. I think it's important to unblock testing for now, though.

This patch worked for you locally, for me locally, and for EWS here when you put the patch in the queue.

But with my patch over in https://bugs.webkit.org/show_bug.cgi?id=232982 it fails to do the link:
https://ews-build.webkit.org/#/builders/9/builds/58765
Comment 10 Alexey Proskuryakov 2021-11-13 08:10:44 PST
I think that this is strictly unacceptable, whether there is another solution or not. I will be reverting this change.
Comment 11 Jonathan Bedard 2021-11-13 13:59:00 PST
Reverted r285772 for reason:

Ownership issues in some XCode installs

Committed r285779 (244223@main): <https://commits.webkit.org/244223@main>
Comment 12 Jonathan Bedard 2021-11-13 14:00:40 PST
Reverted the change because it didn't unblock Brady anyways and I think Brady's patch meant that this change was breaking some EWS queues.

I do think that symlinking is still the right approach, but let's figure out our options next week.
Comment 13 Jonathan Bedard 2021-11-15 10:58:34 PST
Created attachment 444276 [details]
Patch
Comment 14 Jonathan Bedard 2021-11-16 07:50:08 PST
Brady found another way to solve this problem, closing this bug.