-
Notifications
You must be signed in to change notification settings - Fork 568
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
Framework: add SET(LINK_LIBRARIES_ONLY_TARGETS TRUE) to CMakeLists.txt #12339
Conversation
Add an entry detailing what we did to the RELEASE_NOTES, and link to the CMake documentation please. |
CMakeLists.txt
Outdated
@@ -109,6 +109,8 @@ include(SetupTribitsInstall) | |||
|
|||
SET(Trilinos_MUST_FIND_ALL_TPL_LIBS_DEFAULT TRUE) | |||
|
|||
SET(LINK_LIBRARIES_ONLY_TARGETS TRUE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think setting this as a non-cache var is appropriate. It may impact projects like Drekar that use a base Trilinos CMake project but it should be easy to for them to fix any. I am curious to see if all of Trilinos actually configures with this turned on (since raw library names will no longer be accepted).
This should not cause any change in behavior for downstream Trilinos customers just pulling in Trilinos with find_package(Trilinos ...)
or find_package(<TrilinosPackage> ...)
. Therefore, I am not sure it requires a release note.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah good point. This affects developers, not customers. and with how TriBITS automates things it may not even affect developers all that much.
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection is Not Necessary for this Pull Request. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_PR_gcc-8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-8.3.0-serial
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-8.3.0-debug
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_clang-11.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_cuda-11.4.2-uvm-off
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_intel-2021.3
Jenkins Parameters
Using Repos:
Pull Request Author: fryeguy52 |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: Trilinos_PR_gcc-8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-8.3.0-serial
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-8.3.0-debug
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_clang-11.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_cuda-11.4.2-uvm-off
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_intel-2021.3
Jenkins Parameters
|
Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From reading:
LINK_LIBRARIES_ONLY_TARGETS
is a target property, not a variable. I think you need to set:
Please make sure and do a local build and test that this is catching cases like described in:
BTW, when I saw that the PR builds passed above, I knew this option was not getting turned on by this PR because I know there are violations of this in Trilinos. For example: Trilinos/packages/zoltan/src/CMakeLists.txt Lines 671 to 683 in 00cacb8
That passing of of a raw and that shows how to fix this. |
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
2 similar comments
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
@fryeguy52 the reason this appeared to not work was because the variable was being set after the tribits call, which means all of the libraries had already been created/configured. Moving it up in the file fixes it. I actually noticed another variable that was set too low as well, and fixed that as part of #12362. Closing in favor of #12362 (Joe handed this off to me this morning). |
@trilinos/framework
Motivation
Addressees #10081 by adding
SET(LINK_LIBRARIES_ONLY_TARGETS TRUE)
toCMakeLists.txt
Related Issues
#10081