This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/104337/

On March 21st, 2012, 8:54 p.m., Alexander N= eundorf wrote:

I did not=
 yet have time to look through the git branch...

So, kauth, we have a bunch of cmake macros related to this in kdelibs/cmake=
/KDE4Macros.cmake, right ?
What about them ?

On March 21st, 2012, 10:52 p.m., Stephen Kelly wrote:

They are =
already in ${CMAKE_SOURCE_DIR}/staging/kauth/cmake/KAuthMacros.cmake

This change=
 didn't affect them at all, since their sole purpose is handling helper=
 creation and backend selection - this didn't change. The cmake check w=
as mostly to verify I did the correct bits for KCoreAddons (given I simply =
mentioned the binary name when linking), but Stephen already confirmed it&#=
39;s ok.

Of course, having you double-checking the macros just in case would be awes=
ome if you have a couple spare minutes :)

- Dario

On March 18th, 2012, 10:25 p.m., Dario Freddi wrote:

Review request for kdelibs, Kevin Ottens, David Faure, and Alexander N= eundorf.

By Dario Freddi.

Updated March 18, 2012, 10:25 p.m.

Descripti= on

Preamble - sorry for having to name-call people but apparent=
ly we still don't have a frameworks way for reviewing code (which sucks=
). And sorry for the long summary, but it's worth reading. However.

This huge patchsets brings KAuth in the marvelous world of Frameworks. If y=
ou dislike ReviewBoard's way of displaying diffs or simply want to see =
a commit list, please refer to the URL in "Branch".

First of all, I pulled in a dependency on KJob after a chat with Kevin. Thi=
s makes KAuth tier2, but shouldn't be a big issue.

Then there's the hard part: source compatibility is reasonably broken h=
ere. The changes I had to do were mostly for the sake of revamping the inte=
rnal workflow of the library. The main problem KAuth had was the fact it wa=
s completely synchronous, leading to a multitude of problems. After these c=
hanges it's fully asynchronous instead (reason for pulling in KJob), th=
e API was simplified, and some unused features like multiple action executi=
on have been removed.

The main changes at a glance:

 * Some renaming to the enums
 * Moving Action & ActionReply to be implicitly shared
 * Removing ActionWatcher (now useless due to the new semantics of execute
 * Removing some useless APIs from Action, namely executeActions, execute(h=
elper)
 * execute() now returns a KJob
 * helperID() -> helperId()
 * Static action replies are now static accessors returning a new instance.=
 This was a complete mistake in the first place, but it's still there w=
ith a different semantic to ease porting. The main use case for changing th=
is is a failure to handle implicitly shared classes in multithreaded enviro=
nments with that approach.

Of course, while it would be awesome to have all the code reviewed, I under=
stand it's a very big change so I'd like at least some feedback on =
the following points:

 * General sanity of the new API
 * Consistency of the enums. StatusInvalid vs. ExecuteMode vs. Authorizatio=
nDeniedError. While the semantic seems correct to me, I'd like to have =
some feedback on whether consistency is valuable in the ordering of <typ=
e><value> vs. <value><type> and which one should be pr=
eferred in case.
 * Whether to deprecate static accessors such as static const ActionReply S=
uccessReply(). I strongly favor this.
 * Whether the new dependency of kcoreaddons for the sake of using KJob is =
reasonable or I should go for a different alternative.
 * CMake sanity for the new dependency of kcoreaddons.

The code is pretty much unit-tested and it should have a decent coverage, e=
ven if I had no way to check this. For unit tests, I had to create a fake a=
uthorization backend for testing purposes, whereas I managed to reuse the d=
bus backend for helper communication, so that I could even test that. For r=
unning the helper and the client in the same process, in the unit test I am=
 resorting to making the dbus service of the helper live in a separate thre=
ad, to prevent asynchronous DBus calls from failing due to QDBus' local=
-loop optimization. The test is also run on the session bus.

Testing <= /h1>

New unit tests pass 100%

Diffs=

staging/kauth/CMakeLists.txt (PRE-CREATION= )
staging/kauth/autotests/BackendsManager.h = (PRE-CREATION)
staging/kauth/autotests/BackendsManager.cpp (PR= E-CREATION)
staging/kauth/autotests/HelperTest.cpp (PR= E-CREATION)
staging/kauth/autotests/SetupActionTest.cpp (PRE= -CREATION)
staging/kauth/autotests/TestBackend.cpp (P= RE-CREATION)
staging/kauth/autotests/TestHelper.h (PRE-= CREATION)
staging/kauth/autotests/TestHelper.cpp (PR= E-CREATION)
staging/kauth/src/AuthBackend.h (PRE-CREAT= ION)
staging/kauth/src/CMakeLists.txt (PRE-CREA= TION)
staging/kauth/src/HelperProxy.h (PRE-CREAT= ION)
staging/kauth/src/backends/dbus/DBusHelperProxy.h (PRE-CREATION)
staging/kauth/src/backends/dbus/DBusHelperProxy.cpp (PRE-CREATION)
staging/kauth/src/backends/dbus/org.kde.auth.xml (PRE-CREATION)
staging/kauth/src/backends/fake/FakeBackend.cpp (PRE-CREATION)
staging/kauth/src/backends/fakehelper/FakeHelperProxy.h (PRE-CREATION)
staging/kauth/src/backends/fakehelper/FakeHelperProxy.cpp (PRE-CREATION)
staging/kauth/src/backends/mac/AuthServicesBackend.cpp (PRE-CREATION)
staging/kauth/src/backends/policykit/PolicyKitBackend.cpp (PRE-CREATION)
staging/kauth/src/backends/polkit-1/Polkit1Backend.cpp (PRE-CREATION)
staging/kauth/src/kauthaction.h (PRE-CREAT= ION)
staging/kauth/src/kauthaction.cpp (PRE-CRE= ATION)
staging/kauth/src/kauthactionreply.h (PRE-= CREATION)
staging/kauth/src/kauthactionreply.cpp (PR= E-CREATION)
staging/kauth/src/kauthactionwatcher.h (PR= E-CREATION)
staging/kauth/src/kauthactionwatcher.cpp (= PRE-CREATION)
staging/kauth/src/kauthexecutejob.h (PRE-C= REATION)
staging/kauth/src/kauthexecutejob.cpp (PRE= -CREATION)

View Diff