cmake-init icon indicating copy to clipboard operation
cmake-init copied to clipboard

Use target_link_options to pass linker options

Open hbwang15 opened this issue 3 years ago • 5 comments

some spell errors found when i use cmake-init, see the differences below

hbwang15 avatar Jun 16 '21 03:06 hbwang15

Thank you for the PR. However, I would not call this a "spell error". You are replacing one command (target_link_libraries) with another command (target_link_options). This command did not exist before in cmake, it only entered the language in version 3.13 (see https://cmake.org/cmake/help/latest/command/target_link_options.html).

Now, you are right that the new command is semantically the "right" one, now that they finally decided to differentiate it from the former command. However, both commands do the same thing in the end, and if we want to use the new command, we have to raise the minimum version requirement to 3.13 from 3.0, which it is at now. We should discuss if we want to do this for a change that does not have any functional improvement.

@hbwang15: Please raise the mimimum cmake version in the main CMakeLists.txt and add that to the PR.

Everyone: What's your opinion about this change?

sbusch42 avatar Jun 16 '21 03:06 sbusch42

Thank you for the PR. However, I would not call this a "spell error". You are replacing one command (target_link_libraries) with another command (target_link_options). This command did not exist before in cmake, it only entered the language in version 3.13 (see https://cmake.org/cmake/help/latest/command/target_link_options.html).

Now, you are right that the new command is semantically the "right" one, now that they finally decided to differentiate it from the former command. However, both commands do the same thing in the end, and if we want to use the new command, we have to raise the minimum version requirement to 3.13 from 3.0, which it is at now. We should discuss if we want to do this for a change that does not have any functional improvement.

@hbwang15: Please raise the mimimum cmake version in the main CMakeLists.txt and add that to the PR.

Everyone: What's your opinion about this change?

Sorry for my limit knowledge on cmake , I just misunderstand about the command {target_link_libraries} as use for adding link options, As cmake already give more clear command, that is {target_link_options}, so i strongy suggest to upgrade cmake-init to make it more clear for users.

hbwang15 avatar Jun 16 '21 05:06 hbwang15

I'm, too, thinking about our minimum required CMake versions from time to time and for me it depends on the available CMake versions for our target platforms, the availability of fallbacks for modern features/interfaces, and the severity if missing features if a platform does not yet support a more current version. A quick check for the latest Ubuntu LTS (Ubuntu 20.04) shows availability of CMake 3.16.

Following my checklist we would have:

  • [x] Availability of feature on (one/my) target platform
  • [x] Availability of a fallback, that is the current implementation
  • [x] No missing feature as a fallback is available

However, I'm still undecided and suggest to investigate / clarify the following issues:

  • [ ] Is CMake 3.16 available on all our target platforms?
  • [ ] What is the oldest release we want to support? (currently we use features up to CMake 3.2 with graceful degration to 3.0)
  • [ ] How important is it for us to use the most specific interface for the same task, even if it limits portability of our template?

Summarizing, I'm glad that this interface exists in modern CMake but I'm not sure how it should influence this project as our template works completely fine with CMake 3.0.

scheibel avatar Jul 02 '21 10:07 scheibel

Can we provide an own implementation of target_link_options that is implemented in cmake/Custom.cmake if the official version is not available?

scheibel avatar Jul 02 '21 10:07 scheibel

Somehow, this seems to be an implementation to #103.

scheibel avatar Feb 13 '23 22:02 scheibel