Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMake version needs incrementing #5

Closed
PhilipHazel opened this issue Aug 23, 2021 · 9 comments
Closed

CMake version needs incrementing #5

PhilipHazel opened this issue Aug 23, 2021 · 9 comments
Assignees
Labels
bug Something isn't working

Comments

@PhilipHazel
Copy link
Collaborator

This is a consequence of old Bugzilla #2785, by Jan-Willem Blokland . The patch in that issue was applied, but the minimum CMake version in CMakeLists.txt needs updating (it is very old). These are the final comments:

JWB: Interesting that you see this Deprecation Warning. It turns out that CMake deprecated version older than 2.8.12:

Compatibility with versions of CMake older than 2.8.12 is now deprecated and will be removed from a future version. Calls to cmake_minimum_required() or cmake_policy() that set the policy version to an older value now issue a deprecation diagnostic.

For more details see https://cmake.org/cmake/help/v3.20/release/3.19.html. We could decide to increase the minimum required version to something more recent, like version 3.0 to avoid this warning. If we do so, I am willing to make another update to the CMake build configuration.

PH: Yes, I think we can usefully up the number to 3.0.0, which was released in 2014, so it's unlikely to catch anybody. I will do it sometime.

@PhilipHazel PhilipHazel self-assigned this Aug 23, 2021
@PhilipHazel PhilipHazel added the bug Something isn't working label Aug 23, 2021
@PhilipHazel
Copy link
Collaborator Author

CMakeLists.txt has been updated.

@JackBoosY
Copy link

https://github.com/PhilipHazel/pcre2/blob/072717a61fbb139af5a65d06fe23ea8c32189a45/cmake/pcre2-config.cmake.in#L33

This should be replaced to if (@PCRE2_USE_STATIC_LIBS@), set PCRE2_USE_STATIC_LIBS to 1/0 / TRUE/FALSE / ON/OFF in CMakeLists.txt and configure it the correct fixed value in pcre2-config.cmake.

@PhilipHazel
Copy link
Collaborator Author

Thanks for the comment, but I don't think I know enough about CMake to attempt this without some more detailed clues. I do test CMake under Linux (which is all I have) but I'm not experienced in setting up CMake configurations - that's all been done by others.

@JackBoosY
Copy link

I'll try to improve it in PRs.

@PhilipHazel
Copy link
Collaborator Author

Thank you.

@JackBoosY
Copy link

Currently we have 2 ways:

  1. Use pcre2-config.cmake.in, configure it and define PCRE2_USE_STATIC_LIBS before call find_package(pcre2 CONFIG).
  2. Export all pcre2 targets in CMakeLists.txt, according to BUILD_SHARED_LIBS to distinguish between generating dynamic libraries or static libraries, depreciate option BUILD_STATIC_LIBS since it's not provided by cmake by default, don't need to define PCRE2_USE_STATIC_LIBS when using pcre2.

Which way do you prefer to choose?

@PhilipHazel
Copy link
Collaborator Author

As I know almost nothing about CMake and absolutely nothing about find_package(), I don't really know. I test CMake on Linux by running "cmake" or "ccmake" to configure, then "make" to build. Everything ought to be configurable that way, so perhaps I'm going for option 2. I do recall that one of the more recent changes was to allow BOTH static and dynamic libraries to be built together. That is , it isn't "dynamic or static", it's "dynamic AND/OR static".

@JackBoosY
Copy link

The difference between the two is:

  1. Allow to generate dynamic and static libraries at the same time, generate pcre2-config.cmake according to the preset cmake.in file, and need to set and use the relevant cmake options before use.
  2. Only one kind of library is allowed to be generated in the same build, and pcre2-config.cmake is dynamically generated according to the code in CMakeLists.txt, without setting any cmake options before use.

@PhilipHazel
Copy link
Collaborator Author

We'd better go for #1 then, because an earlier contributor specifically wanted to create both static and dynamic libraries at the same time, as is possible with the Autotools build method. This is change #2 for release 10.38 as listed in ChangeLog. There are some notes there (in ChangeLog) about name clashes in MSVC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants