Review Request: Make DBusMenuQt optional

[Aurélien Gâteau]

> On 2010-08-04 16:08:34, Aurélien Gâteau wrote:
> > The change looks OK, but I am wondering why you do not want to link with DBusMenuQt. Having (very) different code paths is potentially a source of trouble for maintenance.
> 
> Marc Mutz wrote:
>     Having that kind of library as a hard dependency of KDE is just gross. Have a look at what other libraries kdelibs depends on, and how many of them are actually required. This library is used for a one-liner, literally. It shouldn't be required, esp. as there doesn't seem to be any release yet.
>     Symptomatic post: http://www.mail-archive.com/kde-buildsystem%40kde.org/msg05473.html
>     IIRC, there once was a policy to not require new (hard) dependencies in new releases. Seems to have slipped off the radar, though.
>     
>     On top of that, on mobile, every dependency we don't have to have on the phone is a good dependency :)
> 
> Aurélien Gâteau wrote:
>     I don't see how depending on such a library is "gross", can you explain a bit more?
>     
>     Regarding the message you refer to, I agree the dependency could have been introduced in a much nicer way and I acknowledged this on my blog ( http://agateau.wordpress.com/2010/04/29/about-dbusmenu-holidays-and-other-hacks/ ). I thought that episode was behind us now.
>     
>     Regarding the lack of releases, here is the url where you can download release tarballs: https://launchpad.net/libdbusmenu-qt/+download . There is a link to the Launchpad page in the FindDBusMenuQt.cmake file.
>     
>     When DBusMenuQt is not used, the application is in charge of showing the menu, not the system tray. This is what I mean with a very different code path. I won't oppose against the change, though. I think the decision should be up to Marco Martin (can you add him to the "People" field?)
> 
> Marc Mutz wrote:
>     > I don't see how depending on such a library is "gross", can you explain a bit more?
>     
>     Sorry, it's not the depending on it per se. I find it gross that DBusMenuQt is required when it's only used in a single file in the notification subsystem of kdeui whereas not even OpenSSL is a required dependency, without which you can't even use half the world's web sites.
>     
>     > Regarding the lack of releases, here is the url where you can download release tarballs:
>     > https://launchpad.net/libdbusmenu-qt/+download .
>     > There is a link to the Launchpad page in the FindDBusMenuQt.cmake file.
>     
>     All that the macro_log_feature() in kdelibs/CMakeLists.txt told me was to git-clone something. I didn't occur to me to look in FindDBusMenuQt.cmake for a link to releases :)
> 
> Marc Mutz wrote:
>     Oh, and what does libdbusmenu-qt do on Windows, WinCE, and OSA X?

> I find it gross that DBusMenuQt is required when it's only used in a single file in the notification subsystem of kdeui

One half of DBusMenuQt is used in kdeui, the other half is used in the systemtray applet (which you would need to patch as well).

> All that the macro_log_feature() in kdelibs/CMakeLists.txt told me was to git-clone something. I didn't occur to me to look in FindDBusMenuQt.cmake for a link to releases :)

The url is an argument to find_package_handle_standard_args(), not a comment. I assumed it would show up when you ran cmake and it failed to find the library. I may be wrong.

> Oh, and what does libdbusmenu-qt do on Windows, WinCE, and OS X?

Honestly I don't know, but I know the KDE Windows people have contributed patches to ensure it builds, so I guess it works.


- Aurélien


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4898/#review6793
-----------------------------------------------------------


On 2010-08-05 04:38:36, Marc Mutz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4898/
> -----------------------------------------------------------
> 
> (Updated 2010-08-05 04:38:36)
> 
> 
> Review request for kdelibs and Marco Martin.
> 
> 
> Summary
> -------
> 
> Make DBusMenuQt optional.
> 
> 
> Diffs
> -----
> 
>   /branches/KDE/4.5/kdelibs/CMakeLists.txt 1159039 
>   /branches/KDE/4.5/kdelibs/ConfigureChecks.cmake 1159039 
>   /branches/KDE/4.5/kdelibs/config.h.cmake 1159039 
>   /branches/KDE/4.5/kdelibs/kdeui/CMakeLists.txt 1159039 
>   /branches/KDE/4.5/kdelibs/kdeui/notifications/kstatusnotifieritem.cpp 1159039 
> 
> Diff: http://reviewboard.kde.org/r/4898/diff
> 
> 
> Testing
> -------
> 
> Compiles w/o DBusMenuQt present.
> 
> 
> Thanks,
> 
> Marc
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20100805/83161653/attachment.htm>