Moving boost to a system include #1137
Comments
|
boost_as_system_lib has been added @bartlettroscoe can you look at this one? |
|
@bathmatt, there should not be any statements like I will rip all of these out and put in optional dependencies on the I should be able to verify this on my local machine by just doing a |
|
@bathmatt, you don't build STKClassic anymore right? So I don't need to worry about fixing up files under that subpakage? |
This has taken care of 173 comile lines but there are another 199 compile lines in STK that are getting included with -I. I am not sure I understand why. It may be that I will need to refactor TriBITS to hanlde the TPL paths separately.
|
I made some progress on this and pushed a trial commit to the branch: However, there are still many compile lines that have the @bathmatt, one easy hack to remove get boost include dirs to use |
|
Correct, we don't use classic anymore.
There is no reason to build that script. I think we can wait until the full
merge.
…On Wed, Mar 15, 2017 at 1:21 PM, Roscoe A. Bartlett < ***@***.***> wrote:
I made some progress on this and pushed a trial commit to the branch:
- https://github.com/trilinos/Trilinos/tree/stk-boost-isystem-1137
However, there are still many compile lines that have the -I form of
Boost include instead of the -isystem form. I am currently digging into
why this might be the case and how to address this.
@bathmatt <https://github.com/bathmatt>, one easy hack to remove get
boost include dirs to use -isystem instead of -I is the post-process the
CMake-generated flags.cmake files and replace -I<boost_include_dir> with -system
<boost_include_dir>. Is that something that would help you in debugging
in the short term? If so, I can likely provide a single script that will
make these replacements pretty fast. If not, I will keep digging trying to
figure out how to get CMake to pu in -isystem <boost_include_dir> instead
of -I<boost_include_dir>.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1137 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AOPDIFpC3-tYje6asns0VYBMB3Ucx1rhks5rmDo0gaJpZM4Mc0Ld>
.
|
) This commit adds the direct TPL dependencies for the 'Boost' and 'BoostLibs' TPLs to all of the STK subpackages. This has been confirmed to result in '-isystem' being used instead of '-I' for the Boost include directory for all object files compiled for Boost libraries (except STKClassic which I did not bother updating since no-one is using anymore). Note that this does *not* currently result in '-isystem' getting used for Boost includes for most of the object files related to test executbles in the STK subpackages under: * stk/stk_doc_tests/ * stk/stk_unit_tests/ This appears to be because this approach only works for exectuables that have upstream libraries in the same SE package. However, I don't think this is worthwile to fix because the driving use case in #1137 should only need the STK libraries built, not the STK tests and executables. The major TriBITS refactoring in TriBITSPub/TriBITS#63 will likely fix this automatically.
|
@Bathmat, if you merge in the branch: (which is only the commit is 644c9dc) and configure Trilinos with Note that this does not result in
But if you don't need to build STK tests and examples, then this should be a problem to you. However, if you need to build the STK tests, please let me know and I will see about getting @bmpersc, can you please work with the STK developers to see if they can accept the path in the commit 644c9dc back in the native STK source repo? @bathmatt, are there other Trilinos packages that are emitting Boost warnings as well? If so, we can make the same changes in their Dependencies.cmake files as well and make them go away. I am putting this Story in review. Please let me know if there is anything more I need to do with this. Otherwise, I will move on to #1140 (part of #1133). DETAILED NOTES: (2017/03/15) I made some progress on this yesterday. I pushed the branch: that has the commit: What that commit actually does is to just add the this casues TriBITS to call However, this only resulted in 173 compile lines including the Boost headers using: as shown by: But there are still 199 compile lines that show that Boost is getting included using: as shown by: Given that 173 + 199 adds up nicely to 372 which is the number object files in stk: That is consistent that there are compile lines that have both forms of Boost includes as shown by: Also note that there are some object files built in the same STK subpackage that have either the It seems that many STK subpackages only have one form or the other of Boost include. Why is this? So I spent about 40 minutes trying to figure out how to get grep and sed to extract out the build directory output from make and could not figure it out. It then spent 5 minutes writing a simple python program Here are all of the directories that use the Here are all of the directories that use the Hum, I wonder if this only impacts test executables? Do the STK subpakages in stk/stk_doc_tests/ and stk/stk_unit_tests/ have any libraries of their own or just test executables? Look at: I think that answers the question. It looks like in order for this appraoch to work with TriBITS currently a TriBITS exectuable must have a TriBITS library added before it. That is why we see the object files built in the directory So is this a TriBITS defect? Perhaps, is it worth fixing? Likely not since the way that TriBITS handles TPLs and TPL include dirs is going to dramatically change as part of TriBITSPub/TriBITS#63. Therefore, for now, I think the solution is to just not built the STK tests and things should be good to go. Going back and configuing STK with no tests enabled, and just building its libraries: Looking for compiles using the Looking for compiles using the So it looks like if you are just building libraries, then the changes made to the STK Dependencies.cmake files in this commit should remove all of the warnings from Boost when building STK libraries. Therefore, I amended this commit log and forced pushed it as: |
bartlettroscoe
added
the
stage: in review
|
@bartlettroscoe this is a huge huge improvement :)... @mwglass can we look at getting this through the chain of commits to get it into the trilinos build? Who takes care of this? Is it @bmpersc |
|
Sure, if someone can send me a patch file with all the necessary changes we can get these pushed to the main STK repo. |
|
Emailed to Mike internally
…On Thu, Mar 16, 2017 at 9:11 AM, mwglass ***@***.***> wrote:
Sure, if someone can send me a patch file with all the necessary changes
we can get these pushed to the main STK repo.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1137 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AOPDIHHMlNvESIjgIAJoU_m90MAzNzQ_ks5rmVEngaJpZM4Mc0Ld>
.
|
The patch can be created by simply doing: The you just apply the patch to the STK native git repo using |
|
Once this is committed to the STK repo and then STK is snapshotted back to Trilinos, then we can delete the branch |
|
The patch was pushed to the Sierra repo today so it will be available in the next STK snapshot. |
|
Thanks!!
…On Fri, Mar 17, 2017 at 2:00 PM, mwglass ***@***.***> wrote:
The patch was pushed to the Sierra repo today so it will be available in
the next STK snapshot.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1137 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AOPDIJd5CIaNJrTZfI12IO5EvwyTDLgjks5rmuZPgaJpZM4Mc0Ld>
.
|
|
I believe this was resolved a long time ago. Closing as complete. |
bartlettroscoe
removed
the
stage: in review
I've been trying to move boost to a system include, however, when I run cmake and build it doesn't look right.
I applied these changes I think everywhere
I make sure I didn't miss any, here is my silly script
and it's clean. but I see
and not the
-Isystemwhere's my mistake?
The text was updated successfully, but these errors were encountered: