Re: [cmake-developers] Invalid/Reserved target names

2013-11-18 Thread Brad King
On 11/15/2013 11:39 AM, Brad King wrote:
 The policy has not been released yet so I think it would be fine
 to extend it to cover this.  Please include documentation and
 test cases.

Thanks for working on this.  Here are some comments.

It looks like you added the reserved check to cmGeneratorExpression
in the IsValidTargetName method.  This method is used in other
places to validate generator expression syntax, and in those places
the reserved target names may be allowed.  Please refactor the
check added to the add_* commands to use a dedicated method that
first checks the reserved names and then calls IsValidTargetName
internally.

In the list of reserved names you also need to add uppercase
INSTALL.  Otherwise I think you got them all.

Good catch on covering add_custom_target with the policy too.

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] Invalid/Reserved target names

2013-11-18 Thread Nils Gladitz

On 11/15/2013 11:39 AM, Brad King wrote:

It looks like you added the reserved check to cmGeneratorExpression
in the IsValidTargetName method.  This method is used in other
places to validate generator expression syntax, and in those places
the reserved target names may be allowed.
Can I keep the extracted method in cmGeneratorExpression or would there 
be a more appropriate location for it?



In the list of reserved names you also need to add uppercase
INSTALL.  Otherwise I think you got them all.

I should be able to get that fixed as soon as I get home from work.

Thank you for looking it over!

Nils
--

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] Invalid/Reserved target names

2013-11-15 Thread Matthew Woehlke

On 2013-11-15 04:05, Nils Gladitz wrote:

I would like to hijack/extend Stephen's changes in
05f5fde0eb83c0e49aab3214f28a098861aa3313
to also disallow target names that have been implicitly reserved by some
of the generators.

This list might not be complete but I assume it would be at least:
 all, help, clean, edit_cache, rebuild_cache, test, package,
package_source, PACKAGE, ZERO_CHECK

Would anyone be opposed to this?
Could I extend the existing policy, should I create a new one or would
it be ok without any policy even?


I haven't done so yet, but I've considered once or twice creating a 
'clean' target in case the generator is ninja. Possibly someone has 
already done that. Maybe only disallow target names that the generator 
actually uses? (Or should we teach CMake to make a 'clean' target for 
ninja? :-) That could probably be done separately, but if 'clean' is 
disallowed unconditionally it feels like that should be on the radar 
of things to do.)


--
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] Invalid/Reserved target names

2013-11-15 Thread Nils Gladitz

On 15.11.2013 17:24, Matthew Woehlke wrote:
I haven't done so yet, but I've considered once or twice creating a 
'clean' target in case the generator is ninja. Possibly someone has 
already done that. Maybe only disallow target names that the generator 
actually uses? (Or should we teach CMake to make a 'clean' target for 
ninja? :-) That could probably be done separately, but if 'clean' is 
disallowed unconditionally it feels like that should be on the radar 
of things to do.)



I would prefer to forbid them regardless of generator.
This would prevent someone using a target that happens to be free for 
his generator but inadvertently breaking something for someone else 
working on the same project with a different generator.


Defining a custom clean target would also break if the Ninja generator 
did start defining a custom clean target ... which obviously can happen 
for any target name not currently used but is more likely with the 
currently reserved names.


Projects that already use these names should still work if I make this 
dependant on the existing policy.


Nils
--

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] Invalid/Reserved target names

2013-11-15 Thread Rolf Eike Beer
Am Freitag, 15. November 2013, 10:05:13 schrieb Nils Gladitz:
 I would like to hijack/extend Stephen's changes in
 05f5fde0eb83c0e49aab3214f28a098861aa3313
 to also disallow target names that have been implicitly reserved by some
 of the generators.
 
 This list might not be complete but I assume it would be at least:
  all, help, clean, edit_cache, rebuild_cache, test, package,
 package_source, PACKAGE, ZERO_CHECK

Anything ending in /fast?

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] Invalid/Reserved target names

2013-11-15 Thread Brad King
On 11/15/2013 03:19 PM, Rolf Eike Beer wrote:
 Anything ending in /fast?

The policy-introduced restrictions already disallow / in names.

-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