Bug 238452 - [Flatpak SDK] Local dependencies override support
Summary: [Flatpak SDK] Local dependencies override support
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philippe Normand
URL:
Keywords: InRadar
Depends on: 238454
Blocks:
  Show dependency treegraph
 
Reported: 2022-03-28 07:24 PDT by Philippe Normand
Modified: 2022-04-05 03:58 PDT (History)
4 users (show)

See Also:


Attachments
Patch (11.97 KB, patch)
2022-03-28 07:47 PDT, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (12.60 KB, patch)
2022-03-30 04:04 PDT, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (12.98 KB, patch)
2022-04-04 10:18 PDT, Philippe Normand
no flags Details | Formatted Diff | Diff
[fast-cq] Patch (12.98 KB, patch)
2022-04-05 03:53 PDT, Philippe Normand
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2022-03-28 07:24:56 PDT
.
Comment 1 Philippe Normand 2022-03-28 07:47:35 PDT
Created attachment 455917 [details]
Patch
Comment 2 Philippe Normand 2022-03-30 04:04:50 PDT
Created attachment 456111 [details]
Patch
Comment 3 Adrian Perez 2022-03-31 01:31:59 PDT
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.
Comment 4 Philippe Normand 2022-04-04 07:41:45 PDT
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.
Comment 5 Philippe Normand 2022-04-04 10:08:23 PDT
(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
Comment 6 Philippe Normand 2022-04-04 10:18:54 PDT
Created attachment 456586 [details]
Patch
Comment 7 Adrian Perez 2022-04-04 14:29:14 PDT
(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 8 Adrian Perez 2022-04-04 14:33:34 PDT
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"'
Comment 9 Adrian Perez 2022-04-04 14:35:12 PDT
(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 ;-)
Comment 10 Philippe Normand 2022-04-05 03:53:16 PDT
Created attachment 456689 [details]
[fast-cq] Patch
Comment 11 EWS 2022-04-05 03:57:02 PDT
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].
Comment 12 Radar WebKit Bug Importer 2022-04-05 03:58:16 PDT
<rdar://problem/91286192>