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? 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
