Re: [cmake-developers] [review] add_jar (UseJava) uses cmake_parse_arguments
On 03/26/2013 01:35 PM, Matthew Woehlke wrote:
>> This should probably be
>> include(${CMAKE_CURRENT_LIST_DIR}/CMakeParseArguments.cmake)
>
> ...or 'include("${CMAKE_ROOT}/Modules/CMakeParseArguments.cmake")'?
>
> *Most* of the find modules appear to use CMAKE_CURRENT_LIST_DIR, but
> there are some (not necessarily find-package) modules that use
> CMAKE_ROOT, and some (mostly non-find) that rely on CMAKE_MODULE_PATH.
We switched to CMAKE_CURRENT_LIST_DIR some time ago.
Fixed:
http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=78b46403
I'll squash that in to the final version before merging to master and
then rebase the rest of your topic on it.
-Brad
--
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] [review] add_jar (UseJava) uses cmake_parse_arguments
On 2013-03-26 13:05, Rolf Eike Beer wrote:
Brad King wrote:
@@ -190,6 +198,8 @@
# (To distribute this file outside of CMake, substitute the full
# License text for the above reference.)
+include(CMakeParseArguments)
+
function (__java_copy_file src dest comment)
add_custom_command(
OUTPUT ${dest}
This should probably be
include(${CMAKE_CURRENT_LIST_DIR}/CMakeParseArguments.cmake) to avoid issues
with peoples having copies of that module lying around somewhere like we do it
for FPHSA.
...or 'include("${CMAKE_ROOT}/Modules/CMakeParseArguments.cmake")'?
*Most* of the find modules appear to use CMAKE_CURRENT_LIST_DIR, but
there are some (not necessarily find-package) modules that use
CMAKE_ROOT, and some (mostly non-find) that rely on CMAKE_MODULE_PATH.
Which is best?
--
Matthew
--
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] [review] add_jar (UseJava) uses cmake_parse_arguments
Brad King wrote:
> @@ -190,6 +198,8 @@
> # (To distribute this file outside of CMake, substitute the full
> # License text for the above reference.)
>
> +include(CMakeParseArguments)
> +
> function (__java_copy_file src dest comment)
> add_custom_command(
> OUTPUT ${dest}
This should probably be
include(${CMAKE_CURRENT_LIST_DIR}/CMakeParseArguments.cmake) to avoid issues
with peoples having copies of that module lying around somewhere like we do it
for FPHSA.
Eike
--
signature.asc
Description: This is a digitally signed message part.
--
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] [review] add_jar (UseJava) uses cmake_parse_arguments
On 03/26/2013 12:25 PM, Matthew Woehlke wrote:
> Done. Please note that I re-pushed the branch such that the end result
> is the same, but it is now broken into two commits, the first one adding
> just INCLUDE_JARS. So presumably only the first commit is wanted, and
> the second can be left along until 2.8.12.
Thanks. I rebased the topic back on the end of the last topic at adeafad4,
force-pushed to the stage, and then merged the first commit to next:
http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=87b3412a
For reference I included its net diff since 45c8e4b1 below.
Andreas, Nicolas, can you please confirm this change does not break
your existing uses, and even better that the new interface works as
you expect?
Thanks,
-Brad
diff --git a/Modules/UseJava.cmake b/Modules/UseJava.cmake
index 7573576..2752cf3 100644
--- a/Modules/UseJava.cmake
+++ b/Modules/UseJava.cmake
@@ -3,11 +3,19 @@
# has already been loaded. See FindJava.cmake for information on how to
# load Java into your CMake project.
#
-# add_jar(TARGET_NAME SRC1 SRC2 .. SRCN RCS1 RCS2 .. RCSN)
-#
-# This command creates a .jar. It compiles the given source
-# files (SRC) and adds the given resource files (RCS) to the jar file.
-# If only resource files are given then just a jar file is created.
+# add_jar(target_name
+# [SOURCES] source1 [source2 ...] [resource1 ...]
+# [INCLUDE_JARS jar1 [jar2 ...]]
+#)
+#
+# This command creates a .jar. It compiles the given source files
+# (source) and adds the given resource files (resource) to the jar file. If
+# only resource files are given then just a jar file is created. The list of
+# include jars are added to the classpath when compiling the java sources and
+# also to the dependencies of the target. INCLUDE_JARS also accepts other
+# target names created by add_jar. For backwards compatibility, jar files
+# listed as sources are ignored (as they have been since the first version of
+# this module).
#
# Additional instructions:
# To add compile flags to the target you can set these flags with
@@ -190,6 +198,8 @@
# (To distribute this file outside of CMake, substitute the full
# License text for the above reference.)
+include(CMakeParseArguments)
+
function (__java_copy_file src dest comment)
add_custom_command(
OUTPUT ${dest}
@@ -205,7 +215,10 @@ set(_JAVA_CLASS_FILELIST_SCRIPT
${CMAKE_CURRENT_LIST_DIR}/UseJavaClassFilelist.c
set(_JAVA_SYMLINK_SCRIPT ${CMAKE_CURRENT_LIST_DIR}/UseJavaSymlinks.cmake)
function(add_jar _TARGET_NAME)
-set(_JAVA_SOURCE_FILES ${ARGN})
+
+cmake_parse_arguments(_add_jar "" "" "SOURCES;INCLUDE_JARS" ${ARGN})
+
+set(_JAVA_SOURCE_FILES ${_add_jar_SOURCES} ${_add_jar_UNPARSED_ARGUMENTS})
if (NOT DEFINED CMAKE_JAVA_TARGET_OUTPUT_DIR)
set(CMAKE_JAVA_TARGET_OUTPUT_DIR ${CMAKE_CURRENT_BINARY_DIR})
@@ -257,6 +270,7 @@ function(add_jar _TARGET_NAME)
set(_JAVA_CLASS_FILES)
set(_JAVA_COMPILE_FILES)
set(_JAVA_DEPENDS)
+set(_JAVA_COMPILE_DEPENDS)
set(_JAVA_RESOURCE_FILES)
foreach(_JAVA_SOURCE_FILE ${_JAVA_SOURCE_FILES})
get_filename_component(_JAVA_EXT ${_JAVA_SOURCE_FILE} EXT)
@@ -284,7 +298,7 @@ function(add_jar _TARGET_NAME)
OR _JAVA_EXT MATCHES ".war"
OR _JAVA_EXT MATCHES ".ear"
OR _JAVA_EXT MATCHES ".sar")
-list(APPEND CMAKE_JAVA_INCLUDE_PATH ${_JAVA_SOURCE_FILE})
+# Ignored for backward compatibility
elseif (_JAVA_EXT STREQUAL "")
list(APPEND CMAKE_JAVA_INCLUDE_PATH
${JAVA_JAR_TARGET_${_JAVA_SOURCE_FILE}}
${JAVA_JAR_TARGET_${_JAVA_SOURCE_FILE}_CLASSPATH})
@@ -298,6 +312,25 @@ function(add_jar _TARGET_NAME)
endif ()
endforeach()
+foreach(_JAVA_INCLUDE_JAR ${_add_jar_INCLUDE_JARS})
+if (TARGET ${_JAVA_INCLUDE_JAR})
+get_target_property(_JAVA_JAR_PATH ${_JAVA_INCLUDE_JAR} JAR_FILE)
+if (_JAVA_JAR_PATH)
+set(CMAKE_JAVA_INCLUDE_PATH_FINAL
"${CMAKE_JAVA_INCLUDE_PATH_FINAL}${CMAKE_JAVA_INCLUDE_FLAG_SEP}${_JAVA_JAR_PATH}")
+list(APPEND CMAKE_JAVA_INCLUDE_PATH ${_JAVA_JAR_PATH})
+list(APPEND _JAVA_DEPENDS ${_JAVA_INCLUDE_JAR})
+list(APPEND _JAVA_COMPILE_DEPENDS ${_JAVA_INCLUDE_JAR})
+else ()
+message(SEND_ERROR "add_jar: INCLUDE_JARS target
${_JAVA_INCLUDE_JAR} is not a jar")
+endif ()
+else ()
+set(CMAKE_JAVA_INCLUDE_PATH_FINAL
"${CMAKE_JAVA_INCLUDE_PATH_FINAL}${CMAKE_JAVA_INCLUDE_FLAG_SEP}${_JAVA_INCLUDE_JAR}")
+list(APPEND CMAKE_JAVA_INCLUDE_PATH "${_JAVA_INCLUDE_JAR}")
+list(APPEND _JAVA_DEPENDS "${_JAVA_INCLUDE_JAR}")
+list(APPEND _JAVA_COMPILE_DEPENDS "${_JAVA_INCLUDE_JAR}")
+endif ()
+endforeach()
+
# create an empty java_class_filelist
if (NOT EXISTS ${CMAKE_JAVA_CLASS_OUTPUT_PATH}/java_class_filelist)
file(WRITE ${C
Re: [cmake-developers] [review] add_jar (UseJava) uses cmake_parse_arguments
On 2013-03-26 08:16, Brad King wrote: On 03/25/2013 05:20 PM, Matthew Woehlke wrote: Actually, there is a fourth option: I could write a patch that ONLY adds INCLUDE_JARS and doesn't touch the rest of the logic. This might be a more reasonable avenue for adding jar dependency support without changing the historic behavior. This sounds good. It adds a minimal amount of the future interface to support functionality missing from the historic interface. Done. Please note that I re-pushed the branch such that the end result is the same, but it is now broken into two commits, the first one adding just INCLUDE_JARS. So presumably only the first commit is wanted, and the second can be left along until 2.8.12. When looking at the changes, it may be also helpful to compare UseJava.cmake in 7d7eab9124 against the version as of 45c8e4b181; this will give the simplest diff of what has changed to support INCLUDE_JARS (versus 2.8.10) without the noise of the effective partial revert of 53a05b8f51 and adeafad488. Thanks, -- Matthew -- 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] [review] add_jar (UseJava) uses cmake_parse_arguments
On 03/25/2013 05:20 PM, Matthew Woehlke wrote: > Actually, there is a fourth option: I could write a patch that ONLY adds > INCLUDE_JARS and doesn't touch the rest of the logic. This might be a > more reasonable avenue for adding jar dependency support without > changing the historic behavior. This sounds good. It adds a minimal amount of the future interface to support functionality missing from the historic interface. Thanks, -Brad -- 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] [review] add_jar (UseJava) uses cmake_parse_arguments
On Tuesday 26 March 2013 10:33:03 Andreas Schneider wrote: > On Monday 25 March 2013 13:14:33 Brad King wrote: > > On 03/25/2013 12:28 PM, Matthew Woehlke wrote: > > > these must now be explicitly specified with INCLUDE_JARS. Other named > > > arguments are SOURCES (optional, to 'reset' to listing sources after > > > 'INCLUDE_JARS'), OUTPUT_DIR, OUTPUT_NAME, VERSION and ENTRY_POINT. These > > > supersede the old variables, which are still supported for backwards > > > compatibility. > > > > That looks like a nicer interface to me. Andreas, Nicholas, does it > > work for your use cases (and not break existing projects)? > > Yes, that's true. > > > > I'm on the fence if this should target 2.8.11. On the plus side, it > > > means the historic behavior of ignoring jar files listed as sources will > > > be preserved. On the down side, it is late in the cycle and this is a > > > less trivial change. > > > > > > If it does not go in, I will redo the change to preserve the behavior > > > from the previous commits (i.e. jars and targets are accepted in the > > > sources list). > > > > This is one reason I don't like rushing such changes in during the > > RC cycle. Now we have to decide whether to rush an even bigger > > change in, revert the original, or have behavior that was preferred > > for exactly one release and have to support it forever. > > I would prefer this change before we define/add more types of the function > and need to support more backward compatibility. > > It is a pain now. But better than more pain later :) > Please take a look at http://cmake.org/gitweb?p=stage/cmake.git;a=shortlog;h=refs/heads/fix_java_test it fixes JNI detection on Linux Distributions, at least openSUSE and Fedora. -- andreas -- Andreas Schneider GPG-ID: F33E3FC6 www.cryptomilk.orga...@cryptomilk.org -- 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] [review] add_jar (UseJava) uses cmake_parse_arguments
On Monday 25 March 2013 13:14:33 Brad King wrote: > On 03/25/2013 12:28 PM, Matthew Woehlke wrote: > > these must now be explicitly specified with INCLUDE_JARS. Other named > > arguments are SOURCES (optional, to 'reset' to listing sources after > > 'INCLUDE_JARS'), OUTPUT_DIR, OUTPUT_NAME, VERSION and ENTRY_POINT. These > > supersede the old variables, which are still supported for backwards > > compatibility. > > That looks like a nicer interface to me. Andreas, Nicholas, does it > work for your use cases (and not break existing projects)? Yes, that's true. > > > I'm on the fence if this should target 2.8.11. On the plus side, it > > means the historic behavior of ignoring jar files listed as sources will > > be preserved. On the down side, it is late in the cycle and this is a > > less trivial change. > > > > If it does not go in, I will redo the change to preserve the behavior > > from the previous commits (i.e. jars and targets are accepted in the > > sources list). > > This is one reason I don't like rushing such changes in during the > RC cycle. Now we have to decide whether to rush an even bigger > change in, revert the original, or have behavior that was preferred > for exactly one release and have to support it forever. I would prefer this change before we define/add more types of the function and need to support more backward compatibility. It is a pain now. But better than more pain later :) -- andreas -- Andreas Schneider GPG-ID: F33E3FC6 www.cryptomilk.orga...@cryptomilk.org -- 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] [review] add_jar (UseJava) uses cmake_parse_arguments
On 2013-03-25 14:07, Brad King wrote: On 03/25/2013 01:46 PM, Matthew Woehlke wrote: As is: - Pros: add_jar accepts jars as it was apparently intended to, as 'source' arguments - Cons: maybe not optimal interface, must support this going forward Even after the proposed interface goes in we will still have to support the old-style interface whether it has this feature or not. The 'current new' interface has not been in any release, has it? (There is not an rc2 yet, and I believe it was merged after rc1?) Users will be able to choose which style to use. Having a way to get the dependencies in the old-style interface will be useful. So long as it is not too complicated to implement both interfaces once the proposed one is added I think we should go with this option. In 2.8.10, there is no way to specify jar dependencies. In current master, they can be listed as sources. In the branch, INCLUDE_JARS is required. I am not 100% sure I follow the above; do you mean you would prefer to accept jar dependencies without requiring INCLUDE_JARS even if support for INCLUDE_JARS were to land in 2.8.11? (The main argument against would be that it is a behavior change versus 2.8.10... but if you think it is a better interface, I would be okay with that.) This branch: - Pros: cleaner interface, stricter compatibility with 2.8.10 - Cons: non-trivial change, late in release cycle The new interface is complex enough that it should be more thoroughly reviewed and discussed by interested parties. Mostly it is using cmake_parse_arguments instead of setting special variables prior to calling add_jar, but generally agreed... Actually, there is a fourth option: I could write a patch that ONLY adds INCLUDE_JARS and doesn't touch the rest of the logic. This might be a more reasonable avenue for adding jar dependency support without changing the historic behavior. -- Matthew -- 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] [review] add_jar (UseJava) uses cmake_parse_arguments
On 03/25/2013 01:46 PM, Matthew Woehlke wrote: > As is: > - Pros: add_jar accepts jars as it was apparently intended to, as > 'source' arguments > - Cons: maybe not optimal interface, must support this going forward Even after the proposed interface goes in we will still have to support the old-style interface whether it has this feature or not. Users will be able to choose which style to use. Having a way to get the dependencies in the old-style interface will be useful. So long as it is not too complicated to implement both interfaces once the proposed one is added I think we should go with this option. > This branch: > - Pros: cleaner interface, stricter compatibility with 2.8.10 > - Cons: non-trivial change, late in release cycle The new interface is complex enough that it should be more thoroughly reviewed and discussed by interested parties. -Brad -- 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] [review] add_jar (UseJava) uses cmake_parse_arguments
On 2013-03-25 13:14, Brad King wrote: On 03/25/2013 12:28 PM, Matthew Woehlke wrote: I'm on the fence if this should target 2.8.11. On the plus side, it means the historic behavior of ignoring jar files listed as sources will be preserved. On the down side, it is late in the cycle and this is a less trivial change. If it does not go in, I will redo the change to preserve the behavior from the previous commits (i.e. jars and targets are accepted in the sources list). This is one reason I don't like rushing such changes in during the RC cycle. Now we have to decide whether to rush an even bigger change in, revert the original, or have behavior that was preferred for exactly one release and have to support it forever. Essentially, yes. Sorry. I do wish I'd thought of this earlier. For what it's worth, I see advantages and disadvantages either way, and can't say I significantly prefer any of them over the others. Revert: - Pros: no behavior change from 2.8.10 - Cons: is a revert; add_jar not improved this release cycle As is: - Pros: add_jar accepts jars as it was apparently intended to, as 'source' arguments - Cons: maybe not optimal interface, must support this going forward This branch: - Pros: cleaner interface, stricter compatibility with 2.8.10 - Cons: non-trivial change, late in release cycle -- Matthew -- 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] [review] add_jar (UseJava) uses cmake_parse_arguments
On 03/25/2013 12:28 PM, Matthew Woehlke wrote: > these must now be explicitly specified with INCLUDE_JARS. Other named > arguments are SOURCES (optional, to 'reset' to listing sources after > 'INCLUDE_JARS'), OUTPUT_DIR, OUTPUT_NAME, VERSION and ENTRY_POINT. These > supersede the old variables, which are still supported for backwards > compatibility. That looks like a nicer interface to me. Andreas, Nicholas, does it work for your use cases (and not break existing projects)? > I'm on the fence if this should target 2.8.11. On the plus side, it > means the historic behavior of ignoring jar files listed as sources will > be preserved. On the down side, it is late in the cycle and this is a > less trivial change. > > If it does not go in, I will redo the change to preserve the behavior > from the previous commits (i.e. jars and targets are accepted in the > sources list). This is one reason I don't like rushing such changes in during the RC cycle. Now we have to decide whether to rush an even bigger change in, revert the original, or have behavior that was preferred for exactly one release and have to support it forever. -Brad -- 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] [review] add_jar (UseJava) uses cmake_parse_arguments
I have pushed a branch (use-java-use-parse-arguments) to stage that converts add_jar to using cmake_parse_arguments. This partly revers the previous change to accept jars and jar targets as sources for 'linking'; these must now be explicitly specified with INCLUDE_JARS. Other named arguments are SOURCES (optional, to 'reset' to listing sources after 'INCLUDE_JARS'), OUTPUT_DIR, OUTPUT_NAME, VERSION and ENTRY_POINT. These supersede the old variables, which are still supported for backwards compatibility. I'm on the fence if this should target 2.8.11. On the plus side, it means the historic behavior of ignoring jar files listed as sources will be preserved. On the down side, it is late in the cycle and this is a less trivial change. If it does not go in, I will redo the change to preserve the behavior from the previous commits (i.e. jars and targets are accepted in the sources list). -- Matthew --- Begin Message --- I have pushed a branch (use-java-use-parse-arguments) to stage that converts add_jar to using cmake_parse_arguments. This partly revers the previous change to accept jars and jar targets as sources for 'linking'; these must now be explicitly specified with INCLUDE_JARS. Other named arguments are SOURCES (optional, to 'reset' to listing sources after 'INCLUDE_JARS'), OUTPUT_DIR, OUTPUT_NAME, VERSION and ENTRY_POINT. These supersede the old variables, which are still supported for backwards compatibility. I'm on the fence if this should target 2.8.11. On the plus side, it means the historic behavior of ignoring jar files listed as sources will be preserved. On the down side, it is late in the cycle and this is a less trivial change. If it does not go in, I will redo the change to preserve the behavior from the previous commits (i.e. jars and targets are accepted in the sources list). -- Matthew --- End Message --- -- 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
