On Sunday 13 February 2011, Eric Noulard wrote: > 2011/2/13 Alexander Neundorf <neund...@kde.org>: > > On Monday 07 February 2011, Alexander Neundorf wrote: > >> On Sunday 06 February 2011, Eric Noulard wrote: > >> > 2011/2/5 Alexander Neundorf <neund...@kde.org>: > >> > > Hi, > >> > > > >> > > in general we keep cmake as backward compatible as possible, so no > >> > > builds are broken. > >> > > > >> > > In a local branch I have a version of cmake with a small improvement > >> > > to the graphviz support in cmake. > >> > > It turns the variable GRAPHVIZ_IGNORE_TARGETS from a list of strings > >> > > into a list of regular expressions. I.e. everything which was before > >> > > in GRAPHVIZ_IGNORE_TARGETS and excluded from the dot-files, is now > >> > > still excluded, but now there may be more targets excluded (e.g. for > >> > > "kio" now not only "kio" is excluded, but also "kio_ftp" etc.) > >> > > Also, with this change, the variable GRAPHVIZ_TARGET_IGNORE_REGEX is > >> > > not supported anymore, so targets previously excluded from the > >> > > dot-files via this (one) regex are now included. > >> > > > >> > > Both issues can be fixed by, for the first, putting "^...$" around > >> > > the string, and for the second by just putting the regex now into > >> > > GRAPHVIZ_IGNORE_TARGETS . > >> > > > >> > > Strictly speaking, this breaks compatibility regarding the graphviz > >> > > support. But, do we care about this ? > >> > > * it is and was completely undocumented (will write a wiki page > >> > > soon) * the changes don't influence the build itself, so no build > >> > > can be broken by this change > >> > > > >> > > What do you think ? > >> > > >> > I think that, I didn't even know the existence of this, so I wouldn't > >> > care about breaking it :-] > >> > Moreover I am basically ok that the breakage of an undocumented > >> > feature is not a real breakage. > >> > > >> > Now may be you can: > >> > 1) rename GRAPHVIZ_IGNORE_TARGETS to > >> > "CMAKE_GRAPHVIZ_IGNORE_TARGETS" which is more consistent with other > >> > var names. > >> > > >> > 2) check GRAPHVIZ_IGNORE_TARGETS and GRAPHVIZ_TARGET_IGNORE_REGEX > >> > and issue a WARNING if they are used. > >> > > >> > 2) may even be unecessary with the new "--warn-unused-vars" option. > >> > > >> > the baseline is if you break it, then break it ALL there won't be any > >> > confusion. > >> > >> I could also make these variables cache variables. > >> That's IMO easier to use and also better discoverable. > >> With "GRAPHVIZ_IGNORE_TARGETS" they would be all in the "GRAPHVIZ" > >> subtree, with "CMAKE_GRAPHVIZ_IGNORE_TARGETS" they would be all together > >> with the other cmake variables in the "CMAKE" subtree. > >> > >> Being in a "GRAPHVIZ" subtree would make them easier to spot, so maybe > >> I'd prefer this. But I don't have a strong opinion. > >> > >> Other comments ? > > > > I guess this means that backward compatibility is no issue here. > > > > Eric, what do you think about the naming and whether make them cache > > variables or not ? > > Naming scheme with GRAPHVIZ prefix is good you are right for the subtree > thing. Caching looks ok too, I don't think such var needs to be seen at
Only issue might be that they would clash with the variables of a potential FindGraphViz.cmake (which would be bad). OTOH, CMake (CTest actually) also already adds the "SLURM" (http://en.wikipedia.org/wiki/Fry_and_the_Slurm_Factory ;-) ) subtree without having a FindSlurm.cmake. > first sight. My opinion may not be that valuable in this area because I'm > not using the feature :-] > > Now the argument of the subtree is a good one even if we can think that > sub-subtree (more than one level tree) may be implemented in cmake-gui too. I also already thought about this, but maybe this would get too much ? Or only do it inside the "CMAKE" subtree ? This would usually give subsubtrees at least for "C", "CXX", "EXE", "MODULE" and "SHARED". Alex _______________________________________________ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers