On Mon, Nov 2, 2009 at 5:07 PM, Hendrik Sattler <p...@hendrik-sattler.de> wrote:
> Zitat von Mathieu Malaterre <mathieu.malate...@gmail.com>:
>
>> Hendrik,
>>
>> On Mon, Nov 2, 2009 at 3:19 PM, Hendrik Sattler  <p...@hendrik-sattler.de>
>> wrote:
>>>
>>> Zitat von Mathieu Malaterre <mathieu.malate...@gmail.com>:
>>>
>>>> I updated the FindJava.cmake. It now implements the VERSION* stuff,
>>>> and I fixed the naming convention as per the readme.txt. I tested on
>>>> linux/gcj, linux/sun-java and linux/openjdk-6
>>>
>>> Why are you using NO_DEFAULT_PATH and but listing /usr/bin and
>>> /usr/local/bin? This is wrong and breaks "/usr/local/bin before
>>> /usr/bin".
>>> Just drop those from the list and also your NO_DEFAULT_PATH. If you want
>>> your list first, use HINTS instead of PATHS.
>>
>> Fixed. Thanks !
>
> Thanks, too.
>
> However, it can still be improved. The current (partial) list is:
>  /usr/lib/java/bin
>  /usr/share/java/bin
>  /usr/local/java/bin
>  /usr/local/java/share/bin
>  /usr/java/j2sdk1.4.2_04
>  /usr/lib/j2sdk1.4-sun/bin
>  /usr/java/j2sdk1.4.2_09/bin
>  /usr/lib/j2sdk1.5-sun/bin
>  /opt/sun-jdk-1.5.0.04/bin
>
> I am still not sure that the path list is good.

This is mostly for backward compat. policy only IMHO. I do not think
*anyone* is actually relying on those.

> Newer version should come
> first to possibly match the version criteria. OTOH, if there is a version
> request for EXACT 1.4, finding version 1.5 is wrong when 1.4 is also
> installed (remember: more than one can be installed at the same time). Same
> applies for the registry entries. The list should be assembled according to
> the version requested. There's no point in looking for other versions than
> the requested one(s).
> The odd paths like /usr/java/j2sdk1.4.2_04 (actually the whole list above)
> should just go away: why only list those specific versions and locations? If
> java is installed there, it is likely to either have symlinks to the normal
> bin locations or to be in PATH or JAVA_HOME.

I do agree for the HKEY* stuff, but deducing version information from
path is very hazardous. Eg on my system, kaffe/java 1.4 is at:
/usr/lib/kaffe/. While Java 1.6 is either
/usr/lib/jvm/java-6-sun-1.6.0.12/ or /usr/lib/jvm/java-6-openjdk/
(easy to read for human, much harder for a machine).

> I also think that caching $ENV{CLASSPATH} should be part of the module. This
> can be done like this:
> if ( NOT Java_CLASSPATH )
>  set ( Java_CLASSPATH $ENV{CLASSPATH} CACHE STRING "java classpath" )
> endif ( NOT Java_CLASSPATH )
> if ( NOT CMAKE_SYSTEM_NAME STREQUAL "Windows" )
>  # Non-Windows classpath may use : instead of ;
>  # so make this a cmake list here by always using ;
>  string ( REPLACE ":" ";" Java_CLASSPATH "${Java_CLASSPATH}" )
> endif ( NOT CMAKE_SYSTEM_NAME STREQUAL "Windows" )
> mark_as_advanced ( Java_CLASSPATH )
>
> This makes it easier to call java with:
> set(foo_CP "${Java_CLASSPATH})
> list(APPEND foo_CP /location/to/foo.jar)
> add_custom_command (
>  OUTPUT ${foo}
>  COMMAND "${Java_JAVA_EXECUTABLE}" -cp "${foo_CP}" ${foo_main_class}
>  DEPENDS "${bar}"
>  VERBATIM
> )

I have to leave now, please open a bug report+assign it to me thanks !

-- 
Mathieu
_______________________________________________
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://www.cmake.org/mailman/listinfo/cmake

Reply via email to