> On Mon, Oct 17, 2011 at 9:18 AM, Rolf Eike Beer <[email protected]> wrote: >>> On 10/6/2011 8:24 AM, Rolf Eike Beer wrote: >>> >>>> Bill, that Watcom stuff was introduced by you in >>>> 9891260a6dab66c9ea44b5c2e244f3128625baf5. >>>> I simply assumed it was a debug leftover as setting a variable to the >>>> value it already has looks pretty pointless to me. >>>> >>>> diff --git a/Tests/LoadCommand/CMakeCommands/CMakeLists.txt >>>> b/Tests/LoadCommand/CMakeCommands/CMakeLists.txt >>>> index 953d05c..5cdbc59 100644 >>>> --- a/Tests/LoadCommand/CMakeCommands/CMakeLists.txt >>>> +++ b/Tests/LoadCommand/CMakeCommands/CMakeLists.txt >>>> @@ -5,9 +5,6 @@ IF (MUDSLIDE_TYPE MATCHES MUCHO) >>>> ADD_DEFINITIONS(-DMUCHO_MUDSLIDE) >>>> ENDIF (MUDSLIDE_TYPE MATCHES MUCHO) >>>> >>>> -IF(WATCOM) >>>> - SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS}") >>>> -ENDIF(WATCOM) >>> >>> Hmmm... It could be some sort of odd escape thing going on that an >>> extra pass by the CMake parser fixes... I guess we can push it into >>> the >>> dashboard and see if it fails. I can not remember at this point, but >>> watcom is a picky odd compiler. >> >> Is anyone going to pick this patch up?
> Why is this patch important? > > Sometimes, some of the "useless" checking that we do in the CMake test > suite is to make sure there's better code coverage on the dashboard, > even if the results of checks are not used. I don't think that's the > case here... but there are sometime non-obvious reasons behind some of > the lines in our tests. > > Since this is just in the test suite, I see no reason to rush it into > a release at the last minute. > > (But if it's important to you, I'm willing to be convinced that it > should be in the release... But you'll have to convince me.) Well, I don't think it's that important that you should do an extra test or whatever for the next release. It's just that this code, well, looks totally wrong. And I would like for this to eventually go upstream. No problem if you put it in some branch and wait for it until the development for 2.8.8 opens. Otherwise this code just looks wrong (tm). If this actually changes anything this looks like a real bug we should find better sooner than later, no? ;) About the coverage: I thought about this, but putting stuff in some totally unrelated tests just doesn't feel right at all. We should try to get some coverage e.g. for the CheckSymbolExists testcase that could prove why this is broken for some gcc optimization settings so that the FindThreads patch I sent didn't work. So if coverage is a problem this should go into it's own test that clearly states what it is testing and why and not being hidden in some other random test. Greetings, Eike -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
