Review Request 112730: Make knewstuff build standalone

[David Faure]

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



knewstuff/CMakeLists.txt
<http://git.reviewboard.kde.org/r/112730/#comment29696>

    Does this really build when building all of kdelibs in one go? I thought we determined that find_package of kf5 components can't work as long as we build everything together? E.g. I see no find_package(KWindowSystem) in tier2/knotifications.
    
    If you want it to build both ways I think you have to add if (NOT kdelibs_SOURCE_DIR) around the find_package(K*).



knewstuff/src/CMakeLists.txt
<http://git.reviewboard.kde.org/r/112730/#comment29697>

    This can be improved:
    
    use LINK_PUBLIC for libs whose classes appear in the knewstuff API, and LINK_PRIVATE for libs which are purely an implementation detail of knewstuff.
    
    In addition, add a comment on each line about why the dependency is required, like
     ${KArchive_LIBRARIES} # for KZip
    
    This helps spotting unnecessary deps, or spotting what could be done to reduce deps.



knewstuff/src/CMakeLists.txt
<http://git.reviewboard.kde.org/r/112730/#comment29698>

    This would be the LINK_PUBLIC stuff, merge with above.



knewstuff/src/downloaddialog.cpp
<http://git.reviewboard.kde.org/r/112730/#comment29699>

    to answer your question about why this is necessary, I typed "make downloaddialog.cpp.i" in my build (i.e. without this patch applied). This shows me the preprocessed file, which gets #include <klocalizedstring.h> from ui_downloadwidget.h.
    
    This is added there by staging/kde4support/src/kde4uic.cmake
    
    So we have a problem. When not using kde4uic, we end up with tr() instead of i18n() (more precisely tr2i18n()) in uic-generated files.
    This is a problem for libs and apps which use ki18n, since I think they need to use either one of the translation frameworks, but not mix them. Right, Chusslove? Albert? Anyone with a clue? :-)
    
    Should we reintroduce kde4uic.cmake in the ki18n framework (with a better name), to have this work like in kde4?
    
    


- David Faure


On Sept. 17, 2013, 7:29 a.m., Jeremy Whiting wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112730/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2013, 7:29 a.m.)
> 
> 
> Review request for KDE Frameworks, Albert Astals Cid, David Faure, and Chusslove Illich.
> 
> 
> Description
> -------
> 
> This makes it so I can mkdir build; cd build; cmake ../; make install from knewstuff sources.
> It's still using KDE4_KIO_LIBS and find_package(KIO) since not all of the kio libraries have been split out apparently.
> I'm not sure why sources had to be changed, but I had to add includes of klocalizedstring where we didn't need them before somehow.
> 
> 
> Diffs
> -----
> 
>   knewstuff/CMakeLists.txt 3fccbc6ee01a1cc8920321ee7125efb0bfc68412 
>   knewstuff/src/CMakeLists.txt c31398159459b79160ef76f193d6208d19953b4d 
>   knewstuff/src/downloaddialog.cpp 3294c7c04c7879320fc0949db0310868bd6fa4fa 
>   knewstuff/src/downloadwidget.cpp 64b7673d67b4e2f15007fc1a3f57d3da844d1dc0 
>   knewstuff/src/ui/entrydetailsdialog.cpp 65b75d79941d9026f368f82c7b6df91d754e0925 
>   knewstuff/src/uploaddialog.cpp dbde573e8c3a477755c8c866d0ca1fccd1a35729 
> 
> Diff: http://git.reviewboard.kde.org/r/112730/diff/
> 
> 
> Testing
> -------
> 
> It builds and installs.
> 
> 
> Thanks,
> 
> Jeremy Whiting
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130917/96d3b7c6/attachment.html>