Add ecm_check_linker_flags function
Needs ReviewPublic

Authored by tcberner on Dec 29 2018, 10:56 PM.

Details

Reviewers
dfaure
apol
cgiboudeaux
Group Reviewers
FreeBSD
Summary

kstars appends -Wl,-z,nodump to the linker flags, which is not supported by lld>=7 (previous versions just discareded unkonwn flags).

Add an ecm module to check whether a linker flag is supported.

Usage:

include(ECMCheckLinkerFlags)
[...]
# Check for nodump support
SET(NODUMP_FLAGS "-Wl,-z,nodump")
ecm_check_linker_flags("${NODUMP_FLAGS}" NODUMP_SUPPORTED)
if (NODUMP_SUPPORTED)
    SET(SEC_LINK_FLAGS "${SEC_LINK_FLAGS} ${NODUMP_FLAGS}")
endif ()

Diff Detail

Repository
R240 Extra CMake Modules
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 6466
Build 6484: arc lint + arc unit
tcberner created this revision.Dec 29 2018, 10:56 PM
Restricted Application added projects: Frameworks, Build System. · View Herald TranscriptDec 29 2018, 10:56 PM
Restricted Application added subscribers: kde-buildsystem, kde-frameworks-devel. · View Herald Transcript
tcberner requested review of this revision.Dec 29 2018, 10:56 PM

Why not fixing kstars instead?

Why not fixing kstars instead?

include(ECMCheckLinkerFlags)
[...]
# Check for nodump support
SET(NODUMP_FLAGS "-Wl,-z,nodump")
ecm_check_linker_flags("${NODUMP_FLAGS}" NODUMP_SUPPORTED)
if (NODUMP_SUPPORTED)
    SET(SEC_LINK_FLAGS "${SEC_LINK_FLAGS} ${NODUMP_FLAGS}")
endif ()

^ that is the fix for kstars. -- Of course kstars could also just stop adding nodump ever :)

Why not fixing kstars instead?

include(ECMCheckLinkerFlags)
[...]
# Check for nodump support
SET(NODUMP_FLAGS "-Wl,-z,nodump")
ecm_check_linker_flags("${NODUMP_FLAGS}" NODUMP_SUPPORTED)
if (NODUMP_SUPPORTED)
    SET(SEC_LINK_FLAGS "${SEC_LINK_FLAGS} ${NODUMP_FLAGS}")
endif ()

Of course kstars could also just stop adding nodump ever :)

agreed, kstars should stop adding random flags and just use what ECM already provides.

For this reason, -1 for having this in ECM. This is a workaround for an app issue (and is not unit tested)

apol added a comment.Feb 20 2019, 5:14 PM

Yep, I'd say let's get it in kstars and if other applications start needing it, we move it to ECM to share.