[KDE/Mac] Review Request 121960: Support an infix for libqtcurve-utils so different versions can co-exist for GTk2, Qt4 and Qt5 support

[Yichao Yu]

> On 一月 9, 2015, 3:21 p.m., Yichao Yu wrote:
> > lib/utils/CMakeLists.txt, line 39
> > <https://git.reviewboard.kde.org/r/121960/diff/1/?file=339398#file339398line39>
> >
> >     I'm still not convinced that this is necessary. But even if it is, please set the OUTPUT_NAME instead of changing the name of the target. That will be a much smaller change.
> 
> Yichao Yu wrote:
>     http://www.cmake.org/cmake/help/v3.0/prop_tgt/OUTPUT_NAME.html
> 
> René J.V. Bertin wrote:
>     Do you mean that I'd only have to do 
>     
>     ```
>     set_target_properties(${QTC_UTILS_LIBRARY} PROPERTIES
>       VERSION 1.0
>       SOVERSION 1
>       COMPILE_FLAGS "-fvisibility=hidden"
>       LIBRARY_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
>       OUTPUT_NAME "${QTC_UTILS_LIBRARY}")
>     ```
>     
>     with QTC_UTILS_LIBRARY set to the same value as currently (e.g. qtcurve-utils-qt4), and cmake will figure it all out without any additional changes? Even if all the dependent CMake files call for "qtcurve-utils" excplicitly?
> 
> Yichao Yu wrote:
>     No
>     
>     you should keep the target name as is and only set OUTPUT_NAME one it.
>     
>     i.e.
>     
>         set_target_properties(qtcurve-utils PROPERTIES
>           VERSION 1.0
>           SOVERSION 1
>           COMPILE_FLAGS "-fvisibility=hidden"
>           LIBRARY_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
>           OUTPUT_NAME "${QTC_UTILS_LIBRARY}")
>     
>     This way you don't need to change the link name used at other places.
>     
>     In another word, the only thing you need to do is (only 1 or 2 lines)
>     
>         set_target_properties(qtcurve-utils PROPERTIES
>           VERSION 1.0
>           SOVERSION 1
>           COMPILE_FLAGS "-fvisibility=hidden"
>           LIBRARY_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
>           OUTPUT_NAME "qtcurve-utils${QTC_UTILSLIB_INFIX}")
>     
>     And specify `-DQTC_UTILSLIB_INFIX=-gtk2` etc at compile time.
>     Everything else can be the same with before.
> 
> René J.V. Bertin wrote:
>     Oops, yes, I forgot to change the target name back to `qtcurve-utils` .
> 
> René J.V. Bertin wrote:
>     Last question: will your example work even if `QTC_UTILSLIB_INFIX` hasn't been specified on the CMake commandline?
> 
> Yichao Yu wrote:
>     AFAIK, yes.
> 
> René J.V. Bertin wrote:
>     OK.
>     
>     In line with the other options, and to "advertise" it to the user, I propose to keep a simple entry in the toplevel CMake file:
>     
>     ```
>     if(NOT DEFINED QTC_UTILSLIB_INFIX)
>       set(QTC_UTILSLIB_INFIX "")
>     endif()
>     ```
>     
>     or is there a better way? `qtc_option` and `option` are only for boolean options, right?

That's also fine.

`option` is only for Boolean option and but `qtc_option` can be used on any options. You can see the definition right below the `## Options` line and it's very similar to yours.


- Yichao


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


On 一月 9, 2015, 3:14 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121960/
> -----------------------------------------------------------
> 
> (Updated 一月 9, 2015, 3:14 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and Yichao Yu.
> 
> 
> Repository: qtcurve
> 
> 
> Description
> -------
> 
> When building separate QtCurve packages for independent GTk2, Qt4/KDE4 and Qt5/KF5 versions of the style, a conflict arises with libqtcurve-utils. Each of the three versions uses this library, and it needs to be linked with the rendering SDK itself.
> 
> This patch introduces a CMake infix option, `QTC_UTILSLIB_INFIX` that can be used to change the library name to something unique for each package. When the option is not defined, the current behaviour is preserved; this is the default.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt e8dd723 
>   gtk2/style/CMakeLists.txt 451ecdd 
>   lib/cairo/CMakeLists.txt 3363e46 
>   lib/utils/CMakeLists.txt 0dd71c1 
>   qt4/config/CMakeLists.txt 3b65e09 
>   qt4/kwin/CMakeLists.txt 52c7c03 
>   qt4/kwinconfig/CMakeLists.txt 1de6dc2 
>   qt4/style/CMakeLists.txt 4ac461f 
>   qt5/config/CMakeLists.txt b4184dd 
>   qt5/kwin/CMakeLists.txt 52c7c03 
>   qt5/kwinconfig/CMakeLists.txt 1de6dc2 
>   qt5/style/CMakeLists.txt c78a73d 
>   test/CMakeLists.txt 007c595 
> 
> Diff: https://git.reviewboard.kde.org/r/121960/diff/
> 
> 
> Testing
> -------
> 
> On OS X 10.9.5 with Qt4.8/kdelibs 4.14.4 (using libqtcurve-utils-qt4.dylib), GTk2/X11 (libqtcurve-utils-gtk2.dylib) and "pure" Qt 5.3.2 (libqtcurve-utils-qt5.dylib).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20150109/6fb64bdd/attachment-0001.html>