libportal icon indicating copy to clipboard operation
libportal copied to clipboard

Is the API/ABI stable?

Open smcv opened this issue 5 years ago • 18 comments

Has libportal reached a point where its API and ABI are stable, in the sense that incompatible changes would result in a SONAME bump to libportal.so.1, as is done in e.g. libhandy, libdazzle and libflatpak?

I'm asking because there were API breaks between 0.2 and 0.3, and as a result, at the moment it's only in Debian experimental, which is appropriate for a library that isn't yet stable (although Ubuntu developers seem to have added it to the package set for Ubuntu 20.10 anyway).

If I put it in Debian testing/unstable where it will become part of Debian 11, then I can use it to get better test coverage in xdg-desktop-portal (and to make it easier to build Flatpak runtimes out of Debian packages), but that comes with higher expectations in terms of not breaking the ABI.

smcv avatar Sep 21 '20 10:09 smcv

Nautilus also grew an (optional but enabled by default) dependency on libportal in version 40:

https://gitlab.gnome.org/GNOME/nautilus/-/merge_requests/589

So it would be nice to get an answer on this. It may be a blocker in moving libportal to Ubuntu's supported package set (main).

jhenstridge avatar Jun 18 '21 09:06 jhenstridge

I would like the answer to become "yes, the API/ABI is stable". Now that I'm a member of the flatpak org on Github, if libportal maintainers (mostly @matthiasclasen and @hadess so far) are OK with me doing merges/commits/releases here, I don't mind doing the work to review changes where people are unsure about API/ABI implications, and bump the SONAME if it becomes necessary.

smcv avatar Jun 18 '21 10:06 smcv

I'm not a maintainer, and I don't think that there's really any need for an ABI stable library when it only gets used (or should only get used) bundled in a Flatpak.

hadess avatar Jun 18 '21 10:06 hadess

@hadess: as I understand it, Nautilus is using it as a desktop neutral method of setting the wallpaper. So if you happen to run Nautilus on a non-GNOME desktop, then the feature will still work provided there is an implementation of org.freedesktop.impl.portal.Wallpaper.

We're already seeing org.freedesktop.portal.ScreenCast being used by non-sandboxed apps as a desktop neutral API for screen capture on Wayland (e.g. Firefox, OBS Studio, etc), so I think it is a bit short sighted to assume libportal will only be used by sandboxed apps.

jhenstridge avatar Jun 18 '21 10:06 jhenstridge

so I think it is a bit short sighted to assume libportal will only be used by sandboxed apps.

That's frankly quite insulting a turn of phrase. I suggest you re-read the xdg-desktop-portal* issues that were open about the behaviour of the portals whem used by non-sandboxed apps, Matthias wasn't keen on opening up that Pandora's box.

org.freedesktop.portal.ScreenCast was always a special-case, as it's the only API available in Wayland, sandboxed or not.

hadess avatar Jun 18 '21 10:06 hadess

I didn't intend it as an insult. But we are clearly seeing developers gravitate towards xdg-desktop-portal to cover "missing" desktop neutral APIs.

If we don't want non-sandboxed applications to use these APIs (and possibly use libportal), then maybe it would make sense to have them error out when called from a non-sandboxed process?

jhenstridge avatar Jun 18 '21 10:06 jhenstridge

@matthiasclasen is the best person to decide on that.

hadess avatar Jun 18 '21 12:06 hadess

I believe the agreed upon situation is:

  • libportal is not API frozen yet
  • libportal will do SONAME bumps on API changes. It has bumped the SONAME since 0.4.
  • libportal, as with xdg-desktop-portal, does and will continue to work on the host outside of any sandbox. Situations where it does not are bugs (mostly in xdg-desktop-portal).

TingPing avatar Aug 25 '21 18:08 TingPing

Then Nautilus and other things should probably not be using this library if it's not ABI stable (if I understand GNOME's policy around this correctly, they aren't supposed to use it by default until it's stabilized).

Conan-Kudo avatar Aug 28 '21 12:08 Conan-Kudo

Libraries like libgnome-desktop are also not API- and ABI-stable; as long as they bump the SONAME correctly when the ABI breaks, downstream distributions can deal with that by doing an ABI transition. (That's why we have SONAMEs.)

smcv avatar Aug 28 '21 13:08 smcv

Yes, but GNOME and Mutter (the other library that does this) are also not intended to be used outside of the GNOME ecosystem and nobody outside of GNOME uses it. Since GNOME applications release together and are adapted together, it's not as much of an issue.

Conan-Kudo avatar Aug 28 '21 14:08 Conan-Kudo

I would also be interested in stable API/ABI as we would like to explore the possibility to use this in WebRTC in the future. We already have our own implementation of ScreenCast portal, but there is an ongoing work for RemoteDesktop portal and we are now in phase where we discuss possibilities how to split ScreenCast portal and re-use some of its bits so we don't duplicate code. Same goes for Camera portal. Being able to use libportal, we can drop significant amount of code, be future proof and support other portals without shuffling existing code around, splitting it and trying to make its API reasonable for other potential portal implementations.

And while I saw mentioned that libportal is mostly meant to be bundled together with applications, I also think that majority of applications will use portal implementations from the toolkit they use and therefore ScreenCast or RemoteDesktop portals can be an important part of libportal where in this case it doesn't necessarily mean libportal will be bundled with the app itself.

grulja avatar Feb 21 '22 07:02 grulja

0.5 included an API break but I don't think any further breaks are planned. @GeorgesStavracas?

TingPing avatar Feb 21 '22 19:02 TingPing

There is at least one change that is probably going to break APIs: moving the camera access request to the Device Access portal, and deprecate the Camera portal. I might be able to do that during Q2 2022. Besides that, there might be a ABI breaks by introducing new portals, but I don't think we'll be changing function signatures that often (after the Camera → Device Access migration anyway).

GeorgesStavracas avatar Feb 21 '22 19:02 GeorgesStavracas

There is at least one change that is probably going to break APIs: moving the camera access request to the Device Access portal, and deprecate the Camera portal. I might be able to do that during Q2 2022. Besides that, there might be a ABI breaks by introducing new portals, but I don't think we'll be changing function signatures that often (after the Camera → Device Access migration anyway).

Adding new API isn't an ABI break. Removing it or changing it would be. I would recommend setting up check-abi in one of the CI jobs: https://gitlab.freedesktop.org/hadess/check-abi

hadess avatar Feb 21 '22 20:02 hadess

There is at least one change that is probably going to break APIs: moving the camera access request to the Device Access portal, and deprecate the Camera portal. I might be able to do that during Q2 2022. Besides that, there might be a ABI breaks by introducing new portals, but I don't think we'll be changing function signatures that often (after the Camera → Device Access migration anyway).

Sounds good. As @hadess said, introducing new portals is fine, changing function signatures is not. Some of the functions take GVariants so these are easily extendable without worrying about breaking API/ABI. I already tested most of the portals when trying to write a Qt wrapper, however, I haven't tested ScreenCast or RemoteDesktop portal implementations, those I would actually like to use in the future. Do you know if there is already an app using them? OBS perhaps? Or should I rather go and write a proof of concept code for myself to verify it works before we announce them stable?

Edit: just to explain myself better, I don't expect it doesn't work, but in the past I had to fix some signal signatures for some portals, otherwise there were not propagated to consumers

grulja avatar Feb 22 '22 07:02 grulja

I would recommend setting up check-abi in one of the CI jobs: https://gitlab.freedesktop.org/hadess/check-abi

Used this to check for ABI breaks between 0.5 and 0.6 before releasing 0.6 and found none.

mwleeds avatar Mar 21 '22 19:03 mwleeds

Used this to check for ABI breaks between 0.5 and 0.6 before releasing 0.6 and found none.

This is the results of running it against 2c6754c2cf1abb038f8621563136d1b953210e75, the last time the library versions seem to have been bumped:

$ check-abi --new-parameters="-Dbackends=gtk3,gtk4"  2c6754c2cf1abb038f8621563136d1b953210e75 `git rev-parse HEAD`
['abidiff', '--headers-dir1', '/home/hadess/Projects/jhbuild/libportal/2c6754c2cf1abb038f8621563136d1b953210e75/usr/include', '--headers-dir2', '/home/hadess/Projects/jhbuild/libportal/3d8b288a7d5e8a7b74908145d5e7cd4f1aced803/usr/include', '--drop-private-types', '--fail-no-debug-info', '--no-added-syms', '/home/hadess/Projects/jhbuild/libportal/2c6754c2cf1abb038f8621563136d1b953210e75/usr/lib/libportal.so', '/home/hadess/Projects/jhbuild/libportal/3d8b288a7d5e8a7b74908145d5e7cd4f1aced803/usr/lib/libportal.so']
Functions changes summary: 0 Removed, 1 Changed (20 filtered out), 0 Added (48 filtered out) functions
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

1 function with some indirect sub-type change:

  [C] 'function void xdp_portal_create_screencast_session(XdpPortal*, XdpOutputType, XdpScreencastFlags, XdpCursorMode, GCancellable*, GAsyncReadyCallback, gpointer)' at remote.c:418:1 has some indirect sub-type changes:
    parameter 5 of type 'GCancellable*' changed:
      entity changed from 'GCancellable*' to 'typedef XdpPersistMode' at remote.h:125:1
      type size changed from 64 to 32 (in bits)
      type alignment changed from 0 to 32
    parameter 6 of type 'typedef GAsyncReadyCallback' changed:
      entity changed from 'typedef GAsyncReadyCallback' to compatible type 'const char*'
        in pointed to type 'function type void (_GObject*, _GAsyncResult*, void*)':
          entity changed from 'function type void (_GObject*, _GAsyncResult*, void*)' to 'const char'
          type size changed from 64 to 8 (in bits)
    parameter 8 of type 'typedef GAsyncReadyCallback' was added
    parameter 9 of type 'typedef gpointer' was added

I've added a CI job for that in https://github.com/flatpak/libportal/pull/85

hadess avatar Mar 22 '22 09:03 hadess

Closing this as resolved based on https://github.com/flatpak/libportal/issues/33#issuecomment-905779376.

smcv avatar Sep 08 '23 00:09 smcv