I would note that while it’s discouraged, it’s just this guys opinion subject to disagreement. It’s like saying operator overloading in c++ is discouraged.
There are perfectly sound cases where wrapping may still make the most sense. *shrug*. The problem he identifies in that article with it is the potential for an infinite loop. That’s not exactly a subtle problem and one we’ve never encountered. The only way it would affect BRL-CAD would be if another project linking BRL-CAD wrapped the same functions as us and those wrappers were actually set up to call each other... That said, I’d agree that we should try to keep it to a minimum. I just wouldn’t sacrifice anything for it until it’s a problem. Like our message() wrapper.. That’s of course just my opinion too and subject to disagreement. :) Cheers! Sean > On Oct 17, 2020, at 12:41 AM, starseeker--- via brlcad-commits > <brlcad-comm...@lists.sourceforge.net> wrote: > > Revision: 77467 > http://sourceforge.net/p/brlcad/code/77467 > Author: starseeker > Date: 2020-10-17 04:41:17 +0000 (Sat, 17 Oct 2020) > Log Message: > ----------- > Wrapping functions as we do here is discouraged > (https://crascit.com/2018/09/14/do-not-redefine-cmake-commands/) but we've > not been able to replace some of the functionality using other means. Since > we'll need to require 3.14 for this new setup to work, and > ExternalProject_Add approaches remove some of the incentive to override > 'standard' functions for bookkeeping purposes, it may not be practical to > eliminate such usages. > > Modified Paths: > -------------- > brlcad/branches/thirdparty_rework/misc/CMake/BRLCAD_Command_Wrappers.cmake > > Modified: > brlcad/branches/thirdparty_rework/misc/CMake/BRLCAD_Command_Wrappers.cmake > =================================================================== > --- > brlcad/branches/thirdparty_rework/misc/CMake/BRLCAD_Command_Wrappers.cmake > 2020-10-17 01:23:35 UTC (rev 77466) > +++ > brlcad/branches/thirdparty_rework/misc/CMake/BRLCAD_Command_Wrappers.cmake > 2020-10-17 04:41:17 UTC (rev 77467) > @@ -1,3 +1,17 @@ > +# TODO: 3.7 has a new property that may help eliminate the need to > +# do the wrappers below (wrapping CMake functions in this manner > +# is officially discouraged, it causes problems..) > +# > +# > https://cmake.org/cmake/help/latest/prop_dir/BUILDSYSTEM_TARGETS.html#prop_dir:BUILDSYSTEM_TARGETS > +# > +# Between that and a couple other notes below, we may be able to > +# eliminate most of the wrappers now. configure_file we may need > +# to turn into our own function, but if we're managing external > +# build systems with ExternalProject_Add now that becomes more > +# practical. Worth doing to get us grounded on officially > +# supported CMake features. > + > + > #--------------------------------------------------------------------- > # By default (as of version 2.8.2) CMake does not provide access to > # global lists of executable and library targets. This is useful > @@ -46,10 +60,7 @@ > endif(${name} MATCHES "^lib*") > > # TODO - the mechanism below should eventually be replaced by a proper > - # feature in CMake, but it is as yet unimplemented: > - # https://cmake.org/pipermail/cmake-developers/2015-July/025682.html > - # https://cmake.org/pipermail/cmake-developers/2016-March/027985.html > - # https://cmake.org/pipermail/cmake-developers/2016-March/027993.html > + # feature in CMake, possibly using BUILDSYSTEM_TARGETS > set(add_lib_to_list 1) > foreach(libarg ${ARGN}) > if("${libarg}" STREQUAL "INTERFACE") > @@ -66,10 +77,7 @@ > _add_executable(${name} ${ARGN}) > > # TODO - the mechanism below should eventually be replaced by a proper > - # feature in CMake, but it is as yet unimplemented: > - # https://cmake.org/pipermail/cmake-developers/2015-July/025682.html > - # https://cmake.org/pipermail/cmake-developers/2016-March/027985.html > - # https://cmake.org/pipermail/cmake-developers/2016-March/027993.html > + # feature in CMake, possibly using BUILDSYSTEM_TARGETS > set_property(GLOBAL APPEND PROPERTY CMAKE_EXEC_TARGET_LIST ${name}) > endfunction(add_executable) > > @@ -78,10 +86,7 @@ > _add_custom_target(${name} ${ARGN}) > > # TODO - the mechanism below should eventually be replaced by a proper > - # feature in CMake, but it is as yet unimplemented: > - # https://cmake.org/pipermail/cmake-developers/2015-July/025682.html > - # https://cmake.org/pipermail/cmake-developers/2016-March/027985.html > - # https://cmake.org/pipermail/cmake-developers/2016-March/027993.html > + # feature in CMake, possibly using BUILDSYSTEM_TARGETS > set_property(GLOBAL APPEND PROPERTY CMAKE_CUSTOM_TARGET_LIST ${name}) > endfunction(add_custom_target) > > > This was sent by the SourceForge.net collaborative development platform, the > world's largest Open Source development site. > > > > _______________________________________________ > BRL-CAD Source Commits mailing list > brlcad-comm...@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/brlcad-commits _______________________________________________ BRL-CAD Developer mailing list brlcad-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/brlcad-devel
