Re: [cmake-developers] User vs CMake include mismatch handling
On Wednesday 15 December 2010, Brad King wrote: On 12/15/2010 05:31 PM, Brad King wrote: + e CMake builtin module\n + includer \n + is including a module from the CMAKE_MODULE_PATH\n + moduleInCMakeModulePath \n + which shadows expected builtin module\n + moduleInCMakeRoot \n + This may cause CMake or build errors later.\n BTW, when I merge the branch to master for testing the FindPackageTest does not actually do anything due to use of ${CMAKE_CURRENT_LIST_DIR} in FindZLIB. -Brad Yes, the plan is to remove the uses of ${CMAKE_CURRENT_LIST_DIR} at this point again. Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On 12/15/2010 04:59 PM, Brad King wrote: On 12/15/2010 04:35 PM, Alexander Neundorf wrote: First get all information, then look and decide what to do. Seemed the cleanest way to me. Searching the filesystem is not cheap. Do it lazily. I tried to incorporate all other comments, and also tried to add some documentation about CMAKE_POLICY_DEFAULT_CMP, but I think you can word it better. I pushed the changes to the branch on stage. I'll take a look when I get more time. The error of CMAKE_POLICY_DEFAULT_CMP=xyz looks better. This hunk should more reliably detect whether the includer is a builtin module: -const char* currentFile = this-GetDefinition(CMAKE_CURRENT_LIST_FILE); -if (currentFile (strstr(currentFile, cmakeRoot) == currentFile)) +const char* includer = this-CallStack.empty()? + 0 : this-CallStack.back().Context-FilePath.c_str(); +if(includer cmSystemTools::IsSubDirectory(includer, cmakeRoot)) And this hunk should improve the warning message: - e File currentFile includes - moduleInCMakeModulePath - (found via CMAKE_MODULE_PATH) which shadows - moduleInCMakeRoot . This may cause errors later on .\n + e CMake builtin module\n + includer \n + is including a module from the CMAKE_MODULE_PATH\n + moduleInCMakeModulePath \n + which shadows expected builtin module\n + moduleInCMakeRoot \n + This may cause CMake or build errors later.\n -Brad ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On 12/15/2010 05:31 PM, Brad King wrote: + e CMake builtin module\n + includer \n + is including a module from the CMAKE_MODULE_PATH\n + moduleInCMakeModulePath \n + which shadows expected builtin module\n + moduleInCMakeRoot \n + This may cause CMake or build errors later.\n BTW, when I merge the branch to master for testing the FindPackageTest does not actually do anything due to use of ${CMAKE_CURRENT_LIST_DIR} in FindZLIB. -Brad ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Tuesday 23 November 2010, Alexander Neundorf wrote: On Tuesday 23 November 2010, Brad King wrote: On 11/23/2010 03:31 PM, Alexander Neundorf wrote: On Monday 22 November 2010, Brad King wrote: On 11/22/2010 04:06 PM, Alexander Neundorf wrote: On Monday 22 November 2010, Brad King wrote: (1) Improve documentation of CMAKE_USER_MAKE_RULES_OVERRIDE[_C] variables (2) Add the Custom-* file inclusion, document it Could you take care of (1) ? Sure, but not until at least next week. The changes don't need to be done in this order anyway. Ok :-) I'll start with the CMAKE_POLICY_DEFAULT_CMP stuff you suggested. I merged this now into next on stage. It contains: - the new policy CMP0017 - the feature to set the default via CMAKE_POLICY_DEFAULT_CMP - a basic test - some documentation Please have a look at it. Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Monday 22 November 2010, Brad King wrote: On 11/22/2010 04:06 PM, Alexander Neundorf wrote: On Monday 22 November 2010, Brad King wrote: ... I think the path forward here is: (1) Improve documentation of CMAKE_USER_MAKE_RULES_OVERRIDE[_C] variables (2) Add the Custom-* file inclusion, document it Do you think both (1) and (2) are necessary ? I suppose just (1) is enough. It can be used to implement (2) in a given project. Could you take care of (1) ? Do you have anything special in mind with (2) or simply that users can set (1) to whatever they want ? Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On 11/23/2010 03:31 PM, Alexander Neundorf wrote: On Monday 22 November 2010, Brad King wrote: On 11/22/2010 04:06 PM, Alexander Neundorf wrote: On Monday 22 November 2010, Brad King wrote: (1) Improve documentation of CMAKE_USER_MAKE_RULES_OVERRIDE[_C] variables (2) Add the Custom-* file inclusion, document it Could you take care of (1) ? Sure, but not until at least next week. The changes don't need to be done in this order anyway. Do you have anything special in mind with (2) or simply that users can set (1) to whatever they want ? I was just thinking that the script specified by (1) can contain code similar to that in CMake*Information.cmake to load Custom-os-id-lang files. If the custom info file code were added to CMake*Information.cmake it would be just before scripts from (1) are included anyway. -Brad ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Tuesday 23 November 2010, Brad King wrote: On 11/23/2010 03:31 PM, Alexander Neundorf wrote: On Monday 22 November 2010, Brad King wrote: On 11/22/2010 04:06 PM, Alexander Neundorf wrote: On Monday 22 November 2010, Brad King wrote: (1) Improve documentation of CMAKE_USER_MAKE_RULES_OVERRIDE[_C] variables (2) Add the Custom-* file inclusion, document it Could you take care of (1) ? Sure, but not until at least next week. The changes don't need to be done in this order anyway. Ok :-) I'll start with the CMAKE_POLICY_DEFAULT_CMP stuff you suggested. Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On 11/22/2010 04:06 PM, Alexander Neundorf wrote: On Monday 22 November 2010, Brad King wrote: The warning should be more specific about what cause errors later on means. It could mean both with future versions of CMake and with this version of CMake later on during *this* configuration. The latter is more important. I think this is hard to know. I just meant that the warning text should elaborate in general. The most important information is to make the reader aware that if there are errors involving the module later in the configuration then this may be the cause. The only problem is how to get users building KDE that see an obscure FPHSA argument error over to instructions to add this cmd line option. Perhaps your proposed shadow warning for CMP0017 that appears just before the breakage will help. Yes, I can add something more there. As above. I think the path forward here is: (1) Improve documentation of CMAKE_USER_MAKE_RULES_OVERRIDE[_C] variables (2) Add the Custom-* file inclusion, document it Do you think both (1) and (2) are necessary ? I suppose just (1) is enough. It can be used to implement (2) in a given project. -Brad ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On 17-11-2010 at 23:28, in message aanlktin6z9rxfqprwdux-qge4b_wych_fvy9dyo7d...@mail.gmail.com, David Cole david.c...@kitware.com wrote: On Wed, Nov 17, 2010 at 4:50 PM, Alexander Neundorf neund...@kde.org wrote: On Wednesday 17 November 2010, David Cole wrote: 2010/11/17 Alexander Neundorf neund...@kde.org: On Thursday 14 October 2010, David Cole wrote: On Thu, Oct 14, 2010 at 1:30 PM, Alexander Neundorf neund...@kde.orgwrote: On Wednesday 13 October 2010, Alexander Neundorf wrote: On Wednesday 13 October 2010, Bill Hoffman wrote: So, I think we have to use the new name approach. Do we want to call it 2? Or should we call it something else? Alex, do you have time to do this? I think it's not a good solution, and the one with CURRENT_LIST_DIR is definitely better and already implemented. And I am still convinced that I am right here, see my other mails. So I would suggest to merge the CURRENT_LIST_DIR branch for 2.8.3, and as soon as 2.8.3 is released, remove the full paths again and enable the new CMP0017 instead (prefer CMAKE_ROOT when include()d from CMAKE_ROOT) and then see what happens during the whole 2.8.4 cycle. I think this (CMP0017) is necessary, because otherwise we can only hope nothing breaks with future releases (independent from FPHSA). Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers I'm ok with this since Alex feels so strongly about it, and the code change is restricted to using CMAKE_CURRENT_LIST_DIR only when including FPHSA.cmake... (i.e. -- it should not affect including *other* files from inside of modules that are presently overridden... which was my concern -- that we'd break some *other* scenario -- since that's not true, I retract my objection.) To review the change yourself: git fetch stage gitk remotes/stage/AddCMAKE_CURRENT_LIST_DIR Look at the top 3 commits. Looks pretty safe. I'd say we should merge it to 'next' and then after a night on the dashboards, merge it to 'master' and do an rc3 release based on that. There is now a branch PreferCMakeModulesByCMakeModulesWithPolicy-NoTrailingWhitespaceCommit on stage. This contains right now only one commit, and that's the commit which adds a policy which decides whether files in CMAKE_ROOT are preferred over CMAKE_MODULE_PATH if include()d from withing CMAKE_ROOT. Currently (in this branch) this new policy is set to WARN, i.e. still the old behaviour by default. As discussed, I think for this policy the default has to be new, because otherwise it doesn't have the desired effect, (with WARN the projects will break, but with warnings). So, are you ok if I set this policy to NEW by default ? I would also remove the full path for the include(FindPackageHandleStandardArgs) in this branch again. If there are no objections, I'll do this in the next days and then merge into next. Alex It doesn't make sense to introduce a policy that defaults to NEW. The reason for adding a policy is to *avoid breaking* existing projects. The default / OLD behavior should not break anything. It is when switched to NEW that stuff might break. How is it that projects will break with the OLD behavior of this policy? If OLD breaks stuff, then it's not really OLD... Hmm, we discussed this here at length already... With no change in behaviour (i.e. old behaviour), if some cmake module inside CMAKE_ROOT does include(SomeFile), and then uses some features of SomeFile.cmake introduced in the same cmake version in a backward compatible way, this breaks if there is another user-supplied SomeFile.cmake in CMAKE_MODULE_PATH (maybe just a copy from the previous cmake release), because that old SomeFile.cmake doesn't have yet the features present in the current cmake version which the SomeFile.cmake-using module expects to be present (because it's not forward-compatible). With this policy a module in CMAKE_ROOT can rely on the fact that it also gets the module it expects, i.e. the one also from CMAKE_ROOT and not another, potentially old and incompatible one, from CMAKE_MODULE_PATH. That's why this policy has to be set to NEW to avoid breakage. The result from the discussion here was to use the full path as a quick fix for 2.8.3, and as soon as this is out, use the new policy and see whether/how much breakage there is. Alex We may have discussed it, but it was a while ago, and my brain can't recall specific details about how we arrived at this point. This policy, and all CMake policies should default to OLD. But if a project says cmake_minimum_required 2.8.4 ... then it can use the NEW behavior. The old behavior is: do not load stuff from CMAKE_MODULE_PATH in a special manner. That should be the
Re: [cmake-developers] User vs CMake include mismatch handling
On Thursday 18 November 2010, Marcel Loose wrote: ... Hi all, I've been following this discussion with interest for quite a while. I was wondering if both worlds could be united (Alex's and David's) if it were possible to set cmake_minimum_required on the command line? That way Alex can be happy, because he doesn't have to modify CMakeLists.txt files - something he cannot do for previous KDE releaeses; and David will be happy, because old projects will not suddenly break due to some incompatible changes in CMake. This is a misunderstanding. This whole discussion is not about old projects breaking due to *incompatible* changes in cmake, but about old projects breaking due to *fully backward compatible* changes in cmake. A command line switch for cmake to switch policies to NEW or OLD sounds good to me. The cygwin guys probably would also like it. Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Thursday 18 November 2010, Brad King wrote: On 11/18/2010 04:29 AM, Marcel Loose wrote: ... This entire issue is about projects using CMAKE_MODULE_PATH to override standard CMake modules (accidentally or intentionally). This policy changes the *granularity* at which that has to happen. Previously a project could just copy out *one* module and modify it to fix something. Now it would need to copy out all modules that depend on that module too. This is typically done to fix a bug or gain a feature without waiting for a new CMake release. Ideally the project should have a separate entry in CMAKE_MODULE_PATH for each version of CMake, and then only add those corresponding to versions of CMake newer than or equal to that running. IOW the outside project takes responsibility for the management of module override versions. Part of the design of policies is that the NEW behavior is clearly superior to the OLD behavior in design and function. IMO, the NEW behavior of the proposed policy does not have this clear distinction. It makes the typical use case harder to do. This discussion should instead refocus on a way to make it easier for projects to deal with the above copy-and-fix-for-this-version use case. The approach should make it easy for CMake to know whether or not to honor a certain project module directory based on its version. Ok. Assume a project includes only a copy of FindZLIB.cmake from cmake 2.8.3 and applies the attached patch (or some similar simple patch). Assume it does: set(CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/CMakeModules/) find_package(ZLIB) find_package(PNG) What should it do to make sure a future cmake release with fully backward compatible changes doesn't break its build ? Alex --- FindZLIB.cmake.orig 2010-11-18 20:33:12.0 +0100 +++ FindZLIB.cmake 2010-11-18 20:33:34.0 +0100 @@ -35,7 +35,7 @@ FIND_PATH(ZLIB_INCLUDE_DIR zlib.h [HKEY_LOCAL_MACHINE\\SOFTWARE\\GnuWin32\\Zlib;InstallPath]/include ) -SET(ZLIB_NAMES z zlib zdll) +SET(ZLIB_NAMES z zlib zdll libz) FIND_LIBRARY(ZLIB_LIBRARY NAMES ${ZLIB_NAMES} ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On 11/17/2010 04:50 PM, Alexander Neundorf wrote: That's why this policy has to be set to NEW to avoid breakage. As Dave said, the entire design of policies is based on defaulting to WARN, as in do what I did before but tell people it is no longer the right way. If that doesn't make sense in this use case then the answer is not a policy. -Brad ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Fri, Oct 15, 2010 at 1:59 PM, David Cole david.c...@kitware.com wrote: On Fri, Oct 15, 2010 at 1:36 PM, David Cole david.c...@kitware.com wrote: On Fri, Oct 15, 2010 at 8:23 AM, Bill Hoffman bill.hoff...@kitware.com wrote: On 10/14/2010 2:18 PM, David Cole wrote: On Thu, Oct 14, 2010 at 1:30 PM, Alexander Neundorf neund...@kde.org mailto:neund...@kde.org wrote: On Wednesday 13 October 2010, Alexander Neundorf wrote: On Wednesday 13 October 2010, Bill Hoffman wrote: So, I think we have to use the new name approach. Do we want to call it 2? Or should we call it something else? Alex, do you have time to do this? I think it's not a good solution, and the one with CURRENT_LIST_DIR is definitely better and already implemented. And I am still convinced that I am right here, see my other mails. So I would suggest to merge the CURRENT_LIST_DIR branch for 2.8.3, and as soon as 2.8.3 is released, remove the full paths again and enable the new CMP0017 instead (prefer CMAKE_ROOT when include()d from CMAKE_ROOT) and then see what happens during the whole 2.8.4 cycle. I think this (CMP0017) is necessary, because otherwise we can only hope nothing breaks with future releases (independent from FPHSA). Alex ___ cmake-developers mailing list cmake-develop...@cmake.org mailto:cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers I'm ok with this since Alex feels so strongly about it, and the code change is restricted to using CMAKE_CURRENT_LIST_DIR only when including FPHSA.cmake... (i.e. -- it should not affect including *other* files from inside of modules that are presently overridden... which was my concern -- that we'd break some *other* scenario -- since that's not true, I retract my objection.) To review the change yourself: git fetch stage gitk remotes/stage/AddCMAKE_CURRENT_LIST_DIR Look at the top 3 commits. Looks pretty safe. I'd say we should merge it to 'next' and then after a night on the dashboards, merge it to 'master' and do an rc3 release based on that. OK, lets try this one. Lets move it to next. -Bill ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers I'm trying to merge this, and getting conflicts when merging to 'next' because of changes in cmMakefile.cxx like this: this-AddDefinition(CMAKE_CURRENT_LIST_FILE, filenametoread); HEAD this-MarkVariableAsUsed(CMAKE_CURRENT_LIST_FILE); === this-AddDefinition(CMAKE_CURRENT_LIST_DIR, cmSystemTools::GetFilenamePath(filenametoread).c_str()); AddCMAKE_CURRENT_LIST_DIR What's the best way to resolve this conflict? Should I delete the MarkVariableAsUsed lines because they're part of the dev/strict-mode branch which is *not* going into 2.8.3? If I leave the MarkVariableAsUsed call in there, does if affect how the commit will merge to 'master'...? Arg. Never mind. I think I resolved this correctly for next (including *both* features) and pushed it just now. The conflict should not occur when merging to master, so there should be nothing to resolve when I merge to master, and the additional lines I added as conflict resolution should not end up in master. (I'm going to verify that locally right now... :-) Thanks, David Just to confirm that the current next fixes the issues I observed with CMake failing to configure Avogadro. I think your merge should be fine for master for the reason you state. Marcus ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Tue, Oct 19, 2010 at 9:46 AM, Marcus D. Hanwell marcus.hanw...@kitware.com wrote: On Fri, Oct 15, 2010 at 1:59 PM, David Cole david.c...@kitware.com wrote: On Fri, Oct 15, 2010 at 1:36 PM, David Cole david.c...@kitware.com wrote: On Fri, Oct 15, 2010 at 8:23 AM, Bill Hoffman bill.hoff...@kitware.com wrote: On 10/14/2010 2:18 PM, David Cole wrote: On Thu, Oct 14, 2010 at 1:30 PM, Alexander Neundorf neund...@kde.org mailto:neund...@kde.org wrote: On Wednesday 13 October 2010, Alexander Neundorf wrote: On Wednesday 13 October 2010, Bill Hoffman wrote: So, I think we have to use the new name approach. Do we want to call it 2? Or should we call it something else? Alex, do you have time to do this? I think it's not a good solution, and the one with CURRENT_LIST_DIR is definitely better and already implemented. And I am still convinced that I am right here, see my other mails. So I would suggest to merge the CURRENT_LIST_DIR branch for 2.8.3, and as soon as 2.8.3 is released, remove the full paths again and enable the new CMP0017 instead (prefer CMAKE_ROOT when include()d from CMAKE_ROOT) and then see what happens during the whole 2.8.4 cycle. I think this (CMP0017) is necessary, because otherwise we can only hope nothing breaks with future releases (independent from FPHSA). Alex ___ cmake-developers mailing list cmake-developers@cmake.org mailto:cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers I'm ok with this since Alex feels so strongly about it, and the code change is restricted to using CMAKE_CURRENT_LIST_DIR only when including FPHSA.cmake... (i.e. -- it should not affect including *other* files from inside of modules that are presently overridden... which was my concern -- that we'd break some *other* scenario -- since that's not true, I retract my objection.) To review the change yourself: git fetch stage gitk remotes/stage/AddCMAKE_CURRENT_LIST_DIR Look at the top 3 commits. Looks pretty safe. I'd say we should merge it to 'next' and then after a night on the dashboards, merge it to 'master' and do an rc3 release based on that. OK, lets try this one. Lets move it to next. -Bill ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers I'm trying to merge this, and getting conflicts when merging to 'next' because of changes in cmMakefile.cxx like this: this-AddDefinition(CMAKE_CURRENT_LIST_FILE, filenametoread); HEAD this-MarkVariableAsUsed(CMAKE_CURRENT_LIST_FILE); === this-AddDefinition(CMAKE_CURRENT_LIST_DIR, cmSystemTools::GetFilenamePath(filenametoread).c_str()); AddCMAKE_CURRENT_LIST_DIR What's the best way to resolve this conflict? Should I delete the MarkVariableAsUsed lines because they're part of the dev/strict-mode branch which is *not* going into 2.8.3? If I leave the MarkVariableAsUsed call in there, does if affect how the commit will merge to 'master'...? Arg. Never mind. I think I resolved this correctly for next (including *both* features) and pushed it just now. The conflict should not occur when merging to master, so there should be nothing to resolve when I merge to master, and the additional lines I added as conflict resolution should not end up in master. (I'm going to verify that locally right now... :-) Thanks, David Just to confirm that the current next fixes the issues I observed with CMake failing to configure Avogadro. I think your merge should be fine for master for the reason you state. Marcus Thanks, Marcus. We can all use a git double-check these days... :-) ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On 10/14/2010 2:18 PM, David Cole wrote: On Thu, Oct 14, 2010 at 1:30 PM, Alexander Neundorf neund...@kde.org mailto:neund...@kde.org wrote: On Wednesday 13 October 2010, Alexander Neundorf wrote: On Wednesday 13 October 2010, Bill Hoffman wrote: So, I think we have to use the new name approach. Do we want to call it 2? Or should we call it something else? Alex, do you have time to do this? I think it's not a good solution, and the one with CURRENT_LIST_DIR is definitely better and already implemented. And I am still convinced that I am right here, see my other mails. So I would suggest to merge the CURRENT_LIST_DIR branch for 2.8.3, and as soon as 2.8.3 is released, remove the full paths again and enable the new CMP0017 instead (prefer CMAKE_ROOT when include()d from CMAKE_ROOT) and then see what happens during the whole 2.8.4 cycle. I think this (CMP0017) is necessary, because otherwise we can only hope nothing breaks with future releases (independent from FPHSA). Alex ___ cmake-developers mailing list cmake-developers@cmake.org mailto:cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers I'm ok with this since Alex feels so strongly about it, and the code change is restricted to using CMAKE_CURRENT_LIST_DIR only when including FPHSA.cmake... (i.e. -- it should not affect including *other* files from inside of modules that are presently overridden... which was my concern -- that we'd break some *other* scenario -- since that's not true, I retract my objection.) To review the change yourself: git fetch stage gitk remotes/stage/AddCMAKE_CURRENT_LIST_DIR Look at the top 3 commits. Looks pretty safe. I'd say we should merge it to 'next' and then after a night on the dashboards, merge it to 'master' and do an rc3 release based on that. OK, lets try this one. Lets move it to next. -Bill ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Fri, Oct 15, 2010 at 1:36 PM, David Cole david.c...@kitware.com wrote: On Fri, Oct 15, 2010 at 8:23 AM, Bill Hoffman bill.hoff...@kitware.comwrote: On 10/14/2010 2:18 PM, David Cole wrote: On Thu, Oct 14, 2010 at 1:30 PM, Alexander Neundorf neund...@kde.org mailto:neund...@kde.org wrote: On Wednesday 13 October 2010, Alexander Neundorf wrote: On Wednesday 13 October 2010, Bill Hoffman wrote: So, I think we have to use the new name approach. Do we want to call it 2? Or should we call it something else? Alex, do you have time to do this? I think it's not a good solution, and the one with CURRENT_LIST_DIR is definitely better and already implemented. And I am still convinced that I am right here, see my other mails. So I would suggest to merge the CURRENT_LIST_DIR branch for 2.8.3, and as soon as 2.8.3 is released, remove the full paths again and enable the new CMP0017 instead (prefer CMAKE_ROOT when include()d from CMAKE_ROOT) and then see what happens during the whole 2.8.4 cycle. I think this (CMP0017) is necessary, because otherwise we can only hope nothing breaks with future releases (independent from FPHSA). Alex ___ cmake-developers mailing list cmake-developers@cmake.org mailto:cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers I'm ok with this since Alex feels so strongly about it, and the code change is restricted to using CMAKE_CURRENT_LIST_DIR only when including FPHSA.cmake... (i.e. -- it should not affect including *other* files from inside of modules that are presently overridden... which was my concern -- that we'd break some *other* scenario -- since that's not true, I retract my objection.) To review the change yourself: git fetch stage gitk remotes/stage/AddCMAKE_CURRENT_LIST_DIR Look at the top 3 commits. Looks pretty safe. I'd say we should merge it to 'next' and then after a night on the dashboards, merge it to 'master' and do an rc3 release based on that. OK, lets try this one. Lets move it to next. -Bill ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers I'm trying to merge this, and getting conflicts when merging to 'next' because of changes in cmMakefile.cxx like this: this-AddDefinition(CMAKE_CURRENT_LIST_FILE, filenametoread); HEAD this-MarkVariableAsUsed(CMAKE_CURRENT_LIST_FILE); === this-AddDefinition(CMAKE_CURRENT_LIST_DIR, cmSystemTools::GetFilenamePath(filenametoread).c_str()); AddCMAKE_CURRENT_LIST_DIR What's the best way to resolve this conflict? Should I delete the MarkVariableAsUsed lines because they're part of the dev/strict-mode branch which is *not* going into 2.8.3? If I leave the MarkVariableAsUsed call in there, does if affect how the commit will merge to 'master'...? Arg. Never mind. I think I resolved this correctly for next (including *both* features) and pushed it just now. The conflict should not occur when merging to master, so there should be nothing to resolve when I merge to master, and the additional lines I added as conflict resolution should not end up in master. (I'm going to verify that locally right now... :-) Thanks, David ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Thu, Oct 14, 2010 at 1:30 PM, Alexander Neundorf neund...@kde.orgwrote: On Wednesday 13 October 2010, Alexander Neundorf wrote: On Wednesday 13 October 2010, Bill Hoffman wrote: So, I think we have to use the new name approach. Do we want to call it 2? Or should we call it something else? Alex, do you have time to do this? I think it's not a good solution, and the one with CURRENT_LIST_DIR is definitely better and already implemented. And I am still convinced that I am right here, see my other mails. So I would suggest to merge the CURRENT_LIST_DIR branch for 2.8.3, and as soon as 2.8.3 is released, remove the full paths again and enable the new CMP0017 instead (prefer CMAKE_ROOT when include()d from CMAKE_ROOT) and then see what happens during the whole 2.8.4 cycle. I think this (CMP0017) is necessary, because otherwise we can only hope nothing breaks with future releases (independent from FPHSA). Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers I'm ok with this since Alex feels so strongly about it, and the code change is restricted to using CMAKE_CURRENT_LIST_DIR only when including FPHSA.cmake... (i.e. -- it should not affect including *other* files from inside of modules that are presently overridden... which was my concern -- that we'd break some *other* scenario -- since that's not true, I retract my objection.) To review the change yourself: git fetch stage gitk remotes/stage/AddCMAKE_CURRENT_LIST_DIR Look at the top 3 commits. Looks pretty safe. I'd say we should merge it to 'next' and then after a night on the dashboards, merge it to 'master' and do an rc3 release based on that. Dave ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
So, I think we have to use the new name approach. Do we want to call it 2? Or should we call it something else? Alex, do you have time to do this? I want to get the CMake release out very soon. Thanks. -Bill ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Tuesday 12 October 2010, David Cole wrote: On Tue, Oct 12, 2010 at 5:21 PM, Brad King brad.k...@kitware.com wrote: ... releases and will get us through this CMake rc cycle safely. Future enhancements to FPHSA2 may need yet another new module, but I think that is in the nature of this particular function. -Brad ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers I really think a second function is the way to go here. It is the least risky and maintains full compatibility with existing module overrides. It does not have to be named FPHSA2 (I am not a big fan of 2 named functions...) but I do think it needs to be a newly-named function, and keep the old function as is. We can come up with better solutions moving forward, but for now, to keep *everything* working well without breaking anything *else*... I think a second function is our only realistic option. No, it's not. Using the full path (as already _completely done_ in the AddCMAKE_CURRENT_LIST_DIR branch) is 100% safe now and in future cmake releases. Adding FPHSA2.cmake now in 2.8.3 is safe now, but not as soon as new features are added to it in 2.8.4 or later versions. Projects will have copies of it and it can break just the same way then. What am I missing here ? Where is the risk (or any problem) when using the full path ? I don't understand the argumentation. FYI, kdelibs 4.5.2 (released 2 weeks ago) has the full-featured FPHSA.cmake, so this one will be compatible with 2.8.3, but I think no matter what we decide for now, 4.5.2 could be ignored if it stays the only release which may have some oddity. Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Tuesday 12 October 2010, Brad King wrote: On 10/12/2010 03:32 PM, Alexander Neundorf wrote: On Tuesday 12 October 2010, Bill Hoffman wrote: Anyway, in the short term, we are going to go with FPHSA2, Alex do you have time to do that? FPHSA2 would have been my last choice. In staging there is already the branch AddCMAKE_CURRENT_LIST_DIR: http://cmake.org/gitweb?p=stage/cmake.git;a=log;h=refs/heads/AddCMAKE_CUR RENT_LIST_DIR where I implemented the option with the hardcoded paths: The original FPHSA floated around outside of CMake in many projects before it was distributed with CMake. It is a long-established API that has been re-implemented (via copying) in many projects. Therefore it is too late to change. See James Bigler's comment earlier in this thread about ABI compatibility approaches. No one proposes changing the ABI of malloc in C because many custom allocation libraries override it at link time. Currently projects have the option to override it with CMAKE_MODULE_PATH to providing a module with the same API but a tweaked implementation. With the CURRENT_LIST_DIR approach that option goes away, and any project that does it already will break. Yes, because this option is what makes cmake break KDE currently. This is the problem, this is what must be changed. Nothing breaks with the CURRENT_LIST_DIR, since this only applies to the modules shipped with cmake, so those get what they expect. The modules shipped with the project still get their own FPHSA.cmake. I can't think of a constellation which could break. Otherwise this means that no matter what new features we add to any of cmake's modules, we cannot use any of them in any other module. Do we want that ? Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Wednesday 13 October 2010, Bill Hoffman wrote: So, I think we have to use the new name approach. Do we want to call it 2? Or should we call it something else? Alex, do you have time to do this? I think it's not a good solution, and the one with CURRENT_LIST_DIR is definitely better and already implemented. And I am still convinced that I am right here, see my other mails. Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Wednesday 13 October 2010, Alexander Neundorf wrote: ... Adding FPHSA2.cmake now in 2.8.3 is safe now, but not as soon as new features are added to it in 2.8.4 or later versions. Projects will have copies of it and it can break just the same way then. Example: assume projects will take a copy of FPHSA2.cmake now. KDE will have one. In cmake 2.8.4 FPHSA2 may have some new option for better handling of the COMPONENTS argument of find_package(). - same breakage again as we have now, if no full path is used. Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Wednesday 13 October 2010, Alexander Neundorf wrote: On Tuesday 12 October 2010, Brad King wrote: ... Currently projects have the option to override it with CMAKE_MODULE_PATH to providing a module with the same API but a tweaked implementation. With the CURRENT_LIST_DIR approach that option goes away, and any project that does it already will break. Yes, because this option is what makes cmake break KDE currently. This is the problem, this is what must be changed. Nothing breaks with the CURRENT_LIST_DIR, since this only applies to the modules shipped with cmake, so those get what they expect. The modules shipped with the project still get their own FPHSA.cmake. I can't think of a constellation which could break. Example: assume a project relies on the fact that FindZLIB.cmake gets their modified copy of FPHSA.cmake, which e.g. sets some additional variable, or looks at some additional variable (realistically I don't think such a modified version, i.e. not just simply an older version, exists). If we rename the file to FPHSA2.cmake, this will not be the case anymore, so their project is broken. The same happens if FindZLIB.cmake uses CURRENT_LIST_DIR. Additionally FPHSA2.cmake has other (likely) chances to break in the future (see my other email). Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Tuesday 12 October 2010, Marcus D. Hanwell wrote: On Mon, Oct 11, 2010 at 5:00 PM, Alexander Neundorf neund...@kde.orgwrote: ... Personally, I would try a rc3 with CMP0017 set to NEW and see how it goes. It works for kdelibs and for our project at work (which doesn't have a lot of fancy cmake stuff). If it is just that one file why not go with the simple solution? I would love to see a policy where includes in the same directory would be considered first, and then the normal search behavior. I wasn't sure how feasible this was in an rc3. Avogadro effectively copied what KDE was doing, and so maybe the issue is not that widespread. I don't have the avogadro sources here. What does it do, i.e. what did it copy from KDE ? Does it a find_package(KDE4) ? Or does it include its own version of FindPackageHandleStandardArgs.cmake ? Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Tuesday 12 October 2010, Marcus D. Hanwell wrote: 2010/10/12 Alexander Neundorf neund...@kde.org On Tuesday 12 October 2010, Marcus D. Hanwell wrote: On Mon, Oct 11, 2010 at 5:00 PM, Alexander Neundorf neund...@kde.org wrote: ... Personally, I would try a rc3 with CMP0017 set to NEW and see how it goes. It works for kdelibs and for our project at work (which doesn't have a lot of fancy cmake stuff). If it is just that one file why not go with the simple solution? I would love to see a policy where includes in the same directory would be considered first, and then the normal search behavior. I wasn't sure how feasible this was in an rc3. Avogadro effectively copied what KDE was doing, and so maybe the issue is not that widespread. I don't have the avogadro sources here. What does it do, i.e. what did it copy from KDE ? Does it a find_package(KDE4) ? Or does it include its own version of FindPackageHandleStandardArgs.cmake ? It has its own copy of FindPackageHandleStandardArgs.cmake, which is used in some of the Find modules. Avogadro is a dependency of Kalzium (KDE Edu), but is C++/Qt based itself. I was more thinking of all the other packages out there that may have done similar, and the fact that we should avoid breaking their builds with a new CMake release. Ok. So setting CMP0017 to NEW in FindKDE4.cmake would not help Avogadro, and KDE4 is not the only project which breaks together with current cmake 2.8.3. This reduces the number of acceptable solutions I think. Remaining are as far as I see: -set new policy CMP0017 to NEW by default Projects with an exotic setup may break, but that's probably better than breaking all KDE 4.5.0/1 builds. One could also argue that these projects relied on internal implementation details of cmake. As a pro, I think this behaviour (include()s from CMAKE_ROOT first check in CMAKE_ROOT) makes sense, since we have that dependency but currently it's not enforced. -hardcode the path to FPHSA.cmake in the find-modules in cmake, probably also to CMakeParseArguments.cmake and FindPackageMessage.cmake IMO not a solution, just a quick fix for the moment, because we have to maintain this forever in the future and basically we need to check *all* other include()s in CMAKE_ROOT. -rename the enhanced FPHSA to FPHSA2 Worse than above, since this doesn't give any guarantee that the same does not happen again in the next cmake release with the renamed function. Did I miss anything ? Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
2010/10/12 Alexander Neundorf neund...@kde.org On Tuesday 12 October 2010, Marcus D. Hanwell wrote: 2010/10/12 Alexander Neundorf neund...@kde.org On Tuesday 12 October 2010, Marcus D. Hanwell wrote: On Mon, Oct 11, 2010 at 5:00 PM, Alexander Neundorf neund...@kde.org wrote: ... Personally, I would try a rc3 with CMP0017 set to NEW and see how it goes. It works for kdelibs and for our project at work (which doesn't have a lot of fancy cmake stuff). If it is just that one file why not go with the simple solution? I would love to see a policy where includes in the same directory would be considered first, and then the normal search behavior. I wasn't sure how feasible this was in an rc3. Avogadro effectively copied what KDE was doing, and so maybe the issue is not that widespread. I don't have the avogadro sources here. What does it do, i.e. what did it copy from KDE ? Does it a find_package(KDE4) ? Or does it include its own version of FindPackageHandleStandardArgs.cmake ? It has its own copy of FindPackageHandleStandardArgs.cmake, which is used in some of the Find modules. Avogadro is a dependency of Kalzium (KDE Edu), but is C++/Qt based itself. I was more thinking of all the other packages out there that may have done similar, and the fact that we should avoid breaking their builds with a new CMake release. Ok. So setting CMP0017 to NEW in FindKDE4.cmake would not help Avogadro, and KDE4 is not the only project which breaks together with current cmake 2.8.3. This reduces the number of acceptable solutions I think. Yes, that is correct. I think it is hard to assess which projects might be broken after release that have not tried an rc. I would not be surprised if quite a few out there might break. Remaining are as far as I see: -set new policy CMP0017 to NEW by default Projects with an exotic setup may break, but that's probably better than breaking all KDE 4.5.0/1 builds. One could also argue that these projects relied on internal implementation details of cmake. As a pro, I think this behaviour (include()s from CMAKE_ROOT first check in CMAKE_ROOT) makes sense, since we have that dependency but currently it's not enforced. I would tend to agree with you there. -hardcode the path to FPHSA.cmake in the find-modules in cmake, probably also to CMakeParseArguments.cmake and FindPackageMessage.cmake IMO not a solution, just a quick fix for the moment, because we have to maintain this forever in the future and basically we need to check *all* other include()s in CMAKE_ROOT. Agreed. -rename the enhanced FPHSA to FPHSA2 Worse than above, since this doesn't give any guarantee that the same does not happen again in the next cmake release with the renamed function. I think this is the simplest, but I can see why you would rather avoid it. Did I miss anything ? I don't think so. I would go with option 1 or 3. Effectively the policy would be the reverse of most though, and so very confusing - setting it to OLD is more likely to cause breakage than setting it to NEW. This would mean that the standard documentation would not really apply well to this one policy, and it could cause a lot of confusion. I wonder if Brad or Bill have any thoughts on this? Marcus ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
Remaining are as far as I see: -set new policy CMP0017 to NEW by default Projects with an exotic setup may break, but that's probably better than breaking all KDE 4.5.0/1 builds. One could also argue that these projects relied on internal implementation details of cmake. As a pro, I think this behaviour (include()s from CMAKE_ROOT first check in CMAKE_ROOT) makes sense, since we have that dependency but currently it's not enforced. -hardcode the path to FPHSA.cmake in the find-modules in cmake, probably also to CMakeParseArguments.cmake and FindPackageMessage.cmake IMO not a solution, just a quick fix for the moment, because we have to maintain this forever in the future and basically we need to check *all* other include()s in CMAKE_ROOT. -rename the enhanced FPHSA to FPHSA2 Worse than above, since this doesn't give any guarantee that the same does not happen again in the next cmake release with the renamed function. I think at this point we are going to have to go with FPHSA2. We can work on a longer term strategy for getting around this problem after this current release is done. I want the release to stay on schedule. In the future, I want to setup a Contract with CMake dashboard for KDE so that things like this don't happen again, and we are not forced to pick a sub-optimal solution. This (FPHSA change) has been in CMake next/master since Aug, and we go to do an RC, and we find out we have a problem! I have started to create contract tests. I have a VTK one that is running each night. It will build a known working version of VTK against CMake next. In a few weeks I will announce this to the lists and create a way for projects to setup a contract with CMake test. Anyway, in the short term, we are going to go with FPHSA2, Alex do you have time to do that? -Bill ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Tuesday 12 October 2010, Alan W. Irwin wrote: ... cmake_minimum_required(VERSION 2.8.3 FATAL_ERROR) because you absolutely need CMP0017. I believe most projects (including PLplot) will eventually need that policy as well because there is a tendency to copy and modify CMake find modules to the project's special needs for any mature project. Thus, I assume PLplot will need CMP0017 in the future, but when that occurs I will bump our minimum version to a high enough value so we are assured of having that policy. Am I missing something? Yes. The (currently known) problem is that FPHSA.cmake in cmake had features added in a backward compatible way, but projects which have a copy of an older (e.g. cmake 2.8.2) FPHSA.cmake break, because their older version is not forward-compatible to the one in cmake 2.8.3, and because of the handling of CMAKE_MODULE_PATH their old, not-forward compatible version is confronted with the parameters for the new version, which they cannot handle. The new CMP0017 would *by default change* the behaviour, in order to avoid that breakage. What could break: projects, which * have a copy of some cmake module shipped with cmake in their CMAKE_MODULE_PATH * AND which rely on the fact that when include()ing/find_package()ing some other module shipped with cmake (where they don't have a copy of), that this called module (from CMAKE_ROOT) gets their own copy of the previously mentioned module loaded when it does a include()/find_package() I think such constellations should be quite rare. Such projects would break. They could be fixed again by either setting CMP0017 to OLD, or by having the full chain of cmake modules as a copy. E.g. if they rely on FindPNG.cmake getting their own copy of FindZLIB.cmake, they would have to include also FindPNG.cmake in their sources. Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Tuesday 12 October 2010, Bill Hoffman wrote: Remaining are as far as I see: -set new policy CMP0017 to NEW by default Projects with an exotic setup may break, but that's probably better than breaking all KDE 4.5.0/1 builds. One could also argue that these projects relied on internal implementation details of cmake. As a pro, I think this behaviour (include()s from CMAKE_ROOT first check in CMAKE_ROOT) makes sense, since we have that dependency but currently it's not enforced. -hardcode the path to FPHSA.cmake in the find-modules in cmake, probably also to CMakeParseArguments.cmake and FindPackageMessage.cmake IMO not a solution, just a quick fix for the moment, because we have to maintain this forever in the future and basically we need to check *all* other include()s in CMAKE_ROOT. -rename the enhanced FPHSA to FPHSA2 Worse than above, since this doesn't give any guarantee that the same does not happen again in the next cmake release with the renamed function. I think at this point we are going to have to go with FPHSA2. We can work on a longer term strategy for getting around this problem after this current release is done. I want the release to stay on schedule. In the future, I want to setup a Contract with CMake dashboard for KDE so that things like this don't happen again, and we are not forced to pick a sub-optimal solution. This (FPHSA change) has been in CMake next/master since Aug, and we go to do an RC, and we find out we have a problem! I have started to create contract tests. I have a VTK one that is running each night. It will build a known working version of VTK against CMake next. In a few weeks I will announce this to the lists and create a way for projects to setup a contract with CMake test. Anyway, in the short term, we are going to go with FPHSA2, Alex do you have time to do that? FPHSA2 would have been my last choice. In staging there is already the branch AddCMAKE_CURRENT_LIST_DIR: http://cmake.org/gitweb?p=stage/cmake.git;a=log;h=refs/heads/AddCMAKE_CURRENT_LIST_DIR where I implemented the option with the hardcoded paths: -INCLUDE(FindPackageHandleStandardArgs) +INCLUDE(${CMAKE_CURRENT_LIST_DIR}/FindPackageHandleStandardArgs.cmake) This doesn't introduce a macro with a new name (i.e. without an appended 2) which we then have to maintain forever, and makes also sure that the correct FPHSA.cmake is loaded by cmake 2.8.3 (which FPHSA2 does only for cmake 2.8.3, but not for coming cmake 2.8.3, and I think it is quite likely that FPHSA()/FPHSA2() will get more options in coming releases) Even while I think this is not a good solution, it is a safe short-term fix, and we can figure out for 2.8.4 how we should handle CMAKE_MODULE_PATH in the long term. Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
2010/10/12 Alexander Neundorf neund...@kde.org: On Tuesday 12 October 2010, Marcus D. Hanwell wrote: 2010/10/12 Alexander Neundorf neund...@kde.org It has its own copy of FindPackageHandleStandardArgs.cmake, which is used in some of the Find modules. Avogadro is a dependency of Kalzium (KDE Edu), but is C++/Qt based itself. I was more thinking of all the other packages out there that may have done similar, and the fact that we should avoid breaking their builds with a new CMake release. Ok. So setting CMP0017 to NEW in FindKDE4.cmake would not help Avogadro, and KDE4 is not the only project which breaks together with current cmake 2.8.3. This reduces the number of acceptable solutions I think. Remaining are as far as I see: -set new policy CMP0017 to NEW by default Projects with an exotic setup may break, but that's probably better than breaking all KDE 4.5.0/1 builds. One could also argue that these projects relied on internal implementation details of cmake. As a pro, I think this behaviour (include()s from CMAKE_ROOT first check in CMAKE_ROOT) makes sense, since we have that dependency but currently it's not enforced. -hardcode the path to FPHSA.cmake in the find-modules in cmake, probably also to CMakeParseArguments.cmake and FindPackageMessage.cmake IMO not a solution, just a quick fix for the moment, because we have to maintain this forever in the future and basically we need to check *all* other include()s in CMAKE_ROOT. -rename the enhanced FPHSA to FPHSA2 Worse than above, since this doesn't give any guarantee that the same does not happen again in the next cmake release with the renamed function. Did I miss anything ? Yes there is another not-so-pleasant option: - revert CMake's FPHSA changes in order to stay compatible with 2.8.2 one and KDE and other currently annoying project w.r.t. FPHSA The incompatible improvement of FPHSA may be postponed to 2.8.4, and we will have to design a good solution and warn all project about bad habits before 2.8.4. Suggesting this I'm don't even know how much impact it has on CMake-shipped modules, i.e. how many modules do currently use the new FPHSA? If the idea is plain stupid just forget my mail, I genuine want to be positive on this issue not trying to shot on others efforts (Alex in this case) to improve CMake. -- Erk Membre de l'April - « promouvoir et défendre le logiciel libre » - http://www.april.org ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On 10/12/2010 03:32 PM, Alexander Neundorf wrote: On Tuesday 12 October 2010, Bill Hoffman wrote: Anyway, in the short term, we are going to go with FPHSA2, Alex do you have time to do that? FPHSA2 would have been my last choice. In staging there is already the branch AddCMAKE_CURRENT_LIST_DIR: http://cmake.org/gitweb?p=stage/cmake.git;a=log;h=refs/heads/AddCMAKE_CURRENT_LIST_DIR where I implemented the option with the hardcoded paths: The original FPHSA floated around outside of CMake in many projects before it was distributed with CMake. It is a long-established API that has been re-implemented (via copying) in many projects. Therefore it is too late to change. See James Bigler's comment earlier in this thread about ABI compatibility approaches. No one proposes changing the ABI of malloc in C because many custom allocation libraries override it at link time. Currently projects have the option to override it with CMAKE_MODULE_PATH to providing a module with the same API but a tweaked implementation. With the CURRENT_LIST_DIR approach that option goes away, and any project that does it already will break. macro with a new name ... which we then have to maintain forever It's not too bad. The new name has the new API. The original FPHSA module can just include the new one and forward calls appropriately. Any call to the original FPHSA will either go through the CMake version of it and forward to FPHSA2 or go through a project-overridden one. Any call to the FPHSA2 will either go through the CMake version or through one overridden by a project aware of the new API. This isn't perfect but it is 100% compatible with current project releases and will get us through this CMake rc cycle safely. Future enhancements to FPHSA2 may need yet another new module, but I think that is in the nature of this particular function. -Brad ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Tuesday 12 October 2010 17:27:31 David Cole wrote: On Tue, Oct 12, 2010 at 5:21 PM, Brad King brad.k...@kitware.com wrote: On 10/12/2010 03:32 PM, Alexander Neundorf wrote: On Tuesday 12 October 2010, Bill Hoffman wrote: Anyway, in the short term, we are going to go with FPHSA2, Alex do you have time to do that? FPHSA2 would have been my last choice. In staging there is already the branch AddCMAKE_CURRENT_LIST_DIR: http://cmake.org/gitweb?p=stage/cmake.git;a=log;h=refs/heads/AddCMAKE_CUR RENT_LIST_DIR where I implemented the option with the hardcoded paths: The original FPHSA floated around outside of CMake in many projects before it was distributed with CMake. It is a long-established API that has been re-implemented (via copying) in many projects. Therefore it is too late to change. See James Bigler's comment earlier in this thread about ABI compatibility approaches. No one proposes changing the ABI of malloc in C because many custom allocation libraries override it at link time. Currently projects have the option to override it with CMAKE_MODULE_PATH to providing a module with the same API but a tweaked implementation. With the CURRENT_LIST_DIR approach that option goes away, and any project that does it already will break. macro with a new name ... which we then have to maintain forever It's not too bad. The new name has the new API. The original FPHSA module can just include the new one and forward calls appropriately. Any call to the original FPHSA will either go through the CMake version of it and forward to FPHSA2 or go through a project-overridden one. Any call to the FPHSA2 will either go through the CMake version or through one overridden by a project aware of the new API. This isn't perfect but it is 100% compatible with current project releases and will get us through this CMake rc cycle safely. Future enhancements to FPHSA2 may need yet another new module, but I think that is in the nature of this particular function. I really think a second function is the way to go here. It is the least risky and maintains full compatibility with existing module overrides. It does not have to be named FPHSA2 (I am not a big fan of 2 named functions...) but I do think it needs to be a newly-named function, and keep the old function as is. We can come up with better solutions moving forward, but for now, to keep *everything* working well without breaking anything *else*... I think a second function is our only realistic option. I like the malloc analogy, and think it holds here. We do have a certain responsibility to maintain API compatibility, and having a new function seems like the best way to achieve that for the release. Going forward, versioned include files, or preference towards local files might be the way to go. Marcus ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Sunday 10 October 2010 14:56:29 Alexander Neundorf wrote: On Friday 08 October 2010, David Cole wrote: On Fri, Oct 8, 2010 at 10:59 AM, Alexander Neundorf neund...@kde.orgwrote: ... Better idea: I'll add a policy which switches this behaviour (prefer CMAKE_ROOT over CMAKE_MODULE_PATH when used from within CMAKE_ROOT), and this policy will, as all other policies, default to the old behaviour, but warn. ... This latest idea does sound better, but I am still not a fan of invisible / magic behavior. I much prefer when things are explicitly spelled out. There is now a branch PreferCMakeModulesByCMakeModulesWithPolicy on staging. It includes this patch, a new policy CMP0017, and a basic test for it. I also verified that this fixes the KDE 4.5 problem. With KDE 4.5.0/4.5.1 there are now a lot of these new CMP0017 warnings, and in the end stuff has not been found which should have been found. Should I handle setting CMP0017 to NEW in kdelibs (- cmake 2.8.3 will not be able to build KDE 4.5.0/1, but complain loud) or in CMake/Modules/FindKDE4.cmake (then cmake 2.8.3 will be able to build KDE 4.5.0/1) ? So is there no chance of fixing this in a backward compatible way? One of the projects I work on has been bitten by this too, and so I guess the old, released versions are doomed to fail with CMake 2.8.3 with the current solution. This seems like quite a regression, and something the policies were supposed to avoid (old projects failing with new CMake versions). I tested your topic branch with this project, and it still fails with the following error, CMake Error at cmake/modules/FindPackageHandleStandardArgs.cmake:53 (MESSAGE): REQUIRED_VARS Call Stack (most recent call first): /usr/local/share/cmake-2.8/Modules/FindZLIB.cmake:70 (FIND_PACKAGE_HANDLE_STANDARD_ARGS) CMakeLists.txt:234 (find_package) Marcus ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Monday 11 October 2010, Marcus D. Hanwell wrote: On Sunday 10 October 2010 14:56:29 Alexander Neundorf wrote: ... So is there no chance of fixing this in a backward compatible way? One of Prefering module in CMAKE_ROOT when include() or find_package() is called from CMAKE_ROOT (which does have the potential to break existing builds, but those projects must have a weird setup I think). the projects I work on has been bitten by this too, and so I guess the old, released versions are doomed to fail with CMake 2.8.3 with the current solution. This seems like quite a regression, and something the policies were supposed to avoid (old projects failing with new CMake versions). Hmm, this would mean that the module which uses FPHSA() would have to check how the new policy is set and depending on this call FPHSA() with the old or the new enhanced syntax. That's also ugly I think, and worse than using include(${CMAKE_CURRENT_LIST_DIR}/FindPackageHandleStandardArgs.cmake) This would fix the current problem, but IMO this is not a real solution, since it can break also for other modules, and then this hardcoded path must be used also in the future in all find-modules. I think prefering the modules from CMAKE_ROOT when included from CMAKE_ROOT is the right thing to do. This expresses the dependency we have (from cmake modules to the other current cmake modules in CMAKE_ROOT). I tested your topic branch with this project, and it still fails with the You should get the policy warning about CMP0017. Is that the case ? following error, CMake Error at cmake/modules/FindPackageHandleStandardArgs.cmake:53 (MESSAGE): REQUIRED_VARS Call Stack (most recent call first): /usr/local/share/cmake-2.8/Modules/FindZLIB.cmake:70 (FIND_PACKAGE_HANDLE_STANDARD_ARGS) CMakeLists.txt:234 (find_package) I see the following options: * set policy 0017 to NEW behaviour by default (is that possible ?) * set policy 0017 to NEW behaviour somewhere locally. What project is failing ? * hardcode ${CMAKE_CURRENT_LIST_DIR}/FindPackageHandleStandardArgs.cmake instead of just using include(FindPackageHandleStandardArgs), problems see above * revert the enhanced syntax :-( or put it into a file FindPackageHandleStandardArgs2.cmake (but the same can happen later on again) :-/ Personally, I would try a rc3 with CMP0017 set to NEW and see how it goes. It works for kdelibs and for our project at work (which doesn't have a lot of fancy cmake stuff). Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Mon, Oct 11, 2010 at 5:00 PM, Alexander Neundorf neund...@kde.orgwrote: On Monday 11 October 2010, Marcus D. Hanwell wrote: On Sunday 10 October 2010 14:56:29 Alexander Neundorf wrote: ... So is there no chance of fixing this in a backward compatible way? One of Prefering module in CMAKE_ROOT when include() or find_package() is called from CMAKE_ROOT (which does have the potential to break existing builds, but those projects must have a weird setup I think). Sounds reasonable to me, although I know other options were floated. the projects I work on has been bitten by this too, and so I guess the old, released versions are doomed to fail with CMake 2.8.3 with the current solution. This seems like quite a regression, and something the policies were supposed to avoid (old projects failing with new CMake versions). Hmm, this would mean that the module which uses FPHSA() would have to check how the new policy is set and depending on this call FPHSA() with the old or the new enhanced syntax. That's also ugly I think, and worse than using include(${CMAKE_CURRENT_LIST_DIR}/FindPackageHandleStandardArgs.cmake) This would fix the current problem, but IMO this is not a real solution, since it can break also for other modules, and then this hardcoded path must be used also in the future in all find-modules. I think prefering the modules from CMAKE_ROOT when included from CMAKE_ROOT is the right thing to do. This expresses the dependency we have (from cmake modules to the other current cmake modules in CMAKE_ROOT). It seems like a reasonable approach to generally prefer the .cmake files in the same directory, before all others. I am not a core CMake developer though, and defer to them on issues like this. I think whatever is done should ensure old projects continue to build, even if new warnings are issues. I tested your topic branch with this project, and it still fails with the You should get the policy warning about CMP0017. Is that the case ? I saw lots of warnings about CMP0017. following error, CMake Error at cmake/modules/FindPackageHandleStandardArgs.cmake:53 (MESSAGE): REQUIRED_VARS Call Stack (most recent call first): /usr/local/share/cmake-2.8/Modules/FindZLIB.cmake:70 (FIND_PACKAGE_HANDLE_STANDARD_ARGS) CMakeLists.txt:234 (find_package) I see the following options: * set policy 0017 to NEW behaviour by default (is that possible ?) I guess this is aimed at someone else, it would be the first I think if it is possible. * set policy 0017 to NEW behaviour somewhere locally. What project is failing ? Avogadro. We can obviously make a new release with this corrected in a number of ways, but it would sidestep the issue that a new CMake release would be incompatible with a source tree that configured/compiled fine with CMake 2.8.2 and earlier. This seems like a major regression to me. Anything done locally doesn't help out when someone downloads this project, and I wonder how many other projects we don't know about might be affected by this issue. * hardcode ${CMAKE_CURRENT_LIST_DIR}/FindPackageHandleStandardArgs.cmake instead of just using include(FindPackageHandleStandardArgs), problems see above * revert the enhanced syntax :-( or put it into a file FindPackageHandleStandardArgs2.cmake (but the same can happen later on again) :-/ Putting it in a FindPackageHandleStandardArgs2.cmake while not ideal seems like one of the better solutions. I agree that it does not fix the problem very elegantly. Personally, I would try a rc3 with CMP0017 set to NEW and see how it goes. It works for kdelibs and for our project at work (which doesn't have a lot of fancy cmake stuff). If it is just that one file why not go with the simple solution? I would love to see a policy where includes in the same directory would be considered first, and then the normal search behavior. I wasn't sure how feasible this was in an rc3. Avogadro effectively copied what KDE was doing, and so maybe the issue is not that widespread. Until now setting a minimum CMake version has always been enough, I would hate for that to change. Marcus ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Friday 08 October 2010, David Cole wrote: On Fri, Oct 8, 2010 at 10:59 AM, Alexander Neundorf neund...@kde.orgwrote: ... Better idea: I'll add a policy which switches this behaviour (prefer CMAKE_ROOT over CMAKE_MODULE_PATH when used from within CMAKE_ROOT), and this policy will, as all other policies, default to the old behaviour, but warn. ... This latest idea does sound better, but I am still not a fan of invisible / magic behavior. I much prefer when things are explicitly spelled out. There is now a branch PreferCMakeModulesByCMakeModulesWithPolicy on staging. It includes this patch, a new policy CMP0017, and a basic test for it. I also verified that this fixes the KDE 4.5 problem. With KDE 4.5.0/4.5.1 there are now a lot of these new CMP0017 warnings, and in the end stuff has not been found which should have been found. Should I handle setting CMP0017 to NEW in kdelibs (- cmake 2.8.3 will not be able to build KDE 4.5.0/1, but complain loud) or in CMake/Modules/FindKDE4.cmake (then cmake 2.8.3 will be able to build KDE 4.5.0/1) ? Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Friday 08 October 2010, David Cole wrote: On Fri, Oct 8, 2010 at 10:59 AM, Alexander Neundorf neund...@kde.orgwrote: ... Better idea: I'll add a policy which switches this behaviour (prefer CMAKE_ROOT over CMAKE_MODULE_PATH when used from within CMAKE_ROOT), and this policy will, as all other policies, default to the old behaviour, but warn. ... This latest idea does sound better, but I am still not a fan of invisible / magic behavior. I much prefer when things are explicitly spelled out. Me definitely too. But I think in this case it will actually be easier to understand with the new behaviour. I had to think twice to understand what happened: it went into CMAKE_ROOT, but from there due to CMAKE_MODULE_PATH magic ;-) again out of it. I also think the comparison to RPATH and LD_LIBRARY_PATH is a good one in this case, and all OSs do have an RPATH to express such dependencies (or equivalent, for Windows it's the current dir). Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Friday 08 October 2010, you wrote: On Fri, Oct 8, 2010 at 10:59 AM, Alexander Neundorf neund...@kde.orgwrote: ... Better idea: I'll add a policy which switches this behaviour (prefer CMAKE_ROOT over CMAKE_MODULE_PATH when used from within CMAKE_ROOT), and this policy will, as all other policies, default to the old behaviour, but warn. ... This latest idea does sound better, but I am still not a fan of invisible / magic behavior. I much prefer when things are explicitly spelled out. Another idea would be the following behaviour (also with a policy): if a file, which has been found via CMAKE_MODULE_PATH or CMAKE_ROOT, includes another files, it looks first in its own directory, then in CMAKE_MODULE_PATH, then in CMAKE_ROOT. This would result in the same new behaviour for files in CMAKE_ROOT, additionally it would also change the search order for files found via CMAKE_MODULE_PATH. So this would be a bigger, but maybe more consistent change from the current behaviour. Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling
On Tuesday 05 October 2010, James Bigler wrote: On Tue, Oct 5, 2010 at 2:08 PM, Alexander Neundorf neund...@kde.org wrote: ... The current behaviour is really like running an executable with a shared library LD_PRELOADED, and hoping that the LD_PRELOADED libs will always be work as the correct one would. Alex This patch breaks backward compatibility, because it changes the include order. I am aware that it can potentially break builds. But, it only breaks compatibility where cmake *hopes* that nobody else breaks it instead of making sure that it gets what it expects. Also I assume that there are very rare, maybe no projects which rely on this very special behaviour. At least for kdelibs it works. Will check the other KDE modules and what we have at work tomorrow. Just like hoping that no one would want to modify the existing set of macros in this way is like assuming that no one will make a copy of a built You still can override them and create your own copies of cmake-supplied modules and use them, no change there. It only changes something for a module shipped with cmake which tries to include another module shipped with cmake. In this case the patch ensures that the including module also gets the module included it expects. IOW, it makes cmake 2.8.3 NOT break the build of applications using installed KDE 4.5.[01] kdelibs. in macro which will not work in a future version of cmake where the macro's api changes. Why can't you create a new version of the FSA macro in question? This is what is typically done to maintain backward compatibility with binary libraries. Alex ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] User vs CMake include mismatch handling [was CMake 2.8.3-rc1 ready for testing!]
On Wednesday 29 September 2010, Alexander Neundorf wrote: ... I have some thoughts, but it's not completely clear yet. Somehow I think if a file is include()d from CMAKE_MODULE_PATH, CMAKE_MODULE_PATH should be considered when it does its own include()s. If it's not included via CMAKE_MODULE_PATH (e.g. from cmake's directory), CMAKE_MODULE_PATH should be also not considered when it doesn its own include()s. Something in that direction might make sense... Thinking more about this... Ok, here we go. The approach comes with a small potential incompatibility problem, which IMO is outweighed by the advantage that it should guarantee that CMake itself *never* breaks a build, not even if it's the projects fault. So, the idea is simple: When a file A calls include/find_package, i.e. cmMakefile::getModulesFile(), its own location is considered. case 1) When A is *not* located in shared/cmake/Modules/, nothing changes. First CMAKE_MODULE_PATH is searched, then shared/cmake/Modules/. No change or potential incompatibility here. case 2) When A *is* located in shared/cmake/Modules/, *first* shared/cmake/Modules/ is checked, CMAKE_MODULE_PATH afterwards. In this case there are the following possibilities: 2.1) file A includes a file which exists only in shared/cmake/Modules/: no change here 2.2) file A includes a file which does not exist in shared/cmake/Modules/, but somewhere in CMAKE_MODULE_PATH (should be a rare case, probably only for custom language or platform modules): no change here And the interesting case, where the behaviour changes: 2.3) file A includes a file which exists both in shared/cmake/Modules/ and in CMAKE_MODULE_PATH. Current behaviour: the file from CMAKE_MODULE_PATH is loaded into file A (which is from shared/cmake/Modules/). This can cause breakage as we can see with the current problem in KDE. Basically because file A in cmake doesn't get the file loaded which it was developed and tested against. New behaviour: the file from shared/cmake/Modules/ is loaded, i.e. the file with which file A was developed and tested, and not a different file provided by a user project. This would fix the problem currently in KDE. Beside that, as a CMake developer, I would strongly prefer this new behaviour, since it ensures that no matter what cmake-using projects do with their modules, the module files provided by cmake will always work, since they will always get the original cmake files with which they were developed and tested (somewhat like a RPATH, while the current handling is more like LD_PRELOAD). Until now I can't think of a way how this could break existing builds. I think this should go into 2.8.3, if necessary with an extended rc-phase. Comments ? Alex P.S. I'll be offline over the weekend, and next week on business trip until Wednesday. ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
[cmake-developers] User vs CMake include mismatch handling [was CMake 2.8.3-rc1 ready for testing!]
Changing subject in order to re-classify the discussion. 2010/9/28 Brad King brad.k...@kitware.com: On 09/28/2010 05:20 PM, Alexander Neundorf wrote: On Tuesday 28 September 2010, Alexander Neundorf wrote: Is this intended this way ? The attached tiny patch seems to make CMAKE_PARENT_LIST_FILE work more like I expected. Yes, but who knows what it will break. I'm not prepared to do this during a release candidate series. Totally agree with that. Moreover I currently lack time for doing fine comments but I think that WE MUST not enforce compatibilities between include cmake files in order to ensure/enforce consistency between CMake provided files and user provided files. I personally thinks that the current behavior which is to let CMake pick-up the first matching file from CMAKE_MODULE_PATH is the best one. Now if ever a project is forking one or several may-become-incompatible version of CMake provided file(s) then the solution shouldn't be to burry some absolute path inside CMake provided files. Being able to patch a particular CMake provided include file is a feature I want to keep: - because user may not be able to wait a new CMake release to get his problem solved - because it's perfectly fine to tune/tweak CMake provided behavior from within a project My point of view is that this is the project responsible of this kind of breakage that should find a fix. For example if the KDE FPHSA was introduced by KDE then KDE project should refer (if needed) to the KDE provided FPHSA using full path (and not require CMake to do so). As Alex saif before: I don't think it's a good idea. People may purposely want to override CMake provided module in order to implement specific behavior and/or to patch locally. This is what we do in KDE, and I think it would be good for us. Using include(FPHSA CURRENT_DIR) in the files in cmake would mean that the find-modules in cmake also get the FPHSA they expect to get, and if we would use that in KDE too, we would be sure we would get our modified version. Again, my opinion is: KDE may ask to explicitely load its own FPHSA but shall not require the CMake provided modules to explicitely load the CMake provided one. May be an ORIGIN property may be automatically added to a MACRO in order to be able to check whether the macros comes from CMake or from a User provided file. Then you may implement your guard using: if (COMMAND FIND_PACKAGE_HANDLE_STANDARD_ARGS) get_property(FPHSA_ORIGIN COMMAND PROPERTY ORIGIN) if (FPHSA_ORIGIN STREQUAL CMake) include(/absolute/path/to/my/FPHSA) endif(FPHSA_ORIGIN STREQUAL CMake) endif(COMMAND FIND_PACKAGE_HANDLE_STANDARD_ARGS) the ORIGIN property may be set automatically by CMake (to User or CMake) when loading a cmake file. using this kind of trick the guard may be more intelligent and check what to do depending on CMAKE_xxx_VERSION. I know that this is NOT solving the current KDE problem but as stated before may be if we fix the KDE problem by enforcing absolute path include within CMake some other projects will break in another way? -- Erk Membre de l'April - « promouvoir et défendre le logiciel libre » - http://www.april.org ___ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers