KDE/kdelibs/cmake/modules

[Ralf Habacker]

Alexander Neundorf schrieb:
> On Tuesday 26 May 2009, Ralf Habacker wrote:
>   
>> SVN commit 972974 by habacker:
>>
>> added initial support for fixing windows vista uac problem by adding a
>> specific manifest file to executables. Vista manifest support is disabled
>> by default and could be enabled by setting the KDE4_ENABLE_UAC_MANIFEST
>> variable. This support requires kdewin32 >= 0.3.9
>>
>> The basic idea of this patch was announced at kde-buildsystem mailing list
>> http://lists.kde.org/?l=kde-buildsystem&m=124220817129087&w=2 without any
>> objections for about two weeks.
>>
>>
>>
>>  M  +27 -1     CMakeLists.txt
>>  M  +46 -3     KDE4Macros.cmake
>>  A             Win32.Manifest.in
>>
>>
>> --- trunk/KDE/kdelibs/cmake/modules/CMakeLists.txt #972973:972974
>> @@ -1,5 +1,23 @@
>> +if (WIN32)
>> +    OPTION(KDE4_ENABLE_UAC_MANIFEST "add manifest to make vista uac happy"
>> OFF) +    if (KDE4_ENABLE_UAC_MANIFEST)
>> +        if (NOT MT_EXECUTABLE)
>> +            find_program(MT_EXECUTABLE mt
>> +                PATHS ${KDEWIN32_INCLUDE_DIR}/../bin
>> +                NO_DEFAULT_PATH
>> +            )
>> +        endif (NOT MT_EXECUTABLE)
>>     
>
> Please use the "KDE4_" prefix also for MT_EXECUTABLE, so that it is obvious 
> that it is set in FindKDE4Internal.cmake:
> http://techbase.kde.org/Policies/CMake_Coding_Style#Writing_CMake_Find-modules
> "If you need other commands to do special things then it should still begin 
> with XXX_."
>   
fixed
> Why do you have the "if (NOT MT_EXECUTABLE)" around the find_program() call ?
> find_program() only searches if MT_EXECUTABLE is not valid, e.g. empty or 
> NOTFOUND.
fixed too
>
>   
>> --- trunk/KDE/kdelibs/cmake/modules/KDE4Macros.cmake #972973:972974
>> @@ -30,7 +30,6 @@
>>  # Redistribution and use is allowed according to the terms of the BSD
>> license. # For details see the accompanying COPYING-CMAKE-SCRIPTS file.
>>
>> -
>>  # This is for versions of automoc4 which don't provide these two macros.
>>  # If such a version is used, just use the "old" style automoc handling.
>>  if(NOT COMMAND _AUTOMOC4_KDE4_PRE_TARGET_HANDLING)
>> @@ -791,6 +790,47 @@
>>  endmacro (KDE4_ADD_UNIT_TEST)
>>
>>
>> +# add a  manifest file to executables. This macro is used by
>> kde4_add_executable 
[1]
>> +#
>> +# In cmake <= 2.6.4 there is a bug which returns a wrong path from
>> +# get_target_property(var <target_name> LOCATION),  when the
>> +# OUTPUT_NAME property of a target is set before.
>> +#
>> +# To workaround  those cases a specific variable should be set before
>> +# calling kde4_add_executable as shown by the following example:
>> +#
>> +# set(xyz_OUTPUT_NAME test)
>> +# kde4_add_executable( xyz <source>)
>> +# set_target_properties( xyz PROPERTIES OUTPUT_NAME ${xyz_OUTPUT_NAME} )
>> +#
>> +macro (KDE4_ADD_MANIFEST _target_NAME)
>>     
>
> Hmm, this is a new macro.
> Please send a patch to kde-buildsystem before adding new macros to these 
> files: http://techbase.kde.org/Policies/CMake_Commit_Policy
>
> Is this a "public" macro or just for internal use by kde4_add_executable() ? 
>   
the latter, see [1]
> If that's the case, please prefix it with an underscore, so this becomes more 
> obvious.
>   
fixed
> If it is a macro for public use, please add documentation to the top of 
> FindKDE4Internal.cmake
>
>   
>> @@ -831,6 +871,10 @@
>>        add_executable(${_target_NAME} ${_add_executable_param} ${_SRCS})
>>     endif (KDE4_ENABLE_FINAL)
>>
>> +   IF (KDE4_ENABLE_UAC_MANIFEST)
>> +       KDE4_ADD_MANIFEST(${_target_NAME})
>> +   ENDIF(KDE4_ENABLE_UAC_MANIFEST)
>>     
>
> Please use consistently lower case:
>   
fixed
> http://techbase.kde.org/Policies/CMake_Coding_Style#Upper.2Flower_casing
>
>   
thanks for your pointers and links. I added a related note into 
kdelibs/cmake/modules/README.

Ralf