Re: [cmake-developers] [PATCH] LoadCommand test: cleanup

2011-12-05 Thread Rolf Eike Beer 
> On Mon, Oct 17, 2011 at 9:18 AM, Rolf Eike Beer  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

Re: [cmake-developers] [PATCH] LoadCommand test: cleanup

2011-12-05 Thread David Cole 
On Mon, Oct 17, 2011 at 9:18 AM, Rolf Eike Beer  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?
>
> 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

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.)

Thanks,
David
--

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

Re: [cmake-developers] [PATCH] LoadCommand test: cleanup

2011-10-17 Thread Rolf Eike Beer 
> 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?

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

Re: [cmake-developers] [PATCH] LoadCommand test: cleanup

2011-10-06 Thread Bill Hoffman 

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.


-Bill

--

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

[cmake-developers] [PATCH] LoadCommand test: cleanup

2011-10-06 Thread Rolf Eike Beer 
>From be44756ad12e28b7640076f485f6a740ca6598d7 Mon Sep 17 00:00:00 2001
From: Rolf Eike Beer 
Date: Thu, 6 Oct 2011 12:04:12 +0200

This removes some useless checking. The results of these things are never
properly checked so they should not count as testcases. At the end they only
needlessly clutter the output.
---
 Tests/LoadCommand/CMakeCommands/CMakeLists.txt |3 ---
 Tests/LoadCommand/CMakeLists.txt   |6 --
 Tests/LoadCommand/LoadedCommand.h.in   |6 --
 3 files changed, 0 insertions(+), 15 deletions(-)



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)
 INCLUDE_DIRECTORIES(${CMAKE_ROOT}/include ${CMAKE_ROOT}/Source)
 
 ADD_LIBRARY(cmCMAKE_TEST_COMMAND MODULE cmTestCommand.c)
diff --git a/Tests/LoadCommand/CMakeLists.txt b/Tests/LoadCommand/CMakeLists.txt
index e99105a..846cbb0 100644
--- a/Tests/LoadCommand/CMakeLists.txt
+++ b/Tests/LoadCommand/CMakeLists.txt
@@ -12,12 +12,6 @@ INCLUDE (CheckFunctionExists)
 CHECK_FUNCTION_EXISTS(printfHAVE_PRINTF)
 CHECK_FUNCTION_EXISTS(vsblabla  HAVE_VSBLABLA)
 
-INCLUDE (${CMAKE_ROOT}/Modules/CheckIncludeFile.cmake)
-CHECK_INCLUDE_FILE("sys/prctl.h"HAVE_SYS_PRCTL_H)
-
-INCLUDE (${CMAKE_ROOT}/Modules/CheckLibraryExists.cmake)
-CHECK_LIBRARY_EXISTS(m ceil "" HAVE_LIBM)
-
 CONFIGURE_FILE(${LoadCommand_SOURCE_DIR}/LoadedCommand.h.in
${LoadCommand_BINARY_DIR}/LoadedCommand.h)
 
diff --git a/Tests/LoadCommand/LoadedCommand.h.in 
b/Tests/LoadCommand/LoadedCommand.h.in
index 7a0a15d..7516a66 100644
--- a/Tests/LoadCommand/LoadedCommand.h.in
+++ b/Tests/LoadCommand/LoadedCommand.h.in
@@ -5,9 +5,3 @@
 /* Check for functions */
 #cmakedefine HAVE_PRINTF
 #cmakedefine HAVE_VSBLABLA
-
-/* Check for headers */
-#cmakedefine HAVE_SYS_PRCTL_H
-
-/* Check for libraries */
-#cmakedefine HAVE_LIBM
-- 
1.7.6.4


--

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