[Tarantool-patches] [PATCH] cmake: set CMP0037 policy to NEW by default
Igor Munkin
imun at tarantool.org
Mon May 25 15:28:47 MSK 2020
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
More information about the Tarantool-patches
mailing list