[KDE/Mac] Review Request 119844: [OS X] backend improvements for kwallet/Keychain integration

[RJVB Bertin]

> On Aug. 20, 2014, 9:22 p.m., RJVB Bertin wrote:
> > An additional change to kwallettest.cpp would be required: the check for $DISPLAY is senseless on OS X:
> > 
> >     void KWalletTest::init()
> >     {
> >     #if !defined(Q_WS_MACX) && !defined(Q_OS_MACX)
> >         if( !qEnvironmentVariableIsSet( "DISPLAY" ) ) {
> >             QSKIP( "$DISPLAY is not set. These tests cannot be done without a graphical system." );
> >         }
> >     #endif
> >     }

Since I have some time waiting for things to happen:
A general remark that I think has been raised before: it'd be good to come to a consensus about what Mac-specific preprocessor flags/macros to use. From what I understand, Q_*_MACX refers to Mac OS X whereas the Q_*_MAC variants refer to Apple Macintosh as a family all the way back to pre-OS X days (and possibly forward to post OS X days). _OS_ vs _WS_ refers to OS versus windowing system details.

The current situation is that MAC implies MACX because there are no pre OS X systems on which KDE code would run. Similarly, the OS and WS flavours are homologous because Qt does not build for X11 anymore under OS X. 

But does that really mean that one can use whichever of the FOUR flags that indicate code is being built for use on a computer running Apple's OS X? Should we prefer MAC instead of MACX because/under the assumption that whatever OS XI will bring is not completely different (I think I would)? In situations like the test above, should we deactivate code with Q_WS_MACX because someday X11 support might be reintroduced (which would probably become Q_WS_MACX11 ...)?


- RJVB


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119844/#review64928
-----------------------------------------------------------


On Aug. 19, 2014, 7:59 p.m., RJVB Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119844/
> -----------------------------------------------------------
> 
> (Updated Aug. 19, 2014, 7:59 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and Valentin Rusu.
> 
> 
> Repository: kwallet
> 
> 
> Description
> -------
> 
> The submitted improvements to kwallet consist of a number of changes to existing files, as well as 2 new files that contain the actual interface to OS X's SecKeychain API.
> 
> With these modifications, KDE wallets are stored in the same location as native OS X keychains, and both can be managed (up to a certain extent) in the OS X Keychain utility as well as the kwalletmanager. In addition, password prompts no longer get posted somewhere in the background.
> 
> 
> Diffs
> -----
> 
>   src/api/KWallet/CMakeLists.txt 9709559 
>   src/api/KWallet/kwallet_mac.cpp d93e5ae 
>   src/api/KWallet/qosxkeychain.h PRE-CREATION 
>   src/api/KWallet/qosxkeychain.cpp PRE-CREATION 
>   tests/KWallet/CMakeLists.txt b155f64 
>   tests/KWallet/kwallettest.cpp 3351a6b 
> 
> Diff: https://git.reviewboard.kde.org/r/119844/diff/
> 
> 
> Testing
> -------
> 
> Testing and development of these was done on OS X 10.6.8 running KDE 4.12.5, which is part of my production environment (https://trac.macports.org/ticket/44473). Since I am not currently able to build (parts of) KF5, porting of my modifications to KF5/KWallet has been done in source only. However, I have good hope that there will be little bugs to review in this request, given the lack of non-esthetic (formatting) modifications to the current kwallet_mac.cpp and kwallettest.cpp.
> 
> 
> Thanks,
> 
> RJVB Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20140829/51312cc5/attachment.html>