Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is the API/ABI stable? #33

Closed
smcv opened this issue Sep 21, 2020 · 19 comments
Closed

Is the API/ABI stable? #33

smcv opened this issue Sep 21, 2020 · 19 comments

Comments

@smcv
Copy link
Contributor

smcv commented Sep 21, 2020

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.

gnomesysadmins pushed a commit to GNOME/epiphany that referenced this issue Oct 1, 2020
It isn't clear whether the API/ABI of libportal are entirely stable yet
(flatpak/libportal#33) so it is not necessarily
appropriate for longer-term-supported OS distributions to include it.
When building a version of epiphany for a distribution package, which is
only intended to be packaged in a format other than as a Flatpak app,
libportal isn't necessary anyway.

libportal is also Linux-specific, so non-Linux OSs will likely want to
disable it (even if it might compile successfully).

Signed-off-by: Simon McVittie <smcv@debian.org>
@jhenstridge
Copy link

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).

@smcv
Copy link
Contributor Author

smcv commented Jun 18, 2021

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.

@hadess
Copy link
Contributor

hadess commented Jun 18, 2021

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.

@jhenstridge
Copy link

@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.

@hadess
Copy link
Contributor

hadess commented Jun 18, 2021

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.

@jhenstridge
Copy link

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?

@hadess
Copy link
Contributor

hadess commented Jun 18, 2021

@matthiasclasen is the best person to decide on that.

@TingPing
Copy link
Member

TingPing commented Aug 25, 2021

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).

@Conan-Kudo
Copy link
Contributor

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).

@smcv
Copy link
Contributor Author

smcv commented Aug 28, 2021

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.)

@Conan-Kudo
Copy link
Contributor

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.

@grulja
Copy link
Contributor

grulja commented Feb 21, 2022

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.

@TingPing
Copy link
Member

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

@GeorgesStavracas
Copy link
Member

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).

@hadess
Copy link
Contributor

hadess commented Feb 21, 2022

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

@grulja
Copy link
Contributor

grulja commented Feb 22, 2022

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

@mwleeds
Copy link
Contributor

mwleeds commented Mar 21, 2022

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.

@hadess
Copy link
Contributor

hadess commented Mar 22, 2022

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 2c6754c, 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 #85

@smcv
Copy link
Contributor Author

smcv commented Sep 8, 2023

Closing this as resolved based on #33 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants