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: Use linker version scripts #9545

Merged

Conversation

Arfrever
Copy link
Contributor

Autotools build system already uses linker version scripts since
commit 13d165d.

Fixes: #6113

@@ -1,5 +1,5 @@
# Minimum CMake required
cmake_minimum_required(VERSION 3.1.3)
cmake_minimum_required(VERSION 3.18)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case anybody is curious, CheckLinkerFlag cmake module (used below) is documented in https://cmake.org/cmake/help/latest/module/CheckLinkerFlag.html that:

New in version 3.18.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of our CI builds is breaking because it's still using CMake 3.10.1. Is there any easy workaround that wouldn't require the CheckLinkerFlag module?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just checked and it doesn't appear to be easy to upgrade our CMake version, because we're using a Windows VM maintained by another team. I think we should probably just enable the behavior conditionally, only if CMAKE_VERSION is greater than or equal to 3.18 (we already do something like this below).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually would you mind setting the minimum version to 3.10.1? Let's do that and see if anyone complains. Maybe we can try to gradually increase the minimum version over time so that we can start to rely on newer CMake features.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added fallback code using check_cxx_source_compiles function from CheckCXXSourceCompiles module.

Autotools build system already uses linker version scripts since
commit 13d165d.

Fixes: protocolbuffers#6113
@acozzette
Copy link
Member

Thanks, @Arfrever!

@Arfrever Arfrever deleted the cmake-linker-version-scripts branch March 2, 2022 01:33
@veblush
Copy link
Contributor

veblush commented Mar 4, 2022

Is it really required to bump the cmake version to 3.10? FYI, gRPC still supports 3.5.1.

@acozzette
Copy link
Member

Let me send out a pull request to lower it to 3.5.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cmake build system does not make linker use version scripts
4 participants