Re: [cmake-developers] Invalid/Reserved target names
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
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
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
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
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
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