Re: [cmake-developers] CMakeParseArguments: Do not skip empty arguments
On 12/04/2013 09:52 AM, Daniele E. Domenichelli wrote:
> Anyway, even with the same name if someone needs the empty arguments
> parsing with an older cmake version, he will have to write it from
> scratch if the version in cmake is empty...
Do you mean he won't be able to copy it out of a newer CMake into his
project to use with an older CMake? IMO that should not influence our
upstream decisions. If you need this you can take the impl you have
in the topic now and use it elsewhere.
> Also all the modules will either not include CMakeParseArguments and use
> the system command, or (according to what I believe it's current policy)
> will include it from ${CMAKE_CURRENT_LIST_DIR}, and therefore the empty
> one will be included
CMake's own modules can be updated to not include the legacy module.
Project code will include it and it will do nothing. I do not see
a problem.
-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] CMakeParseArguments: Do not skip empty arguments
On 04/12/13 14:20, Brad King wrote:
> The module can be provided as an empty file except for documentation.
> Any project including it will expect to be able to call cmake_parse_arguments
> afterwards, which they will be able to since it will be a builtin command.
> With the policy handling compatibility there is no need to keep the old
> macro implementation. If a project has its own copy of the module it
> will simply replace the builtin command and get the original behavior.
I was assuming for some reason that the c++ command would have been
"parse_arguments" instead of "cmake_parse_arguments", probably because
the only "cmake_" commands are strictly related to cmake.
Anyway, even with the same name if someone needs the empty arguments
parsing with an older cmake version, he will have to write it from
scratch if the version in cmake is empty...
Also all the modules will either not include CMakeParseArguments and use
the system command, or (according to what I believe it's current policy)
will include it from ${CMAKE_CURRENT_LIST_DIR}, and therefore the empty
one will be included
so perhaps it could be exactly as it is now in my topic:
if(COMMAND cmake_parse_arguments)
return()
endif()
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] CMakeParseArguments: Do not skip empty arguments
On 04/12/13 14:40, Brad King wrote: > The RunCMake.CMakeParseArguments test fails on the continuous builds. > Please take a look. Should be fixed now. 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] CMakeParseArguments: Do not skip empty arguments
On 12/04/2013 04:57 AM, Daniele E. Domenichelli wrote: > I fixed these issues and merged in next again. The RunCMake.CMakeParseArguments test fails on the continuous builds. Please take a look. Alternatively we can revert this topic pending the proposed C++ re-implementation approach. 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] CMakeParseArguments: Do not skip empty arguments
On 12/04/2013 04:57 AM, Daniele E. Domenichelli wrote: >> Stepping back for a moment: This macro is now a huge amount of >> CMake code to do keyword argument processing for each invocation >> of the calling macro or function. Accumulation of such uses adds >> overhead to CMake language processing. It was previously proposed >> in this thread to convert the implementation of the command to be >> in C++. IMO this is still worth investigating, if only as a follow >> up to this topic. > > +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. > But still it the module should be there for compatibility for > compatibility with scripts already using that, and the old > implementation should be there too in order to be able to backport the > module with older cmake versions... The module can be provided as an empty file except for documentation. Any project including it will expect to be able to call cmake_parse_arguments afterwards, which they will be able to since it will be a builtin command. With the policy handling compatibility there is no need to keep the old macro implementation. If a project has its own copy of the module it will simply replace the builtin command and get the original behavior. -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] CMakeParseArguments: Do not skip empty arguments
On 03/12/13 17:59, Brad King wrote: > Why does CMAKE_PARSE_ARGUMENTS_DEFAULT_SKIP_EMPTY need to be an > inherited directory property instead of a variable? Both have > essentially the same scope and variables are much more common. > I'd like to avoid new uses of define_property if possible. It > is a legacy from the old builtin documentation system. Ok, fixed... I saw it used in ExternalProject and I thought it was something "standard". > We need to be able to test this even prior to the 3.0 release. > No one is going to remember to enable that as part of the release > process, and what if it is broken by then? You might be able to > hack the test with > > set(CMAKE_MINIMUM_REQUIRED_VERSION 3.0.0) > > after the real cmake_minimum_required call. Ok thanks, fixed. I fixed these issues and merged in next again. > Stepping back for a moment: This macro is now a huge amount of > CMake code to do keyword argument processing for each invocation > of the calling macro or function. Accumulation of such uses adds > overhead to CMake language processing. It was previously proposed > in this thread to convert the implementation of the command to be > in C++. IMO this is still worth investigating, if only as a follow > up to this topic. +1, since this is a very useful feature. But still it the module should be there for compatibility for compatibility with scripts already using that, and the old implementation should be there too in order to be able to backport the module with older cmake versions... Perhaps the implementation of the module could be changed to something like if(NOT CMAKE_MINIMUM_REQUIRED_VERSION VERSION_LESS 3.0.0) else() endif() Also all the modules that you should be able to backport should have something like if(NOT CMAKE_MINIMUM_REQUIRED_VERSION VERSION_LESS 3.0.0) else() endif() 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] CMakeParseArguments: Do not skip empty arguments
On 11/27/2013 10:40 AM, Brad King wrote: > Go ahead and merge it to 'next' for testing. I will review it > there Looking at the topic as of commit 10fd6fba, here are some comments. Why does CMAKE_PARSE_ARGUMENTS_DEFAULT_SKIP_EMPTY need to be an inherited directory property instead of a variable? Both have essentially the same scope and variables are much more common. I'd like to avoid new uses of define_property if possible. It is a legacy from the old builtin documentation system. I see this in the tests: +# run_cmake(VERSION-KEEP) # Enable when 3.0.0 is released We need to be able to test this even prior to the 3.0 release. No one is going to remember to enable that as part of the release process, and what if it is broken by then? You might be able to hack the test with set(CMAKE_MINIMUM_REQUIRED_VERSION 3.0.0) after the real cmake_minimum_required call. Stepping back for a moment: This macro is now a huge amount of CMake code to do keyword argument processing for each invocation of the calling macro or function. Accumulation of such uses adds overhead to CMake language processing. It was previously proposed in this thread to convert the implementation of the command to be in C++. IMO this is still worth investigating, if only as a follow up to this topic. 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] CMakeParseArguments: Do not skip empty arguments
Hi Danielle, We use the macro a lot in project like Slicer and CTK. This is a great improvement of the "CMakeParseArguments" module. Thanks for working on this. Jc On Wed, Nov 27, 2013 at 10:40 AM, Brad King wrote: > On 11/27/2013 9:54 AM, Daniele E. Domenichelli wrote: > > Any comment? > > I was hoping others that use this module would respond here. > > > Can I merge it to next? > > Go ahead and merge it to 'next' for testing. I will review it > there when I return from vacation next week. > > 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 > -- +1 919 869 8849 -- 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] CMakeParseArguments: Do not skip empty arguments
On 11/27/2013 9:54 AM, Daniele E. Domenichelli wrote: > Any comment? I was hoping others that use this module would respond here. > Can I merge it to next? Go ahead and merge it to 'next' for testing. I will review it there when I return from vacation next week. 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] CMakeParseArguments: Do not skip empty arguments
On 14/11/13 18:34, Daniele E. Domenichelli wrote: > I updated the CMakeParseArguments_EmptyArgs topic, so that it will work > properly with cmake 3.0.0 (after the branch set_emptyvar_PARENT_SCOPE > topic is merged) and with older releases (I had to add a workaround for > that) > > Can you please review it? Any comment? Can I merge it to next? 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] CMakeParseArguments: Do not skip empty arguments
Hello, I updated the CMakeParseArguments_EmptyArgs topic, so that it will work properly with cmake 3.0.0 (after the branch set_emptyvar_PARENT_SCOPE topic is merged) and with older releases (I had to add a workaround for that) Can you please review it? Thanks! 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] CMakeParseArguments: Do not skip empty arguments
On Friday 08 November 2013, Brad King wrote: > On 11/8/2013 4:19 PM, Alexander Neundorf wrote: > > On Friday 08 November 2013, Daniele E. Domenichelli wrote: > >> So it looks like that "set(FOO "" PARENT_SCOPE)" instead of setting an > >> empty FOO value, unsets it in the parent scope, and the behaviour is > >> definitely different from set for the current scope. > > > > How about adding one new policy "modify handling of empty variables" > > which is used by both ? > > At most the policy should affect PARENT_SCOPE. Yes. I meant "both" set(PARENT_SCOPE) and cmake_parse_arguments(). Alex -- 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] CMakeParseArguments: Do not skip empty arguments
On 11/8/2013 4:19 PM, Alexander Neundorf wrote:
> On Friday 08 November 2013, Daniele E. Domenichelli wrote:
>> So it looks like that "set(FOO "" PARENT_SCOPE)" instead of setting an
>> empty FOO value, unsets it in the parent scope, and the behaviour is
>> definitely different from set for the current scope.
>
> How about adding one new policy "modify handling of empty variables" which is
> used by both ?
At most the policy should affect PARENT_SCOPE. One can see the
difference right in cmSetCommand, where parentScope was added:
http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=fc8ce174#patch6
For parentScope it does
if (value.empty())
{
this->Makefile->RaiseScope(variable, 0);
}
else
{
this->Makefile->RaiseScope(variable, value.c_str());
}
while for normal scope it does just
this->Makefile->AddDefinition(variable, value.c_str());
-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] CMakeParseArguments: Do not skip empty arguments
On Friday 08 November 2013, Daniele E. Domenichelli wrote:
> Hello,
>
> I updated the CMakeParseArguments_EmptyArgs topic, the new behaviour is
> now decided as follows:
>
> * If CMAKE_MINIMUM_REQUIRED_VERSION < 2.8.13, the default behaviour is
> to skip empty arguments, otherwise the default behaviour is to keep
> them.
> * Using the CMAKE_PARSE_ARGUMENTS_DEFAULT_SKIP_EMPTY directory
> property the user can explicitly set the default behaviour for a
> folder and its subfolders.
> * Using CMAKE_PARSE_ARGUMENTS_(SKIP|KEEP)_EMPTY or as the first
> argument of the call after the mandatory ones, the user can specify
> whether to keep or to skip the empty arguments, regardless of the
> default behaviour for that cmake_parse_arguments() call.
>
> Therefore the new syntax is:
>
> cmake_parse_arguments(
>
>
>
>
> [CMAKE_PARSE_ARGUMENTS_(SKIP|KEEP)_EMPTY]
> args...
> )
>
> Does it look better now?
>
>
> Unfortunately there still a big problem, that is probably in the c++
> source code, because "set( "" PARENT_SCOPE)" still fails if
> is empty. This is a small example that shows the problem:
>
> ---
>
> function(_foo)
> set(FOO "" PARENT_SCOPE)
> endfunction()
>
> foo(FOO "foo")
> _foo()
> if(DEFINED FOO)
> message("FOO DEFINED")
> else()
> message("FOO NOT DEFINED")
> endif()
>
> set(BAR "")
> if(DEFINED BAR)
> message("BAR DEFINED")
> else()
> message("BAR NOT DEFINED")
> endif()
>
> ---
>
> The output of this is:
>
> FOO NOT DEFINED
> BAR DEFINED
>
> So it looks like that "set(FOO "" PARENT_SCOPE)" instead of setting an
> empty FOO value, unsets it in the parent scope, and the behaviour is
> definitely different from set for the current scope.
>
> This looks like a bug to me... How should I handle this?
the implementation in set() doesn't differentiate whether it was set to empty
or whether there was no value at all.
In most cases it doesn't matter much whether a variable is empty or not
defined, if it is dereferenced it gives an empty string in both cases, if()
also interprets both as false.
It's more or less only if(DEFINED) which detects the difference, right ?
How about adding one new policy "modify handling of empty variables" which is
used by both ?
Then you don't need the directory property and the extra argument.
Having two separate policies would be cleaner, but it seems a bit overkill to
me, since most probably both changes won't break many, if any, builds.
Alex
--
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] CMakeParseArguments: Do not skip empty arguments
On 08/11/13 17:54, Daniele E. Domenichelli wrote: > foo(FOO "foo") Obviously it was supposed to be set(FOO "foo") Sorry... 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] CMakeParseArguments: Do not skip empty arguments
Hello,
I updated the CMakeParseArguments_EmptyArgs topic, the new behaviour is
now decided as follows:
* If CMAKE_MINIMUM_REQUIRED_VERSION < 2.8.13, the default behaviour is
to skip empty arguments, otherwise the default behaviour is to keep
them.
* Using the CMAKE_PARSE_ARGUMENTS_DEFAULT_SKIP_EMPTY directory
property the user can explicitly set the default behaviour for a
folder and its subfolders.
* Using CMAKE_PARSE_ARGUMENTS_(SKIP|KEEP)_EMPTY or as the first
argument of the call after the mandatory ones, the user can specify
whether to keep or to skip the empty arguments, regardless of the
default behaviour for that cmake_parse_arguments() call.
Therefore the new syntax is:
cmake_parse_arguments(
[CMAKE_PARSE_ARGUMENTS_(SKIP|KEEP)_EMPTY]
args...
)
Does it look better now?
Unfortunately there still a big problem, that is probably in the c++
source code, because "set( "" PARENT_SCOPE)" still fails if
is empty. This is a small example that shows the problem:
---
function(_foo)
set(FOO "" PARENT_SCOPE)
endfunction()
foo(FOO "foo")
_foo()
if(DEFINED FOO)
message("FOO DEFINED")
else()
message("FOO NOT DEFINED")
endif()
set(BAR "")
if(DEFINED BAR)
message("BAR DEFINED")
else()
message("BAR NOT DEFINED")
endif()
---
The output of this is:
FOO NOT DEFINED
BAR DEFINED
So it looks like that "set(FOO "" PARENT_SCOPE)" instead of setting an
empty FOO value, unsets it in the parent scope, and the behaviour is
definitely different from set for the current scope.
This looks like a bug to me... How should I handle this?
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] CMakeParseArguments: Do not skip empty arguments
Hi all.
I hope I don't derail the conversation, but since there's now discussion of
providing a new interface and perhaps behaviour-changing parameters for
cmake_parse_arguments(), I'd like to mention another behaviour switch I
would find useful.
I maintain a CMake framework for a large collection of projects at work,
and this framework provides several functions which (naturally) use
cmake_parse_arguments() internally. My users (and myself when I first saw
it) were quite surprised that repeating the leading keyword for multi-value
parameters overwrites values instead of appending them. What I mean:
function(foo)
cmake_parse_arguments(arg "" "" "MULTI" ${ARGN})
foreach(item IN LISTS arg_MULTI)
message(STATUS "${item}")
endforeach()
endfunction()
foo(MULTI x y MULTI z w)
The above code outputs 'z' and 'w'. I originally expected it to output all
of 'x' 'y' 'z' 'w'. I would certainly welcome an option to switch the
behaviour to the latter one.
Petr
On Wed, Nov 6, 2013 at 9:01 PM, Matthew Woehlke wrote:
> On 2013-11-06 13:57, Brad King wrote:
>
>> On 11/06/2013 01:32 PM, Alexander Neundorf wrote:
>>
>>> Adding proper named argument handling to cmake_parse_arguments() itself
>>> is
>>> somewhat complicated since it can't make use of cmake_parse_arguments()
>>> ;-)
>>>
>>
>> Since the need for this is so common, perhaps we should provide a
>> C++-implemented command to do it, e.g. cmake_command_options().
>> We would need to carefully design the interface here first of course.
>>
>
> FWIW, I prefer the name [cmake_]parse_arguments :-).
>
> Is there not already a C++ implementation something like this? (Or else
> how do the existing C++ commands do argument parsing?) If it is C++, I
> guess it would be possible for it to not have the limitation of being
> unable to parse its own arguments? (Actually, in either case, it seems that
> the implementation should be able to have all the 'real work' in a helper
> that is called once to parse arguments to the command itself, then again to
> parse the arguments the user wants to parse.)
>
> In any case, it would be nice if we could reimplement
> cmake_parse_arguments so that it has the same signature but the
> implementation would newly just wrap the new C++ version.
>
> What if we had something like:
>
> parse_arguments(
> PREFIX "_"
> FLAG_OPTIONS ...
> VALUE_OPTIONS ...
> LIST_OPTIONS ...
> ARGS ...)
>
> ...where everything after ARGS is no longer considered an argument to
> parse_arguments itself. Also, 'parse_arguments(... IN LISTS ...)' (instead
> of using 'ARGS'), taking names of list variables containing arguments to be
> parsed (similar to 'IN LISTS' of foreach).
>
> {FLAG,VALUE,LIST}_OPTIONS would accept one or more strings containing
> names of argument keywords, subject to recursive list expansion (should be
> safe - keywords should not contain ';' - and avoids breakage if option name
> lists are given in quoted variable expansions, which could easily happen by
> forgetting to remove quotes when porting to the new signature).
>
> ...and then of course cmake_parse_arguments can readily accept other
> options to modify its behavior e.g. KEEP_EMPTY (and/or SKIP_EMPTY if
> keeping becomes - or may be, depending on policy - the default).
>
> --
> 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
>
--
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] CMakeParseArguments: Do not skip empty arguments
On 2013-11-06 13:57, Brad King wrote:
On 11/06/2013 01:32 PM, Alexander Neundorf wrote:
Adding proper named argument handling to cmake_parse_arguments() itself is
somewhat complicated since it can't make use of cmake_parse_arguments() ;-)
Since the need for this is so common, perhaps we should provide a
C++-implemented command to do it, e.g. cmake_command_options().
We would need to carefully design the interface here first of course.
FWIW, I prefer the name [cmake_]parse_arguments :-).
Is there not already a C++ implementation something like this? (Or else
how do the existing C++ commands do argument parsing?) If it is C++, I
guess it would be possible for it to not have the limitation of being
unable to parse its own arguments? (Actually, in either case, it seems
that the implementation should be able to have all the 'real work' in a
helper that is called once to parse arguments to the command itself,
then again to parse the arguments the user wants to parse.)
In any case, it would be nice if we could reimplement
cmake_parse_arguments so that it has the same signature but the
implementation would newly just wrap the new C++ version.
What if we had something like:
parse_arguments(
PREFIX "_"
FLAG_OPTIONS ...
VALUE_OPTIONS ...
LIST_OPTIONS ...
ARGS ...)
...where everything after ARGS is no longer considered an argument to
parse_arguments itself. Also, 'parse_arguments(... IN LISTS ...)'
(instead of using 'ARGS'), taking names of list variables containing
arguments to be parsed (similar to 'IN LISTS' of foreach).
{FLAG,VALUE,LIST}_OPTIONS would accept one or more strings containing
names of argument keywords, subject to recursive list expansion (should
be safe - keywords should not contain ';' - and avoids breakage if
option name lists are given in quoted variable expansions, which could
easily happen by forgetting to remove quotes when porting to the new
signature).
...and then of course cmake_parse_arguments can readily accept other
options to modify its behavior e.g. KEEP_EMPTY (and/or SKIP_EMPTY if
keeping becomes - or may be, depending on policy - the default).
--
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] CMakeParseArguments: Do not skip empty arguments
On 11/06/2013 01:32 PM, Alexander Neundorf wrote: > Adding proper named argument handling to cmake_parse_arguments() itself is > somewhat complicated since it can't make use of cmake_parse_arguments() ;-) Since the need for this is so common, perhaps we should provide a C++-implemented command to do it, e.g. cmake_command_options(). We would need to carefully design the interface here first of course. -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] CMakeParseArguments: Do not skip empty arguments
On Wednesday 06 November 2013, Daniele E. Domenichelli wrote:
> On 05/11/13 20:51, Alexander Neundorf wrote:
> >> I don't know if this is to be considered a change of behaviour though,
> >> but I'd rather consider it a bug, and I would like to see it fixed in
> >> the next bugfix release... what do you think?
> >
> > cmake_parse_arguments() is used in quite a few places in the meantime, so
> > I don't really feel good about changing its behaviour in general.
> > Maybe an option for the macro what it should do with empty values ?
>
> Is there any of the other place where cmake_parse_arguments() has to
> deal with with empty arguments?
We cannot know what users are doing with it.
> I believe that someone would have
> complained about that. From what I've seen, modules that are supposed to
> deal with empty arguments (like ExternalProject that can have arguments
> like INSTALL_COMMAND "") do not use CMakeParseArguments...
>
> I don't think that anyone would use something like
> SINGLE "" VALUE
> to assign VALUE to the SINGLE option...
> Also something like
> SINGLE "${EMPTY_VARIABLE}" NEXT_ARG
> would assign to FOO_SINGLE the value NEXT_ARG
> and that is very likely to be an unintended behaviour.
>
> Perhaps someone could use something like
> MULTI "${V1}" "${EMPTY_VARIABLE}" "${V2}"
> and expect to get a list containing only V1 and V2. Nonetheless if he is
> using foreach(arg ${list}), the behaviour won't change because empty
> list elements are skipped. if instead he is using "foreach(arg IN LIST
> list)", he is probably expecting to receive empty arguments, since the
> documentation clearly states so.
>
> Adding an option in the macro arguments would be a bit of a
> non-compatible change, since the name of the option could no longer be
> used as one of the "OPTION/SINGLE/MULTI" value names.
This is all correct.
It seems unlikely that changing this will actually break something.
Adding an option like "DONT_SKIP_EMPTY_ARGUMENTS", e.g at the second argument
position, means that this keyword cannot be used as an option name by users
anymore. By making the keyword longer or more detailled, this becomes less
likely. E.g. I'd assume nobody is using "CMAKE_DONT_SKIP_EMPTY_ARGUMENTS".
Adding proper named argument handling to cmake_parse_arguments() itself is
somewhat complicated since it can't make use of cmake_parse_arguments() ;-)
The macro could check ${CMAKE_MINIMUM_REQUIRED_VERSION} whether 2.8.13/3.0.0
is required, and switch behaviour based on that. Or via a policy. This would
be the normal wy to handle such a change.
I'm not sure whether this wouldn't be a bit much for a change which most
likely doesn't break anything anyway.
But... I've had my share of changes which seemed to be safe, and then somebody
came along with a setup which was broken by this change nevertheless.
So, I'll have a slightly bad feeling if you commit it without additional
checks. Probably it will be ok.
If you commit it with a policy, everything should be fine.
Alex
--
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] CMakeParseArguments: Do not skip empty arguments
On 05/11/13 20:51, Alexander Neundorf wrote:
>> I don't know if this is to be considered a change of behaviour though,
>> but I'd rather consider it a bug, and I would like to see it fixed in
>> the next bugfix release... what do you think?
>
> cmake_parse_arguments() is used in quite a few places in the meantime, so I
> don't really feel good about changing its behaviour in general.
> Maybe an option for the macro what it should do with empty values ?
Is there any of the other place where cmake_parse_arguments() has to
deal with with empty arguments? I believe that someone would have
complained about that. From what I've seen, modules that are supposed to
deal with empty arguments (like ExternalProject that can have arguments
like INSTALL_COMMAND "") do not use CMakeParseArguments...
I don't think that anyone would use something like
SINGLE "" VALUE
to assign VALUE to the SINGLE option...
Also something like
SINGLE "${EMPTY_VARIABLE}" NEXT_ARG
would assign to FOO_SINGLE the value NEXT_ARG
and that is very likely to be an unintended behaviour.
Perhaps someone could use something like
MULTI "${V1}" "${EMPTY_VARIABLE}" "${V2}"
and expect to get a list containing only V1 and V2. Nonetheless if he is
using foreach(arg ${list}), the behaviour won't change because empty
list elements are skipped. if instead he is using "foreach(arg IN LIST
list)", he is probably expecting to receive empty arguments, since the
documentation clearly states so.
Adding an option in the macro arguments would be a bit of a
non-compatible change, since the name of the option could no longer be
used as one of the "OPTION/SINGLE/MULTI" value names.
Also using a global property is not an option, because that would change
the behaviour globally, and if some module is expecting the current
behaviour it will still fail.
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] CMakeParseArguments: Do not skip empty arguments
On Tuesday 05 November 2013, Daniele E. Domenichelli wrote:
> Hello all,
>
> Current implementation of cmake_parse_arguments skips empty arguments.
>
> For example:
>
> cmake_parse_arguments("OPTION" "SINGLE" "MULTI" ${ARGN})
>
> with ARGN that is set using something like:
>
>SINGLE "" # (;SINGLE;;)
>SINGLE "" # (;SINGLE;)
>
> it will fail in 3 different ways:
>
> 1) because the "foreach" skips empty arguments
> 2) because list(APPEND ${currentArg}) will fail to append an
>empty element to the list if currentArg is an empty string.
> 3) because empty arguments are not passed to cmake_parse_arguments
>if not quoted, i.e. if ARGN="a;;b"
>* ${ARGN} will pass the arguments "a", "b"
>* "${ARGN}" will pass the arguments "a", "", "b"
Not sure about the third point. It could be done intentionally that way in
order to ignore empty strings.
> I wrote a small patch to fix this, you can find it in the
> CMakeParseArguments_EmptyArgs topic. This patch fixes the first
> behaviour using foreach(IN LISTS) version, the second by enclosing
> currentArg in quotes where required, and finally fixes the documentation
> in order to suggest to use quoted "${ARGN}" in order not to miss the
> empty arguments at the end of the list.
>
> I don't know if this is to be considered a change of behaviour though,
> but I'd rather consider it a bug, and I would like to see it fixed in
> the next bugfix release... what do you think?
cmake_parse_arguments() is used in quite a few places in the meantime, so I
don't really feel good about changing its behaviour in general.
Maybe an option for the macro what it should do with empty values ?
Alex
--
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
