flatpak / libportal

libportal - Flatpak portal library
https://libportal.org
GNU Lesser General Public License v3.0
82 stars 40 forks source link

meson: default the backends list to 'auto' #104

Closed whot closed 1 year ago

whot commented 2 years ago

This changes the 'backends' option to the special string 'auto' - all backends become optional by default and are built if the dependencies are available.

If need be this can be combined with explicitly listing a backend, so backends=auto,gtk3 will require the GTK3 backend and optionally build the Qt5 and GTK4 backends. A summary is printed to list the actually enabled backends and a warning where no backends are enabled at all.

This makes life easier for new users since the majority of them will not have GTK3, GTK4 and Qt5 headers installed by default and thus are almost guaranteed to run into a meson error on first configure.

Fixes https://github.com/flatpak/libportal/issues/94

whot commented 2 years ago

cc @eli-schwartz for the inevitable meson magic that already exists for this and that I'm not aware of ;)

whot commented 2 years ago

gentle ping

whot commented 2 years ago

gentle ping again

whot commented 2 years ago

monthly ping again

whot commented 2 years ago

@GeorgesStavracas - any chance we can merge this? Not having to manually supply the backends to build libportal from git is a rather nice feature.

whot commented 2 years ago

ping again?

GeorgesStavracas commented 2 years ago

@eli-schwartz I'd actually like to hear your thoughts on the "auto" magic that's happening here. I'm not convinced this is a great idea. Or, at least, I'd love to hear what a Mesonic way to implement this would look like.

GeorgesStavracas commented 2 years ago

@whot sorry, completely dropped the ball here

whot commented 2 years ago

My guess would be that the mesonic way would be to have each backend as separate feature option (instead of a "backends" array with a special string). That'd be easy enough to introduce but changes the build invocations.

eli-schwartz commented 2 years ago

Yeah, adding a separate option for each one would be the most obvious way to handle it.

If you want to keep the existing option, I guess you could make each option default to auto, and then use something like:

backends = get_option('backends')
x_opt = 'x' in backends ? true : get_option('x')
y_opt = 'y' in backends ? true : get_option('y')

The option will then usually be a feature option, but will be a boolean true (equal to enabled when used inside required: kwargs) if the existing option specifies to build that backend. Check for the dependencies using required: x_opt, and they will use:

Whichever one matches last.

whot commented 1 year ago

Changed this PR to use -Dbackend-gtk3=enabled and so on. I tried keeping backends as list for backwards compatibility but tbh the conditions got confusing enough (in particular: -Dbackends=gtk3 must also set the others to disabled) that I didn't really see the value in keeping it, so it's removed now.