Address issues from TriBITS snapshot into Trilinos from PR trilinos/Trilinos#9894 (#299) #433
Comments
|
CC: @marcinwrobel1986, @mperrinel PR #435 should address the configure failures with Albany and Nalu reported in trilinos/Trilinos#9972 and trilinos/Trilinos#9973. |
Turns out some Trilinos configure scripts (including some of the ATDM Trilinos configure scripts) use arbitary linker arguments like '-mkl' that are not -L or -l. These arguments need to be moved forward on the link link in the 'all_libs' target. Since this is not very general, I added a 'NOTE' printed to STDOUT in order to warn about this. NOTE: This now allows an agument like: '-o some-name' and it will move '-o' forward and in the next commit the bare 'some-name' arg will be interepreted as a library name and be handled the same as '-l<libname>'. But obviously splitting up the arguments `-o some-name` by moving '-o' forward by itself and treating `some-name` as a link library is just wrong but I don't see how we can avoid that.
…riBITSPub#299, TriBITSPub#433) Of note: * Added unit test to make that single-char lib name like 'm' will be interepreted as a library. * The arguments '-o some-other-option' that were an error before are now allowed and '-o' is interepreted as its own link option and moved forward and 'some-other-option' is interpreted as a library name.
…riBITSPub#299, TriBITSPub#433) Turns out some Trilinos configure scripts (including some of the ATDM Trilinos configure scripts) use arbitary linker arguments like '-mkl' that are not -L or -l. These arguments need to be moved forward on the link link in the 'all_libs' target. Since this is not very general, I added a 'NOTE' printed to STDOUT in order to warn about this. NOTE: This now allows an agument like: '-o some-name' and it will move '-o' forward and in the next commit the bare 'some-name' arg will be interepreted as a library name and be handled the same as '-l<libname>'. But obviously splitting up the arguments `-o some-name` by moving '-o' forward by itself and treating `some-name` as a link library is just wrong but I don't see how we can avoid that.
…riBITSPub#299, TriBITSPub#433) Of note: * Added unit test to make that single-char lib name like 'm' will be interepreted as a library. * The arguments '-o some-other-option' that were an error before are now allowed and '-o' is interepreted as its own link option and moved forward and 'some-other-option' is interpreted as a library name.
…-linkflags Fix issues with <tplName>Config.cmake generation for TriBITGS TPLs from trilinos/Trilinos#9894 (#433)
…targets-1-again (TriBITSPub/TriBITS#433) Should address all of the issues with the merge of PR trilinos#9894 listed out in TriBITSPub/TriBITS#433 (which is part of TriBITSPub/TriBITS#299). This should resolve the failures reported in trilinos#9972 and trilinos#9973.
TriBITSPub#433) This looks better in the output when it is performed with the rest of TriBITS.
|
I am having to create reference builds of several application codes with Trilinos so that I can compare with the updated TriBITS build system. This is taking a lot of time so I am putting this back in progress. |
|
I have debugged the problem with Albany and the updated TriBITS build system with Trilinos that is in trilinos/Trilinos#9978. The issue is described in detail in #299 (comment). We need to decide to what level of backwards compatibility we are going to maintain (even incorrect usage of TriBITS?). |
|
Next, I am going to bring back deprecated non-namespaced IMPORTED targets as per #299 (comment) before moving on the Nalu and SPARC. |
|
The issue with SPARC has exposed another serious break in backwards compatibility. See #299 (comment) and #443 (and let's not discuss it here). |
|
FYI: We have given SPARC its own gtest implementation to resolve the build issues on the 'van1-tx2' system as implemented in internal MR sparc/sparc-ear99!882. Once they merge that, MR to SPARC 'master', this SPARC issue will no longer be an impediment for this TriBITS work. But we still need to address (at least with documentation) the |
|
With the merge of internal MR sparc/sparc-ear99!890, SPARC now has its own internal build of gtest that will be used instead of any other gtest installed in Trilinos or on the system (because the include dirs for the internally built gtest are included with -I and come before all other include dirs). |
|
FYI: I verified that the internal build of googletest (gtest) with SPARC does not appear to have broken any of their nighty builds. See internal issue tribitsrefactoring#3. Now, I can't do this amount of work for every application code that breaks a build due to this upgrade of TriBITS. But at least now with this SPARC case resolved, I can point to that to prove that the include directory issues can be resolved in a case like gtest by simply building the package's own gtest and putting its include dirs first with -I. |
|
The Trilinos PR #9978 is now updated to resolve everything listed in "Tasks" above. Now we just need to merge that PR to Trilinos 'develop'. As I noted in trilinos/Trilinos#9978 (comment), Trilinos 'develop' is now 6 months behind TriBITS 'master'. |
|
The merge of #10614 a week ago with no related issues reported (but a new set of issues reported) seems to suggest we can close this issue. |
Parent Issue:
Child Issues:
Description
This issue is to track TriBITS work to address the problems that were identified with the merge of the updated TriBITGS snapshot into Trilinos with the merge of Trilinos PR trilinos/Trilinos#9894 which lead the the revert PR trilinos/Trilinos#9977. The fixes will be snapshotted to the branch in Trilinos PR trilinos/Trilinos#9978. All of this is related to #299.
Tasks
TPL_<tplName>_LIBRARIESlike-mkldescribed in WILL NOT MERGE: Fix and reapply: TriBITS: Pull in partial refactoring to modern CMake targets (TriBITSPub/TriBITS#299) (#9894, #9972, #9973) trilinos/Trilinos#9978 (comment)TPL_<tplName>_LIBRARIESdescribed in WILL NOT MERGE: Fix and reapply: TriBITS: Pull in partial refactoring to modern CMake targets (TriBITSPub/TriBITS#299) (#9894, #9972, #9973) trilinos/Trilinos#9978 (comment)-isystemfor IMPORTED targets and workarounds (see Investigate and address issue with change to -isystem includes for IMPORTED targets (#299) #443)The text was updated successfully, but these errors were encountered: