Re: [cmake-developers] Converting cmake_parse_arguments to a builtin command
On 12/06/2013 04:42 PM, Matthew Woehlke wrote: >> An advantage of keeping the name is that existing callers get the >> speed-up immediately. Furthermore there will be less code left in the >> old module to maintain. > > I think that is still true if the module just wraps the new function? Yes, I think so. Be sure that the wrapper forwards empty arguments correctly or the compatibility policy will not be meaningful ;) 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] Converting cmake_parse_arguments to a builtin command
On 2013-12-06 15:42, Brad King wrote: On 12/06/2013 03:11 PM, Matthew Woehlke wrote: On 2013-12-06 14:51, Daniele E. Domenichelli wrote: Are you sure you don't want the command to be renamed to "parse_arguments"? The only commands containing "cmake" looks strictly related to "cmake", and the arguments parsing does not look that much related... FWIW, I was sort-of hoping it would be. If so, CMakeParseArguments.cmake can be left with a simple stub to call the new version. As a bonus, the new version could itself take named arguments instead of positional with a flag whether or not to skip empty (default = keep) with the compatibility wrapper instead specifying to skip, and no policy would be needed (if you want the new behavior, just use parse_arguments). The C++-implemented command would be able to handle both the existing positional or a new proposed keyword-based signature. The name "parse_arguments" is not specific enough about what kind of arguments it parses. By keeping the name as "cmake_parse_arguments" it indicates that it parses cmake-language arguments, though another name such as "process_cmake_command_arguments" would do that too. Sure, I can see the argument. I'm not sure I'm entirely sold on 'cmake_parse_arguments', but 'parse_cmake[_command]_arguments' makes perfect sense to me. (IMHO 'parse' makes more sense than 'process'... the command doesn't actually do anything with the arguments as might be implied with 'process'; just extracts them, which to me is 'parsing'.) I could go either way on 'cmake' vs. 'cmake_command'... the brevity of the former is nice, while the specificity/clarity of the latter is also nice. (Well, maybe not; I think I am actually leaning toward the latter :-).) An advantage of keeping the name is that existing callers get the speed-up immediately. Furthermore there will be less code left in the old module to maintain. I think that is still true if the module just wraps the new function? Matthew, would you have time to work on the C++ impl? I think we could start with reproducing the current signature. Optionally add the policy for handling empty arguments. Then add a proposed keyword-based sig that handles empty arguments always. If someone can sponsor it, maybe? Not for a few days at least, though. -- 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] Converting cmake_parse_arguments to a builtin command
On 12/06/2013 02:51 PM, Daniele E. Domenichelli wrote: > Actually ExternalProject, like several other modules, would probably > benefit on using the C++ implementation instead of having their own > parsing, or using the cmake implementation, but that's for a following > patch. The ExternalProject module does not use cmake_parse_arguments. Its argument parsing is a lightweight state machine and mostly just stores values in target properties for later query. Having to route that through another argument parsing command would not help much. All the values would come back in variables just to turn around and put them in target properties. -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] Converting cmake_parse_arguments to a builtin command
On 12/06/2013 03:11 PM, Matthew Woehlke wrote: > On 2013-12-06 14:51, Daniele E. Domenichelli wrote: >> Are you sure you don't want the command to be renamed to >> "parse_arguments"? The only commands containing "cmake" looks strictly >> related to "cmake", and the arguments parsing does not look that much >> related... > > FWIW, I was sort-of hoping it would be. If so, CMakeParseArguments.cmake > can be left with a simple stub to call the new version. > > As a bonus, the new version could itself take named arguments instead of > positional with a flag whether or not to skip empty (default = keep) > with the compatibility wrapper instead specifying to skip, and no policy > would be needed (if you want the new behavior, just use parse_arguments). The C++-implemented command would be able to handle both the existing positional or a new proposed keyword-based signature. The name "parse_arguments" is not specific enough about what kind of arguments it parses. By keeping the name as "cmake_parse_arguments" it indicates that it parses cmake-language arguments, though another name such as "process_cmake_command_arguments" would do that too. An advantage of keeping the name is that existing callers get the speed-up immediately. Furthermore there will be less code left in the old module to maintain. Matthew, would you have time to work on the C++ impl? I think we could start with reproducing the current signature. Optionally add the policy for handling empty arguments. Then add a proposed keyword-based sig that handles empty arguments always. 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] Converting cmake_parse_arguments to a builtin command
On 12/06/2013 02:51 PM, Daniele E. Domenichelli wrote: > If CMAKE_MINIMUM_REQUIRED_VERSION >= 3.0.0, the NEW policy will be used, > but the author should also be warned that he should no longer include > the CMakeParseArguments.cmake file, since it will be deprecated and > might disappear in the future. I'm not sure if from C++ you can check > that, so an "empty cmake file" won't work... Well, almost empty :) > Does this sound reasonable? Sorry if I stress on keeping the CMake > implementation, but I need a solution that could be backported at least > to CMake 2.8.7 I see no reason to distribute code in the CMake 3.0 version of the module that is meant to work with versions prior to 3.0. It will never be tested or executed while running that version of CMake. It would be an untested fork even though it appears upstream. If you're going to use it with CMake 2.8.7 you will need a copy with your project anyway. There is no reason you can't take your current version of the module from this topic and put it in your project with no change to CMake upstream. > Are you sure you don't want the command to be renamed to > "parse_arguments"? The only commands containing "cmake" looks strictly > related to "cmake", and the arguments parsing does not look that much > related... I'll respond to this part on top of Matthew's response to it. > Please note that ExternalProject does not use CMakeParseArguments and > the ExternalProject-independent-step-targets topic does not depend on > this patch, so I don't see a reason for blocking it on this topic. Ah, I was confused because ExternalProject-independent-step-targets merges CMakeParseArguments_EmptyArgs. I see it does so only for a conflict resolution. I can resolve that as described below. >> I'm convinced that the cmake_parse_arguments command should be >> converted to C++, taught to handle empty arguments, and handle compatibility >> with a policy. Daniele, will you be able to work on this? > > I will try, but I'll have to do it in my free time, and I don't know how > long it will take, because I'm not very confident with CMake "C++" code, If you are not comfortable trying this there is no requirement to do so. Others have expressed interested in this thread and may be able to volunteer. Based on the preceding discussion I do not think we will be changing the upstream module until the C++ implementation is done. Therefore I will revert the CMakeParseArguments_EmptyArgs topic (which will also help in separating ExternalProject-independent-step-targets from it). See my follow-up post in response to Matthew for further discussion of a C++ implementation. 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] Converting cmake_parse_arguments to a builtin command
On 2013-12-06 14:51, Daniele E. Domenichelli wrote: Are you sure you don't want the command to be renamed to "parse_arguments"? The only commands containing "cmake" looks strictly related to "cmake", and the arguments parsing does not look that much related... FWIW, I was sort-of hoping it would be. If so, CMakeParseArguments.cmake can be left with a simple stub to call the new version. As a bonus, the new version could itself take named arguments instead of positional with a flag whether or not to skip empty (default = keep) with the compatibility wrapper instead specifying to skip, and no policy would be needed (if you want the new behavior, just use parse_arguments). -- 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] Converting cmake_parse_arguments to a builtin command
On 05/12/13 14:56, Brad King wrote: > Without a policy project authors will not be warned about the change in > behavior that would be caused by bumping cmake_minimum_required(VERSION). Ok, let's assume that we add a C++ implementation with a CMP policy that does * OLD policy = SKIP_EMPTY * NEW policy = KEEP_EMPTY If CMAKE_MINIMUM_REQUIRED_VERSION < 3.0.0 and CMAKE_VERSION >= 3.0.0, the author will receive a warning about the policy and that the OLD policy is being used. If CMAKE_MINIMUM_REQUIRED_VERSION >= 3.0.0, the NEW policy will be used, but the author should also be warned that he should no longer include the CMakeParseArguments.cmake file, since it will be deprecated and might disappear in the future. I'm not sure if from C++ you can check that, so an "empty cmake file" won't work... If CMAKE_MINIMUM_REQUIRED_VERSION < 3.0.0 and CMAKE_VERSION < 3.0.0, it means that the module was "backported", the OLD policy should be used and no warning issued. So, what about something like this: --- ### Documentation ### ### Copyright statement ### if(NOT CMAKE_MINIMUM_REQUIRED_VERSION VERSION_LESS 3.0.0) message(AUTHOR_WARNING "cmake_parse_arguments is now a builtin function, the CMakeParseArguments is deprecated and could be removed in the future, therefore it should not be included") # Use C++ implementation return() endif() if(COMMAND cmake_parse_arguments) # If CMAKE_VERSION >= 3 will use the C++ implementation, and # therefore will use the policy and when used will print a warning # about the policy. # If CMAKE_VERSION < 3 this file was already included and will avoid # including several times this file. return() endif() # The implementation is left for backwards compatibility, but might # be removed in the future. ### CMake implementation for CMake < 3.0.0 simplified to use the ### ### OLD policy only (SKIP_EMPTY), but that accepts the ### ### CMAKE_PARSE_ARGUMENTS_KEEP_EMPTY argument and ignores the### ### CMAKE_PARSE_ARGUMENTS_SKIP_EMPTY one ### --- Does this sound reasonable? Sorry if I stress on keeping the CMake implementation, but I need a solution that could be backported at least to CMake 2.8.7 (that's the best I was able to achieve, we depended on CMake 2.4 a few months ago in order to support very old systems, and for sure I won't be able to change the dependency on CMake 3.0.0 any time soon), and I hate the idea of "forking" and maintaining my own patched version of the CMakeParseArguments module... Are you sure you don't want the command to be renamed to "parse_arguments"? The only commands containing "cmake" looks strictly related to "cmake", and the arguments parsing does not look that much related... > Currently I do not plan to merge the CMakeParseArguments_EmptyArgs topic or > any topics that depend on it (like ExternalProject-independent-step-targets) > to master. Please note that ExternalProject does not use CMakeParseArguments and the ExternalProject-independent-step-targets topic does not depend on this patch, so I don't see a reason for blocking it on this topic. Actually ExternalProject, like several other modules, would probably benefit on using the C++ implementation instead of having their own parsing, or using the cmake implementation, but that's for a following patch. > I'm convinced that the cmake_parse_arguments command should be > converted to C++, taught to handle empty arguments, and handle compatibility > with a policy. Daniele, will you be able to work on this? I will try, but I'll have to do it in my free time, and I don't know how long it will take, because I'm not very confident with CMake "C++" code, because my time very limited at the moment, and because I have several other patches that I want to work on. If anyone else is in a hurry to have this, and wants to take it, feel free... Cheers, Daniele -- 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] Converting cmake_parse_arguments to a builtin command
On 12/04/2013 08:20 AM, Brad King wrote: > On 12/04/2013 04:57 AM, Daniele E. Domenichelli wrote: >> +1, since this is a very useful feature. > > Actually after thinking about this over night I realized that converting > to a C++ implementation is the best way to fix the empty argument handling > too. The use of "CMAKE_MINIMUM_REQUIRED_VERSION VERSION_LESS 3.0.0" and > the directory-scoped CMAKE_PARSE_ARGUMENTS_DEFAULT_SKIP_EMPTY setting for > compatibility are an approximation of a CMake Policy. We should just use > a policy for it instead so that proper warnings can be given (when empty > arguments are passed) about the change in behavior. While it is possible > to implement a policy in CMake-language code it is much easier in C++ code. > Converting to C++ will have other benefits already discussed. Without a policy project authors will not be warned about the change in behavior that would be caused by bumping cmake_minimum_required(VERSION). With a policy we would be able to fix the NEW behavior test cases to just set the policy instead of hacking the CMAKE_MINIMUM_REQUIRED_VERSION value. Currently I do not plan to merge the CMakeParseArguments_EmptyArgs topic or any topics that depend on it (like ExternalProject-independent-step-targets) to master. I'm convinced that the cmake_parse_arguments command should be converted to C++, taught to handle empty arguments, and handle compatibility with a policy. Daniele, will you be able to work on this? 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
