Bug 238452

Created attachment 455917 [details] Patch

Created attachment 456111 [details] Patch

Comment on attachment 456111 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=456111&action=review > Tools/flatpak/local-projects/meson.build:2 > + version : '0.1.0') Nice trick, using a dummy project with the only goal of letting Meson handle things for us 🤯️ Even when it works only for Meson projects, that will cover a good amount of our dependencies these days, including GTK, GLib, glib-networking and a few others -- we can add later on .wrap files when needed for them.

Thanks for the review! I wanted to update the patch to allow the user to tune meson options with an env var, it's not working yet (for unknown reasons) but whenever I manage to get it working I'd like to update this patch and ask a new review, if that's OK with you. Or I can follow-up in a new patch.

(In reply to Adrian Perez from comment #3) > Even when it works only for Meson projects It could be made to work with different build-systems as well, Meson has an "externalproject" module that I successfully used in another context: https://mesonbuild.com/External-Project-module.html#external-project-module

Created attachment 456586 [details] Patch

(In reply to Philippe Normand from comment #5) > (In reply to Adrian Perez from comment #3) > > > Even when it works only for Meson projects > > It could be made to work with different build-systems as well, Meson has an > "externalproject" module that I successfully used in another context: > https://mesonbuild.com/External-Project-module.html#external-project-module Nice!

Comment on attachment 456586 [details] Patch Patch LGTM, with a small nit that would be nice to tackle before landing :) View in context: https://bugs.webkit.org/attachment.cgi?id=456586&action=review > Tools/flatpak/flatpakutils.py:685 > + options = [o for o in os.environ.get('WEBKIT_SDK_LOCAL_DEPS_OPTIONS', '').strip().split(' ') if o] It would be better to use shlex.split() here, to allow passing options that themselves contain spaces by quoting them inside the environment variable, e.g: export WEBKIT_SDK_LOCAL_DEPS_OPTIONS='-Dsomeproject:someoption="value with spaces"'

(In reply to Philippe Normand from comment #4) > Thanks for the review! I wanted to update the patch to allow the user to > tune meson options with an env var, it's not working yet (for unknown > reasons) but whenever I manage to get it working I'd like to update this > patch and ask a new review, if that's OK with you. Or I can follow-up in a > new patch. I would say it is up to you: if you prefer to update the patch for this bug, I can re-review; if you would rather do a follow-up that's fine as well. The latter approach would allow to get the base functionality in trunk faster, maybe ;-)

Created attachment 456689 [details] [fast-cq] Patch

Committed r292390 (249255@main): <https://commits.webkit.org/249255@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 456689 [details].

<rdar://problem/91286192>