Re: [Tarantool-patches] [PATCH] cmake: set CMP0037 policy to NEW by default

[Olga Arkhangelskaia <arkholga@tarantool.org>]

Igor, I have fixed nits in commit message. Sorry for that. I have lost

somewhere line from vimrc with spellchecker =(

Moreover, I left warning and old behaviour for cmake 3.10 and less.

See diff below.

25.05.2020 15:28, Igor Munkin пишет:
> Olya,
>
> Thanks for the patch! The warning is gone (I checked the patch manually
> on Gentoo with Cmake 3.14.6), but I see configuration failures on some
> platforms (e.g. Ubuntu 18.04[1]). It looks like the problem occurs when
> NEW policy is used for CMake older than 3.11 (Ubuntu 18.04 default repos
> provides 3.10.2). I reproduced the failure manually with Ubuntu docker.
> After reverting your patch, configuration succeeds with no warnings. It
> looks like we need a custom behaviour for different CMake versions,
> doesn't it?
>
> Furthermore, I left several nits regarding commit message below, please
> consider them.
>
> On 21.05.20, Olga Arkhangelskaia wrote:
>> Hi Igor! Thanks for the review.
>>
>> I did not get Sasha's approach. However, according to cmake
>> documentations name test in
>>
>> add_custom_target can be used if the modules CTest or CPAck is enabled.
>>
>> https://cmake.org/cmake/help/latest/policy/CMP0037.html?highlight=cmp0037
>>
>> So we just need to use NEW behaviour and cmake 3.11 and above.
>>
>> See:
> Please also include the commit message whether it was changed:
> | cmake: set CMP0037 policy to NEW
> |
> | To stop deprecation warning CMP0037 policy was changed to NEW.
>
> Typo: s/stop/fix/.
>
> | Deprecation warnings tells us that OLD behaviour will be dropped in the
>
> Minor: s/tells/inform/.
>
> | next versions.
> |
> | CMP0037 old behavior (cmake 2.8.12) allowed taget names such as test,
>
> Typo: s/taget/target/.
>
> | however in versions 3.10 and below names test, help and etc. were
>
> Typo: consider line break right after <were> to fit 72 symbols.
>
> | complitely anavailable because cmake always reserved them.
>
> Typo: s/complitely/completely/.
> Typo: s/anavailable/unavailable/.
> Minor: I guess <always> can be omitted considering the past tense.
>
> | Starting from cmake 3.11 this names only reserved when the corresponding
>
> Typo: s/this names only reserved/these names are only reserved/.
>
> | feature is enabled (e.g. by including the CTest or CPack modules).
> | Tarantool does not use CTest so the name test can be used.
> | Users have to use cmake 3.11 and above.
>
> Side note: Do we need to update Tarantool build requirements anywhere?
>
> |
> | Closes: #3587
>
> Please, don't use ':' in gh tags. It just doesn't respect our commit
> message style.
>
>> -if(POLICY CMP0037)
>> -    cmake_policy(SET CMP0037 OLD)
>> -endif(POLICY CMP0037)
>> -
>>
>> + if(POLICY CMP0037)
>> +    cmake_policy(SET CMP0037 NEW)
>> + endif(POLICY CMP0037)
>> +
>>
> Side note: there is no whitespace change on the branch.
>
New commit message:

   + cmake: set CMP0037 policy to NEW
   +
   + To fix deprecation warning, CMP0037 policy was changed to NEW for 
cmake
   + 3.11 and above.
   +
   + CMP0037 old behavior (cmake 2.8.12) allowed target names such as test.
   + In cmake 3.10 and below names test, help and etc. were reserved.
   +
   + Starting from cmake 3.11 these names are only reserved when the 
corresponding
   + feature is enabled (e.g. by including the CTest or CPack modules).
   + Tarantool does not use CTest so the name test can be used.
   +
   + Closes: #3587

Code diff:

-if(POLICY CMP0037)
-    cmake_policy(SET CMP0037 NEW)
-endif(POLICY CMP0037)
+if(CMAKE_VERSION VERSION_LESS 3.11)
+    if(POLICY CMP0037)
+        cmake_policy(SET CMP0037 OLD)
+    endif(POLICY CMP0037)
+else()
+    if(POLICY CMP0037)
+        cmake_policy(SET CMP0037 NEW)
+    endif(POLICY CMP0037)
+endif()


> <snipped>
>
> [1]: https://gitlab.com/tarantool/tarantool/-/jobs/562530585
>