212524 2020-05-29 07:50:47 -0700 [GTK] Bubblewrap sandbox lacks support for sndio 2020-07-06 05:13:13 -0700 1 1 1 Unclassified WebKit WebKitGTK WebKit Local Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 contact+bugs.webkit.org webkit-unassigned aperez bugs-noreply clopez oldest_to_newest 1657246 0 contact+bugs.webkit.org 2020-05-29 07:50:47 -0700 Loosely related to https://bugs.webkit.org/show_bug.cgi?id=210101 (I'm using sndio as a workaround to this alsa bug) 1657248 1 400580 contact+bugs.webkit.org 2020-05-29 07:53:55 -0700 Created attachment 400580 Patch 1657253 2 400580 clopez 2020-05-29 07:59:32 -0700 Comment on attachment 400580 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=400580&action=review > Source/WebKit/ChangeLog:4 > + BubblewrapLauncher.cpp: Add paths required for sndio > + You have to add the bug number here. Check how other Changelog entries are written. > Source/WebKit/ChangeLog:11 > + No new tests (OOPS!). You have to remove this line and change it by something like "Covered by existing tests" 1657286 3 contact+bugs.webkit.org 2020-05-29 09:27:40 -0700 (In reply to Carlos Alberto Lopez Perez from comment #2) > Comment on attachment 400580 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=400580&action=review > > Source/WebKit/ChangeLog:11 > > + No new tests (OOPS!). > > You have to remove this line and change it by something like "Covered by > existing tests" Well there *is* no tests for sndio into the webkit bubblewrap sandbox so I'm not really sure what to write there. 1657298 4 400580 aperez 2020-05-29 09:48:11 -0700 Comment on attachment 400580 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=400580&action=review > Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:386 > + bindIfExists(args, "/tmp/sndio", BindFlags::ReadWrite); Does the patch work without this bind mount? I think it should be enought to bind the per-user directory (as done in the next two lines). I tried to run “sndiod” in my system and there is only a “/tmp/sndio-1000” directory (my user UID here is 1000), but no “/tmp/sndio” directory. 1657311 5 clopez 2020-05-29 10:21:04 -0700 (In reply to Haelwenn (lanodan) Monnier from comment #3) > (In reply to Carlos Alberto Lopez Perez from comment #2) > > Comment on attachment 400580 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=400580&action=review > > > Source/WebKit/ChangeLog:11 > > > + No new tests (OOPS!). > > > > You have to remove this line and change it by something like "Covered by > > existing tests" > > Well there *is* no tests for sndio into the webkit bubblewrap sandbox so I'm > not really sure what to write there. I know, but you have to remove that line "No new tests (OOPS!)." to make our tooling happy. So saying something along with what I suggested is good enough. You can also say something like "Feature has been manually tested, there is currently not automatic tests for it" 1657619 6 contact+bugs.webkit.org 2020-05-30 06:25:17 -0700 (In reply to Adrian Perez from comment #4) > Comment on attachment 400580 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=400580&action=review > > > Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:386 > > + bindIfExists(args, "/tmp/sndio", BindFlags::ReadWrite); > > Does the patch work without this bind mount? I think it should be enought to > bind the per-user directory (as done in the next two lines). I tried to run > “sndiod” in my system and there is only a “/tmp/sndio-1000” directory (my > user UID here is 1000), but no “/tmp/sndio” directory. Yes, the "/tmp/sndio" is needed when sndio is launched as superuser (which then drops to an unprivileged user like "sndiod"), this allows to share it between multiple users. 1657620 7 400667 contact+bugs.webkit.org 2020-05-30 06:37:28 -0700 Created attachment 400667 Patch 1657642 8 aperez 2020-05-30 11:48:55 -0700 (In reply to Haelwenn (lanodan) Monnier from comment #6) > (In reply to Adrian Perez from comment #4) > > Comment on attachment 400580 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=400580&action=review > > > > > Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:386 > > > + bindIfExists(args, "/tmp/sndio", BindFlags::ReadWrite); > > > > Does the patch work without this bind mount? I think it should be enought to > > bind the per-user directory (as done in the next two lines). I tried to run > > “sndiod” in my system and there is only a “/tmp/sndio-1000” directory (my > > user UID here is 1000), but no “/tmp/sndio” directory. > > Yes, the "/tmp/sndio" is needed when sndio is launched as superuser (which > then drops to an unprivileged user like "sndiod"), this allows to share it > between multiple users. Ah, good to know, thanks for commenting about this :) 1668976 9 contact+bugs.webkit.org 2020-07-05 22:57:21 -0700 Any updates on merging it? 1669020 10 400667 aperez 2020-07-06 05:07:29 -0700 Comment on attachment 400667 Patch Let's land this, patch LGTM now. Thanks for working on the patch! 1669022 11 ews-feeder 2020-07-06 05:13:12 -0700 Committed r263966: <https://trac.webkit.org/changeset/263966> All reviewed patches have been landed. Closing bug and clearing flags on attachment 400667. 400580 2020-05-29 07:53:55 -0700 2020-05-30 06:37:21 -0700 Patch bug-212524-20200529165353.patch text/plain 2344 contact+bugs.webkit.org U3VidmVyc2lvbiBSZXZpc2lvbjogMjYyMjkzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDBlY2RlZDUwNGJjYmExOTRi Y2I1YzM3ZmJjNjc1MDJiZDhhNGJiODUuLmQ1YWM1ZDJlZjU4YzI2NDJjYzg0YmIwNzg4M2RmMWRl MDkyNWEyNGMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTkgQEAKKzIwMjAtMDUtMjkgIEhhZWx3ZW5u IChsYW5vZGFuKSBNb25uaWVyICA8Y29udGFjdEBoYWNrdGl2aXMubWU+CisKKyAgICAgICAgQnVi Ymxld3JhcExhdW5jaGVyLmNwcDogQWRkIHBhdGhzIHJlcXVpcmVkIGZvciBzbmRpbworCisgICAg ICAgIEluZm9ybWF0aW9uIGFib3V0IHRoZSBwYXRocyBjdXJyZW50bHkgaW5zJ3QgaW50byB0aGUg c25kaW8KKyAgICAgICAgZG9jdW1lbnRhdGlvbiBidXQgaXQgY2FuIGJlIGZvdW5kIGluIHRoaXMg ZW1haWw6CisgICAgICAgIGh0dHA6Ly93d3cuc25kaW8ub3JnL2FyY2gvMDA3Ny5odG1sCisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTm8gbmV3IHRlc3Rz IChPT1BTISkuCisKKyAgICAgICAgKiBVSVByb2Nlc3MvTGF1bmNoZXIvZ2xpYi9CdWJibGV3cmFw TGF1bmNoZXIuY3BwOgorICAgICAgICAoV2ViS2l0OjpiaW5kU25kaW8pOgorICAgICAgICAoV2Vi S2l0OjpidWJibGV3cmFwU3Bhd24pOgorCiAyMDIwLTA1LTI5ICBDYXJsb3MgR2FyY2lhIENhbXBv cyAgPGNnYXJjaWFAaWdhbGlhLmNvbT4KIAogICAgICAgICBbR1RLNF0gSW1wbGVtZW50IEhUVFAg YXV0aCBkaWFsb2cKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL0xhdW5jaGVy L2dsaWIvQnViYmxld3JhcExhdW5jaGVyLmNwcCBiL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL0xh dW5jaGVyL2dsaWIvQnViYmxld3JhcExhdW5jaGVyLmNwcAppbmRleCBhZDMwMWFiMjZlYjNlMDMy ZjAzMzRlODgxNjU0YzQ5Y2UzNGJkNTA2Li4wNmRjZDQxYjFkOGEzMjM2MDkzZWY1YjcxZjhlZmNm NTA4OTRkYjQyIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9MYXVuY2hlci9n bGliL0J1YmJsZXdyYXBMYXVuY2hlci5jcHAKKysrIGIvU291cmNlL1dlYktpdC9VSVByb2Nlc3Mv TGF1bmNoZXIvZ2xpYi9CdWJibGV3cmFwTGF1bmNoZXIuY3BwCkBAIC0zODEsNiArMzgxLDE4IEBA IHN0YXRpYyB2b2lkIGJpbmRQdWxzZShWZWN0b3I8Q1N0cmluZz4mIGFyZ3MpCiAgICAgYmluZElm RXhpc3RzKGFyZ3MsICIvZGV2L3NuZCIsIEJpbmRGbGFnczo6RGV2aWNlKTsKIH0KIAorc3RhdGlj IHZvaWQgYmluZFNuZGlvKFZlY3RvcjxDU3RyaW5nPiYgYXJncykKK3sKKyAgICBiaW5kSWZFeGlz dHMoYXJncywgIi90bXAvc25kaW8iLCBCaW5kRmxhZ3M6OlJlYWRXcml0ZSk7CisKKyAgICBHVW5p cXVlUHRyPGNoYXI+IHNuZGlvVWlkRGlyKGdfc3RyZHVwX3ByaW50ZigiL3RtcC9zbmRpby0lZCIs IGdldHVpZCgpKSk7CisgICAgYmluZElmRXhpc3RzKGFyZ3MsIHNuZGlvVWlkRGlyLmdldCgpLCBC aW5kRmxhZ3M6OlJlYWRXcml0ZSk7CisKKyAgICBjb25zdCBjaGFyKiBob21lRGlyID0gZ19nZXRf aG9tZV9kaXIoKTsKKyAgICBHVW5pcXVlUHRyPGNoYXI+IHNuZGlvSG9tZURpcihnX2J1aWxkX2Zp bGVuYW1lKGhvbWVEaXIsICIuc25kaW8iLCBudWxscHRyKSk7CisgICAgYmluZElmRXhpc3RzKGFy Z3MsIHNuZGlvSG9tZURpci5nZXQoKSwgQmluZEZsYWdzOjpSZWFkV3JpdGUpOworfQorCiBzdGF0 aWMgdm9pZCBiaW5kRm9udHMoVmVjdG9yPENTdHJpbmc+JiBhcmdzKQogewogICAgIGNvbnN0IGNo YXIqIGNvbmZpZ0RpciA9IGdfZ2V0X3VzZXJfY29uZmlnX2RpcigpOwpAQCAtODA3LDYgKzgxOSw3 IEBAIEdSZWZQdHI8R1N1YnByb2Nlc3M+IGJ1YmJsZXdyYXBTcGF3bihHU3VicHJvY2Vzc0xhdW5j aGVyKiBsYXVuY2hlciwgY29uc3QgUHJvY2VzCiAgICAgICAgIGJpbmREQnVzU2Vzc2lvbihzYW5k Ym94QXJncywgcHJveHkpOwogICAgICAgICAvLyBGSVhNRTogV2Ugc2hvdWxkIG1vdmUgdG8gUGlw ZXdpcmUgYXMgc29vbiBhcyB2aWFibGUsIFB1bHNlIGRvZXNuJ3QgcmVzdHJpY3QgY2xpZW50cyBh dG0uCiAgICAgICAgIGJpbmRQdWxzZShzYW5kYm94QXJncyk7CisgICAgICAgIGJpbmRTbmRpbyhz YW5kYm94QXJncyk7CiAgICAgICAgIGJpbmRGb250cyhzYW5kYm94QXJncyk7CiAgICAgICAgIGJp bmRHU3RyZWFtZXJEYXRhKHNhbmRib3hBcmdzKTsKICAgICAgICAgYmluZE9wZW5HTChzYW5kYm94 QXJncyk7Cg== 400667 2020-05-30 06:37:28 -0700 2020-07-06 05:13:13 -0700 Patch bug-212524-20200530153727.patch text/plain 2440 contact+bugs.webkit.org U3VidmVyc2lvbiBSZXZpc2lvbjogMjYyMjkzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDBlY2RlZDUwNGJjYmExOTRi Y2I1YzM3ZmJjNjc1MDJiZDhhNGJiODUuLmMwNDlkMjk5NGRjNDgzNDljMzE1ODMwNDhiNmY4YTNi MmQ4YzVmN2MgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjAgQEAKKzIwMjAtMDUtMjkgIEhhZWx3ZW5u IChsYW5vZGFuKSBNb25uaWVyICA8Y29udGFjdEBoYWNrdGl2aXMubWU+CisKKyAgICAgICAgQnVi Ymxld3JhcExhdW5jaGVyLmNwcDogQWRkIHBhdGhzIHJlcXVpcmVkIGZvciBzbmRpbworICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjEyNTI0CisKKyAgICAg ICAgSW5mb3JtYXRpb24gYWJvdXQgdGhlIHBhdGhzIGN1cnJlbnRseSBpbnMndCBpbnRvIHRoZSBz bmRpbworICAgICAgICBkb2N1bWVudGF0aW9uIGJ1dCBpdCBjYW4gYmUgZm91bmQgaW4gdGhpcyBl bWFpbDoKKyAgICAgICAgaHR0cDovL3d3dy5zbmRpby5vcmcvYXJjaC8wMDc3Lmh0bWwKKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBNYW51YWxseSB0ZXN0 ZWQsIHRoZXJlIGlzIGN1cnJlbnRseSBubyBhdXRvbWF0aWMgdGVzdHMgZm9yIGl0CisKKyAgICAg ICAgKiBVSVByb2Nlc3MvTGF1bmNoZXIvZ2xpYi9CdWJibGV3cmFwTGF1bmNoZXIuY3BwOgorICAg ICAgICAoV2ViS2l0OjpiaW5kU25kaW8pOgorICAgICAgICAoV2ViS2l0OjpidWJibGV3cmFwU3Bh d24pOgorCiAyMDIwLTA1LTI5ICBDYXJsb3MgR2FyY2lhIENhbXBvcyAgPGNnYXJjaWFAaWdhbGlh LmNvbT4KIAogICAgICAgICBbR1RLNF0gSW1wbGVtZW50IEhUVFAgYXV0aCBkaWFsb2cKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL0xhdW5jaGVyL2dsaWIvQnViYmxld3JhcExh dW5jaGVyLmNwcCBiL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL0xhdW5jaGVyL2dsaWIvQnViYmxl d3JhcExhdW5jaGVyLmNwcAppbmRleCBhZDMwMWFiMjZlYjNlMDMyZjAzMzRlODgxNjU0YzQ5Y2Uz NGJkNTA2Li4wNmRjZDQxYjFkOGEzMjM2MDkzZWY1YjcxZjhlZmNmNTA4OTRkYjQyIDEwMDY0NAot LS0gYS9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9MYXVuY2hlci9nbGliL0J1YmJsZXdyYXBMYXVu Y2hlci5jcHAKKysrIGIvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvTGF1bmNoZXIvZ2xpYi9CdWJi bGV3cmFwTGF1bmNoZXIuY3BwCkBAIC0zODEsNiArMzgxLDE4IEBAIHN0YXRpYyB2b2lkIGJpbmRQ dWxzZShWZWN0b3I8Q1N0cmluZz4mIGFyZ3MpCiAgICAgYmluZElmRXhpc3RzKGFyZ3MsICIvZGV2 L3NuZCIsIEJpbmRGbGFnczo6RGV2aWNlKTsKIH0KIAorc3RhdGljIHZvaWQgYmluZFNuZGlvKFZl Y3RvcjxDU3RyaW5nPiYgYXJncykKK3sKKyAgICBiaW5kSWZFeGlzdHMoYXJncywgIi90bXAvc25k aW8iLCBCaW5kRmxhZ3M6OlJlYWRXcml0ZSk7CisKKyAgICBHVW5pcXVlUHRyPGNoYXI+IHNuZGlv VWlkRGlyKGdfc3RyZHVwX3ByaW50ZigiL3RtcC9zbmRpby0lZCIsIGdldHVpZCgpKSk7CisgICAg YmluZElmRXhpc3RzKGFyZ3MsIHNuZGlvVWlkRGlyLmdldCgpLCBCaW5kRmxhZ3M6OlJlYWRXcml0 ZSk7CisKKyAgICBjb25zdCBjaGFyKiBob21lRGlyID0gZ19nZXRfaG9tZV9kaXIoKTsKKyAgICBH VW5pcXVlUHRyPGNoYXI+IHNuZGlvSG9tZURpcihnX2J1aWxkX2ZpbGVuYW1lKGhvbWVEaXIsICIu c25kaW8iLCBudWxscHRyKSk7CisgICAgYmluZElmRXhpc3RzKGFyZ3MsIHNuZGlvSG9tZURpci5n ZXQoKSwgQmluZEZsYWdzOjpSZWFkV3JpdGUpOworfQorCiBzdGF0aWMgdm9pZCBiaW5kRm9udHMo VmVjdG9yPENTdHJpbmc+JiBhcmdzKQogewogICAgIGNvbnN0IGNoYXIqIGNvbmZpZ0RpciA9IGdf Z2V0X3VzZXJfY29uZmlnX2RpcigpOwpAQCAtODA3LDYgKzgxOSw3IEBAIEdSZWZQdHI8R1N1YnBy b2Nlc3M+IGJ1YmJsZXdyYXBTcGF3bihHU3VicHJvY2Vzc0xhdW5jaGVyKiBsYXVuY2hlciwgY29u c3QgUHJvY2VzCiAgICAgICAgIGJpbmREQnVzU2Vzc2lvbihzYW5kYm94QXJncywgcHJveHkpOwog ICAgICAgICAvLyBGSVhNRTogV2Ugc2hvdWxkIG1vdmUgdG8gUGlwZXdpcmUgYXMgc29vbiBhcyB2 aWFibGUsIFB1bHNlIGRvZXNuJ3QgcmVzdHJpY3QgY2xpZW50cyBhdG0uCiAgICAgICAgIGJpbmRQ dWxzZShzYW5kYm94QXJncyk7CisgICAgICAgIGJpbmRTbmRpbyhzYW5kYm94QXJncyk7CiAgICAg ICAgIGJpbmRGb250cyhzYW5kYm94QXJncyk7CiAgICAgICAgIGJpbmRHU3RyZWFtZXJEYXRhKHNh bmRib3hBcmdzKTsKICAgICAgICAgYmluZE9wZW5HTChzYW5kYm94QXJncyk7Cg==