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

[Igor Munkin <imun@tarantool.org>]

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.

> 

<snipped>

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

-- 
Best regards,
IM