On 8 May 2024, at 13:14, Juha Vuolle <juvuo...@gmail.com> wrote:
Hi, sorry for just kind of resetting the thread, but we had a quick discussion with Marc and Tor Arne. There's an additional constraint that, at least on iOS/macOS, that the OS/platform callback mechanisms depend on UI libraries (AppKit and UIKit). While making that work in QtCore could *maybe* technically be possible, it'd just add to the effort and risk we'd be talking about here. Hence a concrete suggestion with a few specific questions. Thing 1) In Qt 6.8 we'll add a QtGui dependency to QtNetworkAuth module. With this the new Qt 6.8 features will work. Thing 2) This QtGui dependency will be behind a Qt feature flag. Thing 3) Later in Qt 6.9+, if the three QDS functionalities become available via QtCore, then QtNetworkAuth drops the QtGui dependency. IIUC dropping a private linkage shouldn't cause "transitive" problems. The main questions: Question 1) Do we foresee problems on introducing a Gui dependency to QtNetworkAuth? Question 2) Do we foresee problems in possibly later dropping the Gui dependency? Given all the information, that approach sounds sensible. Adding Qt Gui as a dependency to QtNetworkAuth shouldn’t be a problem, especially if it’s possible to opt out, and as long as it’s documented in the ChangeLog that application using QtNetworkAuth now need to deploy Qt Gui as well. We added that kind of dependency when we linked Qt TextToSpeech against Qt Multimedia (see discussion at https://lists.qt-project.org/pipermail/development/2023-January/043562.html). And I can’t foresee a problem with later on dropping that dependency. One thing to consider is whether you need a QGuiApplication instance for things to work. If things can’t work across platforms if the application only gives you a QCoreApplication instance, then it might be possible to add some virtual function(s) to QCoreApplicationPrivate that you can call from QtNetworkAuth to enable the implicit OAuth flow handling only if GUI is alive. Then things will degrade gracefully if QtGui is not initialized. We have some similar approaches to trigger Qt Widgets specific behaviors from Qt Gui classes (QGuiApplicationPrivate::closeAllPopups, for instance, or even createActionPrivate/createShortcutPrivate). Volker ke 8. toukok. 2024 klo 12.51 Volker Hilsheimer via Development (development@qt-project.org) kirjoitti: On 8 May 2024, at 07:32, Marc Mutz via Development <development@qt-project.org> wrote: On 07.05.24 18:46, Volker Hilsheimer wrote: In the long run, a mechanism in Qt Core makes sense, IMHO. That “it’s a browser” is not true for every possible call of the QDesktopServices API. We need something _now_ for QtNetworkAuth, though. What do these options mean for OAuth support in QtNetworkAuth (my attempt at an analysis below). I question that. QtNetworkAuth’s QOAuth2AuthorizationCodeFlow::authorizeWithBrowser signal is perfectly usable by users, as long as they are either willing to use Qt GUI, or to write their own handler code (possibly inspired but what we have in Qt Gui). However, whether a new QCoreDesktopServices namespace becomes public QtCore API or not in Qt 6.8 is somewhat irrelevant as long as it doesn’t do anything on any platform. The QPA infrastructure in general, and the implementations of QPlatformServices in particular is -as of now - not loaded by an application that doesn’t use QtGui. And we haven’t even started discussing if and how we’d like to make that happen. I do not anticipate that we will get that infrastructure into Qt Core in time for the Qt 6.8 feature freeze in three weeks; while it’s probably not rocket science to write the code, there are evidently too many disagreements on (and probably several devils in) the details. We have a template for this: the permission API. It's a) in QtCore and also highly platform-dependent. I also, honestly, don't see the extraction of the non-Gui code from the QPAs into some QtCore files or plugin as something subject to feature freeze. It's kinda cleanup under the hood. Certainly we can't argue that moving the implementation as private API is ok post-FF, but moving the interface as public API now and moving the implementation as private implementation detail post-FF is not, can we? The point of feature freeze is that we start stabilizing the release. Moving large chunks of code around, while not forbidden explicitly, doesn’t move us to that end. I can see two options: 1) we leave it entirely to the application to implement a Core-only equivalent of QDesktopServices Not fun, but not impossible either, esp given that whoever needs this can take our existing code. We could even have that code in an example, at least for some platforms. This doesn’t invalidate the improved OAuth support in 6.8, esp if we assume that the vast majority of users today will use Qt Gui anyway, and that Qt Core only use cases are rare enough to deal with the alternative. AFAICT, this means one of the following: a) that we have to postpone QtNetworkAuth until new QtCore API is added, provided that any future intents/activities API actually pertains to and supports implementation of what OAuth needs. Danger of "the perfect is the enemy of the good" here. Depending on Qt Gui might not be perfect, but it’s good enough for the vast majority of users. b) that QtNetworkAuth gets the undesired QtGui dependency, which becomes stale once new Core API is merged, but then has to be carried along for BC reasons. QtNetworkAuth doesn’t have to link against Qt Gui. The application does. Qt NetworkAuth emits a signal, the application handles that. Or do I misinterpret what https://code.qt.io/cgit/qt/qtnetworkauth.git/tree/examples/oauth/redditclient/redditwrapper.cpp#n28 (which is the example you referred to a few days ago) is doing and how that relates to how things should be done in the future with new and improved Qt NetworkAuth? Maybe it’s time to point at what new and improved Qt NetworkAuth we are talking about… maybe it’s https://codereview.qt-project.org/c/qt/qtnetworkauth/+/556023? c) that the user has to connect the pieces of the flow together manually. The whole point of the code there is to _implement_ the flow, though. Thanks for finally sharing that bit of information :) Up to now and based on the discussion here, my assumption was not that we replace the responsibility of the application developer (as exemplified by the quoted redditclient example) with a baked-in workflow implementation that handles redirects etc. 2) we put a copy of our implementations into Qt Network, without any public API It could (but doesn’t have to) be a Qt Network specific plugin, and if that plugin exists then we use the implementation in it automatically whenever we’d emit QOAuth2AuthorizationCodeFlow::authorizeWithBrowser. We could add a property of QOAuth2AuthorizationCodeFlow to “useDefaultBrowser” to enable that (and one future day, setting that property will automatically make it go through our new public API instead). Option 2 is not a lot of work, with no impact on public API at this point, while enabling a good out-of-the-box usability of the new OAuth support. The drawback is that we have a duplicate a bunch of code (or find a way to build the relevant sources twice), the most of which seems to be in the Xcb QPA plugin (and not all of that might be relevant if we want to support a QtCore-only-app-on-GUI-less-system scenario). What good could possibly come out of copying that code to QtNetwork? A QtNetwork dependency on QtGui? If not that, we'd need to split the implementation into Gui and non-Gui parts, and if we do that, we might as well put it into Core, where it belongs. I still don’t see why having the *implementation* of QPlatformServices in QtNetwork inevitably requires QtGui to become a dependency. If I was paying y’all’s salary, then I’d strongly suggest to go with option 1, and maybe follow up with Option 2 for 6.9, while we take the time it takes to figure out how to properly wrap intents etc into a cross-platform abstraction. I don't see (1) solving anything for QtNetworkAuth and (2) as being roughly equivalent to putting the code into QtCore. On 8 May 2024, at 10:21, Tor Arne Vestbø via Development <development@qt-project.org> wrote: On 8 May 2024, at 07:32, Marc Mutz via Development <development@qt-project.org> wrote: I'd like an option that actually solves for the needs of QtNetworkAuth. 1. Move the plumbing of the URL handling to private APIs in QtCore, used by QDesktopServices 2a. Add a helper QOAuth2AuthorizationCodeFlow::openChallengeUrl function that uses the QtCore private api 2b. Have QOAuth2AuthorizationCodeFlow automatically open the browser via the QtCore private API No public API changes needed to QDesktopServices needed, which means we’re not blocking the QtNetworkAuth work on the work required to research activities/intents APIs (which is not scheduled for 6.8). That’s more or less what I was having in mind with my proposal of option 2. How much abstraction comes along with the specific implementation of QPlatformServices doesn’t matter all that much. And whether that internal implementation lives in QtCore or QtNetwork(Auth) is also secondary as such; if we want to export it and use it from QPlatformServices implementations rather than than maintain a (temporary) copy, then it obviously has to live in Qt Core. Volker -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
-- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
