add support to LDFLAGS env variable for modules and shared libs #10

Злито
MicheleC злито 1 комітів з feat/linker_flags до master 5 роки тому
efferre прокоментував(ла) 5 роки тому
Співавтор

CMAKE_*_LINKER_FLAGS inherith from LDFLAGS. The current implementation of CMakeLists.txt
overrides this behaviour.

CMAKE_*_LINKER_FLAGS inherith from LDFLAGS. The current implementation of CMakeLists.txt overrides this behaviour.
efferre прокоментував(ла) 5 роки тому
Автор
Співавтор

I have already prepared the patches for all the other repositories. If this patch looks fine for you, I can proceed by pushing the feat/linker_flags of all the other repositories creating the respective pull requests.

I have already prepared the patches for all the other repositories. If this patch looks fine for you, I can proceed by pushing the feat/linker_flags of all the other repositories creating the respective pull requests.
SlavekB прокоментував(ла) 5 роки тому
Власник

@efferre, now I finally understand the commits that have begun to appear here today!

First I was confused when a branch feat/linker_flags appeared on the main TDE/tde repository and then a strange commit in TDE/experimental repository. Because the branch on the main TDE/tde repository was messing up for automation scripts on VPS, I removed it.

Now your commit already makes good sense. Please, have you tested whether CMAKE_SHARED_LINKER_FLAGS is treated as a list? If it is used as a list, it would be better to use

list( APPEND CMAKE_SHARED_LINKER_FLAGS "-Wl,-no-undefined" )

instead of

set( CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} ..." )

If it is used as a string, then your commit looks like an appropriate solution.

@efferre, now I finally understand the commits that have begun to appear here today! First I was confused when a branch feat/linker_flags appeared on the main [TDE/tde](../tde) repository and then a strange commit in [TDE/experimental](../experimental) repository. Because the branch on the main TDE/tde repository was messing up for automation scripts on VPS, I removed it. Now your commit already makes good sense. Please, have you tested whether CMAKE_SHARED_LINKER_FLAGS is treated as a list? If it is used as a list, it would be better to use ``` list( APPEND CMAKE_SHARED_LINKER_FLAGS "-Wl,-no-undefined" ) ``` instead of ``` set( CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} ..." ) ``` If it is used as a string, then your commit looks like an appropriate solution.
efferre прокоментував(ла) 5 роки тому
Автор
Співавтор

I am sorry for the noise but I have been making experience with git submodules and with gitea REST API :-P Right now I have pushed only changes to tdelibs repo as pilot test, I can easily continue with the others now that I have a script to iterate over all the 65 modified submodules :-)

The CMAKE_*_LINKER_FLAGS variables are here used as strings. I could use the

string( APPEND ... )

form in the patches but I don't think it makes a big difference comparing to how is implemented now.

If you agree I will continue with the pushes and pull requests

I am sorry for the noise but I have been making experience with git submodules and with gitea REST API :-P Right now I have pushed only changes to tdelibs repo as pilot test, I can easily continue with the others now that I have a script to iterate over all the 65 modified submodules :-) The CMAKE_*_LINKER_FLAGS variables are [here](https://github.com/Kitware/CMake/blob/master/Modules/CMakeCommonLanguageInclude.cmake) used as strings. I could use the string( APPEND ... ) form in the patches but I don't think it makes a big difference comparing to how is implemented now. If you agree I will continue with the pushes and pull requests
SlavekB прокоментував(ла) 5 роки тому
Власник

Thank you for verifying the list × string. In this case, the existing patch is ok. Using string( APPEND ... ) is not possible because it is a new in CMake 3.4.0 while we hold CMake 2.8.x support.

You can go ahead and push the remaining pull requests.

Thank you for verifying the list × string. In this case, the existing patch is ok. Using `string( APPEND ... )` is not possible because it is a new in CMake 3.4.0 while we hold CMake 2.8.x support. You can go ahead and push the remaining pull requests.
SlavekB прокоментував(ла) 5 роки тому
Власник

By the way, did you think about GPG signing?

By the way, did you think about GPG signing?
MicheleC додав(ла) до R14.0.6 release етапу 5 роки тому
MicheleC прокоментував(ла) 5 роки тому
Власник

Fabio, when you create the other 64 PRs of this type, please add the milestone R14.0.6 to them, since we will also backport this change to the current stable branch.

And keep up the good work :-) 👍

Fabio, when you create the other 64 PRs of this type, please add the milestone R14.0.6 to them, since we will also backport this change to the current stable branch. And keep up the good work :-) :+1:
efferre прокоментував(ла) 5 роки тому
Автор
Співавтор

I have completed the pull requests targeting to milestone R14.0.6.

Right now I am not using GPG, in case I'll consider it in the future.

I have completed the pull requests targeting to milestone R14.0.6. Right now I am not using GPG, in case I'll consider it in the future.
MicheleC закрив цей запит на злиття 5 роки тому
MicheleC видалена гілка feat/linker_flags 5 роки тому
Запит на злиття був влитиий як d8177b4058.
Підпишіться щоб приєднатися до обговорення.
Немає рецензентів
Етап відсутній
Немає виконавця
3 учасників
Сповіщення
Дата завершення

Термін виконання не встановлений.

Залежності

No dependencies set.

Reference: TDE/tdelibs#10
Завантаження…
Тут ще немає жодного змісту.