[iOS] Copy sandbox profiles into $BUILT_PRODUCTS_DIR during engineering builds
Created attachment 364156 [details] Patch
Is there any way to retrofit the existing Sandbox Profiles target do this in engineering builds?
Perhaps just changing the shared scheme (and engineering makefile) to build Sandbox Profiles in addition to All?
Comment on attachment 364156 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364156&action=review > Source/WebKit/ChangeLog:11 > + To make this task easier, this patch teaches WebKit to copy the iOS sandbox profiles into Does this mean that perf testing results on iOS will be misleading when using release builds? When discussing a change like this in the past, some people thought that that would be a show stopper.
(In reply to Alexey Proskuryakov from comment #4) > Comment on attachment 364156 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=364156&action=review > > > Source/WebKit/ChangeLog:11 > > + To make this task easier, this patch teaches WebKit to copy the iOS sandbox profiles into > > Does this mean that perf testing results on iOS will be misleading when > using release builds? When discussing a change like this in the past, some > people thought that that would be a show stopper. I don't think this would change iOS perf testing results at all, because nothing in this patch causes the copied sandbox profiles to be installed on an iOS device. That still requires a manual step. (Of course, it's not clear to me how iOS perf testing is dealing with this problem now. Surely not applying a necessary sandbox profile change on a device will affect perf testing results.)
Comment on attachment 364156 [details] Patch Attachment 364156 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11444112 New failing tests: imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-constructor.html
Created attachment 364165 [details] Archive of layout-test-results from ews107 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 364156 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364156&action=review >>> Source/WebKit/ChangeLog:11 >>> + To make this task easier, this patch teaches WebKit to copy the iOS sandbox profiles into >> >> Does this mean that perf testing results on iOS will be misleading when using release builds? When discussing a change like this in the past, some people thought that that would be a show stopper. > > I don't think this would change iOS perf testing results at all, because nothing in this patch causes the copied sandbox profiles to be installed on an iOS device. That still requires a manual step. > > (Of course, it's not clear to me how iOS perf testing is dealing with this problem now. Surely not applying a necessary sandbox profile change on a device will affect perf testing results.) Perhaps I don’t understand what the patch does, going from the ChangeLog. But if it puts profiles into /usr/local/share/sandbox in the built archive, then what is the missing manual step? As for perf impact - not installing profiles means that we are missing on testing the impact of changes, but installing them into this directory results in enormous process launch slowdown. At least that’s what I’ve been told. So it’s better to not perf test these changes than to run all testing in a wildly different scenario.
Perhaps you are saying that you don’t expect package-root to be the def in this scenario?
(In reply to Alexey Proskuryakov from comment #9) > Perhaps you are saying that you don’t expect package-root to be the def in > this scenario? Yes, the manual steps would be to run package-root (or otherwise package your built products into a root, install the root on a device, and run the sbutil command (or reboot I think). I think for performance testing we build with build-root, and my change has no impact on Production builds.
Comment on attachment 364156 [details] Patch r- just as a reminder to address my comment #2 and comment #3. If it turns out that there’s no way to reuse the existing target, you can r? this version again.
> I think for performance testing we build with build-root, and my change has no impact on Production builds. Bots do, but I've heard it many times from folks who require preserving the ability to do perf testing with local release builds.