230969 2021-09-29 10:43:46 -0700 [GTK][WPE] Enable bwrap launcher build on bots 2021-10-03 08:06:39 -0700 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 pnormand pnormand annulen bugs-noreply clopez ews-watchlist gyuyoung.kim mcatanzaro ryuan.choi sergio oldest_to_newest 1798444 0 pnormand 2021-09-29 10:43:46 -0700 So that it's code does not bitrot. 1798445 1 439626 pnormand 2021-09-29 10:44:56 -0700 Created attachment 439626 Patch 1798510 2 439626 mcatanzaro 2021-09-29 13:06:22 -0700 Comment on attachment 439626 Patch Ehh, I can't decide if this is a good change or not. It seems reasonable, so r=me if you decide to proceed. Here's what I'm thinking: building bubblewrap sandbox enabled on buildbots and EWS is definitely good to do. We surely want all tests to run with the sandbox enabled, because they are not meaningfully testing the code that users actually run otherwise (or, in cases where the sandbox is not yet enabled, the code that we want users to actually run). So we surely want to do that. The downside is that now we need to modify each downstream flatpak runtime and each flatpak app that bundles WebKitGTK to build with -DENABLE_BUBBLEWRAP_SANDBOX=OFF (because bubblewrap will not be available in these environments, since it is useless under flatpak). That is hardly a big deal. For the GNOME runtime, it would be a one-line change. It might confuse some app developers, but hopefully most app developers inherit WebKitGTK from a runtime instead of building it themselves. So... I think that's OK. The alternative way to accomplish this would be to enable -DENABLE_BUBBLEWRAP_SANDBOX at the buildbot config or build-webkit level rather than here in CMake. Modifying the config of every bot would be annoying, but perhaps this could be something that build-webkit just knows to pass, because build-webkit is the one special time that we want to build with bubblewrap sandbox enabled inside a flatpak environment? Whatever. Your call. 1798513 3 mcatanzaro 2021-09-29 13:10:21 -0700 (In reply to Michael Catanzaro from comment #2) > We surely > want all tests to run with the sandbox enabled, because they are not > meaningfully testing the code that users actually run otherwise (or, in > cases where the sandbox is not yet enabled, the code that we want users to > actually run). So we surely want to do that. Also I know we want to avoid mistakes like: spend months developing feature using build-webkit, then discover it doesn't work with the sandbox enabled and is really hard to redesign. I understand you've been there, done that with a web process feature that was going to depend on host network access. That sounds horribly disheartening. Much better to notice the problems immediately instead. 1798515 4 pnormand 2021-09-29 13:13:38 -0700 In g-b-m you already build-depend on bwrap: https://gitlab.gnome.org/GNOME/gnome-build-meta/-/blob/master/elements/sdk/webkitgtk.bst#L12 Was that an oversight? IIUC the bwrap process launcher won't be used at runtime anyway if the UIProcess is running in a flatpak runtime. 1798523 5 pnormand 2021-09-29 13:21:03 -0700 (In reply to Philippe Normand from comment #4) > In g-b-m you already build-depend on bwrap: > > https://gitlab.gnome.org/GNOME/gnome-build-meta/-/blob/master/elements/sdk/ > webkitgtk.bst#L12 > > Was that an oversight? > > IIUC the bwrap process launcher won't be used at runtime anyway if the > UIProcess is running in a flatpak runtime. Or maybe I am misunderstanding this: https://github.com/WebKit/WebKit/blob/main/Source/WebKit/UIProcess/Launcher/glib/ProcessLauncherGLib.cpp#L174..L184 1798855 6 mcatanzaro 2021-09-30 07:11:01 -0700 (In reply to Philippe Normand from comment #4) > In g-b-m you already build-depend on bwrap: > > https://gitlab.gnome.org/GNOME/gnome-build-meta/-/blob/master/elements/sdk/ > webkitgtk.bst#L12 > > Was that an oversight? An oversight in my previous comment, yes! But not in gnome-build-meta. That's there for GNOME OS. > IIUC the bwrap process launcher won't be used at runtime anyway if the > UIProcess is running in a flatpak runtime. Right, and gnome-build-meta proves that it works. I'm typing this comment in Tech Preview with bubblewrap sandbox enabled at build time but disabled at runtime. Just didn't realize it was enabled before you noticed. That said, it's still not available in the flatpak runtime (you'll notice we have it in sdk-deps to ensure it doesn't appear in the SDK) so every app bundling WebKit will have to pass -DENABLE_BUBBLEWRAP_SANDBOX=OFF. But apps should really probably not bundle WebKit. So I think it's fine. Still your call. 1799719 7 ews-feeder 2021-10-02 02:04:22 -0700 Committed r283436 (242423@main): <https://commits.webkit.org/242423@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 439626. 1799722 8 clopez 2021-10-02 03:03:27 -0700 (In reply to Philippe Normand from comment #4) > IIUC the bwrap process launcher won't be used at runtime anyway if the > UIProcess is running in a flatpak runtime. How this works? It may be interesting to do the same when running inside docker. Do you know which part of the code does this check? 1799723 9 pnormand 2021-10-02 03:15:55 -0700 https://github.com/WebKit/WebKit/blob/main/Source/WebKit/UIProcess/Launcher/glib/ProcessLauncherGLib.cpp#L174..L184 :) 1799850 10 mcatanzaro 2021-10-03 08:06:39 -0700 As you can see, the check is there for docker already, but I'm not sure if it actually works for podman, it might be limited to moby only. 439626 2021-09-29 10:44:56 -0700 2021-10-02 02:04:24 -0700 Patch bug-230969-20210929104455.patch text/plain 3581 pnormand U3VidmVyc2lvbiBSZXZpc2lvbjogMjgzMjMzCmRpZmYgLS1naXQgYS9Tb3VyY2UvY21ha2UvT3B0 aW9uc0dUSy5jbWFrZSBiL1NvdXJjZS9jbWFrZS9PcHRpb25zR1RLLmNtYWtlCmluZGV4IGY5ZWRj OTE4ZTQ2OWZmZmE4NzMzMDkzMWU0N2ZmOWU2Mzc5MjYwNTYuLmMyOTA5ZGI4ZGFiNDk5ZGNjODVj MzY4OWRlMTYyNWIxOGQ1ZmZlNDQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9jbWFrZS9PcHRpb25zR1RL LmNtYWtlCisrKyBiL1NvdXJjZS9jbWFrZS9PcHRpb25zR1RLLmNtYWtlCkBAIC05OCwxOSArOTgs MTUgQEAgZWxzZSAoKQogZW5kaWYgKCkKIAogaWYgKENNQUtFX1NZU1RFTV9OQU1FIE1BVENIRVMg IkxpbnV4IikKKyAgICBXRUJLSVRfT1BUSU9OX0RFRkFVTFRfUE9SVF9WQUxVRShFTkFCTEVfQlVC QkxFV1JBUF9TQU5EQk9YIFBVQkxJQyBPTikKICAgICBXRUJLSVRfT1BUSU9OX0RFRkFVTFRfUE9S VF9WQUxVRShFTkFCTEVfTUVNT1JZX1NBTVBMRVIgUFJJVkFURSBPTikKICAgICBXRUJLSVRfT1BU SU9OX0RFRkFVTFRfUE9SVF9WQUxVRShFTkFCTEVfUkVTT1VSQ0VfVVNBR0UgUFJJVkFURSBPTikK IGVsc2UgKCkKKyAgICBXRUJLSVRfT1BUSU9OX0RFRkFVTFRfUE9SVF9WQUxVRShFTkFCTEVfQlVC QkxFV1JBUF9TQU5EQk9YIFBVQkxJQyBPRkYpCiAgICAgV0VCS0lUX09QVElPTl9ERUZBVUxUX1BP UlRfVkFMVUUoRU5BQkxFX01FTU9SWV9TQU1QTEVSIFBSSVZBVEUgT0ZGKQogICAgIFdFQktJVF9P UFRJT05fREVGQVVMVF9QT1JUX1ZBTFVFKEVOQUJMRV9SRVNPVVJDRV9VU0FHRSBQUklWQVRFIE9G RikKIGVuZGlmICgpCiAKLWlmIChDTUFLRV9TWVNURU1fTkFNRSBNQVRDSEVTICJMaW51eCIgQU5E IE5PVCBFWElTVFMgIi8uZmxhdHBhay1pbmZvIikKLSAgICBXRUJLSVRfT1BUSU9OX0RFRkFVTFRf UE9SVF9WQUxVRShFTkFCTEVfQlVCQkxFV1JBUF9TQU5EQk9YIFBVQkxJQyBPTikKLWVsc2UgKCkK LSAgICBXRUJLSVRfT1BUSU9OX0RFRkFVTFRfUE9SVF9WQUxVRShFTkFCTEVfQlVCQkxFV1JBUF9T QU5EQk9YIFBVQkxJQyBPRkYpCi1lbmRpZiAoKQotCiAjIEVuYWJsZSB2YXJpYXRpb24gZm9udHMg d2hlbiBjYWlybyA+PSAxLjE2LCBmb250Y29uZmlnID49IDIuMTMuMCwgZnJlZXR5cGUgPj0gMi45 LjAgYW5kIGhhcmZidXp6ID49IDEuNC4yLgogaWYgKCgiJHtQQ19DQUlST19WRVJTSU9OfSIgVkVS U0lPTl9HUkVBVEVSICIxLjE2LjAiIE9SICIke1BDX0NBSVJPX1ZFUlNJT059IiBTVFJFUVVBTCAi MS4xNi4wIikKICAgICBBTkQgKCIke1BDX0ZPTlRDT05GSUdfVkVSU0lPTn0iIFZFUlNJT05fR1JF QVRFUiAiMi4xMy4wIiBPUiAiJHtQQ19GT05UQ09ORklHX1ZFUlNJT059IiBTVFJFUVVBTCAiMi4x My4wIikKZGlmZiAtLWdpdCBhL1NvdXJjZS9jbWFrZS9PcHRpb25zV1BFLmNtYWtlIGIvU291cmNl L2NtYWtlL09wdGlvbnNXUEUuY21ha2UKaW5kZXggNjEwNDE1ZTUyMjQ1NzBiZWU0Y2E4YWYyNWMz N2VhNWM1ZmE5YWFlOC4uZDUwMDA3OTYxMmYzZDBlMTAyOGM1YmRjNWIxMTg2MGFhMDQ0YTg1YyAx MDA2NDQKLS0tIGEvU291cmNlL2NtYWtlL09wdGlvbnNXUEUuY21ha2UKKysrIGIvU291cmNlL2Nt YWtlL09wdGlvbnNXUEUuY21ha2UKQEAgLTg3LDkgKzg3LDExIEBAIFdFQktJVF9PUFRJT05fREVG SU5FKFVTRV9HU1RSRUFNRVJfSE9MRVBVTkNIICJXaGV0aGVyIHRvIGVuYWJsZSBHU3RyZWFtZXIg aG9sZXB1CiBXRUJLSVRfT1BUSU9OX0RFRklORShVU0VfRVhURVJOQUxfSE9MRVBVTkNIICJXaGV0 aGVyIHRvIGVuYWJsZSBleHRlcm5hbCBob2xlcHVuY2giIFBSSVZBVEUgT0ZGKQogCiBpZiAoQ01B S0VfU1lTVEVNX05BTUUgTUFUQ0hFUyAiTGludXgiKQorICAgIFdFQktJVF9PUFRJT05fREVGQVVM VF9QT1JUX1ZBTFVFKEVOQUJMRV9CVUJCTEVXUkFQX1NBTkRCT1ggUFVCTElDIE9OKQogICAgIFdF QktJVF9PUFRJT05fREVGQVVMVF9QT1JUX1ZBTFVFKEVOQUJMRV9NRU1PUllfU0FNUExFUiBQUklW QVRFIE9OKQogICAgIFdFQktJVF9PUFRJT05fREVGQVVMVF9QT1JUX1ZBTFVFKEVOQUJMRV9SRVNP VVJDRV9VU0FHRSBQUklWQVRFIE9OKQogZWxzZSAoKQorICAgIFdFQktJVF9PUFRJT05fREVGQVVM VF9QT1JUX1ZBTFVFKEVOQUJMRV9CVUJCTEVXUkFQX1NBTkRCT1ggUFVCTElDIE9GRikKICAgICBX RUJLSVRfT1BUSU9OX0RFRkFVTFRfUE9SVF9WQUxVRShFTkFCTEVfTUVNT1JZX1NBTVBMRVIgUFJJ VkFURSBPRkYpCiAgICAgV0VCS0lUX09QVElPTl9ERUZBVUxUX1BPUlRfVkFMVUUoRU5BQkxFX1JF U09VUkNFX1VTQUdFIFBSSVZBVEUgT0ZGKQogZW5kaWYgKCkKQEAgLTEwMCwxMiArMTAyLDYgQEAg aWYgKEVOQUJMRV9ERVZFTE9QRVJfTU9ERSkKICAgICBXRUJLSVRfT1BUSU9OX0RFRkFVTFRfUE9S VF9WQUxVRShFTkFCTEVfQ09HIFBSSVZBVEUgT04pCiBlbmRpZiAoKQogCi1pZiAoQ01BS0VfU1lT VEVNX05BTUUgTUFUQ0hFUyAiTGludXgiIEFORCBOT1QgRVhJU1RTICIvLmZsYXRwYWstaW5mbyIp Ci0gICAgV0VCS0lUX09QVElPTl9ERUZBVUxUX1BPUlRfVkFMVUUoRU5BQkxFX0JVQkJMRVdSQVBf U0FOREJPWCBQVUJMSUMgT04pCi1lbHNlICgpCi0gICAgV0VCS0lUX09QVElPTl9ERUZBVUxUX1BP UlRfVkFMVUUoRU5BQkxFX0JVQkJMRVdSQVBfU0FOREJPWCBQVUJMSUMgT0ZGKQotZW5kaWYgKCkK LQogIyBFbmFibGUgdmFyaWF0aW9uIGZvbnRzIHdoZW4gY2Fpcm8gPj0gMS4xNiwgZm9udGNvbmZp ZyA+PSAyLjEzLjAsIGZyZWV0eXBlID49IDIuOS4wIGFuZCBoYXJmYnV6eiA+PSAxLjQuMi4KIGlm ICgoIiR7UENfQ0FJUk9fVkVSU0lPTn0iIFZFUlNJT05fR1JFQVRFUiAiMS4xNi4wIiBPUiAiJHtQ Q19DQUlST19WRVJTSU9OfSIgU1RSRVFVQUwgIjEuMTYuMCIpCiAgICAgQU5EICgiJHtQQ19GT05U Q09ORklHX1ZFUlNJT059IiBWRVJTSU9OX0dSRUFURVIgIjIuMTMuMCIgT1IgIiR7UENfRk9OVENP TkZJR19WRVJTSU9OfSIgU1RSRVFVQUwgIjIuMTMuMCIpCmRpZmYgLS1naXQgYS9DaGFuZ2VMb2cg Yi9DaGFuZ2VMb2cKaW5kZXggODdjNzAzOWI1MGI5NjIwZGQ3YjRhNTM1OWI1MWZhYjdkOTg1M2Yx ZC4uNTYyOTBlNGNlZmRlZGQ1NGRjNjk0N2NjZWQ0MmMxOTdjM2YzYTY2YiAxMDA2NDQKLS0tIGEv Q2hhbmdlTG9nCisrKyBiL0NoYW5nZUxvZwpAQCAtMSwzICsxLDEzIEBACisyMDIxLTA5LTI5ICBQ aGlsaXBwZSBOb3JtYW5kICA8cG5vcm1hbmRAaWdhbGlhLmNvbT4KKworICAgICAgICBbR1RLXVtX UEVdIEVuYWJsZSBid3JhcCBsYXVuY2hlciBidWlsZCBvbiBib3RzCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMzA5NjkKKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFNvdXJjZS9jbWFrZS9PcHRpb25zR1RL LmNtYWtlOgorICAgICAgICAqIFNvdXJjZS9jbWFrZS9PcHRpb25zV1BFLmNtYWtlOgorCiAyMDIx LTA5LTI4ICBDYXJsb3MgR2FyY2lhIENhbXBvcyAgPGNnYXJjaWFAaWdhbGlhLmNvbT4KIAogICAg ICAgICBbR1RLXVtXUEVdIEJ1bXAgbGlic291cDMgdmVyc2lvbiB0byAzLjAuMAo=